[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 3/3] Change other funcitons referring to feat

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v3 3/3] Change other funcitons referring to feature_word_info[]
Date: Wed, 5 Sep 2018 10:32:33 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

On 09/05/2018 09:10 AM, Eduardo Habkost wrote:
Question to the QAPI schema maintainers below:

-{ 'struct': 'X86CPUFeatureWordInfo',
-  'data': { 'cpuid-input-eax': 'int',
-            '*cpuid-input-ecx': 'int',
-            'cpuid-register': 'X86CPURegister32',
+{ 'struct': 'X86CPUIDFeatureWordInfo',
+  'data': { 'input-eax': 'int',
+            '*input-ecx': 'int',
+            'register': 'X86CPURegister32',
              'features': 'int' } }

You are renaming the struct (which is not visible over the wire), as well as renaming members within the struct (which is visible, if this type appears on the wire).

However, 'grep FeatureWordInfo qapi/qapi-introspect.c' has no hits either before or after this patch (well, first you have to build with my currently-pending patch as part of Markus' pull request for this trick to work, https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg05968.html). Or, even without my patch, grepping for 'input-eax' has no hits as a member name in any struct. Which means that there are no QMP commands currently exposing this struct over the wire.

Question to the QAPI maintainers: is this really allowed?  With
this change, data that conforms to the new schema may not conform
to the old schema, because now the "input-eax" field will become

Yes, you can do whatever you want to QAPI structs that are not part of the QMP API, since they exist merely to make your C code easier. You don't have to worry about backwards compatibility, because the only clients of such structs are being compiled at the same time as you make modifications to that struct. Only once the struct appears in QMP introspection do you start having to worry about back-compat. There, the rules are the struct names don't matter, but member names do; if the struct is used on input, then removing members is bad, adding mandatory members is bad, but adding optional members is okay (older clients don't know to pass in the new members, and may continue to pass in the member name you no longer want to use); if the struct is used on output, then removing non-optional members is bad, removing optional members is okay, and adding members is okay (clients are supposed to ignore unknown members, but may break if a previously mandatory member disappears).

AFAICS, this will break the existing libvirt code: it will make
qemuMonitorJSONParseCPUx86Features() error out because it won't
find the "input-eax" field on all X86CPUFeatureWordInfo elements.

No, this change can't break libvirt, since I just proved there is no QMP command that libvirt could be using that either supplies an input member or expects an output member named "input-eax" in the first place.

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]