| Message ID | 20180424214550.32549-6-lersek@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Laszlo Ersek <lersek@redhat.com> writes: > @CpuInfo and @CpuInfoFast duplicate the following four fields: @qom-path, > @thread-id, @props and @arch. From these, extract the first three to a > common structure called @CpuInfoCommon. (More on @arch later.) > > Introduce two new mid-layer structures, @CpuInfoBase and @CpuInfoFastBase, > to soak up the union base struct fields on top of @CpuInfoCommon that are > specific to @query-cpus and @query-cpus-fast, respectively. This is > necessary because the base struct spec in union definitions does not let > us mix named fields with a recursive base struct. (In other words, we > couldn't directly use @CpuInfoCommon *plus* some other fields within > @CpuInfo and @CpuInfoFast as base struct). Yes, you can either specify a base type or inline common members. If "union's common members = base type plus additional inline members" turns out to be desirable in more places, we can try to add the feature. I'm not asking *you* to find out, let alone try :) > @arch cannot be hoisted higher than to @CpuInfoBase and @CpuInfoFastBase > because the union descriminator is only accepted from a direct base > struct, not from an indirect one. That's a bit of a blemish. Again, I'm not asking you to do anything about it. > Cc: Eric Blake <eblake@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > > Notes: > PATCHv1: > > - new patch > > qapi/misc.json | 94 +++++++++++++++++++++++++------------ > qapi/qapi-schema.json | 2 +- > tests/test-x86-cpuid-compat.c | 2 +- > tests/migration/guestperf/engine.py | 2 +- > 4 files changed, 68 insertions(+), 32 deletions(-) > > diff --git a/qapi/misc.json b/qapi/misc.json > index 460866cf542f..d7b776a5af37 100644 > --- a/qapi/misc.json > +++ b/qapi/misc.json > @@ -348,52 +348,81 @@ > # > # @s390: since 2.12 > # > # @riscv: since 2.12 > # > # Since: 2.6 > ## > { 'enum': 'CpuInfoArch', > 'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 'other' ] } > > ## > -# @CpuInfo: > +# @CpuInfoCommon: > # > -# Information about a virtual CPU > +# Collects fields common to @CpuInfoBase and @CpuInfoFastBase; that is, > +# fields that are shared by @query-cpus and @query-cpus-fast, and not > +# specific to the target architecture. > +# > +# @qom-path: path to the CPU object in the QOM tree (since 2.4) > +# > +# @thread-id: ID of the underlying host thread > +# > +# @props: properties describing which node/socket/core/thread the > +# virtual CPU belongs to, if supported by the board (since 2.10) > +# > +# Since: 2.13 > +## > +{ 'struct' : 'CpuInfoCommon', > + 'data' : { 'qom-path' : 'str', > + 'thread-id' : 'int', > + '*props' : 'CpuInstanceProperties' } } > + > +## > +# @CpuInfoBase: > +# > +# Extends @CpuInfoCommon with fields that are specific to the @query-cpus > +# command, but not specific to the target architecture. > # > # @CPU: the index of the virtual CPU > # > # @current: this only exists for backwards compatibility and should be ignored > # > # @halted: true if the virtual CPU is in the halt state. Halt usually refers > # to a processor specific low power mode. > # > -# @qom_path: path to the CPU object in the QOM tree (since 2.4) > -# > -# @thread_id: ID of the underlying host thread > -# > -# @props: properties describing to which node/socket/core/thread > -# virtual CPU belongs to, provided if supported by board (since 2.10) > -# > # @arch: architecture of the cpu, which determines which additional fields > # will be listed (since 2.6) > # > -# Since: 0.14.0 > +# Since: 2.13 > # > # Notes: @halted is a transient state that changes frequently. By the time the > # data is sent to the client, the guest may no longer be halted. > +# Moreover, @arch cannot be moved up to @CpuInfoCommon because > +# that would prevent its use as the discriminator in @CpuInfo. > +## > +{ 'struct' : 'CpuInfoBase', > + 'base' : 'CpuInfoCommon', > + 'data' : { 'CPU' : 'int', > + 'current' : 'bool', > + 'halted' : 'bool', > + 'arch' : 'CpuInfoArch' } } > + > +## > +# @CpuInfo: > +# > +# Information about a virtual CPU > +# > +# Since: 0.14.0 > ## > { 'union': 'CpuInfo', > - 'base': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', > - 'qom_path': 'str', 'thread_id': 'int', > - '*props': 'CpuInstanceProperties', 'arch': 'CpuInfoArch' }, > + 'base': 'CpuInfoBase', > 'discriminator': 'arch', > 'data': { 'x86': 'CpuInfoX86', > 'sparc': 'CpuInfoSPARC', > 'ppc': 'CpuInfoPPC', > 'mips': 'CpuInfoMIPS', > 'tricore': 'CpuInfoTricore', > 's390': 'CpuInfoS390', > 'riscv': 'CpuInfoRISCV', > 'other': 'CpuInfoOther' } } > > ## > @@ -512,70 +541,77 @@ > # > # Since: 0.14.0 > # > # Example: > # > # -> { "execute": "query-cpus" } > # <- { "return": [ > # { > # "CPU":0, > # "current":true, > # "halted":false, > -# "qom_path":"/machine/unattached/device[0]", > +# "qom-path":"/machine/unattached/device[0]", > # "arch":"x86", > # "pc":3227107138, > -# "thread_id":3134 > +# "thread-id":3134 > # }, > # { > # "CPU":1, > # "current":false, > # "halted":true, > -# "qom_path":"/machine/unattached/device[2]", > +# "qom-path":"/machine/unattached/device[2]", > # "arch":"x86", > # "pc":7108165, > -# "thread_id":3135 > +# "thread-id":3135 > # } > # ] > # } Compatibility break, whoops! CpuInfo and CpuInfoFast do share qom-path and thread-id *values*, but the keys differ in '_' vs. '-'. Sad. What now? Is there enough common stuff left to justify the refactoring? > # > # Notes: This interface is deprecated (since 2.12.0), and it is strongly > # recommended that you avoid using it. Use @query-cpus-fast to > # obtain information about virtual CPUs. > # > ## > { 'command': 'query-cpus', 'returns': ['CpuInfo'] } > > ## > -# @CpuInfoFast: > +# @CpuInfoFastBase: > # > -# Information about a virtual CPU > +# Extends @CpuInfoCommon with fields that are specific to the > +# @query-cpus-fast command, but not specific to the target architecture. > # > # @cpu-index: index of the virtual CPU > # > -# @qom-path: path to the CPU object in the QOM tree > -# > -# @thread-id: ID of the underlying host thread > -# > -# @props: properties describing to which node/socket/core/thread > -# virtual CPU belongs to, provided if supported by board > -# > # @arch: architecture of the cpu, which determines which additional fields > # will be listed > # > +# Since: 2.13 > +# > +# Notes: @arch cannot be moved up to @CpuInfoCommon because that would > +# prevent its use as the discriminator in @CpuInfoFast. > +## > +{ 'struct' : 'CpuInfoFastBase', > + 'base' : 'CpuInfoCommon', > + 'data' : { 'cpu-index' : 'int', > + 'arch' : 'CpuInfoArch' } } > + > +## > +# @CpuInfoFast: > +# > +# Information about a virtual CPU > +# > # Since: 2.12 > # > ## > { 'union': 'CpuInfoFast', > - 'base': {'cpu-index': 'int', 'qom-path': 'str', > - 'thread-id': 'int', '*props': 'CpuInstanceProperties', > - 'arch': 'CpuInfoArch' }, > + 'base': 'CpuInfoFastBase', > 'discriminator': 'arch', > 'data': { 'x86': 'CpuInfoOther', > 'sparc': 'CpuInfoOther', > 'ppc': 'CpuInfoOther', > 'mips': 'CpuInfoOther', > 'tricore': 'CpuInfoOther', > 's390': 'CpuInfoS390', > 'riscv': 'CpuInfoOther', > 'other': 'CpuInfoOther' } } > > ## > diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json > index 25bce78352b8..5bfd2ef1dd3b 100644 > --- a/qapi/qapi-schema.json > +++ b/qapi/qapi-schema.json > @@ -61,23 +61,23 @@ > 'query-migrate-cache-size', > 'query-tpm-models', > 'query-tpm-types', > 'ringbuf-read' ], > 'name-case-whitelist': [ > 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status > 'CpuInfoMIPS', # PC, visible through query-cpu > 'CpuInfoTricore', # PC, visible through query-cpu > 'QapiErrorClass', # all members, visible through errors > 'UuidInfo', # UUID, visible through query-uuid > 'X86CPURegister32', # all members, visible indirectly through qom-get > - 'q_obj_CpuInfo-base' # CPU, visible through query-cpu > + 'CpuInfoBase' # CPU, visible through query-cpu Let's update this to "visible through query-cpus, query-cpus-fast" while there. > ] } } > > # Documentation generated with qapi-gen.py is in source order, with > # included sub-schemas inserted at the first include directive > # (subsequent include directives have no effect). To get a sane and > # stable order, it's best to include each sub-schema just once, or > # include it first right here. > > { 'include': 'common.json' } > { 'include': 'sockets.json' } > { 'include': 'run-state.json' } [...]
On 04/25/18 09:06, Markus Armbruster wrote: > Laszlo Ersek <lersek@redhat.com> writes: > >> @CpuInfo and @CpuInfoFast duplicate the following four fields: @qom-path, >> @thread-id, @props and @arch. From these, extract the first three to a >> common structure called @CpuInfoCommon. (More on @arch later.) >> >> Introduce two new mid-layer structures, @CpuInfoBase and @CpuInfoFastBase, >> to soak up the union base struct fields on top of @CpuInfoCommon that are >> specific to @query-cpus and @query-cpus-fast, respectively. [[ >> This is >> necessary because the base struct spec in union definitions does not let >> us mix named fields with a recursive base struct. (In other words, we >> couldn't directly use @CpuInfoCommon *plus* some other fields within >> @CpuInfo and @CpuInfoFast as base struct). ]] > > Yes, you can either specify a base type or inline common members. If > "union's common members = base type plus additional inline members" > turns out to be desirable in more places, we can try to add the feature. > I'm not asking *you* to find out, let alone try :) [[ >> @arch cannot be hoisted higher than to @CpuInfoBase and @CpuInfoFastBase >> because the union descriminator is only accepted from a direct base >> struct, not from an indirect one. ]] > That's a bit of a blemish. Again, I'm not asking you to do anything > about it. If these characteristics of the generator are operating knowledge for QAPI developers, I can trim the commit message -- those traits don't bother me at all, I just felt that I needed to justify the code. IOW, should I drop: - the sentences marked with [[ ]] above, - and/or the "Notes:" on @arch in the schema itself (which are updated to @target, in the next patch) ? [snip] >> @@ -512,70 +541,77 @@ >> # >> # Since: 0.14.0 >> # >> # Example: >> # >> # -> { "execute": "query-cpus" } >> # <- { "return": [ >> # { >> # "CPU":0, >> # "current":true, >> # "halted":false, >> -# "qom_path":"/machine/unattached/device[0]", >> +# "qom-path":"/machine/unattached/device[0]", >> # "arch":"x86", >> # "pc":3227107138, >> -# "thread_id":3134 >> +# "thread-id":3134 >> # }, >> # { >> # "CPU":1, >> # "current":false, >> # "halted":true, >> -# "qom_path":"/machine/unattached/device[2]", >> +# "qom-path":"/machine/unattached/device[2]", >> # "arch":"x86", >> # "pc":7108165, >> -# "thread_id":3135 >> +# "thread-id":3135 >> # } >> # ] >> # } > > Compatibility break, whoops! But, at least, I updated the example! :) > > CpuInfo and CpuInfoFast do share qom-path and thread-id *values*, but > the keys differ in '_' vs. '-'. Sad. > > What now? Is there enough common stuff left to justify the refactoring? While working on the schema changes, I saw the '_' vs. '-' difference immediately, but I thought these two characters were treated equivalently by all QAPI clients. Later I found (in the test suite) that this wasn't the case, so I thought my memories must have applied to libvirtd only. (Because, libvirt does map "_" to "-", right?) Anyway, I figured the best way to ask was to post the patch like this :) So, if I drop @qom-path and @thread-id from CpuInfoCommon, then only @props remains subject to hoisting, in this patch. In the next patch, @arch joins @props. I believe the refactoring is still worth doing, because otherwise, after the addition of @target, we'd end up with: { 'union' : 'CpuInfo', 'base' : { 'CPU' : 'int', 'current' : 'bool', 'halted' : 'bool', 'qom_path' : 'str', 'thread_id' : 'int', '*props' : 'CpuInstanceProperties', 'arch' : 'CpuInfoArch', 'target' : 'SysEmuTarget' }, ... { 'union' : 'CpuInfoFast', 'base' : { 'cpu-index' : 'int', 'qom-path' : 'str', 'thread-id' : 'int', '*props' : 'CpuInstanceProperties', 'arch' : 'CpuInfoArch', 'target' : 'SysEmuTarget' }, ... and people would ask themselves ever after, "are there some common fields in there that we could extract ... hmmm, @props and @arch, okay, maybe, maybe not, grey area". Let's do it now and save them the thinking. [snip] >> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json >> index 25bce78352b8..5bfd2ef1dd3b 100644 >> --- a/qapi/qapi-schema.json >> +++ b/qapi/qapi-schema.json >> @@ -61,23 +61,23 @@ >> 'query-migrate-cache-size', >> 'query-tpm-models', >> 'query-tpm-types', >> 'ringbuf-read' ], >> 'name-case-whitelist': [ >> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status >> 'CpuInfoMIPS', # PC, visible through query-cpu >> 'CpuInfoTricore', # PC, visible through query-cpu >> 'QapiErrorClass', # all members, visible through errors >> 'UuidInfo', # UUID, visible through query-uuid >> 'X86CPURegister32', # all members, visible indirectly through qom-get >> - 'q_obj_CpuInfo-base' # CPU, visible through query-cpu >> + 'CpuInfoBase' # CPU, visible through query-cpu > > Let's update this to "visible through query-cpus, query-cpus-fast" while > there. Right, I noticed the typo in the preexistent comment too late (and I didn't notice the "query-cpus-fast" omission at all). Thanks! Laszlo
Laszlo Ersek <lersek@redhat.com> writes: > On 04/25/18 09:06, Markus Armbruster wrote: >> Laszlo Ersek <lersek@redhat.com> writes: >> >>> @CpuInfo and @CpuInfoFast duplicate the following four fields: @qom-path, >>> @thread-id, @props and @arch. From these, extract the first three to a >>> common structure called @CpuInfoCommon. (More on @arch later.) >>> >>> Introduce two new mid-layer structures, @CpuInfoBase and @CpuInfoFastBase, >>> to soak up the union base struct fields on top of @CpuInfoCommon that are >>> specific to @query-cpus and @query-cpus-fast, respectively. > > [[ > >>> This is >>> necessary because the base struct spec in union definitions does not let >>> us mix named fields with a recursive base struct. (In other words, we >>> couldn't directly use @CpuInfoCommon *plus* some other fields within >>> @CpuInfo and @CpuInfoFast as base struct). > > ]] > >> >> Yes, you can either specify a base type or inline common members. If >> "union's common members = base type plus additional inline members" >> turns out to be desirable in more places, we can try to add the feature. >> I'm not asking *you* to find out, let alone try :) > > [[ > >>> @arch cannot be hoisted higher than to @CpuInfoBase and @CpuInfoFastBase >>> because the union descriminator is only accepted from a direct base >>> struct, not from an indirect one. > > ]] > >> That's a bit of a blemish. Again, I'm not asking you to do anything >> about it. > > If these characteristics of the generator are operating knowledge for > QAPI developers, I can trim the commit message -- those traits don't > bother me at all, I just felt that I needed to justify the code. > > IOW, should I drop: > - the sentences marked with [[ ]] above, > - and/or the "Notes:" on @arch in the schema itself (which are updated > to @target, in the next patch) > ? Let's keep them. > > [snip] > >>> @@ -512,70 +541,77 @@ >>> # >>> # Since: 0.14.0 >>> # >>> # Example: >>> # >>> # -> { "execute": "query-cpus" } >>> # <- { "return": [ >>> # { >>> # "CPU":0, >>> # "current":true, >>> # "halted":false, >>> -# "qom_path":"/machine/unattached/device[0]", >>> +# "qom-path":"/machine/unattached/device[0]", >>> # "arch":"x86", >>> # "pc":3227107138, >>> -# "thread_id":3134 >>> +# "thread-id":3134 >>> # }, >>> # { >>> # "CPU":1, >>> # "current":false, >>> # "halted":true, >>> -# "qom_path":"/machine/unattached/device[2]", >>> +# "qom-path":"/machine/unattached/device[2]", >>> # "arch":"x86", >>> # "pc":7108165, >>> -# "thread_id":3135 >>> +# "thread-id":3135 >>> # } >>> # ] >>> # } >> >> Compatibility break, whoops! > > But, at least, I updated the example! :) Thanks :) >> CpuInfo and CpuInfoFast do share qom-path and thread-id *values*, but >> the keys differ in '_' vs. '-'. Sad. >> >> What now? Is there enough common stuff left to justify the refactoring? > > While working on the schema changes, I saw the '_' vs. '-' difference > immediately, but I thought these two characters were treated > equivalently by all QAPI clients. I wish... QEMU could do that on QMP input. We've talked about it, but no patches. On QMP output, we're screwed. Tolerating '_' was one of the dumber mistakes in QAPI. > Later I found (in the test suite) that this wasn't the case, so I > thought my memories must have applied to libvirtd only. (Because, > libvirt does map "_" to "-", right?) Anyway, I figured the best way to > ask was to post the patch like this :) Heh. I would've appreciated a hint in the commit message, though. > So, if I drop @qom-path and @thread-id from CpuInfoCommon, then only > @props remains subject to hoisting, in this patch. In the next patch, > @arch joins @props. > > I believe the refactoring is still worth doing, because otherwise, after > the addition of @target, we'd end up with: > > { 'union' : 'CpuInfo', > 'base' : { 'CPU' : 'int', > 'current' : 'bool', > 'halted' : 'bool', > 'qom_path' : 'str', > 'thread_id' : 'int', > '*props' : 'CpuInstanceProperties', > 'arch' : 'CpuInfoArch', > 'target' : 'SysEmuTarget' }, > ... > > { 'union' : 'CpuInfoFast', > 'base' : { 'cpu-index' : 'int', > 'qom-path' : 'str', > 'thread-id' : 'int', > '*props' : 'CpuInstanceProperties', > 'arch' : 'CpuInfoArch', > 'target' : 'SysEmuTarget' }, > ... > > and people would ask themselves ever after, "are there some common > fields in there that we could extract ... hmmm, @props and @arch, okay, > maybe, maybe not, grey area". Let's do it now and save them the thinking. Works for me. > [snip] > >>> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json >>> index 25bce78352b8..5bfd2ef1dd3b 100644 >>> --- a/qapi/qapi-schema.json >>> +++ b/qapi/qapi-schema.json >>> @@ -61,23 +61,23 @@ >>> 'query-migrate-cache-size', >>> 'query-tpm-models', >>> 'query-tpm-types', >>> 'ringbuf-read' ], >>> 'name-case-whitelist': [ >>> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status >>> 'CpuInfoMIPS', # PC, visible through query-cpu >>> 'CpuInfoTricore', # PC, visible through query-cpu >>> 'QapiErrorClass', # all members, visible through errors >>> 'UuidInfo', # UUID, visible through query-uuid >>> 'X86CPURegister32', # all members, visible indirectly through qom-get >>> - 'q_obj_CpuInfo-base' # CPU, visible through query-cpu >>> + 'CpuInfoBase' # CPU, visible through query-cpu >> >> Let's update this to "visible through query-cpus, query-cpus-fast" while >> there. > > Right, I noticed the typo in the preexistent comment too late (and I > didn't notice the "query-cpus-fast" omission at all). > > Thanks! You're welcome!
On 04/25/2018 08:20 AM, Laszlo Ersek wrote: > ... > > and people would ask themselves ever after, "are there some common > fields in there that we could extract ... hmmm, @props and @arch, okay, > maybe, maybe not, grey area". Let's do it now and save them the thinking. No, CpuInfo is slated for death in the next year or so; per commit ff9a9156. Once it disappears (in 2.14 or 2.15?), we will ONLY have CpuInfoFast (although we might rename it at that time, as the name of QMP structs is not part of the introspection interface). So, my personal inclination is to just live with the mindless near-duplication until the deprecation period ends, rather than wasting cycles refactoring things just to refactor it back out when removing the dead code later.
On 04/25/18 21:12, Eric Blake wrote: > On 04/25/2018 08:20 AM, Laszlo Ersek wrote: > >> ... >> >> and people would ask themselves ever after, "are there some common >> fields in there that we could extract ... hmmm, @props and @arch, okay, >> maybe, maybe not, grey area". Let's do it now and save them the thinking. > > No, CpuInfo is slated for death in the next year or so; per commit > ff9a9156. Once it disappears (in 2.14 or 2.15?), we will ONLY have > CpuInfoFast (although we might rename it at that time, as the name of > QMP structs is not part of the introspection interface). > > So, my personal inclination is to just live with the mindless > near-duplication until the deprecation period ends, rather than wasting > cycles refactoring things just to refactor it back out when removing the > dead code later. > This is an important update; thank you for it. Because, it tells me that we might not need to add @target to CpuInfo at all. Why *extend* an interface that is deprecated to the point that we're reluctant even to *refactor* it? (BTW, I had noticed the deprecation note in the schema source code, from what you've now identified as commit ff9a9156; I didn't know what it meant -- I didn't know it carried a removal sentence.) The consequence is that I could drop the painful modifications for qmp_query_cpus() altogether, and just keep the simple ones for qmp_query_cpus_fast(). Markus, does that work for you? Forget about @CpuInfo for good? Thanks, Laszlo
Laszlo Ersek <lersek@redhat.com> writes: > On 04/25/18 21:12, Eric Blake wrote: >> On 04/25/2018 08:20 AM, Laszlo Ersek wrote: >> >>> ... >>> >>> and people would ask themselves ever after, "are there some common >>> fields in there that we could extract ... hmmm, @props and @arch, okay, >>> maybe, maybe not, grey area". Let's do it now and save them the thinking. >> >> No, CpuInfo is slated for death in the next year or so; per commit >> ff9a9156. Good catch, I missed it. >> Once it disappears (in 2.14 or 2.15?), we will ONLY have >> CpuInfoFast (although we might rename it at that time, as the name of >> QMP structs is not part of the introspection interface). >> >> So, my personal inclination is to just live with the mindless >> near-duplication until the deprecation period ends, rather than wasting >> cycles refactoring things just to refactor it back out when removing the >> dead code later. >> > > This is an important update; thank you for it. Because, it tells me that > we might not need to add @target to CpuInfo at all. Why *extend* an > interface that is deprecated to the point that we're reluctant even to > *refactor* it? > > (BTW, I had noticed the deprecation note in the schema source code, from > what you've now identified as commit ff9a9156; I didn't know what it > meant -- I didn't know it carried a removal sentence.) No need to enhance it even without a removal sentence. > The consequence is that I could drop the painful modifications for > qmp_query_cpus() altogether, and just keep the simple ones for > qmp_query_cpus_fast(). > > Markus, does that work for you? Forget about @CpuInfo for good? Absolutely.
diff --git a/qapi/misc.json b/qapi/misc.json index 460866cf542f..d7b776a5af37 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -348,52 +348,81 @@ # # @s390: since 2.12 # # @riscv: since 2.12 # # Since: 2.6 ## { 'enum': 'CpuInfoArch', 'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 'other' ] } ## -# @CpuInfo: +# @CpuInfoCommon: # -# Information about a virtual CPU +# Collects fields common to @CpuInfoBase and @CpuInfoFastBase; that is, +# fields that are shared by @query-cpus and @query-cpus-fast, and not +# specific to the target architecture. +# +# @qom-path: path to the CPU object in the QOM tree (since 2.4) +# +# @thread-id: ID of the underlying host thread +# +# @props: properties describing which node/socket/core/thread the +# virtual CPU belongs to, if supported by the board (since 2.10) +# +# Since: 2.13 +## +{ 'struct' : 'CpuInfoCommon', + 'data' : { 'qom-path' : 'str', + 'thread-id' : 'int', + '*props' : 'CpuInstanceProperties' } } + +## +# @CpuInfoBase: +# +# Extends @CpuInfoCommon with fields that are specific to the @query-cpus +# command, but not specific to the target architecture. # # @CPU: the index of the virtual CPU # # @current: this only exists for backwards compatibility and should be ignored # # @halted: true if the virtual CPU is in the halt state. Halt usually refers # to a processor specific low power mode. # -# @qom_path: path to the CPU object in the QOM tree (since 2.4) -# -# @thread_id: ID of the underlying host thread -# -# @props: properties describing to which node/socket/core/thread -# virtual CPU belongs to, provided if supported by board (since 2.10) -# # @arch: architecture of the cpu, which determines which additional fields # will be listed (since 2.6) # -# Since: 0.14.0 +# Since: 2.13 # # Notes: @halted is a transient state that changes frequently. By the time the # data is sent to the client, the guest may no longer be halted. +# Moreover, @arch cannot be moved up to @CpuInfoCommon because +# that would prevent its use as the discriminator in @CpuInfo. +## +{ 'struct' : 'CpuInfoBase', + 'base' : 'CpuInfoCommon', + 'data' : { 'CPU' : 'int', + 'current' : 'bool', + 'halted' : 'bool', + 'arch' : 'CpuInfoArch' } } + +## +# @CpuInfo: +# +# Information about a virtual CPU +# +# Since: 0.14.0 ## { 'union': 'CpuInfo', - 'base': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', - 'qom_path': 'str', 'thread_id': 'int', - '*props': 'CpuInstanceProperties', 'arch': 'CpuInfoArch' }, + 'base': 'CpuInfoBase', 'discriminator': 'arch', 'data': { 'x86': 'CpuInfoX86', 'sparc': 'CpuInfoSPARC', 'ppc': 'CpuInfoPPC', 'mips': 'CpuInfoMIPS', 'tricore': 'CpuInfoTricore', 's390': 'CpuInfoS390', 'riscv': 'CpuInfoRISCV', 'other': 'CpuInfoOther' } } ## @@ -512,70 +541,77 @@ # # Since: 0.14.0 # # Example: # # -> { "execute": "query-cpus" } # <- { "return": [ # { # "CPU":0, # "current":true, # "halted":false, -# "qom_path":"/machine/unattached/device[0]", +# "qom-path":"/machine/unattached/device[0]", # "arch":"x86", # "pc":3227107138, -# "thread_id":3134 +# "thread-id":3134 # }, # { # "CPU":1, # "current":false, # "halted":true, -# "qom_path":"/machine/unattached/device[2]", +# "qom-path":"/machine/unattached/device[2]", # "arch":"x86", # "pc":7108165, -# "thread_id":3135 +# "thread-id":3135 # } # ] # } # # Notes: This interface is deprecated (since 2.12.0), and it is strongly # recommended that you avoid using it. Use @query-cpus-fast to # obtain information about virtual CPUs. # ## { 'command': 'query-cpus', 'returns': ['CpuInfo'] } ## -# @CpuInfoFast: +# @CpuInfoFastBase: # -# Information about a virtual CPU +# Extends @CpuInfoCommon with fields that are specific to the +# @query-cpus-fast command, but not specific to the target architecture. # # @cpu-index: index of the virtual CPU # -# @qom-path: path to the CPU object in the QOM tree -# -# @thread-id: ID of the underlying host thread -# -# @props: properties describing to which node/socket/core/thread -# virtual CPU belongs to, provided if supported by board -# # @arch: architecture of the cpu, which determines which additional fields # will be listed # +# Since: 2.13 +# +# Notes: @arch cannot be moved up to @CpuInfoCommon because that would +# prevent its use as the discriminator in @CpuInfoFast. +## +{ 'struct' : 'CpuInfoFastBase', + 'base' : 'CpuInfoCommon', + 'data' : { 'cpu-index' : 'int', + 'arch' : 'CpuInfoArch' } } + +## +# @CpuInfoFast: +# +# Information about a virtual CPU +# # Since: 2.12 # ## { 'union': 'CpuInfoFast', - 'base': {'cpu-index': 'int', 'qom-path': 'str', - 'thread-id': 'int', '*props': 'CpuInstanceProperties', - 'arch': 'CpuInfoArch' }, + 'base': 'CpuInfoFastBase', 'discriminator': 'arch', 'data': { 'x86': 'CpuInfoOther', 'sparc': 'CpuInfoOther', 'ppc': 'CpuInfoOther', 'mips': 'CpuInfoOther', 'tricore': 'CpuInfoOther', 's390': 'CpuInfoS390', 'riscv': 'CpuInfoOther', 'other': 'CpuInfoOther' } } ## diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json index 25bce78352b8..5bfd2ef1dd3b 100644 --- a/qapi/qapi-schema.json +++ b/qapi/qapi-schema.json @@ -61,23 +61,23 @@ 'query-migrate-cache-size', 'query-tpm-models', 'query-tpm-types', 'ringbuf-read' ], 'name-case-whitelist': [ 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status 'CpuInfoMIPS', # PC, visible through query-cpu 'CpuInfoTricore', # PC, visible through query-cpu 'QapiErrorClass', # all members, visible through errors 'UuidInfo', # UUID, visible through query-uuid 'X86CPURegister32', # all members, visible indirectly through qom-get - 'q_obj_CpuInfo-base' # CPU, visible through query-cpu + 'CpuInfoBase' # CPU, visible through query-cpu ] } } # Documentation generated with qapi-gen.py is in source order, with # included sub-schemas inserted at the first include directive # (subsequent include directives have no effect). To get a sane and # stable order, it's best to include each sub-schema just once, or # include it first right here. { 'include': 'common.json' } { 'include': 'sockets.json' } { 'include': 'run-state.json' } diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c index 02e41843fc9c..df51b3bbacbc 100644 --- a/tests/test-x86-cpuid-compat.c +++ b/tests/test-x86-cpuid-compat.c @@ -10,23 +10,23 @@ static char *get_cpu0_qom_path(void) { QDict *resp; QList *ret; QDict *cpu0; char *path; resp = qmp("{'execute': 'query-cpus', 'arguments': {}}"); g_assert(qdict_haskey(resp, "return")); ret = qdict_get_qlist(resp, "return"); cpu0 = qobject_to(QDict, qlist_peek(ret)); - path = g_strdup(qdict_get_str(cpu0, "qom_path")); + path = g_strdup(qdict_get_str(cpu0, "qom-path")); QDECREF(resp); return path; } static QObject *qom_get(const char *path, const char *prop) { QDict *resp = qmp("{ 'execute': 'qom-get'," " 'arguments': { 'path': %s," " 'property': %s } }", path, prop); QObject *ret = qdict_get(resp, "return"); diff --git a/tests/migration/guestperf/engine.py b/tests/migration/guestperf/engine.py index e14d4320b239..663881c163e9 100644 --- a/tests/migration/guestperf/engine.py +++ b/tests/migration/guestperf/engine.py @@ -104,23 +104,23 @@ class Engine(object): info.get("x-cpu-throttle-percentage", 0), ) def _migrate(self, hardware, scenario, src, dst, connect_uri): src_qemu_time = [] src_vcpu_time = [] src_pid = src.get_pid() vcpus = src.command("query-cpus") src_threads = [] for vcpu in vcpus: - src_threads.append(vcpu["thread_id"]) + src_threads.append(vcpu["thread-id"]) # XXX how to get dst timings on remote host ? if self._verbose: print "Sleeping %d seconds for initial guest workload run" % self._sleep sleep_secs = self._sleep while sleep_secs > 1: src_qemu_time.append(self._cpu_timing(src_pid)) src_vcpu_time.extend(self._vcpu_timing(src_pid, src_threads)) time.sleep(1) sleep_secs -= 1
@CpuInfo and @CpuInfoFast duplicate the following four fields: @qom-path, @thread-id, @props and @arch. From these, extract the first three to a common structure called @CpuInfoCommon. (More on @arch later.) Introduce two new mid-layer structures, @CpuInfoBase and @CpuInfoFastBase, to soak up the union base struct fields on top of @CpuInfoCommon that are specific to @query-cpus and @query-cpus-fast, respectively. This is necessary because the base struct spec in union definitions does not let us mix named fields with a recursive base struct. (In other words, we couldn't directly use @CpuInfoCommon *plus* some other fields within @CpuInfo and @CpuInfoFast as base struct). @arch cannot be hoisted higher than to @CpuInfoBase and @CpuInfoFastBase because the union descriminator is only accepted from a direct base struct, not from an indirect one. Cc: Eric Blake <eblake@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- Notes: PATCHv1: - new patch qapi/misc.json | 94 +++++++++++++++++++++++++------------ qapi/qapi-schema.json | 2 +- tests/test-x86-cpuid-compat.c | 2 +- tests/migration/guestperf/engine.py | 2 +- 4 files changed, 68 insertions(+), 32 deletions(-)