Re: [libvirt] [Qemu-devel] [PATCH v3 for 2.0] update names in option tables to match with actual command-line spelling
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 <eblake(a)redhat.com> writes:
...
> > Reviewed-by: Eric Blake <eblake(a)redhat.com>
>
> 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.
Attachments:
- 0002-query-command-line-options-query-all-the-options-in-.patch (text/plain — 6.7 KB)
- 0001-add-alias_index-to-QemuOptsList.patch (text/plain — 4.2 KB)