diff mbox

[3/6] qapi: add SysEmuTarget to "common.json"

Message ID 20180424214550.32549-4-lersek@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laszlo Ersek April 24, 2018, 9:45 p.m. UTC
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>
---

Notes:
    PATCHv1:
    
    - pick up R-b's from Markus and Kashyap, no changes
    
    RFCv3:
    
    - The patch is new in this version. [Dan, Markus]
    
    - The original idea was to call the new enum @Target; however, @Target
      generates exactly the TARGET_AARCH64, TARGET_ALPHA, TARGET_ARM, ...
      enumeration constants that conflict with the poisoned preprocessing
      macros of the same names. Hence @SysEmuTarget -- it's more accurate
      anyway, since we want it to stand for system emulation targets.
    
    - Also, we discussed defining the new type in either "common.json" or
      "misc.json". "misc.json" turned out to be a problem: "firmware.json"
      would then include "misc.json" for the new type's sake, but that
      inclusion would become the first appearance of "misc.json" -- within
      "firmware.json". That messed up the generated documentation. By adding
      the new type to "common.json", "misc.json" (see the 2nd patch) and
      "firmware.json" (see the 3rd patch) can both consume the new type
      without problems.

 qapi/common.json | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Eric Blake April 24, 2018, 11:11 p.m. UTC | #1
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
Daniel P. Berrangé April 25, 2018, 12:54 p.m. UTC | #2
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.


Regards,
Daniel
Laszlo Ersek April 25, 2018, 7:05 p.m. UTC | #3
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
Eric Blake April 25, 2018, 7:08 p.m. UTC | #4
On 04/25/2018 02:05 PM, Laszlo Ersek wrote:

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

Then worth a mention in the commit message to make it obvious that our
choice of _ for over-the-wire QMP is intentional.
Laszlo Ersek April 25, 2018, 10:57 p.m. UTC | #5
On 04/25/18 21:08, Eric Blake wrote:
> On 04/25/2018 02:05 PM, Laszlo Ersek wrote:
> 
>>>>> +             '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.
> 
> Then worth a mention in the commit message to make it obvious that our
> choice of _ for over-the-wire QMP is intentional.
> 

Right; I'll add a note to the schema too.

Thanks!
Laszlo
diff mbox

Patch

diff --git a/qapi/common.json b/qapi/common.json
index d9b14dd429f3..1fd63172754a 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -118,11 +118,30 @@ 
 #
 # @bar3: PCI BAR3 is used for the feature
 #
 # @bar4: PCI BAR4 is used for the feature
 #
 # @bar5: PCI BAR5 is used for the feature
 #
 # Since: 2.12
 ##
 { 'enum': 'OffAutoPCIBAR',
   'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
+
+##
+# @SysEmuTarget:
+#
+# The comprehensive enumeration of QEMU system emulation ("softmmu")
+# targets. Run "./configure --help" in the project root directory, and
+# look for the *-softmmu targets near the "--target-list" option. The
+# individual target constants are not documented here, for the time
+# being.
+#
+# Since: 2.13
+##
+{ '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' ] }