Hi there, I also work with @mdhaduk at UT, and we came up with a rough implementation over trying to resolve this issue of implementing the discard logic and wanted to run it through you guys @acatangiu @ShadowCurse.

1. Expose the Discard Feature (VIRTIO_BLK_F_DISCARD)

We need to advertise discard support to the guest by enabling the VIRTIO_BLK_F_DISCARD feature in the device’s available features. For example, during the virtio block device initialization (likely in the device setup file), update the feature negotiation code as follows:

// in /src/vmm/src/devices/virtio/block/virtio/device.rs:
let mut avail_features = (1u64 << VIRTIO_F_VERSION_1)
    | (1u64 << VIRTIO_RING_F_EVENT_IDX)
    | VIRTIO_BLK_F_DISCARD; // Enable discard support

if config.cache_type == CacheType::Writeback {
    avail_features |= 1u64 << VIRTIO_BLK_F_FLUSH;
}

2. Parse Discard Requests

To our understanding, since this is an unimplemented RequestType, we need to add this variant in the RequestType enum.

// in request.rs
pub enum RequestType {
    In,
    Out,
    Flush,
    GetDeviceID,
    Discard,   // New variant for discard
    Unsupported(u32),
}

impl From<u32> for RequestType {
    fn from(value: u32) -> Self {
        match value {
            VIRTIO_BLK_T_IN      => RequestType::In,
            VIRTIO_BLK_T_OUT     => RequestType::Out,
            VIRTIO_BLK_T_FLUSH   => RequestType::Flush,
            VIRTIO_BLK_T_GET_ID  => RequestType::GetDeviceID,
            VIRTIO_BLK_T_DISCARD => RequestType::Discard,  // Map discard request
            t                    => RequestType::Unsupported(t),
        }
    }
}
...

Additionally, we must create a new match arm to handle RequestType::Discard and invoke a helper function that performs the discard operation with the actual logic).

// in request.rs:
pub(crate) fn process(
    self,
    disk: &mut DiskProperties,
    desc_idx: u16,
    mem: &GuestMemoryMmap,
    block_metrics: &BlockDeviceMetrics,
) -> ProcessingResult {
    // ... existing processing logic ...

    match request_type {
        // ... other request types ...
        RequestType::Discard => {
            // Retrieve offset and length from the request descriptor
            let offset = /* extract offset from request */;
            let len = /* extract length from request */;
            self.handle_discard(offset, len)
                .map_err(|e| /* map error accordingly */)?;
        }
        // ... handle other cases ...
    }
}

3. Implement Backend for Discarding

From here, we use the fallocate syscall with FALLOC_FL_PUNCH_HOLE as stated in previous discussions.

// in sync_io.rs:
fn handle_discard(&self, offset: u64, len: u32) -> Result<()> {
    let fd = self.file.as_raw_fd();
    let result = fallocate(
        fd,
        FallocateFlags::FALLOC_FL_PUNCH_HOLE | FallocateFlags::FALLOC_FL_KEEP_SIZE,
        offset as i64,
        len as i64,
    );
    if let Err(e) = result {
        eprintln!("Discard failed: {}", e);
        return Err(Error::IoError);
    }
    Ok(())
}

// define fallocate & flags (not sure where this would go).
use libc::{c_int, off64_t};
use std::io::Error as IoError;

bitflags::bitflags! {
    pub struct FallocateFlags: c_int {
        const FALLOC_FL_KEEP_SIZE = 0x01;
        const FALLOC_FL_PUNCH_HOLE = 0x02;
    }
}

// Wrapper for library call
pub fn fallocate(fd: c_int, mode: FallocateFlags, offset: off64_t, len: off64_t) -> Result<(), IoError> {
    let ret = unsafe { libc::fallocate64(fd, mode.bits(), offset, len) };
    if ret == 0 {
        Ok(())
    } else {
        Err(IoError::last_os_error())
    }
}

Please let us know if there's anything we aren't accounting for in our implementation. We're still figuring out some details (ex. actually fully parsing the command, as we need some logic in the parse() function as well) but I wanted to come out with a rough draft. If needed, I can also make a draft PR.

Unmap/discard/fstrim #2708

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions