Patchwork [v3,2/6] block: Take advantage of QemuOpt default integers

login
register
mail settings
Submitter Eric Blake
Date Jan. 10, 2019, 7:18 p.m.
Message ID <20190110191901.5082-3-eblake@redhat.com>
Download mbox | patch
Permalink /patch/697227/
State New
Headers show

Comments

Eric Blake - Jan. 10, 2019, 7:18 p.m.
Instead of defining an integer to a default string value (where we
have to be careful how we spelled the integer because of the use of
stringify), populate a default integer value instead.

Drop a useless stringify(0); a missing default is just as easy to
interpret as 0 as an explicit string 0.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/parallels.c | 2 +-
 block/qcow2.c     | 2 +-
 block/qed.c       | 2 +-
 block/vdi.c       | 2 +-
 block/vhdx.c      | 3 +--
 5 files changed, 5 insertions(+), 6 deletions(-)
Kevin Wolf - Jan. 11, 2019, 10:38 a.m.
Am 10.01.2019 um 20:18 hat Eric Blake geschrieben:
> Instead of defining an integer to a default string value (where we
> have to be careful how we spelled the integer because of the use of
> stringify), populate a default integer value instead.
> 
> Drop a useless stringify(0); a missing default is just as easy to
> interpret as 0 as an explicit string 0.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

> diff --git a/block/vhdx.c b/block/vhdx.c
> index b785aef4b7b..54bf6805fc6 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -2085,13 +2085,12 @@ static QemuOptsList vhdx_create_opts = {
>         {
>             .name = VHDX_BLOCK_OPT_LOG_SIZE,
>             .type = QEMU_OPT_SIZE,
> -           .def_value_str = stringify(DEFAULT_LOG_SIZE),
> +           .def_value_int = DEFAULT_LOG_SIZE,
>             .help = "Log size; min 1MB."
>         },
>         {
>             .name = VHDX_BLOCK_OPT_BLOCK_SIZE,
>             .type = QEMU_OPT_SIZE,
> -           .def_value_str = stringify(0),
>             .help = "Block Size; min 1MB, max 256MB. " \
>                     "0 means auto-calculate based on image size."
>         },

Before the patch:
$ ./qemu-img create -f vhdx /tmp/test.vhdx 64M
Formatting '/tmp/test.vhdx', fmt=vhdx size=67108864 log_size=1048576 block_size=0

After the patch:
$ ./qemu-img create -f vhdx /tmp/test.vhdx 64M
Formatting '/tmp/test.vhdx', fmt=vhdx size=67108864 log_size=1048576

Intentional?

Kevin
Eric Blake - Jan. 11, 2019, 4:28 p.m.
On 1/11/19 4:38 AM, Kevin Wolf wrote:
> Am 10.01.2019 um 20:18 hat Eric Blake geschrieben:
>> Instead of defining an integer to a default string value (where we
>> have to be careful how we spelled the integer because of the use of
>> stringify), populate a default integer value instead.
>>
>> Drop a useless stringify(0); a missing default is just as easy to
>> interpret as 0 as an explicit string 0.
>>

>>         {
>>             .name = VHDX_BLOCK_OPT_BLOCK_SIZE,
>>             .type = QEMU_OPT_SIZE,
>> -           .def_value_str = stringify(0),
>>             .help = "Block Size; min 1MB, max 256MB. " \
>>                     "0 means auto-calculate based on image size."
>>         },
> 
> Before the patch:
> $ ./qemu-img create -f vhdx /tmp/test.vhdx 64M
> Formatting '/tmp/test.vhdx', fmt=vhdx size=67108864 log_size=1048576 block_size=0
> 
> After the patch:
> $ ./qemu-img create -f vhdx /tmp/test.vhdx 64M
> Formatting '/tmp/test.vhdx', fmt=vhdx size=67108864 log_size=1048576
> 
> Intentional?

Yes. Well, sort of.  A default of 0 is automatic, so right now, our help
output omits all variables with an implicit default of 0.  We have no
way to tell the difference between an explicit integer default of 0 and
an implicit one (we did with strings, but the point of the series is to
store default integers as integers rather than as strings), so we could
work around that by adding even more to QemuOpts to record a bool of
whether an int of 0 should be listed as an explicit zero in our help
output.  Or we could state that listing a default of zero doesn't help
anyone.  Or we could list ALL variable defaults (even the ones that
default to implicit 0) rather than the current code that special-cases
output to omit variables left at their implicit default, but that may
change output in the other direction (more output where we are now
silent - although that may not be a bad thing).

Patch

diff --git a/block/parallels.c b/block/parallels.c
index cc9445879d7..a7593310332 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -96,7 +96,7 @@  static QemuOptsList parallels_create_opts = {
             .name = BLOCK_OPT_CLUSTER_SIZE,
             .type = QEMU_OPT_SIZE,
             .help = "Parallels image cluster size",
-            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE),
+            .def_value_int = DEFAULT_CLUSTER_SIZE,
         },
         { /* end of list */ }
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 4897abae5e8..c9b76a9ea93 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4892,7 +4892,7 @@  static QemuOptsList qcow2_create_opts = {
             .name = BLOCK_OPT_CLUSTER_SIZE,
             .type = QEMU_OPT_SIZE,
             .help = "qcow2 cluster size",
-            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
+            .def_value_int = DEFAULT_CLUSTER_SIZE,
         },
         {
             .name = BLOCK_OPT_PREALLOC,
diff --git a/block/qed.c b/block/qed.c
index 9377c0b7ad8..9db36c39066 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1651,7 +1651,7 @@  static QemuOptsList qed_create_opts = {
             .name = BLOCK_OPT_CLUSTER_SIZE,
             .type = QEMU_OPT_SIZE,
             .help = "Cluster size (in bytes)",
-            .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE)
+            .def_value_int = QED_DEFAULT_CLUSTER_SIZE,
         },
         {
             .name = BLOCK_OPT_TABLE_SIZE,
diff --git a/block/vdi.c b/block/vdi.c
index 2380daa583e..65659c9b179 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -985,7 +985,7 @@  static QemuOptsList vdi_create_opts = {
             .name = BLOCK_OPT_CLUSTER_SIZE,
             .type = QEMU_OPT_SIZE,
             .help = "VDI cluster (block) size",
-            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
+            .def_value_int = DEFAULT_CLUSTER_SIZE,
         },
 #endif
 #if defined(CONFIG_VDI_STATIC_IMAGE)
diff --git a/block/vhdx.c b/block/vhdx.c
index b785aef4b7b..54bf6805fc6 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -2085,13 +2085,12 @@  static QemuOptsList vhdx_create_opts = {
        {
            .name = VHDX_BLOCK_OPT_LOG_SIZE,
            .type = QEMU_OPT_SIZE,
-           .def_value_str = stringify(DEFAULT_LOG_SIZE),
+           .def_value_int = DEFAULT_LOG_SIZE,
            .help = "Log size; min 1MB."
        },
        {
            .name = VHDX_BLOCK_OPT_BLOCK_SIZE,
            .type = QEMU_OPT_SIZE,
-           .def_value_str = stringify(0),
            .help = "Block Size; min 1MB, max 256MB. " \
                    "0 means auto-calculate based on image size."
        },