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

[Amos Kong]

On Thu, Mar 27, 2014 at 10:16:44AM +0800, Amos Kong wrote:
> On Wed, Mar 26, 2014 at 05:12:08PM +0100, Markus Armbruster wrote:
> > Eric Blake <address@hidden> writes:

...
> > > Reviewed-by: Eric Blake <address@hidden>
> > 
> > I'm not thrilled about the ABI break, but avoiding it would probably
> > take too much code for too little gain.
 
Hi Markus, Eric

> 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.

* Option 1
Attached two RFC patches, it added a new field ('alias_index') to
QemuOptsList, and record QEMU_OPTION enum-index to it when group
name of option table doesn't match option name.

And try to find list by index before find list by option name in
qmp_query_command_line_options().

This can avoid the ABI breaking, it also introduced some trouble.
We also need to check if alias_index of unmatched option is set
to a positive number, and report error (option name doesn't match, and
alias_index isn't set) in error state.

* Option 2
OR pass enum-index to qemu_add_opts(), and set enum-index for all the options.
   -void qemu_add_opts(QemuOptsList *list)
   +void qemu_add_opts(QemuOptsList *list, int index)

* Option 3 break ABI

Let's make a decision.

> > 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.

0001-add-alias_index-to-QemuOptsList.patch
Description: Text document

0002-query-command-line-options-query-all-the-options-in-.patch
Description: Text document