[libvirt] [PATCH 0/6] Use query-cpus-fast instead of query-cpus

The QEMU monitor commmand query-cpus is deprecated starting with QEMU 2.12.0 because it can adversely affect the performance of a running virtual machine. This series enables libvirt to use the new query-cpus-fast interface if supported by the local QEMU instance and is required in order to support QEMU once the interface has been removed. query-cpus-fast doesn't return the halted state for a virtual CPU, meaning that the vcpu.<n>.halted value won't be reported with 'virsh domstats' anymore. This is OK, as stats values are not guaranteed to be reported under all circumstances and on all architectures. Upstream discussion consensus was that the halted state was problematic anyway, as it had different semantics per architecture. The only known exploitation happened for s390, for this architecture the halted state will be computed based on an architecture-specific cpu value returned in query-cpus-fast. Viktor Mihajlovski (6): qemu: add capability detection for query-cpus-fast qemu: use query-cpus-fast in JSON monitor tests: add qemumonitorjson tests for query-cpus-fast qemu: add architecture-specific CPU info handling tests: add testcase for s390 query-cpus-fast qemu: refresh vcpu halted state only via query-cpus-fast src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 9 +- src/qemu/qemu_monitor.c | 14 +- src/qemu/qemu_monitor_json.c | 149 +++++++++++++++++++-- src/qemu/qemu_monitor_json.h | 3 +- .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 ++ .../qemumonitorjson-cpuinfo-s390-fast-cpus.json | 21 +++ .../qemumonitorjson-cpuinfo-s390-fast-hotplug.json | 21 +++ .../qemumonitorjson-cpuinfo-s390-fast.data | 19 +++ ...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 5 + ...qemumonitorjson-cpuinfo-x86-full-fast-cpus.json | 71 ++++++++++ ...umonitorjson-cpuinfo-x86-full-fast-hotplug.json | 115 ++++++++++++++++ .../qemumonitorjson-cpuinfo-x86-full-fast.data | 109 +++++++++++++++ .../qemumonitorjson-cpuinfo-x86-node-full.data | 2 + tests/qemumonitorjsontest.c | 121 +++++++++++++---- tests/qemumonitortestutils.c | 6 + tests/qemumonitortestutils.h | 1 + 18 files changed, 637 insertions(+), 42 deletions(-) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data -- 1.9.1

Detect whether QEMU supports the QMP query-cpus-fast API and set QEMU_CAPS_QUERY_CPUS_FAST in this case. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b5eb8cf..6635f5e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "query-cpus-fast", ); @@ -1579,7 +1580,8 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "query-qmp-schema", QEMU_CAPS_QUERY_QMP_SCHEMA }, { "query-cpu-model-expansion", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION}, { "query-cpu-definitions", QEMU_CAPS_QUERY_CPU_DEFINITIONS}, - { "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES} + { "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES}, + { "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST} }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be..e3c31ab 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 1.9.1

On Fri, Mar 02, 2018 at 10:29:06 +0100, Viktor Mihajlovski wrote:
Detect whether QEMU supports the QMP query-cpus-fast API and set QEMU_CAPS_QUERY_CPUS_FAST in this case.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-)
ACK, although you should add a qemucapabilitiestest case for the new qemu supporting this command first probably.

On 03/02/2018 12:44 PM, Peter Krempa wrote:
On Fri, Mar 02, 2018 at 10:29:06 +0100, Viktor Mihajlovski wrote:
Detect whether QEMU supports the QMP query-cpus-fast API and set QEMU_CAPS_QUERY_CPUS_FAST in this case.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 4 +++- src/qemu/qemu_capabilities.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-)
ACK, although you should add a qemucapabilitiestest case for the new qemu supporting this command first probably. Shouldn't this test come along automatically when the QEMU 2.12 replies/xml files containing the new qmp method is introduced? Or are you suggesting to create new QEMU 2.11_patched replies/xml files to test this?
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Use query-cpus-fast instead of query-cpus if supported by QEMU. Based on the QEMU_CAPS_QUERY_CPUS_FAST capability. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 10 ++++++++-- src/qemu/qemu_monitor_json.c | 26 +++++++++++++++++--------- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ad5c572..dad1383 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2012,6 +2012,7 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int ret = -1; int rc; qemuMonitorCPUInfoPtr info = NULL; + bool fast; QEMU_CHECK_MONITOR(mon); @@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0) goto cleanup; + fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST); + if (mon->json) - rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug); + rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug, + fast); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries); @@ -2082,7 +2087,8 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon, QEMU_CHECK_MONITOR_NULL(mon); if (mon->json) - rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false); + rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false, + false); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a09e93e..2ecdf0a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1466,7 +1466,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) static int qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, struct qemuMonitorQueryCpusEntry **entries, - size_t *nentries) + size_t *nentries, + bool fast) { struct qemuMonitorQueryCpusEntry *cpus = NULL; int ret = -1; @@ -1491,11 +1492,13 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, } /* Some older qemu versions don't report the thread_id so treat this as - * non-fatal, simply returning no data */ - ignore_value(virJSONValueObjectGetNumberInt(entry, "CPU", &cpuid)); - ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread)); + * non-fatal, simply returning no data. + * The return data of query-cpus-fast has different field names + */ + ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "cpu-index" : "CPU", &cpuid)); + ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "thread-id" : "thread_id", &thread)); ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted)); - qom_path = virJSONValueObjectGetString(entry, "qom_path"); + qom_path = virJSONValueObjectGetString(entry, fast ? "qom-path" : "qom_path"); cpus[i].qemu_id = cpuid; cpus[i].tid = thread; @@ -1520,10 +1523,12 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, * @mon: monitor object * @entries: filled with detected entries on success * @nentries: number of entries returned + * @force: force exit on error + * @fast: use query-cpus-fast * * Queries qemu for cpu-related information. Failure to execute the command or * extract results does not produce an error as libvirt can continue without - * this information. + * this information, unless the caller has specified @force == true. * * Returns 0 on success, -1 on a fatal error (oom ...) and -2 if the * query failed gracefully. @@ -1532,10 +1537,13 @@ int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon, struct qemuMonitorQueryCpusEntry **entries, size_t *nentries, - bool force) + bool force, + bool fast) { int ret = -1; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL); + virJSONValuePtr cmd = + qemuMonitorJSONMakeCommand(fast ? "query-cpus-fast" : "query-cpus", + NULL); virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -1553,7 +1561,7 @@ qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon, goto cleanup; } - ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries); + ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries, fast); cleanup: virJSONValueFree(cmd); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ec243be..e03299a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -60,7 +60,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon); int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon, struct qemuMonitorQueryCpusEntry **entries, size_t *nentries, - bool force); + bool force, + bool fast); int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, virDomainVirtType *virtType); int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 908ec3a..6824623 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1420,7 +1420,7 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data) goto cleanup; if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test), - &cpudata, &ncpudata, true) < 0) + &cpudata, &ncpudata, true, false) < 0) goto cleanup; if (ncpudata != 4) { -- 1.9.1

On Fri, Mar 02, 2018 at 10:29:07 +0100, Viktor Mihajlovski wrote:
Use query-cpus-fast instead of query-cpus if supported by QEMU. Based on the QEMU_CAPS_QUERY_CPUS_FAST capability.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 10 ++++++++-- src/qemu/qemu_monitor_json.c | 26 +++++++++++++++++--------- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ad5c572..dad1383 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2012,6 +2012,7 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int ret = -1; int rc; qemuMonitorCPUInfoPtr info = NULL; + bool fast;
QEMU_CHECK_MONITOR(mon);
@@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0) goto cleanup;
+ fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
Don't do this. You'll need to determine it sooner. The main reason being that once you are in the monitor code, 'vm' is unlocked and should not be accesssed. Also we don't access the VM object trhough the mon object usually.
+ QEMU_CAPS_QUERY_CPUS_FAST); + if (mon->json) - rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug); + rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug, + fast); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
@@ -2082,7 +2087,8 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon, QEMU_CHECK_MONITOR_NULL(mon);
if (mon->json) - rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false); + rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false, + false); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a09e93e..2ecdf0a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1466,7 +1466,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) static int qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, struct qemuMonitorQueryCpusEntry **entries, - size_t *nentries) + size_t *nentries, + bool fast) { struct qemuMonitorQueryCpusEntry *cpus = NULL; int ret = -1; @@ -1491,11 +1492,13 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, }
/* Some older qemu versions don't report the thread_id so treat this as - * non-fatal, simply returning no data */ - ignore_value(virJSONValueObjectGetNumberInt(entry, "CPU", &cpuid)); - ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread)); + * non-fatal, simply returning no data. + * The return data of query-cpus-fast has different field names + */ + ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "cpu-index" : "CPU", &cpuid)); + ignore_value(virJSONValueObjectGetNumberInt(entry, fast ? "thread-id" : "thread_id", &thread));
No ternary operators please. Add an if-block and two code paths.
ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted)); - qom_path = virJSONValueObjectGetString(entry, "qom_path"); + qom_path = virJSONValueObjectGetString(entry, fast ? "qom-path" : "qom_path");
cpus[i].qemu_id = cpuid; cpus[i].tid = thread; @@ -1520,10 +1523,12 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, * @mon: monitor object * @entries: filled with detected entries on success * @nentries: number of entries returned + * @force: force exit on error + * @fast: use query-cpus-fast * * Queries qemu for cpu-related information. Failure to execute the command or * extract results does not produce an error as libvirt can continue without - * this information. + * this information, unless the caller has specified @force == true. * * Returns 0 on success, -1 on a fatal error (oom ...) and -2 if the * query failed gracefully. @@ -1532,10 +1537,13 @@ int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon, struct qemuMonitorQueryCpusEntry **entries, size_t *nentries, - bool force) + bool force, + bool fast) { int ret = -1; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL); + virJSONValuePtr cmd = + qemuMonitorJSONMakeCommand(fast ? "query-cpus-fast" : "query-cpus", + NULL);
Same here, no ternaries please.
virJSONValuePtr reply = NULL; virJSONValuePtr data;
@@ -1553,7 +1561,7 @@ qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon, goto cleanup; }
- ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries); + ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries, fast);
cleanup: virJSONValueFree(cmd); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ec243be..e03299a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -60,7 +60,8 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon); int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon, struct qemuMonitorQueryCpusEntry **entries, size_t *nentries, - bool force); + bool force, + bool fast); int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, virDomainVirtType *virtType); int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 908ec3a..6824623 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1420,7 +1420,7 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data) goto cleanup;
if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test), - &cpudata, &ncpudata, true) < 0) + &cpudata, &ncpudata, true, false) < 0) goto cleanup;
if (ncpudata != 4) { -- 1.9.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 02.03.2018 12:33, Peter Krempa wrote: [...]
@@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0) goto cleanup;
+ fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
Don't do this. You'll need to determine it sooner. The main reason being that once you are in the monitor code, 'vm' is unlocked and should not be accesssed. Also we don't access the VM object trhough the mon object usually. OK. I've been struggling over this part since the availability of query-cpus-fast is tied to the QEMU instance and I didn't want to duplicate the capability in the qemuMonitor struct. Looks like I have to ...
I'll rework the series (including your other comments). [...] -- Regards, Viktor Mihajlovski

On Fri, Mar 02, 2018 at 13:32:19 +0100, Viktor Mihajlovski wrote:
On 02.03.2018 12:33, Peter Krempa wrote: [...]
@@ -2028,8 +2029,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, (qemuMonitorJSONGetHotpluggableCPUs(mon, &hotplugcpus, &nhotplugcpus)) < 0) goto cleanup;
+ fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
Don't do this. You'll need to determine it sooner. The main reason being that once you are in the monitor code, 'vm' is unlocked and should not be accesssed. Also we don't access the VM object trhough the mon object usually. OK. I've been struggling over this part since the availability of query-cpus-fast is tied to the QEMU instance and I didn't want to duplicate the capability in the qemuMonitor struct. Looks like I have to ...
You can pass it from the caller as we do in more than one place.

Extended the json monitor test program with support for query-cpus-fast and added a sample file set for x86 data obtained using the it. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- ...qemumonitorjson-cpuinfo-x86-full-fast-cpus.json | 71 +++++++++++++ ...umonitorjson-cpuinfo-x86-full-fast-hotplug.json | 115 ++++++++++++++++++++ .../qemumonitorjson-cpuinfo-x86-full-fast.data | 109 +++++++++++++++++++ tests/qemumonitorjsontest.c | 116 ++++++++++++++++----- tests/qemumonitortestutils.c | 6 ++ tests/qemumonitortestutils.h | 1 + 6 files changed, 393 insertions(+), 25 deletions(-) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json new file mode 100644 index 0000000..88fd2b8 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json @@ -0,0 +1,71 @@ +{ + "return": [ + { + "arch": "x86", + "cpu-index": 0, + "qom-path": "/machine/unattached/device[0]", + "thread-id": 895040 + }, + { + "arch": "x86", + "cpu-index": 1, + "qom-path": "/machine/peripheral/vcpu1", + "thread-id": 895056 + }, + { + "arch": "x86", + "cpu-index": 2, + "qom-path": "/machine/peripheral/vcpu2", + "thread-id": 895057 + }, + { + "arch": "x86", + "cpu-index": 3, + "qom-path": "/machine/peripheral/vcpu3", + "thread-id": 895058 + }, + { + "arch": "x86", + "cpu-index": 4, + "qom-path": "/machine/peripheral/vcpu4", + "thread-id": 895059 + }, + { + "arch": "x86", + "cpu-index": 5, + "qom-path": "/machine/peripheral/vcpu5", + "thread-id": 895060 + }, + { + "arch": "x86", + "cpu-index": 6, + "qom-path": "/machine/peripheral/vcpu6", + "thread-id": 895061 + }, + { + "arch": "x86", + "cpu-index": 7, + "qom-path": "/machine/peripheral/vcpu7", + "thread-id": 895062 + }, + { + "arch": "x86", + "cpu-index": 8, + "qom-path": "/machine/peripheral/vcpu8", + "thread-id": 895063 + }, + { + "arch": "x86", + "cpu-index": 9, + "qom-path": "/machine/peripheral/vcpu9", + "thread-id": 895064 + }, + { + "arch": "x86", + "cpu-index": 10, + "qom-path": "/machine/peripheral/vcpu10", + "thread-id": 895065 + } + ], + "id": "libvirt-52" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json new file mode 100644 index 0000000..aff5aa3 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json @@ -0,0 +1,115 @@ +{ + "return": [ + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 10 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu10", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 9 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu9", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 8 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu8", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 7 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu7", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 6 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu6", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 5 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu5", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 4 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu4", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 3 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu3", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 2 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu2", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 1 + }, + "vcpus-count": 1, + "qom-path": "/machine/peripheral/vcpu1", + "type": "Broadwell-x86_64-cpu" + }, + { + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 0 + }, + "vcpus-count": 1, + "qom-path": "/machine/unattached/device[0]", + "type": "Broadwell-x86_64-cpu" + } + ], + "id": "libvirt-51" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data new file mode 100644 index 0000000..1908e39 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data @@ -0,0 +1,109 @@ +[vcpu libvirt-id='0'] + online=yes + hotpluggable=no + thread-id='895040' + enable-id='1' + query-cpus-id='0' + type='Broadwell-x86_64-cpu' + qom_path='/machine/unattached/device[0]' + topology: socket='0' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='1'] + online=yes + hotpluggable=yes + thread-id='895056' + enable-id='2' + query-cpus-id='1' + type='Broadwell-x86_64-cpu' + alias='vcpu1' + qom_path='/machine/peripheral/vcpu1' + topology: socket='1' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='2'] + online=yes + hotpluggable=yes + thread-id='895057' + enable-id='3' + query-cpus-id='2' + type='Broadwell-x86_64-cpu' + alias='vcpu2' + qom_path='/machine/peripheral/vcpu2' + topology: socket='2' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='3'] + online=yes + hotpluggable=yes + thread-id='895058' + enable-id='4' + query-cpus-id='3' + type='Broadwell-x86_64-cpu' + alias='vcpu3' + qom_path='/machine/peripheral/vcpu3' + topology: socket='3' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='4'] + online=yes + hotpluggable=yes + thread-id='895059' + enable-id='5' + query-cpus-id='4' + type='Broadwell-x86_64-cpu' + alias='vcpu4' + qom_path='/machine/peripheral/vcpu4' + topology: socket='4' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='5'] + online=yes + hotpluggable=yes + thread-id='895060' + enable-id='6' + query-cpus-id='5' + type='Broadwell-x86_64-cpu' + alias='vcpu5' + qom_path='/machine/peripheral/vcpu5' + topology: socket='5' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='6'] + online=yes + hotpluggable=yes + thread-id='895061' + enable-id='7' + query-cpus-id='6' + type='Broadwell-x86_64-cpu' + alias='vcpu6' + qom_path='/machine/peripheral/vcpu6' + topology: socket='6' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='7'] + online=yes + hotpluggable=yes + thread-id='895062' + enable-id='8' + query-cpus-id='7' + type='Broadwell-x86_64-cpu' + alias='vcpu7' + qom_path='/machine/peripheral/vcpu7' + topology: socket='7' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='8'] + online=yes + hotpluggable=yes + thread-id='895063' + enable-id='9' + query-cpus-id='8' + type='Broadwell-x86_64-cpu' + alias='vcpu8' + qom_path='/machine/peripheral/vcpu8' + topology: socket='8' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='9'] + online=yes + hotpluggable=yes + thread-id='895064' + enable-id='10' + query-cpus-id='9' + type='Broadwell-x86_64-cpu' + alias='vcpu9' + qom_path='/machine/peripheral/vcpu9' + topology: socket='9' core='0' thread='0' vcpus='1' +[vcpu libvirt-id='10'] + online=yes + hotpluggable=yes + thread-id='895065' + enable-id='11' + query-cpus-id='10' + type='Broadwell-x86_64-cpu' + alias='vcpu10' + qom_path='/machine/peripheral/vcpu10' + topology: socket='10' core='0' thread='0' vcpus='1' diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 6824623..f23614e 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1360,6 +1360,41 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntr return true; } +static int +testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTestPtr test, + struct qemuMonitorQueryCpusEntry *expect, + bool fast, size_t num) +{ + struct qemuMonitorQueryCpusEntry *cpudata = NULL; + size_t ncpudata = 0; + size_t i; + int ret = -1; + + if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test), + &cpudata, &ncpudata, true, fast) < 0) + goto cleanup; + + if (ncpudata != num) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expecting ncpupids = %zu but got %zu", num, ncpudata); + goto cleanup; + } + + for (i = 0; i < ncpudata; i++) { + if (!testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(cpudata + i, + expect + i)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "vcpu entry %zu does not match expected data", i); + goto cleanup; + } + } + + ret = 0; + + cleanup: + qemuMonitorQueryCpusFree(cpudata, ncpudata); + return ret; +} static int testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data) @@ -1367,15 +1402,16 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data) virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt); int ret = -1; - struct qemuMonitorQueryCpusEntry *cpudata = NULL; - struct qemuMonitorQueryCpusEntry expect[] = { - {0, 17622, (char *) "/machine/unattached/device[0]", true}, - {1, 17624, (char *) "/machine/unattached/device[1]", true}, - {2, 17626, (char *) "/machine/unattached/device[2]", true}, - {3, 17628, NULL, true}, + struct qemuMonitorQueryCpusEntry expect_slow[] = { + {0, 17622, (char *) "/machine/unattached/device[0]", true}, + {1, 17624, (char *) "/machine/unattached/device[1]", true}, + {2, 17626, (char *) "/machine/unattached/device[2]", true}, + {3, 17628, NULL, true}, + }; + struct qemuMonitorQueryCpusEntry expect_fast[] = { + {0, 17629, (char *) "/machine/unattached/device[0]", false}, + {1, 17630, (char *) "/machine/unattached/device[1]", false}, }; - size_t ncpudata = 0; - size_t i; if (!test) return -1; @@ -1419,29 +1455,37 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data) "}") < 0) goto cleanup; - if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test), - &cpudata, &ncpudata, true, false) < 0) + if (qemuMonitorTestAddItem(test, "query-cpus-fast", + "{" + " \"return\": [" + " {" + " \"cpu-index\": 0," + " \"qom-path\": \"/machine/unattached/device[0]\"," + " \"thread-id\": 17629" + " }," + " {" + " \"cpu-index\": 1," + " \"qom-path\": \"/machine/unattached/device[1]\"," + " \"thread-id\": 17630" + " }" + " ]," + " \"id\": \"libvirt-8\"" + "}") < 0) goto cleanup; - if (ncpudata != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "Expecting ncpupids = 4 but got %zu", ncpudata); + /* query-cpus */ + if (testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(test, expect_slow, + false, 4)) goto cleanup; - } - for (i = 0; i < ncpudata; i++) { - if (!testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(cpudata + i, - expect + i)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "vcpu entry %zu does not match expected data", i); - goto cleanup; - } - } + /* query-cpus-fast */ + if (testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(test, expect_fast, + true, 2)) + goto cleanup; ret = 0; cleanup: - qemuMonitorQueryCpusFree(cpudata, ncpudata); qemuMonitorTestFree(test); return ret; } @@ -2612,6 +2656,7 @@ struct testCPUInfoData { const char *name; size_t maxvcpus; virDomainXMLOptionPtr xmlopt; + bool fast; }; @@ -2678,6 +2723,7 @@ testQemuMonitorCPUInfo(const void *opaque) { const struct testCPUInfoData *data = opaque; qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, data->xmlopt); + virDomainObjPtr vm = NULL; char *queryCpusFile = NULL; char *queryHotpluggableFile = NULL; char *dataFile = NULL; @@ -2712,9 +2758,20 @@ testQemuMonitorCPUInfo(const void *opaque) queryHotpluggableStr) < 0) goto cleanup; - if (qemuMonitorTestAddItem(test, "query-cpus", queryCpusStr) < 0) + if (qemuMonitorTestAddItem(test, data->fast ? "query-cpus-fast" : "query-cpus", + queryCpusStr) < 0) goto cleanup; + vm = qemuMonitorTestGetDomainObj(test); + if (!vm) + return -1; + + if (data->fast) { + QEMU_DOMAIN_PRIVATE(vm)->qemuCaps = virQEMUCapsNew(); + virQEMUCapsSet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST); + } + rc = qemuMonitorGetCPUInfo(qemuMonitorTestGetMonitor(test), &vcpus, data->maxvcpus, true); @@ -2872,7 +2929,15 @@ mymain(void) #define DO_TEST_CPU_INFO(name, maxvcpus) \ do { \ - struct testCPUInfoData data = {name, maxvcpus, driver.xmlopt}; \ + struct testCPUInfoData data = {name, maxvcpus, driver.xmlopt, false}; \ + if (virTestRun("GetCPUInfo(" name ")", testQemuMonitorCPUInfo, \ + &data) < 0) \ + ret = -1; \ + } while (0) + +#define DO_TEST_CPU_INFO_FAST(name, maxvcpus) \ + do { \ + struct testCPUInfoData data = {name, maxvcpus, driver.xmlopt, true}; \ if (virTestRun("GetCPUInfo(" name ")", testQemuMonitorCPUInfo, \ &data) < 0) \ ret = -1; \ @@ -2956,6 +3021,7 @@ mymain(void) DO_TEST_CPU_INFO("x86-basic-pluggable", 8); DO_TEST_CPU_INFO("x86-full", 11); DO_TEST_CPU_INFO("x86-node-full", 8); + DO_TEST_CPU_INFO_FAST("x86-full-fast", 11); DO_TEST_CPU_INFO("ppc64-basic", 24); DO_TEST_CPU_INFO("ppc64-hotplug-1", 24); diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 5e30fb0..b20d59a 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1439,3 +1439,9 @@ qemuMonitorTestGetAgent(qemuMonitorTestPtr test) { return test->agent; } + +virDomainObjPtr +qemuMonitorTestGetDomainObj(qemuMonitorTestPtr test) +{ + return test->vm; +} diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index 8b19b37..3604a1b 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -96,5 +96,6 @@ void qemuMonitorTestFree(qemuMonitorTestPtr test); qemuMonitorPtr qemuMonitorTestGetMonitor(qemuMonitorTestPtr test); qemuAgentPtr qemuMonitorTestGetAgent(qemuMonitorTestPtr test); +virDomainObjPtr qemuMonitorTestGetDomainObj(qemuMonitorTestPtr test); #endif /* __VIR_QEMU_MONITOR_TEST_UTILS_H__ */ -- 1.9.1

Extract architecture specific data from query-cpus[-fast] if available. A new function qemuMonitorJSONExtractCPUArchInfo() uses a call-back table to find and call architecture-specific extraction handlers. Initially, there's a handler for s390 cpu info to set the halted property depending on the s390 cpu state returned by QEMU. With this it's still possible to report the halted condition even when using query-cpus-fast. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 6 ++- src/qemu/qemu_monitor_json.c | 123 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dad1383..5a0e8a7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2083,12 +2083,16 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon, size_t i; int rc; virBitmapPtr ret = NULL; + bool fast; QEMU_CHECK_MONITOR_NULL(mon); + fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST); + if (mon->json) rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false, - false); + fast); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2ecdf0a..a408cfd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1451,15 +1451,128 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) } -/* +typedef struct { + const char *state; + bool halted; +} s390CpuStateMap; + +/** + * qemuMonitorJSONExtractCPUS390Info: + * @jsoncpu: pointer to a single JSON cpu entry + * @cpu: pointer to a single cpu entry + * + * Derive the legacy cpu info 'halted' information + * from the more accurate s390 cpu state. @cpu is + * modified only on success. + * + * Note: the 'uninitialized' s390 cpu state can't be + * mapped to halted yes/no. + * + * A s390 cpu entry could look like this + * { "arch": "s390", + * "cpu-index": 0, + * "qom-path": "/machine/unattached/device[0]", + * "thread_id": 3081, + * "cpu-state": "operating" } + * + * Returns 0 on success, and -2 if no cpu-state field was + * found or the state value is unknown. + */ +static int +qemuMonitorJSONExtractCPUS390Info(virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu) +{ + const char *cpu_state; + s390CpuStateMap states[] = { + { "operating", false}, + { "load", false}, + { "stopped", true}, + { "check-stop", true}, + }; + size_t i; + + if ((cpu_state = virJSONValueObjectGetString(jsoncpu, "cpu-state"))) { + for (i = 0; i < ARRAY_CARDINALITY(states); i++) { + if (STREQ(states[i].state, cpu_state)) { + cpu->halted = states[i].halted; + return 0; + } + } + } + + return -2; +} + +typedef struct { + const char *arch; + int (*extract)(virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu); +} qemuCpuArchInfoHandler; + + +/** + * qemuMonitorJSONExtractCPUArchInfo: + * @arch: virtual CPU's architecture + * @jsoncpu: pointer to a single JSON cpu entry + * @cpu: pointer to a single cpu entry + * + * Extracts architecure specific virtual CPU data for a single + * CPU from the QAPI response using an architecture specific + * callback. * + * To add a support for a new architecture, extend the array + * 'handlers' with a line like + * ... + * { "newarch", qemuMonitorJSONExtractCPUNewarch }, + * ... + * and implement the extraction callback. + * Check the QEMU QAPI schema for valid architecture names. + * + * Returns the called handler's return value or 0, if no handler + * was called. + */ +static int +qemuMonitorJSONExtractCPUArchInfo(const char *arch, virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu) +{ + qemuCpuArchInfoHandler handlers[] = { + { "s390", qemuMonitorJSONExtractCPUS390Info }, + }; + size_t i; + + for (i = 0; i < ARRAY_CARDINALITY(handlers); i++) { + if (STREQ(handlers[i].arch, arch)) + return handlers[i].extract(jsoncpu, cpu); + } + + return 0; +} + +/** + * qemuMonitorJSONExtractCPUArchInfo: + * @data: JSON response data + * @entries: filled with detected cpu entries on success + * @nentries: number of entries returned + * @fast: true if this is a response from query-cpus-fast + * + * The JSON response @data will have the following format + * in case @fast == false * [{ "arch": "x86", * "current": true, * "CPU": 0, * "qom_path": "/machine/unattached/device[0]", * "pc": -2130415978, * "halted": true, - * "thread_id": 2631237}, + * "thread_id": 2631237, + * ...}, + * {...} + * ] + * and for @fast == true + * [{ "arch": "x86", + * "cpu-index": 0, + * "qom-path": "/machine/unattached/device[0]", + * "thread_id": 2631237, + * ...}, * {...} * ] */ @@ -1486,6 +1599,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, int thread = 0; bool halted = false; const char *qom_path; + const char *arch; if (!entry) { ret = -2; goto cleanup; @@ -1505,6 +1619,11 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, cpus[i].halted = halted; if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0) goto cleanup; + + /* process optional architecture-specific data */ + if ((arch = virJSONValueObjectGetString(entry, "arch"))) + ignore_value(qemuMonitorJSONExtractCPUArchInfo(arch, entry, + cpus + i)); } VIR_STEAL_PTR(*entries, cpus); -- 1.9.1

On Fri, Mar 02, 2018 at 10:29:09 +0100, Viktor Mihajlovski wrote:
Extract architecture specific data from query-cpus[-fast] if available. A new function qemuMonitorJSONExtractCPUArchInfo() uses a call-back table to find and call architecture-specific extraction handlers.
Initially, there's a handler for s390 cpu info to set the halted property depending on the s390 cpu state returned by QEMU. With this it's still possible to report the halted condition even when using query-cpus-fast.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 6 ++- src/qemu/qemu_monitor_json.c | 123 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dad1383..5a0e8a7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2083,12 +2083,16 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon, size_t i; int rc; virBitmapPtr ret = NULL; + bool fast;
QEMU_CHECK_MONITOR_NULL(mon);
+ fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(mon->vm)->qemuCaps,
As said, mon->vm should not be used here.
+ QEMU_CAPS_QUERY_CPUS_FAST); + if (mon->json) rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, false, - false); + fast); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2ecdf0a..a408cfd 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1451,15 +1451,128 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) }
-/* +typedef struct { + const char *state; + bool halted; +} s390CpuStateMap; + +/** + * qemuMonitorJSONExtractCPUS390Info: + * @jsoncpu: pointer to a single JSON cpu entry + * @cpu: pointer to a single cpu entry + * + * Derive the legacy cpu info 'halted' information + * from the more accurate s390 cpu state. @cpu is + * modified only on success. + * + * Note: the 'uninitialized' s390 cpu state can't be + * mapped to halted yes/no. + * + * A s390 cpu entry could look like this + * { "arch": "s390", + * "cpu-index": 0, + * "qom-path": "/machine/unattached/device[0]", + * "thread_id": 3081, + * "cpu-state": "operating" } + * + * Returns 0 on success, and -2 if no cpu-state field was + * found or the state value is unknown. + */ +static int +qemuMonitorJSONExtractCPUS390Info(virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu) +{ + const char *cpu_state; + s390CpuStateMap states[] = { + { "operating", false}, + { "load", false}, + { "stopped", true}, + { "check-stop", true}, + }; + size_t i; + + if ((cpu_state = virJSONValueObjectGetString(jsoncpu, "cpu-state"))) { + for (i = 0; i < ARRAY_CARDINALITY(states); i++) { + if (STREQ(states[i].state, cpu_state)) { + cpu->halted = states[i].halted; + return 0; + } + } + } + + return -2; +} + +typedef struct { + const char *arch; + int (*extract)(virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu); +} qemuCpuArchInfoHandler; + + +/** + * qemuMonitorJSONExtractCPUArchInfo: + * @arch: virtual CPU's architecture + * @jsoncpu: pointer to a single JSON cpu entry + * @cpu: pointer to a single cpu entry + * + * Extracts architecure specific virtual CPU data for a single + * CPU from the QAPI response using an architecture specific + * callback. * + * To add a support for a new architecture, extend the array + * 'handlers' with a line like + * ... + * { "newarch", qemuMonitorJSONExtractCPUNewarch },
This should be obvious from the code.
+ * ... + * and implement the extraction callback. + * Check the QEMU QAPI schema for valid architecture names. + * + * Returns the called handler's return value or 0, if no handler + * was called. + */ +static int +qemuMonitorJSONExtractCPUArchInfo(const char *arch, virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu)
Please don't mix header alignment styles.
+{ + qemuCpuArchInfoHandler handlers[] = { + { "s390", qemuMonitorJSONExtractCPUS390Info }, + }; + size_t i; + + for (i = 0; i < ARRAY_CARDINALITY(handlers); i++) { + if (STREQ(handlers[i].arch, arch)) + return handlers[i].extract(jsoncpu, cpu);
You can remove the callback topology and hard-code the STREQs. It will result in the same execution complexity, but be clearer to understand than the callback-topology.
+ } + + return 0; +} + +/** + * qemuMonitorJSONExtractCPUArchInfo: + * @data: JSON response data + * @entries: filled with detected cpu entries on success + * @nentries: number of entries returned + * @fast: true if this is a response from query-cpus-fast + * + * The JSON response @data will have the following format + * in case @fast == false * [{ "arch": "x86", * "current": true, * "CPU": 0, * "qom_path": "/machine/unattached/device[0]", * "pc": -2130415978, * "halted": true, - * "thread_id": 2631237}, + * "thread_id": 2631237, + * ...}, + * {...} + * ] + * and for @fast == true + * [{ "arch": "x86", + * "cpu-index": 0, + * "qom-path": "/machine/unattached/device[0]", + * "thread_id": 2631237, + * ...}, * {...} * ] */ @@ -1486,6 +1599,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, int thread = 0; bool halted = false; const char *qom_path; + const char *arch; if (!entry) { ret = -2; goto cleanup; @@ -1505,6 +1619,11 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, cpus[i].halted = halted; if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0) goto cleanup; + + /* process optional architecture-specific data */ + if ((arch = virJSONValueObjectGetString(entry, "arch"))) + ignore_value(qemuMonitorJSONExtractCPUArchInfo(arch, entry, + cpus + i));
So why does this function try to return something if the only caller does not care?
}
VIR_STEAL_PTR(*entries, cpus); -- 1.9.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

The s390 testcase verifies that the s390-specific cpu-state field is correctly mapped to the halted property. Since a few of the x86 and ppc testcases return "halted": "true" it was necessary to update the respective .data files. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 ++++++++ .../qemumonitorjson-cpuinfo-s390-fast-cpus.json | 21 +++++++++++++++++++++ .../qemumonitorjson-cpuinfo-s390-fast-hotplug.json | 21 +++++++++++++++++++++ .../qemumonitorjson-cpuinfo-s390-fast.data | 19 +++++++++++++++++++ ...qemumonitorjson-cpuinfo-x86-basic-pluggable.data | 5 +++++ .../qemumonitorjson-cpuinfo-x86-node-full.data | 2 ++ tests/qemumonitorjsontest.c | 5 +++++ 7 files changed, 81 insertions(+) create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data index 7c90889..5f6b865 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data @@ -52,41 +52,49 @@ alias='vcpu0' qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' + halted [vcpu libvirt-id='9'] online=yes hotpluggable=yes thread-id='23171' query-cpus-id='17' + halted [vcpu libvirt-id='10'] online=yes hotpluggable=yes thread-id='23172' query-cpus-id='18' + halted [vcpu libvirt-id='11'] online=yes hotpluggable=yes thread-id='23173' query-cpus-id='19' + halted [vcpu libvirt-id='12'] online=yes hotpluggable=yes thread-id='23174' query-cpus-id='20' + halted [vcpu libvirt-id='13'] online=yes hotpluggable=yes thread-id='23175' query-cpus-id='21' + halted [vcpu libvirt-id='14'] online=yes hotpluggable=yes thread-id='23176' query-cpus-id='22' + halted [vcpu libvirt-id='15'] online=yes hotpluggable=yes thread-id='23177' query-cpus-id='23' + halted [vcpu libvirt-id='16'] online=yes hotpluggable=yes diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json new file mode 100644 index 0000000..4082e0e --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json @@ -0,0 +1,21 @@ +{ + "return": [ + { + "arch": "s390", + "cpu-index": 0, + "props": {"core-id": 0}, + "qom-path": "/machine/unattached/device[0]", + "thread-id": 89504, + "cpu-state": "operating" + }, + { + "arch": "s390", + "cpu-index": 1, + "props": {"core-id": 1}, + "qom-path": "/machine/unattached/device[1]", + "thread-id": 89505, + "cpu-state": "stopped" + } + ], + "id": "libvirt-42" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json new file mode 100644 index 0000000..8016b5b --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json @@ -0,0 +1,21 @@ +{ + "return": [ + { + "props": { + "core-id": 1 + }, + "vcpus-count": 1, + "qom-path": "/machine/unattached/device[1]", + "type": "host-s390x-cpu" + }, + { + "props": { + "core-id": 0 + }, + "vcpus-count": 1, + "qom-path": "/machine/unattached/device[0]", + "type": "host-s390x-cpu" + } + ], + "id": "libvirt-41" +} diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data new file mode 100644 index 0000000..9fc7041 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data @@ -0,0 +1,19 @@ +[vcpu libvirt-id='0'] + online=yes + hotpluggable=no + thread-id='89504' + enable-id='1' + query-cpus-id='0' + type='host-s390x-cpu' + qom_path='/machine/unattached/device[0]' + topology: core='0' vcpus='1' +[vcpu libvirt-id='1'] + online=yes + hotpluggable=no + thread-id='89505' + enable-id='2' + query-cpus-id='1' + type='host-s390x-cpu' + qom_path='/machine/unattached/device[1]' + topology: core='1' vcpus='1' + halted diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data index 93cefb9..9a1788d 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data @@ -7,6 +7,7 @@ type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[0]' topology: socket='0' core='0' thread='0' vcpus='1' + halted [vcpu libvirt-id='1'] online=yes hotpluggable=no @@ -16,6 +17,7 @@ type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[2]' topology: socket='0' core='0' thread='1' vcpus='1' + halted [vcpu libvirt-id='2'] online=yes hotpluggable=no @@ -25,6 +27,7 @@ type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[3]' topology: socket='0' core='1' thread='0' vcpus='1' + halted [vcpu libvirt-id='3'] online=yes hotpluggable=no @@ -34,6 +37,7 @@ type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[4]' topology: socket='0' core='1' thread='1' vcpus='1' + halted [vcpu libvirt-id='4'] online=yes hotpluggable=no @@ -43,6 +47,7 @@ type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[5]' topology: socket='1' core='0' thread='0' vcpus='1' + halted [vcpu libvirt-id='5'] online=no hotpluggable=yes diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-node-full.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-node-full.data index 070ea08..0f7dbf1 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-node-full.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-node-full.data @@ -7,6 +7,7 @@ type='Broadwell-x86_64-cpu' qom_path='/machine/unattached/device[0]' topology: socket='0' core='0' thread='0' node='0' vcpus='1' + halted [vcpu libvirt-id='1'] online=yes hotpluggable=no @@ -16,6 +17,7 @@ type='Broadwell-x86_64-cpu' qom_path='/machine/unattached/device[2]' topology: socket='0' core='0' thread='1' node='1' vcpus='1' + halted [vcpu libvirt-id='2'] online=no hotpluggable=yes diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index f23614e..6fe9544 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2711,6 +2711,9 @@ testQemuMonitorCPUInfoFormat(qemuMonitorCPUInfoPtr vcpus, virBufferAddLit(&buf, "\n"); } + if (vcpu->halted) + virBufferAddLit(&buf, "halted\n"); + virBufferAdjustIndent(&buf, -4); } @@ -3029,6 +3032,8 @@ mymain(void) DO_TEST_CPU_INFO("ppc64-hotplug-4", 24); DO_TEST_CPU_INFO("ppc64-no-threads", 16); + DO_TEST_CPU_INFO_FAST("s390-fast", 2); + #define DO_TEST_BLOCK_NODE_DETECT(testname) \ do { \ if (virTestRun("node-name-detect(" testname ")", \ -- 1.9.1

In order to not affect running VMs, refreshing the halted state is only performed if QEMU supports the query-cpus-fast QAPI. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8b4efc8..2eda5c8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8796,8 +8796,13 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, return 0; /* The halted state is interresting only on s390(x). On other platforms - * the data would be stale at the time when it would be used. */ - if (!ARCH_IS_S390(vm->def->os.arch)) + * the data would be stale at the time when it would be used. + * Calling qemuMonitorGetCpuHalted() can adversely affect the running + * VM's performance unless QEMU supports query-cpus-fast. + */ + if (!ARCH_IS_S390(vm->def->os.arch) || + !virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST)) return 0; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) -- 1.9.1
participants (3)
-
Boris Fiuczynski
-
Peter Krempa
-
Viktor Mihajlovski