On Thu, Sep 19, 2024 at 08:20:56AM -0300, Daniel Henrique Barboza wrote:
> Add a QMP command that shows all specific properties of the current
> accelerator in use.
Why do we need to expose /everything/ ?
>
> This can be used as a complement of other APIs like query-machines and
> query-cpu-model-expansion, allowing management to get a more complete
> picture of the running QEMU process.
query-machines doesn't return every single QOM property, just
a hand selected set of information pieces.
The query-cpu-model-expansion does return everything, but I
consider that command to be bad design, as it doesn't distinguish
between hardware CPU features, and QEMU QOM properties
>
> This is the output with a x86_64 TCG guest:
>
> $ ./build/qemu-system-x86_64 -S -display none -accel tcg -qmp
> tcp:localhost:1234,server,wait=off
>
> $ ./scripts/qmp/qmp-shell localhost:1234
> Welcome to the QMP low-level shell!
> Connected to QEMU 9.1.50
>
> (QEMU) query-accelerator
> {"return": {"name": "tcg", "props": {"one-insn-per-tb": false, "thread":
> "multi", "tb-size": 0, "split-wx": false, "type": "tcg-accel"}}}
>
> And for a x86_64 KVM guest:
>
> $ ./build/qemu-system-x86_64 -S -display none -accel kvm -qmp
> tcp:localhost:1234,server,wait=off
>
> $ ./scripts/qmp/qmp-shell localhost:1234
> Welcome to the QMP low-level shell!
> Connected to QEMU 9.1.50
>
> (QEMU) query-accelerator
> {"return": {"name": "KVM", "props": {"mem-container-smram[0]": "",
> "xen-gnttab-max-frames": 64, "device": "", "xen-version": 0, "mem-smram[0]":
> "", "notify-window": 0, "dirty-ring-size": 0, "kvm-shadow-mem": -1, "type":
> "kvm-accel", "notify-vmexit": "run", "xen-evtchn-max-pirq": 256}}}
>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> ---
> hw/core/machine-qmp-cmds.c | 34 ++++++++++++++++++++++++++++++++++
> qapi/machine.json | 27 +++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 130217da8f..eac803bf36 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> +AccelInfo *qmp_query_accelerator(Error **errp)
> +{
> + AccelState *accel = current_accel();
> + AccelClass *acc = ACCEL_GET_CLASS(accel);
> + AccelInfo *info = g_new0(AccelInfo, 1);
> + QDict *qdict_out = qdict_new();
> + ObjectPropertyIterator iter;
> + ObjectProperty *prop;
> +
> + info->name = g_strdup(acc->name);
> +
> + object_property_iter_init(&iter, OBJECT(accel));
> + while ((prop = object_property_iter_next(&iter))) {
> + QObject *value;
> +
> + if (!prop->get) {
> + continue;
> + }
> +
> + value = object_property_get_qobject(OBJECT(accel), prop->name,
> + &error_abort);
> + qdict_put_obj(qdict_out, prop->name, value);
> + }
I'm not at all convinced trhat we should be exposing every single
QOM property on the accelerator class as public QMP data
> +
> + if (!qdict_size(qdict_out)) {
> + qobject_unref(qdict_out);
> + } else {
> + info->props = QOBJECT(qdict_out);
> + }
> +
> + return info;
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index a6b8795b09..d0d527d1eb 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1898,3 +1898,30 @@
> { 'command': 'x-query-interrupt-controllers',
> 'returns': 'HumanReadableText',
> 'features': [ 'unstable' ]}
> +
> +##
> +# @AccelInfo:
> +#
> +# Information about the current accelerator.
> +#
> +# @name: the name of the current accelerator being used
> +#
> +# @props: a dictionary of the accelerator properties
> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'AccelInfo',
> + 'data': { 'name': 'str',
> + '*props': 'any' } }
This is way too open ended. IMHO ideally we would never add more
instances of the 'any' type, as it has many downsides
- zero documentation about what is available
- no version info about when each prop was introduced
- no ability to tag fields as deprecated
For this new API, IMHO 'name' should be an enumeration of the
accelerator types, and thenm 'props' should be a discrinated
union of accelerator specific structs
> +
> +##
> +# @query-accelerator:
> +#
> +# Shows information about the accelerator in use.
> +#
> +# Returns: a CpuModelExpansionInfo describing the expanded CPU model
> +#
> +# Since: 9.2
> +##
> +{ 'command': 'query-accelerator',
> + 'returns': 'AccelInfo' }
> --
> 2.45.2
>
>
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 :|
