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