> -----Original Message----- > From: Eric Blake [mailto:ebl...@redhat.com] > Sent: Thursday, September 6, 2018 1:41 > To: Eduardo Habkost <ehabk...@redhat.com> > Cc: Robert Hoo <robert...@linux.intel.com>; Paolo Bonzini > <pbonz...@redhat.com>; Hu, Robert <robert...@intel.com>; r...@twiddle.net; > thomas.lenda...@amd.com; qemu-devel@nongnu.org; Liu, Jingqi > <jingqi....@intel.com>; Markus Armbruster <arm...@redhat.com>; Jiri > Denemark <jdene...@redhat.com> > 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