Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Amos Kong]

On Thu, Mar 06, 2014 at 02:23:15PM -0700, Eric Blake wrote:
> On 03/05/2014 07:36 PM, Amos Kong wrote:
> > vm_config_groups[] only contains part of the options which have
> > argument, and all options which have no argument aren't added
> > to vm_config_groups[]. Current query-command-line-options only
> > checks options from vm_config_groups[], so some options will
> > be lost.
> > 
> > We have macro in qemu-options.hx to generate a table that
> > contains all the options. This patch tries to query options
> > from the table.
> > 
> > Then we won't lose the legacy options that weren't added to
> > vm_config_groups[] (eg: -vnc, -smbios). The options that have
> > no argument will also be returned (eg: -enable-fips)
> > 
> > Some options that have argument have a NULL desc list, some
> > options don't have argument, and "parameters" is mandatory
> > in the past. So we add a new field "argument" to present
> > if the option takes unspecified arguments.
> 
> I like Markus' suggestion of naming the new field
> 'unspecified-parameters' rather than 'argument'.
> 
> > 
> > This patch also fixes options to match their actual command-line
> > spelling rather than an alternate name associated with the
> > option table in use by the command.
> 
> Should we independently patch hw/acpi/core.c to rename qemu_acpi_opts
> from "acpi" to "acpitable" to match the command line option?  Same for
> vl.c and qemu_boot_opts from "boot-opts" to "boot"?  Same for vl.c and
> qemu_smp_opts from "smp-opts" to "smp"?

Yes, we should.

> Those were the obvious
> mismatches I found where the command line was spelled differently than
> the vm_config_groups entry.
> 
> This is a bug fix patch, so let's shoot to get it into 2.0.
> 
> > 
> > Signed-off-by: Amos Kong <address@hidden>
> > ---
> >  qapi-schema.json   |  8 ++++++--
> >  qemu-options.h     | 10 ++++++++++
> >  util/qemu-config.c | 44 ++++++++++++++++++++++++++++++++++++++------
> >  vl.c               | 15 ---------------
> >  4 files changed, 54 insertions(+), 23 deletions(-)
> 
> > 
> > +++ b/util/qemu-config.c
> > @@ -6,6 +6,16 @@
> >  #include "hw/qdev.h"
> >  #include "qapi/error.h"
> >  #include "qmp-commands.h"
> > +#include "qemu-options.h"
> > +
> > +#define HAS_ARG 0x0001
> 
> Hmm, we are now duplicating this macro between here and vl.c.  I'd
> prefer it gets hoisted into the .h file, so that it doesn't get out of
> sync between the two clients.
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 



-- 
                        Amos.

pgp7YQ_uGHhgF.pgp
Description: PGP signature