diff options
author | Daniel Verkamp <dverkamp@chromium.org> | 2019-11-08 10:11:16 -0800 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-11-18 09:30:58 +0000 |
commit | e73c80f355099a38293108baf0aed9666664a6e7 (patch) | |
tree | e325810bd77f52f6f525fe1255029e8d16e45982 | |
parent | 40a721b434f8bc59aac0010d5612ef3ac39f20c8 (diff) | |
download | crosvm-e73c80f355099a38293108baf0aed9666664a6e7.tar crosvm-e73c80f355099a38293108baf0aed9666664a6e7.tar.gz crosvm-e73c80f355099a38293108baf0aed9666664a6e7.tar.bz2 crosvm-e73c80f355099a38293108baf0aed9666664a6e7.tar.lz crosvm-e73c80f355099a38293108baf0aed9666664a6e7.tar.xz crosvm-e73c80f355099a38293108baf0aed9666664a6e7.tar.zst crosvm-e73c80f355099a38293108baf0aed9666664a6e7.zip |
devices: block: add option to control sparseness
Extend the --disk option and other related options to allow a particular disk to have the sparse operations (virtio-blk's discard command) enabled or disabled. By default, the sparse flag will be enabled for virtio-blk devices, matching current behavior. BUG=chromium:858815 TEST=Run `crosvm with --rwdisk file.img,sparse=false` and try to discard Change-Id: Ib72c949711fbe869a3f444d7f929a80d0e039f72 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1906750 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
-rw-r--r-- | devices/src/virtio/block.rs | 36 | ||||
-rw-r--r-- | src/crosvm.rs | 1 | ||||
-rw-r--r-- | src/linux.rs | 9 | ||||
-rw-r--r-- | src/main.rs | 67 |
4 files changed, 98 insertions, 15 deletions
diff --git a/devices/src/virtio/block.rs b/devices/src/virtio/block.rs index 2a515af..987cb03 100644 --- a/devices/src/virtio/block.rs +++ b/devices/src/virtio/block.rs @@ -248,12 +248,14 @@ struct Worker { disk_image: Box<dyn DiskFile>, disk_size: Arc<Mutex<u64>>, read_only: bool, + sparse: bool, } impl Worker { fn process_one_request( avail_desc: DescriptorChain, read_only: bool, + sparse: bool, disk: &mut dyn DiskFile, disk_size: u64, flush_timer: &mut TimerFd, @@ -278,6 +280,7 @@ impl Worker { &mut reader, &mut writer, read_only, + sparse, disk, disk_size, flush_timer, @@ -313,6 +316,7 @@ impl Worker { let len = match Worker::process_one_request( avail_desc, self.read_only, + self.sparse, &mut *self.disk_image, *disk_size, flush_timer, @@ -464,6 +468,7 @@ pub struct Block { disk_size: Arc<Mutex<u64>>, avail_features: u64, read_only: bool, + sparse: bool, seg_max: u32, control_socket: Option<DiskControlResponseSocket>, } @@ -491,6 +496,7 @@ impl Block { pub fn new( mut disk_image: Box<dyn DiskFile>, read_only: bool, + sparse: bool, control_socket: Option<DiskControlResponseSocket>, ) -> SysResult<Block> { let disk_size = disk_image.seek(SeekFrom::End(0))? as u64; @@ -506,7 +512,9 @@ impl Block { if read_only { avail_features |= 1 << VIRTIO_BLK_F_RO; } else { - avail_features |= 1 << VIRTIO_BLK_F_DISCARD; + if sparse { + avail_features |= 1 << VIRTIO_BLK_F_DISCARD; + } avail_features |= 1 << VIRTIO_BLK_F_WRITE_ZEROES; } avail_features |= 1 << VIRTIO_F_VERSION_1; @@ -527,6 +535,7 @@ impl Block { disk_size: Arc::new(Mutex::new(disk_size)), avail_features, read_only, + sparse, seg_max, control_socket, }) @@ -540,6 +549,7 @@ impl Block { reader: &mut Reader, writer: &mut Writer, read_only: bool, + sparse: bool, disk: &mut dyn DiskFile, disk_size: u64, flush_timer: &mut TimerFd, @@ -611,6 +621,10 @@ impl Block { } } VIRTIO_BLK_T_DISCARD | VIRTIO_BLK_T_WRITE_ZEROES => { + if req_type == VIRTIO_BLK_T_DISCARD && !sparse { + return Err(ExecuteError::Unsupported(req_type)); + } + while reader.available_bytes() >= size_of::<virtio_blk_discard_write_zeroes>() { let seg: virtio_blk_discard_write_zeroes = reader.read_obj().map_err(ExecuteError::Read)?; @@ -744,6 +758,7 @@ impl VirtioDevice for Block { self.kill_evt = Some(self_kill_evt); let read_only = self.read_only; + let sparse = self.sparse; let disk_size = self.disk_size.clone(); if let Some(disk_image) = self.disk_image.take() { if let Some(control_socket) = self.control_socket.take() { @@ -758,6 +773,7 @@ impl VirtioDevice for Block { disk_image, disk_size, read_only, + sparse, }; worker.run(queue_evts.remove(0), kill_evt, control_socket); }); @@ -795,7 +811,7 @@ mod tests { let f = File::create(&path).unwrap(); f.set_len(0x1000).unwrap(); - let b = Block::new(Box::new(f), true, None).unwrap(); + let b = Block::new(Box::new(f), true, false, None).unwrap(); let mut num_sectors = [0u8; 4]; b.read_config(0, &mut num_sectors); // size is 0x1000, so num_sectors is 8 (4096/512). @@ -815,17 +831,27 @@ mod tests { // read-write block device { let f = File::create(&path).unwrap(); - let b = Block::new(Box::new(f), false, None).unwrap(); + let b = Block::new(Box::new(f), false, true, None).unwrap(); // writable device should set VIRTIO_BLK_F_FLUSH + VIRTIO_BLK_F_DISCARD // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE // + VIRTIO_BLK_F_SEG_MAX assert_eq!(0x100006244, b.features()); } + // read-write block device, non-sparse + { + let f = File::create(&path).unwrap(); + let b = Block::new(Box::new(f), false, false, None).unwrap(); + // writable device should set VIRTIO_BLK_F_FLUSH + // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE + // + VIRTIO_BLK_F_SEG_MAX + assert_eq!(0x100004244, b.features()); + } + // read-only block device { let f = File::create(&path).unwrap(); - let b = Block::new(Box::new(f), true, None).unwrap(); + let b = Block::new(Box::new(f), true, true, None).unwrap(); // read-only device should set VIRTIO_BLK_F_FLUSH and VIRTIO_BLK_F_RO // + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE + VIRTIO_BLK_F_SEG_MAX assert_eq!(0x100000264, b.features()); @@ -879,6 +905,7 @@ mod tests { Worker::process_one_request( avail_desc, false, + true, &mut f, disk_size, &mut flush_timer, @@ -939,6 +966,7 @@ mod tests { Worker::process_one_request( avail_desc, false, + true, &mut f, disk_size, &mut flush_timer, diff --git a/src/crosvm.rs b/src/crosvm.rs index 649b0fe..c8b67bb 100644 --- a/src/crosvm.rs +++ b/src/crosvm.rs @@ -33,6 +33,7 @@ pub enum Executable { pub struct DiskOption { pub path: PathBuf, pub read_only: bool, + pub sparse: bool, } /// A bind mount for directories in the plugin process. diff --git a/src/linux.rs b/src/linux.rs index 1177377..25aba0f 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -358,8 +358,13 @@ fn create_block_device( flock(&raw_image, lock_op, true).map_err(Error::DiskImageLock)?; let disk_file = disk::create_disk_file(raw_image).map_err(Error::CreateDiskError)?; - let dev = virtio::Block::new(disk_file, disk.read_only, Some(disk_device_socket)) - .map_err(Error::BlockDeviceNew)?; + let dev = virtio::Block::new( + disk_file, + disk.read_only, + disk.sparse, + Some(disk_device_socket), + ) + .map_err(Error::BlockDeviceNew)?; Ok(VirtioDeviceStub { dev: Box::new(dev), diff --git a/src/main.rs b/src/main.rs index a864833..d6d82a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -299,11 +299,21 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: cfg.syslog_tag = Some(value.unwrap().to_owned()); } "root" | "rwroot" | "disk" | "rwdisk" | "qcow" | "rwqcow" => { + let param = value.unwrap(); + let mut components = param.split(','); let read_only = !name.starts_with("rw"); - let disk_path = PathBuf::from(value.unwrap()); + let disk_path = + PathBuf::from( + components + .next() + .ok_or_else(|| argument::Error::InvalidValue { + value: param.to_owned(), + expected: "missing disk path", + })?, + ); if !disk_path.exists() { return Err(argument::Error::InvalidValue { - value: value.unwrap().to_owned(), + value: param.to_owned(), expected: "this disk path does not exist", }); } @@ -319,10 +329,42 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: if read_only { "ro" } else { "rw" } )); } - cfg.disks.push(DiskOption { + + let mut disk = DiskOption { path: disk_path, read_only, - }); + sparse: true, + }; + + for opt in components { + let mut o = opt.splitn(2, '='); + let kind = o.next().ok_or_else(|| argument::Error::InvalidValue { + value: opt.to_owned(), + expected: "disk options must not be empty", + })?; + let value = o.next().ok_or_else(|| argument::Error::InvalidValue { + value: opt.to_owned(), + expected: "disk options must be of the form `kind=value`", + })?; + + match kind { + "sparse" => { + let sparse = value.parse().map_err(|_| argument::Error::InvalidValue { + value: value.to_owned(), + expected: "`sparse` must be a boolean", + })?; + disk.sparse = sparse; + } + _ => { + return Err(argument::Error::InvalidValue { + value: kind.to_owned(), + expected: "unrecognized disk option", + }); + } + } + } + + cfg.disks.push(disk); } "pmem-device" | "rw-pmem-device" => { let disk_path = PathBuf::from(value.unwrap()); @@ -336,6 +378,7 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: cfg.pmem_devices.push(DiskOption { path: disk_path, read_only: !name.starts_with("rw"), + sparse: false, }); } "host_ip" => { @@ -726,12 +769,18 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> { "Amount of guest memory in MiB. (default: 256)"), Argument::short_value('r', "root", - "PATH", - "Path to a root disk image. Like `--disk` but adds appropriate kernel command line option."), - Argument::value("rwroot", "PATH", "Path to a writable root disk image."), - Argument::short_value('d', "disk", "PATH", "Path to a disk image."), + "PATH[,key=value[,key=value[,...]]", + "Path to a root disk image followed by optional comma-separated options. + Like `--disk` but adds appropriate kernel command line option. + See --disk for valid options."), + Argument::value("rwroot", "PATH[,key=value[,key=value[,...]]", "Path to a writable root disk image followed by optional comma-separated options. + See --disk for valid options."), + Argument::short_value('d', "disk", "PATH[,key=value[,key=value[,...]]", "Path to a disk image followed by optional comma-separated options. + Valid keys: + sparse=BOOL - Indicates whether the disk should support the discard operation (default: true)"), Argument::value("qcow", "PATH", "Path to a qcow2 disk image. (Deprecated; use --disk instead.)"), - Argument::value("rwdisk", "PATH", "Path to a writable disk image."), + Argument::value("rwdisk", "PATH[,key=value[,key=value[,...]]", "Path to a writable disk image followed by optional comma-separated options. + See --disk for valid options."), Argument::value("rwqcow", "PATH", "Path to a writable qcow2 disk image. (Deprecated; use --rwdisk instead.)"), Argument::value("rw-pmem-device", "PATH", "Path to a writable disk image."), Argument::value("pmem-device", "PATH", "Path to a disk image."), |