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

[Eric Blake]

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
> optional.

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