Re: [Qemu-devel] [PATCH 3/6] qapi: add SysEmuTarget to "common.json"

[Laszlo Ersek <lersek@redhat.com>]

On 04/25/18 14:54, Daniel P. Berrangé wrote:
> On Tue, Apr 24, 2018 at 06:11:05PM -0500, Eric Blake wrote:
>> On 04/24/2018 04:45 PM, Laszlo Ersek wrote:
>>> We'll soon need an enumeration type that lists all the softmmu targets
>>> that QEMU (the project) supports. Introduce @SysEmuTarget to
>>> "common.json".
>>>
>>> Cc: "Daniel P. Berrange" <berrange@redhat.com>
>>> Cc: David Gibson <dgibson@redhat.com>
>>> Cc: Eric Blake <eblake@redhat.com>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Cc: Kashyap Chamarthy <kchamart@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>> Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
>>> ---
>>>
>>
>>> +##
>>> +{ 'enum' : 'SysEmuTarget',
>>> +  'data' : [ 'aarch64', 'alpha', 'arm', 'cris', 'hppa', 'i386', 'lm32',
>>> +             'm68k', 'microblaze', 'microblazeel', 'mips', 'mips64',
>>> +             'mips64el', 'mipsel', 'moxie', 'nios2', 'or1k', 'ppc',
>>> +             'ppc64', 'ppcemb', 'riscv32', 'riscv64', 's390x', 'sh4',
>>> +             'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
>>> +             'x86_64', 'xtensa', 'xtensaeb' ] }
>>
>> x86_64 doesn't match our typical conventions of preferring '-' over '_';
>> also, wikipedia mentions both spellings but under the page name
>> 'x86-64'.  Is it worth switching that enum constant?
>>
>> https://en.wikipedia.org/wiki/X86-64
> 
> I would not want that - SysEmuTarget is supposed to be correlated with
> the qemu-system-$TARGET  binary names and we use qemu-system-x86_64
> there.

Right; also TARGET_NAME is looked up as follows in patch #4, in
qmp_query_target():

+    info->arch = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME, -1,
+                                 &error_abort);

That would fail if we used "x86-64" here.

Thanks!
Laszlo