Re: [libvirt] [Qemu-devel] [PATCH v3 for 2.0] update names in option tables to match with actual command-line spelling
On Wed, Mar 26, 2014 at 05:12:08PM +0100, Markus Armbruster wrote:
Eric Blake <eblake(a)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(a)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(a)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.