On Thu, Jan 05, 2023 at 03:53:11PM +0100, Pierre Morel wrote:
> Reporting the current topology informations to the admin through
> the QEMU monitor.
>
> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
> ---
> qapi/machine-target.json | 66 ++++++++++++++++++++++++++++++++++
> include/monitor/hmp.h | 1 +
> hw/s390x/cpu-topology.c | 76 ++++++++++++++++++++++++++++++++++++++++
> hmp-commands-info.hx | 16 +++++++++
> 4 files changed, 159 insertions(+)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 75b0aa254d..927618a78f 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -371,3 +371,69 @@
> },
> 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
> }
> +
> +##
> +# @S390CpuTopology:
> +#
> +# CPU Topology information
> +#
> +# @drawer: the destination drawer where to move the vCPU
> +#
> +# @book: the destination book where to move the vCPU
> +#
> +# @socket: the destination socket where to move the vCPU
> +#
> +# @polarity: optional polarity, default is last polarity set by the guest
> +#
> +# @dedicated: optional, if the vCPU is dedicated to a real CPU
> +#
> +# @origin: offset of the first bit of the core mask
> +#
> +# @mask: mask of the cores sharing the same topology
> +#
> +# Since: 8.0
> +##
> +{ 'struct': 'S390CpuTopology',
> + 'data': {
> + 'drawer': 'int',
> + 'book': 'int',
> + 'socket': 'int',
> + 'polarity': 'int',
> + 'dedicated': 'bool',
> + 'origin': 'int',
> + 'mask': 'str'
> + },
> + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
> +}
> +
> +##
> +# @query-topology:
> +#
> +# Return information about CPU Topology
> +#
> +# Returns a @CpuTopology instance describing the CPU Toplogy
> +# being currently used by QEMU.
> +#
> +# Since: 8.0
> +#
> +# Example:
> +#
> +# -> { "execute": "cpu-topology" }
> +# <- {"return": [
> +# {
> +# "drawer": 0,
> +# "book": 0,
> +# "socket": 0,
> +# "polarity": 0,
> +# "dedicated": true,
> +# "origin": 0,
> +# "mask": 0xc000000000000000,
> +# },
> +# ]
> +# }
> +#
> +##
> +{ 'command': 'query-topology',
> + 'returns': ['S390CpuTopology'],
> + 'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
> +}
IIUC, you're using @mask as a way to compress the array returned
from query-topology, so that it doesn't have any repeated elements
with the same data. I guess I can understand that desire when the
core count can get very large, this can have a large saving.
The downside of using @mask, is that now you require the caller
to parse the string to turn it into a bitmask and expand the
data. Generally this is considered a bit of an anti-pattern in
QAPI design - we don't want callers to have to further parse
the data to extract information, we want to directly consumable
from the parsed JSON doc.
We already have 'query-cpus-fast' wich returns one entry for
each CPU. In fact why do we need to add query-topology at all.
Can't we just add book-id / drawer-id / polarity / dedicated
to the query-cpus-fast result ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
