Review feedback

oxidecomputer · Oct 24, 2023 · c161858 · c161858
1 parent cc42a72
commit c161858
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 12 deletions.
diff --git a/lib/propolis/src/block/in_memory.rs b/lib/propolis/src/block/in_memory.rs
@@ -125,7 +125,7 @@ impl InMemoryBackend {
             });
 
             let _join = std::thread::Builder::new()
-                .name(format!("file worker {n}"))
+                .name(format!("in-memory worker {n}"))
                 .spawn(move || {
                     worker_state.processing_loop(worker_acc);
                 })?;

diff --git a/lib/propolis/src/block/mod.rs b/lib/propolis/src/block/mod.rs
@@ -103,10 +103,7 @@ pub enum Result {
 }
 impl Result {
     pub const fn is_err(&self) -> bool {
-        match self {
-            Result::Success => false,
-            _ => true,
-        }
+        !matches!(self, Result::Success)
     }
 }
 
@@ -217,7 +214,7 @@ pub struct BackendOpts {
 pub trait Device: Send + Sync + 'static {
     fn attachment(&self) -> &device::Attachment;
 
-    /// Retreive the next request (if any)
+    /// Retrieve the next request (if any)
     fn next(&self) -> Option<Request>;
 
     /// Complete processing of result
@@ -226,9 +223,8 @@ pub trait Device: Send + Sync + 'static {
     /// Get an accessor to guest memory via the underlying device
     fn accessor_mem(&self) -> MemAccessor;
 
-    fn attach(&self, _info: DeviceInfo) {
-        // optional attach handler
-    }
+    /// Optional on-attach handler to update device state with new `DeviceInfo`
+    fn attach(&self, _info: DeviceInfo) {}
 }
 
 pub trait Backend: Send + Sync + 'static {

diff --git a/lib/propolis/src/hw/nvme/mod.rs b/lib/propolis/src/hw/nvme/mod.rs
@@ -839,9 +839,8 @@ impl PciNvme {
 
                     // We may have skipped pulling entries off some SQ due to this
                     // CQ having no available entry slots. Since we've just freed
-                    // up some slots, kick the SQs (excl. admin) here just in case.
-                    // TODO: worth kicking only the SQs specifically associated
-                    //       with this CQ?
+                    // up some slots, notify any attached block backend that
+                    // there may be new requests available.
                     self.block_attach.notify();
                 } else {
                     // Submission Queue y Tail Doorbell