On Thu, Sep 04, 2025 at 16:46:55 +0200, Jiri Denemark via Devel wrote:
From: Jiri Denemark <jdenemar@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 <jdenemar@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 <pkrempa@redhat.com>