disk: remove non-async disk image support

With AsyncDiskFileWrapper, we can use all disk image formats via the
async API.

The create_async_disk_file() function is removed, and the existing
create_disk_file() function can be used to create async-capable disk
files, as the DiskFile trait now includes ToAsyncDisk.

BUG=b:219595052
TEST=cargo build --features=composite-disk
TEST=Boot x86-64 Linux in crosvm with a qcow2 disk attached

Change-Id: I430d306fe71c325ab529a7031353d20f8a9aa2f3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3824067
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>

devices/src/virtio/block/asynchronous.rs

@@ -36,7 +36,7 @@ use cros_async::SelectResult;
 use cros_async::TimerAsync;
 use data_model::DataInit;
 use disk::AsyncDisk;
-use disk::ToAsyncDisk;
+use disk::DiskFile;
 use futures::pin_mut;
 use futures::stream::FuturesUnordered;
 use futures::stream::StreamExt;
@@ -502,7 +502,7 @@ fn run_worker(
 /// Virtio device for exposing block level read/write operations on a host file.
 pub struct BlockAsync {
     // We keep these members crate-public as they are accessed by the vhost-user device.
-    pub(crate) disk_image: Option<Box<dyn ToAsyncDisk>>,
+    pub(crate) disk_image: Option<Box<dyn DiskFile>>,
     pub(crate) disk_size: Arc<AtomicU64>,
     pub(crate) avail_features: u64,
     pub(crate) read_only: bool,
@@ -512,14 +512,14 @@ pub struct BlockAsync {
     pub(crate) id: Option<BlockId>,
     pub(crate) control_tube: Option<Tube>,
     kill_evt: Option<Event>,
-    worker_thread: Option<thread::JoinHandle<(Box<dyn ToAsyncDisk>, Option<Tube>)>>,
+    worker_thread: Option<thread::JoinHandle<(Box<dyn DiskFile>, Option<Tube>)>>,
 }
 
 impl BlockAsync {
     /// Create a new virtio block device that operates on the given AsyncDisk.
     pub fn new(
         base_features: u64,
-        disk_image: Box<dyn ToAsyncDisk>,
+        disk_image: Box<dyn DiskFile>,
         read_only: bool,
         sparse: bool,
         block_size: u32,

devices/src/virtio/block/sys/unix.rs

@@ -13,7 +13,7 @@ use base::flock;
 use base::iov_max;
 use base::open_file;
 use base::FlockOperation;
-use disk::ToAsyncDisk;
+use disk::DiskFile;
 
 use crate::virtio::block::block::DiskOption;
 
@@ -27,8 +27,8 @@ pub fn get_seg_max(queue_size: u16) -> u32 {
 }
 
 impl DiskOption {
-    /// Open the specified disk file as a raw image.
-    pub fn open_as_raw_image(&self) -> anyhow::Result<File> {
+    /// Open the specified disk file.
+    pub fn open(&self) -> anyhow::Result<Box<dyn DiskFile>> {
         let mut options = OpenOptions::new();
         options.read(true).write(!self.read_only);
 
@@ -47,14 +47,7 @@ impl DiskOption {
         flock(&raw_image, lock_op, true)
             .with_context(|| format!("failed to lock disk image {}", self.path.display()))?;
 
-        Ok(raw_image)
-    }
-
-    pub fn open_as_async_file(&self) -> anyhow::Result<Box<dyn ToAsyncDisk>> {
-        let raw_image = self.open_as_raw_image()?;
-        let async_file = disk::create_async_disk_file(raw_image)
-            .context("failed to create async virtual disk")?;
-
-        Ok(async_file)
+        disk::create_disk_file(raw_image, self.sparse, disk::MAX_NESTING_DEPTH, &self.path)
+            .context("create_disk_file failed")
     }
 }

devices/src/virtio/vhost/user/device/block/sys/unix.rs

@@ -53,7 +53,7 @@ pub fn start_device(opts: Options) -> anyhow::Result<()> {
 
     let block = Box::new(BlockAsync::new(
         base_features(ProtectionType::Unprotected),
-        disk.open_as_async_file()?,
+        disk.open()?,
         disk.read_only,
         disk.sparse,
         disk.block_size,

devices/src/virtio/vhost/user/device/block/sys/windows.rs

@@ -16,7 +16,6 @@ use base::RawDescriptor;
 use broker_ipc::common_child_setup;
 use broker_ipc::CommonChildStartupArgs;
 use cros_async::Executor;
-use disk::async_ok;
 use disk::AsyncDisk;
 use disk::SingleFileDisk;
 use hypervisor::ProtectionType;
@@ -117,10 +116,6 @@ pub fn start_device(opts: Options) -> anyhow::Result<()> {
         files.push(open_disk_file(&disk_option, take_write_lock)?);
     }
 
-    if !async_ok(&files[0])? {
-        bail!("found non-raw image; only raw images are supported.");
-    }
-
     let base_features = base_features(ProtectionType::Unprotected);
 
     let ex = Executor::new().context("failed to create executor")?;

disk/src/asynchronous.rs

@@ -23,7 +23,6 @@ use crate::DiskFile;
 use crate::DiskGetLen;
 use crate::Error;
 use crate::Result;
-use crate::ToAsyncDisk;
 
 /// Async wrapper around a non-async `DiskFile` using a `BlockingPool`.
 ///
@@ -68,8 +67,8 @@ impl<T: DiskFile + Send> AsRawDescriptors for AsyncDiskFileWrapper<T> {
 }
 
 #[async_trait(?Send)]
-impl<T: 'static + DiskFile + Send + ToAsyncDisk> AsyncDisk for AsyncDiskFileWrapper<T> {
-    fn into_inner(self: Box<Self>) -> Box<dyn ToAsyncDisk> {
+impl<T: 'static + DiskFile + Send> AsyncDisk for AsyncDiskFileWrapper<T> {
+    fn into_inner(self: Box<Self>) -> Box<dyn DiskFile> {
         self.blocking_pool
             .shutdown(None)
             .expect("AsyncDiskFile pool shutdown failed");

disk/src/disk.rs

@@ -151,6 +151,7 @@ pub trait DiskFile:
     + PunchHole
     + WriteZeroesAt
     + FileAllocate
+    + ToAsyncDisk
     + Send
     + AsRawDescriptors
     + Debug
@@ -164,6 +165,7 @@ impl<
             + FileReadWriteAtVolatile
             + WriteZeroesAt
             + FileAllocate
+            + ToAsyncDisk
             + Send
             + AsRawDescriptors
             + Debug,
@@ -253,26 +255,6 @@ pub fn detect_image_type(file: &File) -> Result<ImageType> {
     Ok(ImageType::Raw)
 }
 
-/// Check if the image file type can be used for async disk access.
-pub fn async_ok(raw_image: &File) -> Result<bool> {
-    let image_type = detect_image_type(raw_image)?;
-    Ok(match image_type {
-        ImageType::Raw => true,
-        ImageType::Qcow2 | ImageType::AndroidSparse | ImageType::CompositeDisk => false,
-    })
-}
-
-/// Inspect the image file type and create an appropriate disk file to match it.
-pub fn create_async_disk_file(raw_image: File) -> Result<Box<dyn ToAsyncDisk>> {
-    let image_type = detect_image_type(&raw_image)?;
-    Ok(match image_type {
-        ImageType::Raw => Box::new(raw_image) as Box<dyn ToAsyncDisk>,
-        ImageType::Qcow2 | ImageType::AndroidSparse | ImageType::CompositeDisk => {
-            return Err(Error::UnknownType)
-        }
-    })
-}
-
 /// Inspect the image file type and create an appropriate disk file to match it.
 pub fn create_disk_file(
     raw_image: File,
@@ -327,7 +309,7 @@ pub fn create_disk_file(
 #[async_trait(?Send)]
 pub trait AsyncDisk: DiskGetLen + FileSetLen + FileAllocate {
     /// Returns the inner file consuming self.
-    fn into_inner(self: Box<Self>) -> Box<dyn ToAsyncDisk>;
+    fn into_inner(self: Box<Self>) -> Box<dyn DiskFile>;
 
     /// Asynchronously fsyncs any completed operations to the disk.
     async fn fsync(&self) -> Result<()>;
@@ -389,7 +371,7 @@ impl FileAllocate for SingleFileDisk {
 
 #[async_trait(?Send)]
 impl AsyncDisk for SingleFileDisk {
-    fn into_inner(self: Box<Self>) -> Box<dyn ToAsyncDisk> {
+    fn into_inner(self: Box<Self>) -> Box<dyn DiskFile> {
         Box::new(self.inner.into_source())
     }
 

src/crosvm/sys/unix/device_helpers.rs

@@ -240,46 +240,25 @@ impl<'a> VirtioDeviceBuilder for DiskConfig<'a> {
         &self,
         protection_type: ProtectionType,
     ) -> anyhow::Result<Box<dyn VirtioDevice>> {
-        let disk = self.disk;
+        info!(
+            "Trying to attach block device: {}",
+            self.disk.path.display(),
+        );
+        let disk_image = self.disk.open()?;
+
         let disk_device_tube = self.device_tube.take();
-
-        let raw_image = disk.open_as_raw_image()?;
-
-        info!("Trying to attach block device: {}", disk.path.display());
-        let dev = if disk::async_ok(&raw_image).context("failed to check disk async_ok")? {
-            let async_file = disk::create_async_disk_file(raw_image)
-                .context("failed to create async virtual disk")?;
-            Box::new(
-                virtio::BlockAsync::new(
-                    virtio::base_features(protection_type),
-                    async_file,
-                    disk.read_only,
-                    disk.sparse,
-                    disk.block_size,
-                    disk.id,
-                    disk_device_tube,
-                )
-                .context("failed to create block device")?,
-            ) as Box<dyn VirtioDevice>
-        } else {
-            let disk_file =
-                disk::create_disk_file(raw_image, disk.sparse, disk::MAX_NESTING_DEPTH, &disk.path)
-                    .context("failed to create virtual disk")?;
-            Box::new(
-                virtio::Block::new(
-                    virtio::base_features(protection_type),
-                    disk_file,
-                    disk.read_only,
-                    disk.sparse,
-                    disk.block_size,
-                    disk.id,
-                    disk_device_tube,
-                )
-                .context("failed to create block device")?,
+        Ok(Box::new(
+            virtio::BlockAsync::new(
+                virtio::base_features(protection_type),
+                disk_image,
+                self.disk.read_only,
+                self.disk.sparse,
+                self.disk.block_size,
+                self.disk.id,
+                disk_device_tube,
             )
-        };
-
-        Ok(dev)
+            .context("failed to create block device")?,
+        ))
     }
 
     fn create_vhost_user_device(
@@ -293,13 +272,13 @@ impl<'a> VirtioDeviceBuilder for DiskConfig<'a> {
             keep_rds.push(device_tube.as_raw_descriptor());
         }
 
-        let async_file = disk.open_as_async_file()?;
-        keep_rds.extend(async_file.as_raw_descriptors());
+        let disk_image = disk.open()?;
+        keep_rds.extend(disk_image.as_raw_descriptors());
 
         let block = Box::new(
             virtio::BlockAsync::new(
                 virtio::base_features(ProtectionType::Unprotected),
-                async_file,
+                disk_image,
                 disk.read_only,
                 disk.sparse,
                 disk.block_size,