From 2767223fdbcf189ccebbffbec8b3a0f254d9d40e Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 6 Dec 2019 17:26:55 +1100 Subject: [PATCH] devices: block: add block_size option for disks This allows overriding the default logical block size (512 bytes) with other values, such as 4096 for 4K block size disks. BUG=chromium:942700 TEST=crosvm run -r vm_rootfs,block_size=4096 vm_kernel TEST=verify block size with lsblk --output-all Change-Id: Ia6db05f369a76557a2afb8b48b5cc2b66cf84b01 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1954220 Reviewed-by: Zach Reizner Tested-by: kokoro Commit-Queue: Daniel Verkamp --- devices/src/virtio/block.rs | 45 ++++++++++++++++++++++++++++--------- src/crosvm.rs | 1 + src/linux.rs | 1 + src/main.rs | 13 ++++++++++- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/devices/src/virtio/block.rs b/devices/src/virtio/block.rs index 3dab8b9acc..dd37f09b26 100644 --- a/devices/src/virtio/block.rs +++ b/devices/src/virtio/block.rs @@ -463,15 +463,16 @@ pub struct Block { read_only: bool, sparse: bool, seg_max: u32, + block_size: u32, control_socket: Option, } -fn build_config_space(disk_size: u64, seg_max: u32) -> virtio_blk_config { +fn build_config_space(disk_size: u64, seg_max: u32, block_size: u32) -> virtio_blk_config { virtio_blk_config { // If the image is not a multiple of the sector size, the tail bits are not exposed. capacity: Le64::from(disk_size >> SECTOR_SHIFT), seg_max: Le32::from(seg_max), - blk_size: Le32::from(SECTOR_SIZE as u32), + blk_size: Le32::from(block_size), max_discard_sectors: Le32::from(MAX_DISCARD_SECTORS), discard_sector_alignment: Le32::from(DISCARD_SECTOR_ALIGNMENT), max_write_zeroes_sectors: Le32::from(MAX_WRITE_ZEROES_SECTORS), @@ -488,14 +489,22 @@ impl Block { disk_image: Box, read_only: bool, sparse: bool, + block_size: u32, control_socket: Option, ) -> SysResult { + if block_size % SECTOR_SIZE as u32 != 0 { + error!( + "Block size {} is not a multiple of {}.", + block_size, SECTOR_SIZE, + ); + return Err(SysError::new(libc::EINVAL)); + } let disk_size = disk_image.get_len()?; - if disk_size % SECTOR_SIZE != 0 { + if disk_size % block_size as u64 != 0 { warn!( - "Disk size {} is not a multiple of sector size {}; \ + "Disk size {} is not a multiple of block size {}; \ the remainder will not be visible to the guest.", - disk_size, SECTOR_SIZE + disk_size, block_size, ); } @@ -528,6 +537,7 @@ impl Block { read_only, sparse, seg_max, + block_size, control_socket, }) } @@ -716,7 +726,7 @@ impl VirtioDevice for Block { fn read_config(&self, offset: u64, data: &mut [u8]) { let config_space = { let disk_size = self.disk_size.lock(); - build_config_space(*disk_size, self.seg_max) + build_config_space(*disk_size, self.seg_max, self.block_size) }; copy_config(data, 0, config_space.as_slice(), offset); } @@ -795,7 +805,7 @@ mod tests { let f = File::create(&path).unwrap(); f.set_len(0x1000).unwrap(); - let b = Block::new(Box::new(f), true, false, None).unwrap(); + let b = Block::new(Box::new(f), true, false, 512, 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). @@ -806,6 +816,21 @@ mod tests { assert_eq!([0x00, 0x00, 0x00, 0x00], msw_sectors); } + #[test] + fn read_block_size() { + let tempdir = TempDir::new().unwrap(); + let mut path = tempdir.path().to_owned(); + path.push("disk_image"); + let f = File::create(&path).unwrap(); + f.set_len(0x1000).unwrap(); + + let b = Block::new(Box::new(f), true, false, 4096, None).unwrap(); + let mut blk_size = [0u8; 4]; + b.read_config(20, &mut blk_size); + // blk_size should be 4096 (0x1000). + assert_eq!([0x00, 0x10, 0x00, 0x00], blk_size); + } + #[test] fn read_features() { let tempdir = TempDir::new().unwrap(); @@ -815,7 +840,7 @@ mod tests { // read-write block device { let f = File::create(&path).unwrap(); - let b = Block::new(Box::new(f), false, true, None).unwrap(); + let b = Block::new(Box::new(f), false, true, 512, 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 @@ -825,7 +850,7 @@ mod tests { // read-write block device, non-sparse { let f = File::create(&path).unwrap(); - let b = Block::new(Box::new(f), false, false, None).unwrap(); + let b = Block::new(Box::new(f), false, false, 512, 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 @@ -835,7 +860,7 @@ mod tests { // read-only block device { let f = File::create(&path).unwrap(); - let b = Block::new(Box::new(f), true, true, None).unwrap(); + let b = Block::new(Box::new(f), true, true, 512, 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()); diff --git a/src/crosvm.rs b/src/crosvm.rs index de235739bd..0e0e218f46 100644 --- a/src/crosvm.rs +++ b/src/crosvm.rs @@ -39,6 +39,7 @@ pub struct DiskOption { pub path: PathBuf, pub read_only: bool, pub sparse: bool, + pub block_size: u32, } /// A bind mount for directories in the plugin process. diff --git a/src/linux.rs b/src/linux.rs index 006af42fa9..a9a3445755 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -386,6 +386,7 @@ fn create_block_device( disk_file, disk.read_only, disk.sparse, + disk.block_size, Some(disk_device_socket), ) .map_err(Error::BlockDeviceNew)?; diff --git a/src/main.rs b/src/main.rs index 8d9c8906be..9c97c4dbaf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -378,6 +378,7 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: path: disk_path, read_only, sparse: true, + block_size: 512, }; for opt in components { @@ -399,6 +400,14 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: })?; disk.sparse = sparse; } + "block_size" => { + let block_size = + value.parse().map_err(|_| argument::Error::InvalidValue { + value: value.to_owned(), + expected: "`block_size` must be an integer", + })?; + disk.block_size = block_size; + } _ => { return Err(argument::Error::InvalidValue { value: kind.to_owned(), @@ -423,6 +432,7 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument:: path: disk_path, read_only: !name.starts_with("rw"), sparse: false, + block_size: sys_util::pagesize() as u32, }); } "host_ip" => { @@ -893,7 +903,8 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> { 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)"), + sparse=BOOL - Indicates whether the disk should support the discard operation (default: true) + block_size=BYTES - Set the reported block size of the disk (default: 512)"), Argument::value("qcow", "PATH", "Path to a qcow2 disk image. (Deprecated; use --disk instead.)"), 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."),