[PATCH 2/7] block: Improve errors about block sizes

Markus Armbruster posted 7 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 2/7] block: Improve errors about block sizes
Posted by Markus Armbruster 1 month, 2 weeks ago
Block sizes need to be a power of two between 512 and an arbitrary
limit, currently 2MiB.

Commit 5937835ac4c factored block size checking out of set_blocksize()
into new check_block_size(), for reuse in block/export/.

Its two error messages are okay for the original purpose:

    $ qemu-system-x86_64 -device ide-hd,physical_block_size=1
    qemu-system-x86_64: -device ide-hd,physical_block_size=1: Property .physical_block_size doesn't take value 1 (minimum: 512, maximum: 2097152)
    $ qemu-system-x86_64 -device ide-hd,physical_block_size=513
    qemu-system-x86_64: -device ide-hd,physical_block_size=513: Property .physical_block_size doesn't take value '513', it's not a power of 2

They're mildly off for block exports:

    $ qemu-storage-daemon --blockdev node-name=nod0,driver=file,filename=foo.img --export type=vduse-blk,id=exp0,node-name=nod0,name=foo,logical-block-size=1
    qemu-storage-daemon: --export type=vduse-blk,id=exp0,node-name=nod0,name=foo,logical-block-size=1: Property exp0.logical-block-size doesn't take value 1 (minimum: 512, maximum: 2097152)

The error message talks about a property.  CLI options like --export
don't have properties, they have parameters.

Replace the two error messages by a single one that's okay for both
purposes.  Looks like this:

    qemu-storage-daemon: --export type=vduse-blk,id=exp0,node-name=nod0,name=foo,logical-block-size=1: parameter logical-block-size must be a power of 2 between 512 and 2097152

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/block-helpers.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/util/block-helpers.c b/util/block-helpers.c
index c4851432f5..fb5de348e2 100644
--- a/util/block-helpers.c
+++ b/util/block-helpers.c
@@ -10,7 +10,6 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "qapi/qmp/qerror.h"
 #include "block-helpers.h"
 
 /**
@@ -28,19 +27,16 @@
 void check_block_size(const char *id, const char *name, int64_t value,
                       Error **errp)
 {
-    /* value of 0 means "unset" */
-    if (value && (value < MIN_BLOCK_SIZE || value > MAX_BLOCK_SIZE)) {
-        error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-                   id, name, value, MIN_BLOCK_SIZE, MAX_BLOCK_SIZE);
+    if (!value) {
+        /* unset */
         return;
     }
 
-    /* We rely on power-of-2 blocksizes for bitmasks */
-    if ((value & (value - 1)) != 0) {
+    if (value < MIN_BLOCK_SIZE || value > MAX_BLOCK_SIZE
+        || (value & (value - 1))) {
         error_setg(errp,
-                   "Property %s.%s doesn't take value '%" PRId64
-                   "', it's not a power of 2",
-                   id, name, value);
-        return;
+                   "parameter %s must be a power of 2 between %" PRId64
+                   " and %" PRId64,
+                   name, MIN_BLOCK_SIZE, MAX_BLOCK_SIZE);
     }
 }
-- 
2.46.0
Re: [PATCH 2/7] block: Improve errors about block sizes
Posted by Daniel P. Berrangé 1 month, 2 weeks ago
On Thu, Oct 10, 2024 at 04:56:25PM +0200, Markus Armbruster wrote:
> Block sizes need to be a power of two between 512 and an arbitrary
> limit, currently 2MiB.
> 
> Commit 5937835ac4c factored block size checking out of set_blocksize()
> into new check_block_size(), for reuse in block/export/.
> 
> Its two error messages are okay for the original purpose:
> 
>     $ qemu-system-x86_64 -device ide-hd,physical_block_size=1
>     qemu-system-x86_64: -device ide-hd,physical_block_size=1: Property .physical_block_size doesn't take value 1 (minimum: 512, maximum: 2097152)
>     $ qemu-system-x86_64 -device ide-hd,physical_block_size=513
>     qemu-system-x86_64: -device ide-hd,physical_block_size=513: Property .physical_block_size doesn't take value '513', it's not a power of 2
> 
> They're mildly off for block exports:
> 
>     $ qemu-storage-daemon --blockdev node-name=nod0,driver=file,filename=foo.img --export type=vduse-blk,id=exp0,node-name=nod0,name=foo,logical-block-size=1
>     qemu-storage-daemon: --export type=vduse-blk,id=exp0,node-name=nod0,name=foo,logical-block-size=1: Property exp0.logical-block-size doesn't take value 1 (minimum: 512, maximum: 2097152)
> 
> The error message talks about a property.  CLI options like --export
> don't have properties, they have parameters.
> 
> Replace the two error messages by a single one that's okay for both
> purposes.  Looks like this:
> 
>     qemu-storage-daemon: --export type=vduse-blk,id=exp0,node-name=nod0,name=foo,logical-block-size=1: parameter logical-block-size must be a power of 2 between 512 and 2097152
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  util/block-helpers.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|