[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 4/5] block: make size-related BlockConf properties accept size suffixes



On 5/27/20 7:45 AM, Roman Kagan wrote:
Several BlockConf properties represent respective sizes in bytes so it
makes sense to accept size suffixes for them.

Turn them all into uint32_t and use size-suffix-capable setters/getters
on them; introduce DEFINE_PROP_SIZE32 and adjust DEFINE_PROP_BLOCKSIZE
for that. (Making them uint64_t and reusing DEFINE_PROP_SIZE isn't
justified because guests expect at most 32bit values.)

Also, since min_io_size is exposed to the guest by scsi and virtio-blk
devices as an uint16_t in units of logical blocks, introduce an
additional check in blkconf_blocksizes to prevent its silent truncation.

Signed-off-by: Roman Kagan <rvkagan@xxxxxxxxxxxxxx>
---
v5 -> v6:
- add prop_size32 instead of going with 64bit

Would it be worth adding prop_size32 as its own patch, before using it here? But I'll review this as-is.

+++ b/hw/block/block.c
@@ -96,6 +96,17 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp)
          return false;
      }
+ /*
+     * all devices which support min_io_size (scsi and virtio-blk) expose it to
+     * the guest as a uint16_t in units of logical blocks
+     */
+    if (conf->min_io_size > conf->logical_block_size * UINT16_MAX) {

This risks overflow.  Better would be:

if (conf->min_io_size / conf->logical_block-size > UINT16_MAX)

+        error_setg(errp,
+                   "min_io_size must not exceed " stringify(UINT16_MAX)
+                   " logical blocks");
+        return false;
+    }
+
      if (!QEMU_IS_ALIGNED(conf->opt_io_size, conf->logical_block_size)) {
          error_setg(errp,
                     "opt_io_size must be a multiple of logical_block_size");

+++ b/tests/qemu-iotests/172.out
@@ -24,11 +24,11 @@ Testing:
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
-                logical_block_size = 512 (0x200)
-                physical_block_size = 512 (0x200)
-                min_io_size = 0 (0x0)
-                opt_io_size = 0 (0x0)
-                discard_granularity = 4294967295 (0xffffffff)
+                logical_block_size = 512 (512 B)
+                physical_block_size = 512 (512 B)
+                min_io_size = 0 (0 B)
+                opt_io_size = 0 (0 B)
+                discard_granularity = 4294967295 (4 GiB)

Although 4 GiB is not quite the same as 4294967295, the exact byte value next to the approximate size is not too bad. The mechanical fallout from the change from int to size is fine to me.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.