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

[Hu, Robert]

> -----Original Message-----
> From: Eric Blake [mailto:address@hidden
> Sent: Thursday, September 6, 2018 1:41
> To: Eduardo Habkost <address@hidden>
> Cc: Robert Hoo <address@hidden>; Paolo Bonzini
> <address@hidden>; Hu, Robert <address@hidden>; address@hidden;
> address@hidden; address@hidden; Liu, Jingqi
> <address@hidden>; Markus Armbruster <address@hidden>; Jiri
> Denemark <address@hidden>
> Subject: Re: [PATCH v3 3/3] Change other funcitons referring to
> feature_word_info[]
> 
> On 09/05/2018 11:44 AM, Eduardo Habkost wrote:
> > On Wed, Sep 05, 2018 at 10:32:33AM -0500, Eric Blake wrote:
> >> 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.
> >
> > There is one: qom-get.
> 
> Oh, right, the nasty exception that is not visible to introspection :(
> 
> >>> 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.
> >
> > I'm sure it will break libvirt.  libvirt uses "qom-get path=<cpu-path>
> > property=feature-words" to get X86FeatureWordInfo data, and it expects
> > the returned data to have a "input-eax" field.
> 
> Yeah, since this type is used in the qom-get backdoor that evades 
> introspection,
> it's even more important that you follow the backwards-compatibility rules and
> not rename or delete any previously-mandatory members, since libvirt CAN'T
> introspect if such changes have happened.
> 
[Robert Hoo] 
Oh, this is more complicated than my thought.
So, Eduardo, how about leave the QAPI expansion for MSR based features to other
professional people?

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