qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Amos Kong <akong@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com,
	jyang@redhat.com, pbonzini@redhat.com, libvirt-list@redhat.com
Subject: Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx
Date: Fri, 7 Mar 2014 14:09:54 +0800	[thread overview]
Message-ID: <20140307060954.GA3487@amosk.info> (raw)
In-Reply-To: <5318E743.7070609@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]

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 <akong@redhat.com>
> > ---
> >  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.

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2014-03-07  6:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-06  2:36 [Qemu-devel] [PATCH v4 0/2] fix query-command-line-options Amos Kong
2014-03-06  2:36 ` [Qemu-devel] [PATCH v4 1/2] qmp: rename query_option_descs() to get_param_infolist() Amos Kong
2014-03-06  2:36 ` [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx Amos Kong
2014-03-06 10:50   ` Markus Armbruster
2014-03-06 21:23   ` Eric Blake
2014-03-07  6:09     ` Amos Kong [this message]
2014-03-07  9:54     ` Markus Armbruster
2014-03-10 17:41       ` Eric Blake
2014-03-11  9:04         ` Markus Armbruster
2014-03-11 14:46           ` Eric Blake
2014-03-20 14:12           ` Amos Kong
2014-03-27  5:09             ` Amos Kong
2014-03-20 14:03       ` Amos Kong
2014-03-20 14:51         ` Amos Kong
2014-03-26 13:15         ` Markus Armbruster
2014-03-27  5:04           ` Amos Kong
2014-03-27  9:46             ` Markus Armbruster
2014-03-07  9:56   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140307060954.GA3487@amosk.info \
    --to=akong@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jyang@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=libvirt-list@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).