bulk update of docs related to virtio queue

This commit takes care of all outdated comments related to
Queue/QueueState. Also, added a few lines about saving and
restoring state.

Signed-off-by: Andreea Florescu <[email protected]>

Author: andreeaflorescu

crates/virtio-device/src/lib.rs

@@ -56,14 +56,6 @@ pub mod status {
     pub const DEVICE_NEEDS_RESET: u8 = 64;
 }
 
-// Adding a `M: GuestAddressSpace` generic type parameter here as well until we sort out the
-// current discussion about how a memory object/reference gets passed to a queue.
-// We might end up with the queue type as an associated type here in the future, if it makes
-// sense to define an interface for queues which abstracts away whether they are split or packed.
-// On the other hand, if we split the queue into a config/state part and a live part, then we'd
-// only need to work with the configuration here, and there's no need for generic type parameters
-// or associated types related to that.
-
 // We're currently relying on macros from the `log` crate to output messages (as do other modules
 // from `vm-virtio`). This is temporary solution until we agree on a better alternative going
 // forward since different customers most likely have different expectation around levels,

crates/virtio-device/src/virtio_config.rs

@@ -190,7 +190,6 @@ where
 
 impl<T> WithDriverSelect for T
 where
-    // Added a `static bound here while `M` is around to simplify dealing with lifetimes.
     T: BorrowMut<VirtioConfig<Queue>> + VirtioDevice,
 {
     fn queue_select(&self) -> u16 {

crates/virtio-queue-ser/src/state.rs

@@ -31,17 +31,6 @@ pub struct QueueStateSer {
 
 // The following `From` implementations can be used to convert from a `QueueStateSer` to the
 // `QueueState` from the base crate and vice versa.
-// WARNING: They don't check for any invalid data, that would otherwise be checked when initializing
-// a queue object from scratch (for example setting a queue size greater than its max possible
-// size). The problem with the current `QueueState` implementation is that it can be used as the
-// queue object itself. `QueueState`'s fields are public, which allows the user to set up and use an
-// invalid queue. Once we fix https://github.com/rust-vmm/vm-virtio/issues/143, `QueueState` will be
-// renamed to `Queue`, its fields will no longer be public, and `QueueState` will consist of the
-// actual state. This way we can also check against any invalid data when trying to get a `Queue`
-// from the state object.
-// Nevertheless, we don't make any assumptions in the virtio-queue code about the queue's state that
-// would otherwise result in a panic, when initialized with random data, so from this point of view
-// these conversions are safe to use.
 impl From<&QueueStateSer> for QueueState {
     fn from(state: &QueueStateSer) -> Self {
         QueueState {
@@ -121,11 +110,6 @@ mod tests {
     fn test_ser_with_len_zero() {
         // This is a regression test that tests that a queue where the size is set to 0 does not
         // cause any problems when poping the descriptor chain.
-        //
-        // In the future this should be updated such that the Queue does not have the fields as
-        // public, and the way to obtain a Queue from a serialized Queue is by using a `try_from`
-        // function which then makes sure that the deserialized values are valid before creating
-        // a queue that might be invalid.
         let queue_ser = QueueStateSer {
             max_size: 16,
             next_avail: 0,

crates/virtio-queue/README.md

@@ -9,9 +9,7 @@ format.
 The purpose of the virtio-queue API is to be consumed by virtio device
 implementations (such as the block device or vsock device).
 The main abstraction is the `Queue`. The crate is also defining a state object
-for the queue, i.e. `QueueState`. The `Queue` objects are always created from a
-state (even if it’s an empty one) in order to avoid branching in the calling
-functions.
+for the queue, i.e. `QueueState`.
 
 ## Usage
 
@@ -40,9 +38,7 @@ Each virtqueue consists of three parts:
 
 Before booting the virtual machine (VM), the VMM does the following set up:
 
-1. initialize an array of Queues based on a `max_size` and a reference to the
-   memory object, by using `Queue::new(mem: M, max_size: u16)`, the queue
-   objects are created from a default state.
+1. initialize an array of Queues using the Queue constructor.
 2. register the device to the MMIO bus, so that the driver can later send
    read/write requests from/to the MMIO space, some of those requests also set
    up the queues’ state.
@@ -135,17 +131,13 @@ of the next descriptor if VIRTQ_DESC_F_NEXT is set.
 
 ## Design
 
-`QueueStateT` is a trait that allows different implementations for a `Queue`
+`QueueT` is a trait that allows different implementations for a `Queue`
 object for single-threaded context and multi-threaded context. The
 implementations provided in `virtio-queue` are:
 
-1. `QueueState` → it is used for the single-threaded context, and keeps the
-   state of a virtio queue.
-2. `QueueStateSync` → it is used for the multi-threaded context, and is simply
-   a wrapper over an `Arc<Mutex<QueueState>>`.
-
-`Queue` is a wrapper over a `QueueState` that also holds the guest memory
-object associated with the queue.
+1. `Queue` → it is used for the single-threaded context.
+2. `QueueSync` → it is used for the multi-threaded context, and is simply
+   a wrapper over an `Arc<Mutex<Queue>>`.
 
 Besides the above abstractions, the `virtio-queue` crate provides also the
 following ones:
@@ -159,6 +151,13 @@ following ones:
 * `AvailIter` - is a consuming iterator over all available descriptor chain
   heads in the queue.
 
+## Save/Restore Queue
+
+The `Queue` allows saving the state through the `state` function which returns
+a `QueueState`. `Queue` objects can be created from a previously saved state by
+using `QueueState::try_from`. The VMM should check for errors when restoring
+a `Queue` from a previously saved state.
+
 ### Notification suppression
 
 A big part of the `virtio-queue` crate consists of the notification suppression

crates/virtio-queue/src/lib.rs

@@ -107,7 +107,7 @@ pub trait QueueGuard<'a> {
 /// provided for single-threaded context and multi-threaded context.
 ///
 /// Using Higher-Rank Trait Bounds (HRTBs) to effectively define an associated type that has a
-/// lifetime parameter, without tagging the `QueueStateT` trait with a lifetime as well.
+/// lifetime parameter, without tagging the `QueueT` trait with a lifetime as well.
 pub trait QueueT: for<'a> QueueGuard<'a> {
     /// Construct an empty virtio queue state object with the given `max_size`.
     ///