virtio/block: wrap disk_image in a Mutex

Imago's discard_to_* operations require a mutable reference to Storage.
Let's wrap disk_image in a Mutex so we can obtain such reference when
needed. Since, in practice, we won't have multiple threads accessing the
lock, the overhead should be very small (one or two operations on
atomics).

Signed-off-by: Sergio Lopez <[email protected]>

Author: slp

src/devices/src/virtio/block/device.rs

@@ -15,7 +15,7 @@ use std::os::linux::fs::MetadataExt;
 use std::os::macos::fs::MetadataExt;
 use std::path::PathBuf;
 use std::result;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 use std::thread::JoinHandle;
 
 use imago::{
@@ -65,18 +65,18 @@ impl CacheType {
 /// Helper object for setting up all `Block` fields derived from its backing file.
 pub(crate) struct DiskProperties {
     cache_type: CacheType,
-    pub(crate) file: Arc<SyncFormatAccess<Box<dyn DynStorage>>>,
+    pub(crate) file: Arc<Mutex<SyncFormatAccess<Box<dyn DynStorage>>>>,
     nsectors: u64,
     image_id: Vec<u8>,
 }
 
 impl DiskProperties {
     pub fn new(
-        disk_image: Arc<SyncFormatAccess<Box<dyn DynStorage>>>,
+        disk_image: Arc<Mutex<SyncFormatAccess<Box<dyn DynStorage>>>>,
         disk_image_id: Vec<u8>,
         cache_type: CacheType,
     ) -> io::Result<Self> {
-        let disk_size = disk_image.size();
+        let disk_size = disk_image.lock().unwrap().size();
 
         // We only support disk size, which uses the first two words of the configuration space.
         // If the image is not a multiple of the sector size, the tail bits are not exposed.
@@ -95,10 +95,6 @@ impl DiskProperties {
         })
     }
 
-    pub fn file(&self) -> &SyncFormatAccess<Box<dyn DynStorage>> {
-        self.file.as_ref()
-    }
-
     pub fn nsectors(&self) -> u64 {
         self.nsectors
     }
@@ -146,11 +142,11 @@ impl Drop for DiskProperties {
         match self.cache_type {
             CacheType::Writeback => {
                 // flush() first to force any cached data out.
-                if self.file.flush().is_err() {
+                if self.file.lock().unwrap().flush().is_err() {
                     error!("Failed to flush block data on drop.");
                 }
                 // Sync data out to physical media on host.
-                if self.file.sync().is_err() {
+                if self.file.lock().unwrap().sync().is_err() {
                     error!("Failed to sync block data on drop.")
                 }
             }
@@ -206,7 +202,7 @@ pub struct Block {
     // Host file and properties.
     disk: Option<DiskProperties>,
     cache_type: CacheType,
-    disk_image: Arc<SyncFormatAccess<Box<dyn DynStorage>>>,
+    disk_image: Arc<Mutex<SyncFormatAccess<Box<dyn DynStorage>>>>,
     disk_image_id: Vec<u8>,
     worker_thread: Option<JoinHandle<()>>,
     worker_stopfd: EventFd,
@@ -271,10 +267,10 @@ impl Block {
             }
         };
 
-        let disk_image = Arc::new(disk_image);
+        let disk_image = Arc::new(Mutex::new(disk_image));
 
         let disk_properties =
-            DiskProperties::new(Arc::clone(&disk_image), disk_image_id.clone(), cache_type)?;
+            DiskProperties::new(disk_image.clone(), disk_image_id.clone(), cache_type)?;
 
         let mut avail_features = (1u64 << VIRTIO_F_VERSION_1)
             | (1u64 << VIRTIO_BLK_F_FLUSH)

src/devices/src/virtio/block/worker.rs

@@ -228,7 +228,7 @@ impl BlockWorker {
             }
             VIRTIO_BLK_T_FLUSH => match self.disk.cache_type() {
                 CacheType::Writeback => {
-                    let diskfile = self.disk.file();
+                    let diskfile = self.disk.file.lock().unwrap();
                     diskfile.flush().map_err(RequestError::FlushingToDisk)?;
                     diskfile.sync().map_err(RequestError::FlushingToDisk)?;
                     Ok(0)

src/devices/src/virtio/file_traits.rs

@@ -432,7 +432,7 @@ impl FileReadWriteAtVolatile for DiskProperties {
             .len()
             .try_into()
             .map_err(|e| Error::new(ErrorKind::InvalidData, e))?;
-        self.file().readv(iovec, offset)?;
+        self.file.lock().unwrap().readv(iovec, offset)?;
         Ok(full_length)
     }
 
@@ -450,7 +450,7 @@ impl FileReadWriteAtVolatile for DiskProperties {
             .len()
             .try_into()
             .map_err(|e| Error::new(ErrorKind::InvalidData, e))?;
-        self.file().writev(iovec, offset)?;
+        self.file.lock().unwrap().writev(iovec, offset)?;
         Ok(full_length)
     }
 }