Repository: firecracker-microvm/firecracker
File: src/vmm/src/devices/virtio/queue.rs (Rust)

Code before:
        fence(Ordering::Acquire);

        // We'll need to find the first available descriptor, that we haven't yet popped.
        // In a naive notation, that would be:
        // `descriptor_table[avail_ring[next_avail]]`.
        //
        // We use `self.next_avail` to store the position, in `ring`, of the next available
        // descriptor index, with a twist: we always only increment `self.next_avail`, so the
        // actual position will be `self.next_avail % self.actual_size()`.
        let idx = self.next_avail.0 % self.actual_size();
        // SAFETY:
        // index is bound by the queue size
        let desc_index = unsafe { self.avail_ring_ring_get(usize::from(idx)) };

        DescriptorChain::checked_new(self.desc_table_ptr, self.actual_size(), desc_index).map(
            |dc| {
                self.next_avail += Wrapping(1);
                dc
            },
        )
    }

    /// Undo the effects of the last `self.pop()` call.
    /// The caller can use this, if it was unable to consume the last popped descriptor chain.
    pub fn undo_pop(&mut self) {
        self.next_avail -= Wrapping(1);
    }

    /// Write used element into used_ring ring.
    pub fn write_used_element(
        &mut self,
        ring_index: u16,
        desc_index: u16,
        len: u32,
    ) -> Result<(), QueueError> {
        if self.actual_size() <= desc_index {
            error!(
                "attempted to add out of bounds descriptor to used ring: {}",
                desc_index
            );
            return Err(QueueError::DescIndexOutOfBounds(desc_index));
        }

        let next_used = ring_index % self.actual_size();
        let used_element = UsedElement {
            id: u32::from(desc_index),
            len,
        };
        // SAFETY:
        // index is bound by the queue size
        unsafe {

Review comment (suggestion):
Could you please re-word the first commit into something like:

```
feat(virtio): split `add_used()` into 2 separate methods

`Queue::add_used()` method first writes a Descriptor head in the used descriptor
ring buffer and then advances the index of this buffer to let the guest know we
used one or more Descriptors.

Carve out each one of these steps in their own function so that we can add multiple
descriptors in the used ring and advance the index only once we finish handling
descriptors in one step.

This will be useful when, in later commits, we will implement mergeable buffers for
the RX queue of the network device

Code after:
        fence(Ordering::Acquire);

        // We'll need to find the first available descriptor, that we haven't yet popped.
        // In a naive notation, that would be:
        // `descriptor_table[avail_ring[next_avail]]`.
        //
        // We use `self.next_avail` to store the position, in `ring`, of the next available
        // descriptor index, with a twist: we always only increment `self.next_avail`, so the
        // actual position will be `self.next_avail % self.actual_size()`.
        let idx = self.next_avail.0 % self.actual_size();
        // SAFETY:
        // index is bound by the queue size
        let desc_index = unsafe { self.avail_ring_ring_get(usize::from(idx)) };

        DescriptorChain::checked_new(self.desc_table_ptr, self.actual_size(), desc_index).map(
            |dc| {
                self.next_avail += Wrapping(1);
                dc
            },
        )
    }

    /// Undo the effects of the last `self.pop()` call.
    /// The caller can use this, if it was unable to consume the last popped descriptor chain.
    pub fn undo_pop(&mut self) {
        self.next_avail -= Wrapping(1);
    }

    /// Write used element into used_ring ring.
    /// - [`ring_index_offset`] is an offset added to
    /// the current [`self.next_used`] to obtain actual index
    /// into used_ring.
    pub fn write_used_element(
        &mut self,
        ring_index_offset: u16,
        desc_index: u16,
        len: u32,
    ) -> Result<(), QueueError> {
        if self.actual_size() <= desc_index {
            error!(
                "attempted to add out of bounds descriptor to used ring: {}",
                desc_index
            );
            return Err(QueueError::DescIndexOutOfBounds(desc_index));
        }

        let next_used = (self.next_used + Wrapping(ring_index_offset)).0 % self.actual_size();
        let used_element = UsedElement {
            id: u32::from(desc_index),
            len,
        };