On Thu, Sep 04, 2025 at 16:46:55 +0200, Jiri Denemark via Devel wrote:
> From: Jiri Denemark <jdene...@redhat.com>
>
> qom-list-get is a new QMP command (since QEMU 10.1) that combines
> qom-list for listing properties of a specified object with qom-get for
> getting a value of a given property. The new command provides an array
> of all properties and their values, which allows us to dramatically
> reduce the number of QMP commands we have to call when starting a domain
> to check which CPU features were actually enabled.
>
> A simple domain with no disk can now be started with only 15 QMP
> commands in about 200 ms compared to 485 commands and 400 ms startup
> time without this patch.
>
> https://issues.redhat.com/browse/RHEL-7038
>
> Signed-off-by: Jiri Denemark <jdene...@redhat.com>
> ---
> src/qemu/qemu_capabilities.c | 2 +
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_monitor.c | 13 +++-
> src/qemu/qemu_monitor.h | 1 +
> src/qemu/qemu_monitor_json.c | 73 ++++++++++++++++---
> src/qemu/qemu_monitor_json.h | 7 +-
> src/qemu/qemu_process.c | 1 +
> .../caps_10.1.0_x86_64.xml | 1 +
> .../caps_10.2.0_x86_64.xml | 1 +
> ...=> get-guest-cpu-SierraForest-legacy.json} | 0
> ...> get-guest-cpu-SkylakeClient-legacy.json} | 0
> tests/qemumonitorjsontest.c | 18 +++--
> 12 files changed, 92 insertions(+), 26 deletions(-)
> rename tests/qemumonitorjsondata/{get-guest-cpu-SierraForest.json =>
> get-guest-cpu-SierraForest-legacy.json} (100%)
> rename tests/qemumonitorjsondata/{get-guest-cpu-SkylakeClient.json =>
> get-guest-cpu-SkylakeClient-legacy.json} (100%)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 688d100b01..b7174c657d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -742,6 +742,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
> "amd-iommu.pci-id", /* QEMU_CAPS_AMD_IOMMU_PCI_ID */
> "usb-bot", /* QEMU_CAPS_DEVICE_USB_BOT */
> "tdx-guest", /* QEMU_CAPS_TDX_GUEST */
> + "qom-list-get", /* QEMU_CAPS_QOM_LIST_GET */
> );
>
>
> @@ -1256,6 +1257,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
> { "query-stats-schemas", QEMU_CAPS_QUERY_STATS_SCHEMAS },
> { "display-reload", QEMU_CAPS_DISPLAY_RELOAD },
> { "blockdev-set-active", QEMU_CAPS_BLOCKDEV_SET_ACTIVE },
> + { "qom-list-get", QEMU_CAPS_QOM_LIST_GET },
> };
>
> struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 8916973364..f50d908b3f 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -723,6 +723,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
> syntax-check */
> QEMU_CAPS_AMD_IOMMU_PCI_ID, /* amd-iommu.pci-id */
> QEMU_CAPS_DEVICE_USB_BOT, /* -device usb-bot */
> QEMU_CAPS_TDX_GUEST, /* -object tdx-guest,... */
> + QEMU_CAPS_QOM_LIST_GET, /* qom-list-get QMP command */
>
> QEMU_CAPS_LAST /* this must always be the last item */
> } virQEMUCapsFlags;
Preferrably introduce new capabilities in separate patch.
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 0213bd5af8..176651eab4 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3673,6 +3673,8 @@ qemuMonitorSetDomainLog(qemuMonitor *mon,
> * qemuMonitorGetGuestCPU:
> * @mon: Pointer to the monitor
> * @arch: CPU architecture
> + * @qomListGet: QEMU supports getting list of features and their values using
> + * a single qom-list-get QMP command
> * @cpuQOMPath: QOM path of a CPU to probe
> * @translate: callback for translating CPU feature names from QEMU to
> libvirt
> * @opaque: data for @translate callback
> @@ -3687,13 +3689,16 @@ qemuMonitorSetDomainLog(qemuMonitor *mon,
> int
> qemuMonitorGetGuestCPU(qemuMonitor *mon,
> virArch arch,
> + bool qomListGet,
> const char *cpuQOMPath,
> qemuMonitorCPUFeatureTranslationCallback translate,
> virCPUData **enabled,
> virCPUData **disabled)
> {
> - VIR_DEBUG("arch=%s cpuQOMPath=%s translate=%p enabled=%p disabled=%p",
> - virArchToString(arch), cpuQOMPath, translate, enabled,
> disabled);
> + VIR_DEBUG("arch=%s qomListGet%d cpuQOMPath=%s translate=%p "
qomListGet=%d instead of qomListGet%d
> + "enabled=%p disabled=%p",
> + virArchToString(arch), qomListGet, cpuQOMPath, translate,
> + enabled, disabled);
>
> QEMU_CHECK_MONITOR(mon);
>
[...]
> @@ -6631,14 +6648,29 @@ qemuMonitorJSONGetCPUProperties(qemuMonitor *mon,
> virJSONValue *array;
> struct _qemuMonitorJSONCPUPropsFilterData filterData = {
> .mon = mon,
> + .values = qomListGet,
> .cpuQOMPath = cpuQOMPath,
> };
>
> *props = NULL;
>
> - if (!(cmd = qemuMonitorJSONMakeCommand("qom-list",
> - "s:path", cpuQOMPath,
> - NULL)))
> + if (qomListGet) {
> + g_autoptr(virJSONValue) paths = virJSONValueNewArray();
> + g_autoptr(virJSONValue) path =
> virJSONValueNewString(g_strdup(cpuQOMPath));
> +
> + if (virJSONValueArrayAppend(paths, &path) < 0)
> + return -1;
How about 'virJSONValueArrayAppendString'? instead of the manual
construction?
> +
> + cmd = qemuMonitorJSONMakeCommand("qom-list-get",
> + "a:paths", &paths,
> + NULL);
> + } else {
> + cmd = qemuMonitorJSONMakeCommand("qom-list",
> + "s:path", cpuQOMPath,
> + NULL);
> + }
[...]
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index f076e637ba..f17769f7fe 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -558,15 +558,10 @@ int
> qemuMonitorJSONGetDeviceAliases(qemuMonitor *mon,
> char ***aliases);
>
> -int
> -qemuMonitorJSONGetGuestCPUx86(qemuMonitor *mon,
> - const char *cpuQOMPath,
> - virCPUData **data,
> - virCPUData **disabled);
> -
Leftover from 1/15?
> int
> qemuMonitorJSONGetGuestCPU(qemuMonitor *mon,
> virArch arch,
> + bool qomListGet,
> const char *cpuQOMPath,
> qemuMonitorCPUFeatureTranslationCallback
> translate,
> virCPUData **enabled,
[...]
> --- a/tests/qemumonitorjsontest.c
> +++ b/tests/qemumonitorjsontest.c
> @@ -2773,6 +2773,7 @@ testQemuMonitorJSONGetSEVInfo(const void *opaque)
>
> struct testQemuMonitorJSONGetGuestCPUData {
> const char *name;
> + bool qomListGet;
> virQEMUDriver driver;
> GHashTable *schema;
> };
> @@ -2791,8 +2792,9 @@ testQemuMonitorJSONGetGuestCPU(const void *opaque)
> g_autofree char *enabled = NULL;
> g_autofree char *disabled = NULL;
> bool failed = false;
> + const char *legacy = data->qomListGet ? "" : "-legacy";
>
> - fileJSON = g_strdup_printf("%s-%s.json", base, data->name);
> + fileJSON = g_strdup_printf("%s-%s%s.json", base, data->name, legacy);
> fileEnabled = g_strdup_printf("%s-%s-enabled.xml", base, data->name);
> fileDisabled = g_strdup_printf("%s-%s-disabled.xml", base, data->name);
>
> @@ -2802,6 +2804,7 @@ testQemuMonitorJSONGetGuestCPU(const void *opaque)
>
> if (qemuMonitorJSONGetGuestCPU(qemuMonitorTestGetMonitor(mon),
> VIR_ARCH_X86_64,
> + data->qomListGet,
> "/machine/unattached/device[0]",
> virQEMUCapsCPUFeatureFromQEMU,
> &dataEnabled, &dataDisabled) < 0)
> @@ -2884,11 +2887,14 @@ mymain(void)
> ret = -1; \
> } while (0)
>
> -#define DO_TEST_GET_GUEST_CPU(name) \
> +#define DO_TEST_GET_GUEST_CPU(name, qomListGet) \
> do { \
> struct testQemuMonitorJSONGetGuestCPUData data = { \
> - name, driver, qapiData.schema }; \
> - if (virTestRun("GetGuestCPU(" name ")", \
> + name, qomListGet, driver, qapiData.schema }; \
> + g_autofree char *label = NULL; \
> + label = g_strdup_printf("GetGuestCPU(%s, legacy=%s)", \
> + name, qomListGet ? "false" : "true"); \
Not converting it to a string can save you the ternary use.
With the caps separated and the nits addressed:
Reviewed-by: Peter Krempa <pkre...@redhat.com>
