Patchwork [2/2] qemu-config: convert all QemuOpts to use dashes instead of underscores

login
register
mail settings
Submitter Anthony Liguori
Date July 25, 2012, 4:25 p.m.
Message ID <1343233543-18561-3-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/173210/
State New
Headers show

Comments

Anthony Liguori - July 25, 2012, 4:25 p.m.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qemu-config.c |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)
Luiz Capitulino - July 25, 2012, 5:32 p.m.
On Wed, 25 Jul 2012 11:25:43 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

What about the man page? Should it be updated or are we going to let it
list options with "_"?

> ---
>  qemu-config.c |   38 +++++++++++++++++++-------------------
>  1 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..1616547 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -91,11 +91,11 @@ static QemuOptsList qemu_drive_opts = {
>              .type = QEMU_OPT_NUMBER,
>              .help = "limit total I/O operations per second",
>          },{
> -            .name = "iops_rd",
> +            .name = "iops-rd",
>              .type = QEMU_OPT_NUMBER,
>              .help = "limit read operations per second",
>          },{
> -            .name = "iops_wr",
> +            .name = "iops-wr",
>              .type = QEMU_OPT_NUMBER,
>              .help = "limit write operations per second",
>          },{
> @@ -103,11 +103,11 @@ static QemuOptsList qemu_drive_opts = {
>              .type = QEMU_OPT_NUMBER,
>              .help = "limit total bytes per second",
>          },{
> -            .name = "bps_rd",
> +            .name = "bps-rd",
>              .type = QEMU_OPT_NUMBER,
>              .help = "limit read bytes per second",
>          },{
> -            .name = "bps_wr",
> +            .name = "bps-wr",
>              .type = QEMU_OPT_NUMBER,
>              .help = "limit write bytes per second",
>          },{
> @@ -230,7 +230,7 @@ QemuOptsList qemu_fsdev_opts = {
>              .name = "path",
>              .type = QEMU_OPT_STRING,
>          }, {
> -            .name = "security_model",
> +            .name = "security-model",
>              .type = QEMU_OPT_STRING,
>          }, {
>              .name = "writeout",
> @@ -243,7 +243,7 @@ QemuOptsList qemu_fsdev_opts = {
>              .name = "socket",
>              .type = QEMU_OPT_STRING,
>          }, {
> -            .name = "sock_fd",
> +            .name = "sock-fd",
>              .type = QEMU_OPT_NUMBER,
>          },
>  
> @@ -263,10 +263,10 @@ QemuOptsList qemu_virtfs_opts = {
>              .name = "path",
>              .type = QEMU_OPT_STRING,
>          }, {
> -            .name = "mount_tag",
> +            .name = "mount-tag",
>              .type = QEMU_OPT_STRING,
>          }, {
> -            .name = "security_model",
> +            .name = "security-model",
>              .type = QEMU_OPT_STRING,
>          }, {
>              .name = "writeout",
> @@ -278,7 +278,7 @@ QemuOptsList qemu_virtfs_opts = {
>              .name = "socket",
>              .type = QEMU_OPT_STRING,
>          }, {
> -            .name = "sock_fd",
> +            .name = "sock-fd",
>              .type = QEMU_OPT_NUMBER,
>          },
>  
> @@ -423,25 +423,25 @@ static QemuOptsList qemu_cpudef_opts = {
>              .name = "stepping",
>              .type = QEMU_OPT_NUMBER,
>          },{
> -            .name = "feature_edx",      /* cpuid 0000_0001.edx */
> +            .name = "feature-edx",
>              .type = QEMU_OPT_STRING,
>          },{
> -            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
> +            .name = "feature-ecx",
>              .type = QEMU_OPT_STRING,
>          },{
> -            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
> +            .name = "extfeature-edx",
>              .type = QEMU_OPT_STRING,
>          },{
> -            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
> +            .name = "extfeature-ecx",
>              .type = QEMU_OPT_STRING,
>          },{
>              .name = "xlevel",
>              .type = QEMU_OPT_NUMBER,
>          },{
> -            .name = "model_id",
> +            .name = "model-id",
>              .type = QEMU_OPT_STRING,
>          },{
> -            .name = "vendor_override",
> +            .name = "vendor-override",
>              .type = QEMU_OPT_NUMBER,
>          },
>          { /* end of list */ }
> @@ -560,11 +560,11 @@ static QemuOptsList qemu_machine_opts = {
>              .type = QEMU_OPT_STRING,
>              .help = "accelerator list",
>          }, {
> -            .name = "kernel_irqchip",
> +            .name = "kernel-irqchip",
>              .type = QEMU_OPT_BOOL,
>              .help = "use KVM in-kernel irqchip",
>          }, {
> -            .name = "kvm_shadow_mem",
> +            .name = "kvm_shadow-mem",
>              .type = QEMU_OPT_SIZE,
>              .help = "KVM shadow MMU size",
>          }, {
> @@ -588,11 +588,11 @@ static QemuOptsList qemu_machine_opts = {
>              .type = QEMU_OPT_STRING,
>              .help = "Dump current dtb to a file and quit",
>          }, {
> -            .name = "phandle_start",
> +            .name = "phandle-start",
>              .type = QEMU_OPT_STRING,
>              .help = "The first phandle ID we may generate dynamically",
>          }, {
> -            .name = "dt_compatible",
> +            .name = "dt-compatible",
>              .type = QEMU_OPT_STRING,
>              .help = "Overrides the \"compatible\" property of the dt root node",
>          },
Anthony Liguori - July 25, 2012, 6:20 p.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Wed, 25 Jul 2012 11:25:43 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> What about the man page? Should it be updated or are we going to let it
> list options with "_"?

The man page is generated from qemu-options.hx which also generates the
help output.

Unfortunately, we're still stuck in the "everyone yells at Anthony when
he tries to change the help output because of libvirt" state.

So we need to keep the option names in --help with '_'s.

Regards,

Anthony Liguori

>
>> ---
>>  qemu-config.c |   38 +++++++++++++++++++-------------------
>>  1 files changed, 19 insertions(+), 19 deletions(-)
>> 
>> diff --git a/qemu-config.c b/qemu-config.c
>> index 5c3296b..1616547 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -91,11 +91,11 @@ static QemuOptsList qemu_drive_opts = {
>>              .type = QEMU_OPT_NUMBER,
>>              .help = "limit total I/O operations per second",
>>          },{
>> -            .name = "iops_rd",
>> +            .name = "iops-rd",
>>              .type = QEMU_OPT_NUMBER,
>>              .help = "limit read operations per second",
>>          },{
>> -            .name = "iops_wr",
>> +            .name = "iops-wr",
>>              .type = QEMU_OPT_NUMBER,
>>              .help = "limit write operations per second",
>>          },{
>> @@ -103,11 +103,11 @@ static QemuOptsList qemu_drive_opts = {
>>              .type = QEMU_OPT_NUMBER,
>>              .help = "limit total bytes per second",
>>          },{
>> -            .name = "bps_rd",
>> +            .name = "bps-rd",
>>              .type = QEMU_OPT_NUMBER,
>>              .help = "limit read bytes per second",
>>          },{
>> -            .name = "bps_wr",
>> +            .name = "bps-wr",
>>              .type = QEMU_OPT_NUMBER,
>>              .help = "limit write bytes per second",
>>          },{
>> @@ -230,7 +230,7 @@ QemuOptsList qemu_fsdev_opts = {
>>              .name = "path",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>> -            .name = "security_model",
>> +            .name = "security-model",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>>              .name = "writeout",
>> @@ -243,7 +243,7 @@ QemuOptsList qemu_fsdev_opts = {
>>              .name = "socket",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>> -            .name = "sock_fd",
>> +            .name = "sock-fd",
>>              .type = QEMU_OPT_NUMBER,
>>          },
>>  
>> @@ -263,10 +263,10 @@ QemuOptsList qemu_virtfs_opts = {
>>              .name = "path",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>> -            .name = "mount_tag",
>> +            .name = "mount-tag",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>> -            .name = "security_model",
>> +            .name = "security-model",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>>              .name = "writeout",
>> @@ -278,7 +278,7 @@ QemuOptsList qemu_virtfs_opts = {
>>              .name = "socket",
>>              .type = QEMU_OPT_STRING,
>>          }, {
>> -            .name = "sock_fd",
>> +            .name = "sock-fd",
>>              .type = QEMU_OPT_NUMBER,
>>          },
>>  
>> @@ -423,25 +423,25 @@ static QemuOptsList qemu_cpudef_opts = {
>>              .name = "stepping",
>>              .type = QEMU_OPT_NUMBER,
>>          },{
>> -            .name = "feature_edx",      /* cpuid 0000_0001.edx */
>> +            .name = "feature-edx",
>>              .type = QEMU_OPT_STRING,
>>          },{
>> -            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
>> +            .name = "feature-ecx",
>>              .type = QEMU_OPT_STRING,
>>          },{
>> -            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
>> +            .name = "extfeature-edx",
>>              .type = QEMU_OPT_STRING,
>>          },{
>> -            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
>> +            .name = "extfeature-ecx",
>>              .type = QEMU_OPT_STRING,
>>          },{
>>              .name = "xlevel",
>>              .type = QEMU_OPT_NUMBER,
>>          },{
>> -            .name = "model_id",
>> +            .name = "model-id",
>>              .type = QEMU_OPT_STRING,
>>          },{
>> -            .name = "vendor_override",
>> +            .name = "vendor-override",
>>              .type = QEMU_OPT_NUMBER,
>>          },
>>          { /* end of list */ }
>> @@ -560,11 +560,11 @@ static QemuOptsList qemu_machine_opts = {
>>              .type = QEMU_OPT_STRING,
>>              .help = "accelerator list",
>>          }, {
>> -            .name = "kernel_irqchip",
>> +            .name = "kernel-irqchip",
>>              .type = QEMU_OPT_BOOL,
>>              .help = "use KVM in-kernel irqchip",
>>          }, {
>> -            .name = "kvm_shadow_mem",
>> +            .name = "kvm_shadow-mem",
>>              .type = QEMU_OPT_SIZE,
>>              .help = "KVM shadow MMU size",
>>          }, {
>> @@ -588,11 +588,11 @@ static QemuOptsList qemu_machine_opts = {
>>              .type = QEMU_OPT_STRING,
>>              .help = "Dump current dtb to a file and quit",
>>          }, {
>> -            .name = "phandle_start",
>> +            .name = "phandle-start",
>>              .type = QEMU_OPT_STRING,
>>              .help = "The first phandle ID we may generate dynamically",
>>          }, {
>> -            .name = "dt_compatible",
>> +            .name = "dt-compatible",
>>              .type = QEMU_OPT_STRING,
>>              .help = "Overrides the \"compatible\" property of the dt root node",
>>          },
Peter Maydell - July 25, 2012, 6:58 p.m.
On 25 July 2012 19:20, Anthony Liguori <aliguori@us.ibm.com> wrote:
> The man page is generated from qemu-options.hx which also generates the
> help output.
>
> Unfortunately, we're still stuck in the "everyone yells at Anthony when
> he tries to change the help output because of libvirt" state.

I think we should simply say "no, parsing -help is broken and wrong and it
was obviously broken and wrong and we are in fact going to change the
help output for QEMU 1.2, and you will need a new libvirt that can
cope with that". We can't be held hostage forever to really bad decisions
like that.

-- PMM
Luiz Capitulino - July 25, 2012, 7:02 p.m.
On Wed, 25 Jul 2012 19:58:02 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 25 July 2012 19:20, Anthony Liguori <aliguori@us.ibm.com> wrote:
> > The man page is generated from qemu-options.hx which also generates the
> > help output.
> >
> > Unfortunately, we're still stuck in the "everyone yells at Anthony when
> > he tries to change the help output because of libvirt" state.
> 
> I think we should simply say "no, parsing -help is broken and wrong and it
> was obviously broken and wrong and we are in fact going to change the
> help output for QEMU 1.2, and you will need a new libvirt that can
> cope with that". We can't be held hostage forever to really bad decisions
> like that.

We have to provide an alternative before doing that.
Peter Maydell - July 25, 2012, 7:02 p.m.
On 25 July 2012 20:02, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Wed, 25 Jul 2012 19:58:02 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> I think we should simply say "no, parsing -help is broken and wrong and it
>> was obviously broken and wrong and we are in fact going to change the
>> help output for QEMU 1.2, and you will need a new libvirt that can
>> cope with that". We can't be held hostage forever to really bad decisions
>> like that.
>
> We have to provide an alternative before doing that.

Try whatever it is you wanted to try, see if it barfs. Or don't use it.

-- PMM
Luiz Capitulino - July 25, 2012, 7:09 p.m.
On Wed, 25 Jul 2012 20:02:53 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 25 July 2012 20:02, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Wed, 25 Jul 2012 19:58:02 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >> I think we should simply say "no, parsing -help is broken and wrong and it
> >> was obviously broken and wrong and we are in fact going to change the
> >> help output for QEMU 1.2, and you will need a new libvirt that can
> >> cope with that". We can't be held hostage forever to really bad decisions
> >> like that.
> >
> > We have to provide an alternative before doing that.
> 
> Try whatever it is you wanted to try, see if it barfs. Or don't use it.

Libvirt folks can answer if this is feasible (CC'ing them), I'd guess it's not.
Eric Blake - July 25, 2012, 9:21 p.m.
On 07/25/2012 01:09 PM, Luiz Capitulino wrote:
> On Wed, 25 Jul 2012 20:02:53 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
>> On 25 July 2012 20:02, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>> On Wed, 25 Jul 2012 19:58:02 +0100
>>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> I think we should simply say "no, parsing -help is broken and wrong and it
>>>> was obviously broken and wrong and we are in fact going to change the
>>>> help output for QEMU 1.2, and you will need a new libvirt that can
>>>> cope with that". We can't be held hostage forever to really bad decisions
>>>> like that.
>>>
>>> We have to provide an alternative before doing that.
>>
>> Try whatever it is you wanted to try, see if it barfs. Or don't use it.
> 
> Libvirt folks can answer if this is feasible (CC'ing them), I'd guess it's not.

I'm all for breaking -help output, provided we have something more
reliable to use in its place.  The way I see it, we have these scenarios
to think about:

old libvirt, old qemu => works
new libvirt, new qemu => works
new libvirt, old qemu => works (and if it doesn't, it's libvirt's fault,
so this is irrelevant to qemu)
old libvirt, new qemu => this is what _might_ break if -help output
changes; but if you can afford to upgrade qemu, you should also be able
to upgrade your libvirt.  A historical example of this was when qemu
upgraded to 1.0, but older libvirt was still expecting to parse a x.y.z
version string, so the reality was that no one upgraded to qemu 1.0
unless they also upgraded libvirt.

We've already known for some time that parsing -help output is fragile;
the best we can do is make sure that new libvirt can handle all
historical forms of output, but I think it is reasonable to tell users
that as soon as a new form of output is added to the mix (because qemu
was upgraded), then you also have to upgrade libvirt to handle that new
format.  Older libvirt's inability to predict the future of what newer
qemu output will be should not penalize innovation in newer qemu.
Markus Armbruster - July 27, 2012, 7:45 a.m.
Anthony Liguori <aliguori@us.ibm.com> writes:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> On Wed, 25 Jul 2012 11:25:43 -0500
>> Anthony Liguori <aliguori@us.ibm.com> wrote:
>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>> What about the man page? Should it be updated or are we going to let it
>> list options with "_"?
>
> The man page is generated from qemu-options.hx which also generates the
> help output.
>
> Unfortunately, we're still stuck in the "everyone yells at Anthony when
> he tries to change the help output because of libvirt" state.
>
> So we need to keep the option names in --help with '_'s.

Put a TODO there to remind us to fix it up after the help message
gridlock clears?
Andreas Färber - July 27, 2012, 11:10 a.m.
Am 25.07.2012 18:25, schrieb Anthony Liguori:
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qemu-config.c |   38 +++++++++++++++++++-------------------
>  1 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..1616547 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
[...]
> @@ -560,11 +560,11 @@ static QemuOptsList qemu_machine_opts = {
>              .type = QEMU_OPT_STRING,
>              .help = "accelerator list",
>          }, {
> -            .name = "kernel_irqchip",
> +            .name = "kernel-irqchip",
>              .type = QEMU_OPT_BOOL,
>              .help = "use KVM in-kernel irqchip",
>          }, {
> -            .name = "kvm_shadow_mem",
> +            .name = "kvm_shadow-mem",

Was the remaining underscore an oversight or intentional?

>              .type = QEMU_OPT_SIZE,
>              .help = "KVM shadow MMU size",
>          }, {

Andreas

Patch

diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..1616547 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -91,11 +91,11 @@  static QemuOptsList qemu_drive_opts = {
             .type = QEMU_OPT_NUMBER,
             .help = "limit total I/O operations per second",
         },{
-            .name = "iops_rd",
+            .name = "iops-rd",
             .type = QEMU_OPT_NUMBER,
             .help = "limit read operations per second",
         },{
-            .name = "iops_wr",
+            .name = "iops-wr",
             .type = QEMU_OPT_NUMBER,
             .help = "limit write operations per second",
         },{
@@ -103,11 +103,11 @@  static QemuOptsList qemu_drive_opts = {
             .type = QEMU_OPT_NUMBER,
             .help = "limit total bytes per second",
         },{
-            .name = "bps_rd",
+            .name = "bps-rd",
             .type = QEMU_OPT_NUMBER,
             .help = "limit read bytes per second",
         },{
-            .name = "bps_wr",
+            .name = "bps-wr",
             .type = QEMU_OPT_NUMBER,
             .help = "limit write bytes per second",
         },{
@@ -230,7 +230,7 @@  QemuOptsList qemu_fsdev_opts = {
             .name = "path",
             .type = QEMU_OPT_STRING,
         }, {
-            .name = "security_model",
+            .name = "security-model",
             .type = QEMU_OPT_STRING,
         }, {
             .name = "writeout",
@@ -243,7 +243,7 @@  QemuOptsList qemu_fsdev_opts = {
             .name = "socket",
             .type = QEMU_OPT_STRING,
         }, {
-            .name = "sock_fd",
+            .name = "sock-fd",
             .type = QEMU_OPT_NUMBER,
         },
 
@@ -263,10 +263,10 @@  QemuOptsList qemu_virtfs_opts = {
             .name = "path",
             .type = QEMU_OPT_STRING,
         }, {
-            .name = "mount_tag",
+            .name = "mount-tag",
             .type = QEMU_OPT_STRING,
         }, {
-            .name = "security_model",
+            .name = "security-model",
             .type = QEMU_OPT_STRING,
         }, {
             .name = "writeout",
@@ -278,7 +278,7 @@  QemuOptsList qemu_virtfs_opts = {
             .name = "socket",
             .type = QEMU_OPT_STRING,
         }, {
-            .name = "sock_fd",
+            .name = "sock-fd",
             .type = QEMU_OPT_NUMBER,
         },
 
@@ -423,25 +423,25 @@  static QemuOptsList qemu_cpudef_opts = {
             .name = "stepping",
             .type = QEMU_OPT_NUMBER,
         },{
-            .name = "feature_edx",      /* cpuid 0000_0001.edx */
+            .name = "feature-edx",
             .type = QEMU_OPT_STRING,
         },{
-            .name = "feature_ecx",      /* cpuid 0000_0001.ecx */
+            .name = "feature-ecx",
             .type = QEMU_OPT_STRING,
         },{
-            .name = "extfeature_edx",   /* cpuid 8000_0001.edx */
+            .name = "extfeature-edx",
             .type = QEMU_OPT_STRING,
         },{
-            .name = "extfeature_ecx",   /* cpuid 8000_0001.ecx */
+            .name = "extfeature-ecx",
             .type = QEMU_OPT_STRING,
         },{
             .name = "xlevel",
             .type = QEMU_OPT_NUMBER,
         },{
-            .name = "model_id",
+            .name = "model-id",
             .type = QEMU_OPT_STRING,
         },{
-            .name = "vendor_override",
+            .name = "vendor-override",
             .type = QEMU_OPT_NUMBER,
         },
         { /* end of list */ }
@@ -560,11 +560,11 @@  static QemuOptsList qemu_machine_opts = {
             .type = QEMU_OPT_STRING,
             .help = "accelerator list",
         }, {
-            .name = "kernel_irqchip",
+            .name = "kernel-irqchip",
             .type = QEMU_OPT_BOOL,
             .help = "use KVM in-kernel irqchip",
         }, {
-            .name = "kvm_shadow_mem",
+            .name = "kvm_shadow-mem",
             .type = QEMU_OPT_SIZE,
             .help = "KVM shadow MMU size",
         }, {
@@ -588,11 +588,11 @@  static QemuOptsList qemu_machine_opts = {
             .type = QEMU_OPT_STRING,
             .help = "Dump current dtb to a file and quit",
         }, {
-            .name = "phandle_start",
+            .name = "phandle-start",
             .type = QEMU_OPT_STRING,
             .help = "The first phandle ID we may generate dynamically",
         }, {
-            .name = "dt_compatible",
+            .name = "dt-compatible",
             .type = QEMU_OPT_STRING,
             .help = "Overrides the \"compatible\" property of the dt root node",
         },