Re: [Qemu-devel] [PATCH v3 for 2.0] update names in option tables to match with actual command-line spelling

[Amos Kong <akong@redhat.com>]

On Wed, Mar 26, 2014 at 05:12:08PM +0100, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 03/20/2014 07:07 AM, Amos Kong wrote:
> >> We want to establish a mapping between option name and option table,
> >> then we can search related option table by option name.
> >> 
> >> This patch makes all the member name of QemuOptsList to match with
> >> actual command-line spelling(option name).
> >> 
> >> [ Important Note ]
> >> 
> >> The QemuOptsList member name values are ABI, changing them can break
> >> existing -readconfig configuration files.
> >> 
> >> This patch changes:
> >> 
> >>     from        to          introduced in
> >>     acpi        acpitable   0c764a9 v1.5.0
> >>     boot-opts   boot        3d3b830 v1.0
> >>     smp-opts    smp         12b7f57 v1.6.0
> >> 
> >> All three have calcified into ABI already.
> >> 
> >> I have updated the release note of 2.0
> >> http://wiki.qemu.org/ChangeLog/2.0#ABI_breaking
> >> 
> >> Signed-off-by: Amos Kong <akong@redhat.com>
> >> ---
> >
> > The benefit of this patch is that 'query-command-line-options' gains a
> > fix where three bogus entries are replaced by their actual command line
> > spelling.  The drawback is that anyone that doesn't pay attention to the
> > ABI break announcement, and expects -readconfig and friends to work
> > while using the old spelling, is in for a surprise.  But since we have
> > prominently documented the change, and since consistency makes life
> > nicer, I'm in favor of this patch.
> >
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> I'm not thrilled about the ABI break, but avoiding it would probably
> take too much code for too little gain.

We can add an 'alias_index' field to QemuOptsList, and init it to -1
in qemu_add_opts().

And we only re-set 'alias_index' to 'popt->index' in vl.c(option parsing part)
then we can find opts by qemu_options[i].index.

We also need to give a warning () if it's group name of added QemuOptsList
doesn't match with defined option name.

If someone add a new QemuOpts and the group name doesn't match, he/she will
get a warning, and call a help function to re-set 'alias_index'.
 
I can send a RFC patch for this.


> How can we prevent future violations of the convention "QemuOptsList
> member name matches the name of the (primary) option using it for its
> argument"?  Right now, all we have is /* option name */ tacked to the
> member.  Which is at best a reminder for those who already know.

It's absolutely necessary!
 
> I'd ask for a test catching violations if I could think of an easy way
> to code it.

Currently I prefer this option, so I will send a V3 with strict checking.

-- 
			Amos.