| Message ID | 1343233543-18561-3-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
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", > },
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", >> },
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
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.
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
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.
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.
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?
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
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", },
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- qemu-config.c | 38 +++++++++++++++++++------------------- 1 files changed, 19 insertions(+), 19 deletions(-)