Message ID | CAFEAcA-3AertMzzpF=jdC__7QAa2RKneS3OitqNDJwXetRbiPA@mail.gmail.com |
---|---|
State | Superseded |
Headers | show |
On 08/09/2012 03:02 PM, Peter Maydell wrote: > On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote: >> On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote: >>> Peter Maydell <peter.maydell@linaro.org> writes: >>>> For command line options which permit '?' meaning 'please list the >>>> permitted values', add support for 'help' as a synonym, by abstracting >>>> the check out into a helper function. > >>> Applied. Thanks. >> >> I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and >> "-cpu ?model": > > These options appear to be completely undocumented. They're also pretty > ugly syntax and seem to be x86 specific. Thankfully, libvirt is not using them. If there is a reason for libvirt to start using them, then libvirt would rather go through QMP than any new syntax when talking to newer qemu (that is, I'm okay if we completely lose the spelling as long as we don't lose the feature). > > Any suggestions for what the sane syntax for these options would be? > (ie the analogous change to having '?' go to 'help'). -cpu help=dump -cpu help=cpuid
On Thu, Aug 09, 2012 at 10:02:22PM +0100, Peter Maydell wrote: > On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote: > >> Peter Maydell <peter.maydell@linaro.org> writes: > >> > For command line options which permit '?' meaning 'please list the > >> > permitted values', add support for 'help' as a synonym, by abstracting > >> > the check out into a helper function. > > >> Applied. Thanks. > > > > I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and > > "-cpu ?model": > > These options appear to be completely undocumented. They're also pretty > ugly syntax and seem to be x86 specific. Agreed. I wasn't aware it was completely undocumented, I thought there was documentation somewhere. > However we can unbreak them > if we must with a patch like this: > > --- a/vl.c > +++ b/vl.c > @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp) > */ > cpudef_init(); > > - if (cpu_model && is_help_option(cpu_model)) { > + /* We have to check for "starts with '?' as well as is_help_option > + * to support targets which implement various weird help options > + * via '?thingy' syntax. > + */ > + if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) { > list_cpus(stdout, &fprintf, cpu_model); > exit(0); > } > > (will send as a proper patch with commit message and signoff tomorrow). > > Any suggestions for what the sane syntax for these options would be? > (ie the analogous change to having '?' go to 'help'). What about "-cpu help,dump" or "-cpu help=dump"?
Eduardo Habkost <ehabkost@redhat.com> writes: > On Thu, Aug 09, 2012 at 10:02:22PM +0100, Peter Maydell wrote: >> On 9 August 2012 20:25, Eduardo Habkost <ehabkost@redhat.com> wrote: >> > On Fri, Aug 03, 2012 at 03:42:39PM -0500, Anthony Liguori wrote: >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > For command line options which permit '?' meaning 'please list the >> >> > permitted values', add support for 'help' as a synonym, by abstracting >> >> > the check out into a helper function. >> >> >> Applied. Thanks. >> > >> > I just found out that this patch broke "-cpu ?dump", "-cpu ?cpuid", and >> > "-cpu ?model": >> >> These options appear to be completely undocumented. They're also pretty >> ugly syntax and seem to be x86 specific. > > Agreed. I wasn't aware it was completely undocumented, I thought there > was documentation somewhere. > > >> However we can unbreak them >> if we must with a patch like this: >> >> --- a/vl.c >> +++ b/vl.c >> @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp) >> */ >> cpudef_init(); >> >> - if (cpu_model && is_help_option(cpu_model)) { >> + /* We have to check for "starts with '?' as well as is_help_option >> + * to support targets which implement various weird help options >> + * via '?thingy' syntax. >> + */ >> + if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) { >> list_cpus(stdout, &fprintf, cpu_model); >> exit(0); >> } >> >> (will send as a proper patch with commit message and signoff tomorrow). >> >> Any suggestions for what the sane syntax for these options would be? >> (ie the analogous change to having '?' go to 'help'). > > What about "-cpu help,dump" or "-cpu help=dump"? Let's just drop the feature. I doubt a user would ever do this. For 1.3, I'd like to introduce glib option groups and allow for group specific help options. IOW, --help-cpu Regards, Anthony Liguori > > -- > Eduardo
--- a/vl.c +++ b/vl.c @@ -3215,7 +3215,11 @@ int main(int argc, char **argv, char **envp) */ cpudef_init(); - if (cpu_model && is_help_option(cpu_model)) { + /* We have to check for "starts with '?' as well as is_help_option + * to support targets which implement various weird help options + * via '?thingy' syntax. + */ + if (cpu_model && (is_help_option(cpu_model) || *cpu_model == '?')) { list_cpus(stdout, &fprintf, cpu_model); exit(0); }