[libvirt] [PATCHv3 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. v2 -> v3: ========= Patch changes: 1/6: - formatting changes as requested by John Ferlan - updated the qemucapabilitiestest XML files - dropped A/R-bys (due to change above) 2/6: - fixed comment to also account for query-cpus-fast 3/6: - updated json monitor tests to account for props returned by query-cpus and query-cpus-fast - updated json monitor tests to handle the halted property (moved here from patch 5) 4/6: - removed stale text (about callbacks) from commit message - simplified qemuMonitorJSONExtractCPUArchInfo call - enhanced sample JSON response in comment - fixed inter-function spacing 5/6: - kept s390-specific tests and moved the rest to patch 3 - reformatted the JSON responses (indented "props") - dropped R-b v1 -> v2: ========= Patch changes: 1/6: - add Acked-by: Peter Krempa 2/6: - evaluate capability outside of monitor code (changes signatures of qemuMonitorGetCPUInfo and qemuMonitorGetCpuHalted) - remove ternary expressions as requested by review - remove Reviewed-bys due to code changes 3/6 - adapt test cases to changes above - remove Reviewed-bys due to code changes 4/6 - replace callbacks for architecture data extraction with direct function calls - remove Reviewed-bys due to code changes 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 | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 23 +++- src/qemu/qemu_monitor.c | 30 +++-- src/qemu/qemu_monitor.h | 7 +- src/qemu/qemu_monitor_json.c | 133 +++++++++++++++++++-- src/qemu/qemu_monitor_json.h | 3 +- tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 ++ .../qemumonitorjson-cpuinfo-s390-fast-cpus.json | 25 ++++ .../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 | 126 +++++++++++++++++++ ...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 | 123 +++++++++++++++---- tests/qemumonitortestutils.c | 7 ++ tests/qemumonitortestutils.h | 1 + 23 files changed, 706 insertions(+), 58 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> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 6 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 959c27f..4034f1f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -467,6 +467,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio-mouse-ccw", "virtio-tablet-ccw", "qcow2-luks", + "query-cpus-fast", ); @@ -1588,6 +1589,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "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-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 2203c28..db6d42f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -451,6 +451,7 @@ typedef enum { QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, /* -device virtio-mouse-ccw */ QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */ QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */ + QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index ec2eec1..bf79777 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -188,6 +188,7 @@ <flag name='pl011'/> <flag name='dump-completed'/> <flag name='qcow2-luks'/> + <flag name='query-cpus-fast'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342346</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index 1122d64..9958d36 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -186,6 +186,7 @@ <flag name='machine.pseries.max-cpu-compat'/> <flag name='dump-completed'/> <flag name='qcow2-luks'/> + <flag name='query-cpus-fast'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419215</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 191b1e0..272b77d 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -151,6 +151,7 @@ <flag name='virtio-mouse-ccw'/> <flag name='virtio-tablet-ccw'/> <flag name='qcow2-luks'/> + <flag name='query-cpus-fast'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index aa5de81..b5f40cd 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -226,6 +226,7 @@ <flag name='isa-serial'/> <flag name='dump-completed'/> <flag name='qcow2-luks'/> + <flag name='query-cpus-fast'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390060</microcodeVersion> -- 1.9.1
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> --- src/qemu/qemu_domain.c | 14 +++++++++++--- src/qemu/qemu_monitor.c | 30 ++++++++++++++++++------------ src/qemu/qemu_monitor.h | 7 +++++-- src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++---------- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 4 ++-- 6 files changed, 65 insertions(+), 30 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9d1c33b..662937b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9006,7 +9006,12 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug); + rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), + &info, + maxvcpus, + hotplug, + virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST)); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -9025,7 +9030,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, * thread, but it runs every vCPU in that same thread. So it * is impossible to setup different affinity per thread. * - * What's more the 'query-cpus' command returns bizarre + * What's more the 'query-cpus[-fast]' command returns bizarre * data for the threads. It gives the TCG thread for the * vCPU 0, but for vCPUs 1-> N, it actually replies with * the main process thread ID. @@ -9126,7 +9131,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus); + haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), + maxvcpus, + virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST)); if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7b64752..2b4b7c7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1852,15 +1852,16 @@ qemuMonitorGetCPUInfoLegacy(struct qemuMonitorQueryCpusEntry *cpuentries, * * This function stitches together data retrieved via query-hotpluggable-cpus * which returns entities on the hotpluggable level (which may describe more - * than one guest logical vcpu) with the output of query-cpus, having an entry - * per enabled guest logical vcpu. + * than one guest logical vcpu) with the output of query-cpus (or + * query-cpus-fast), having an entry per enabled guest logical vcpu. * * query-hotpluggable-cpus conveys following information: * - topology information and number of logical vcpus this entry creates * - device type name of the entry that needs to be used when hotplugging - * - qom path in qemu which can be used to map the entry against query-cpus + * - qom path in qemu which can be used to map the entry against + * query-cpus[-fast] * - * query-cpus conveys following information: + * query-cpus[-fast] conveys following information: * - thread id of a given guest logical vcpu * - order in which the vcpus were inserted * - qom path to allow mapping the two together @@ -1895,7 +1896,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl for (i = 0; i < nhotplugvcpus; i++) totalvcpus += hotplugvcpus[i].vcpus; - /* trim '/thread...' suffix from the data returned by query-cpus */ + /* trim '/thread...' suffix from the data returned by query-cpus[-fast] */ for (i = 0; i < ncpuentries; i++) { if (cpuentries[i].qom_path && (tmp = strstr(cpuentries[i].qom_path, "/thread"))) @@ -1908,7 +1909,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl } /* Note the order in which the hotpluggable entities are inserted by - * matching them to the query-cpus entries */ + * matching them to the query-cpus[-fast] entries */ for (i = 0; i < ncpuentries; i++) { for (j = 0; j < nhotplugvcpus; j++) { if (!cpuentries[i].qom_path || @@ -1963,7 +1964,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl } if (anyvcpu == maxvcpus) { - VIR_DEBUG("too many query-cpus entries for a given " + VIR_DEBUG("too many query-cpus[-fast] entries for a given " "query-hotpluggable-cpus entry"); return -1; } @@ -1991,6 +1992,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl * @vcpus: pointer filled by array of qemuMonitorCPUInfo structures * @maxvcpus: total possible number of vcpus * @hotplug: query data relevant for hotplug support + * @fast: use QMP query-cpus-fast if supported * * Detects VCPU information. If qemu doesn't support or fails reporting * information this function will return success as other parts of libvirt @@ -2003,7 +2005,8 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, qemuMonitorCPUInfoPtr *vcpus, size_t maxvcpus, - bool hotplug) + bool hotplug, + bool fast) { struct qemuMonitorQueryHotpluggableCpusEntry *hotplugcpus = NULL; size_t nhotplugcpus = 0; @@ -2029,7 +2032,8 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, goto cleanup; if (mon->json) - rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug); + rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries, hotplug, + fast); else rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries); @@ -2067,11 +2071,12 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, * qemuMonitorGetCpuHalted: * * Returns a bitmap of vcpu id's that are halted. The id's correspond to the - * 'CPU' field as reported by query-cpus'. + * 'CPU' field as reported by query-cpus[-fast]'. */ virBitmapPtr qemuMonitorGetCpuHalted(qemuMonitorPtr mon, - size_t maxvcpus) + size_t maxvcpus, + bool fast ATTRIBUTE_UNUSED) { struct qemuMonitorQueryCpusEntry *cpuentries = NULL; size_t ncpuentries = 0; @@ -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.h b/src/qemu/qemu_monitor.h index d04148e..8b4353c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -542,8 +542,11 @@ void qemuMonitorCPUInfoFree(qemuMonitorCPUInfoPtr list, int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, qemuMonitorCPUInfoPtr *vcpus, size_t maxvcpus, - bool hotplug); -virBitmapPtr qemuMonitorGetCpuHalted(qemuMonitorPtr mon, size_t maxvcpus); + bool hotplug, + bool fast); +virBitmapPtr qemuMonitorGetCpuHalted(qemuMonitorPtr mon, + size_t maxvcpus, + bool fast); int qemuMonitorGetVirtType(qemuMonitorPtr mon, virDomainVirtType *virtType); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 57c2c4d..4f2018d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1532,7 +1532,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; @@ -1557,11 +1558,19 @@ 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)); - ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted)); - qom_path = virJSONValueObjectGetString(entry, "qom_path"); + * non-fatal, simply returning no data. + * The return data of query-cpus-fast has different field names + */ + if (fast) { + ignore_value(virJSONValueObjectGetNumberInt(entry, "cpu-index", &cpuid)); + ignore_value(virJSONValueObjectGetNumberInt(entry, "thread-id", &thread)); + qom_path = virJSONValueObjectGetString(entry, "qom-path"); + } else { + ignore_value(virJSONValueObjectGetNumberInt(entry, "CPU", &cpuid)); + ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread)); + ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted)); + qom_path = virJSONValueObjectGetString(entry, "qom_path"); + } cpus[i].qemu_id = cpuid; cpus[i].tid = thread; @@ -1586,10 +1595,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. @@ -1598,13 +1609,19 @@ 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; virJSONValuePtr reply = NULL; virJSONValuePtr data; + if (fast) + cmd = qemuMonitorJSONMakeCommand("query-cpus-fast", NULL); + else + cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL); + if (!cmd) return -1; @@ -1619,7 +1636,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 045df49..c9fc727 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 0afdc80..0add50a 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1423,7 +1423,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) { @@ -2719,7 +2719,7 @@ testQemuMonitorCPUInfo(const void *opaque) goto cleanup; rc = qemuMonitorGetCPUInfo(qemuMonitorTestGetMonitor(test), - &vcpus, data->maxvcpus, true); + &vcpus, data->maxvcpus, true, false); if (rc < 0) goto cleanup; -- 1.9.1
On 04/04/2018 10:45 AM, 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> --- src/qemu/qemu_domain.c | 14 +++++++++++--- src/qemu/qemu_monitor.c | 30 ++++++++++++++++++------------ src/qemu/qemu_monitor.h | 7 +++++-- src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++---------- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 4 ++-- 6 files changed, 65 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9d1c33b..662937b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9006,7 +9006,12 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1;
Count me as one of those that would prefer to see: bool fast; ... fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, QEMU_CAPS_QUERY_CPUS_FAST); ...
- rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug); + rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), + &info, + maxvcpus, + hotplug, + virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST));
rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug, fast);
if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -9025,7 +9030,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, * thread, but it runs every vCPU in that same thread. So it * is impossible to setup different affinity per thread. * - * What's more the 'query-cpus' command returns bizarre + * What's more the 'query-cpus[-fast]' command returns bizarre * data for the threads. It gives the TCG thread for the * vCPU 0, but for vCPUs 1-> N, it actually replies with * the main process thread ID. @@ -9126,7 +9131,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1;
- haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);
+ haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), + maxvcpus, + virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST));
Likewise, bool fast; ... fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, QEMU_CAPS_QUERY_CPUS_FAST); haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus, fast);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap) goto cleanup;
[...] John If you're OK with this I can make the alterations... What you have works - it's just one of those preferential things related to seeing and/or relying on function calls within function calls.
On 12.04.2018 17:52, John Ferlan wrote:
On 04/04/2018 10:45 AM, 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> --- src/qemu/qemu_domain.c | 14 +++++++++++--- src/qemu/qemu_monitor.c | 30 ++++++++++++++++++------------ src/qemu/qemu_monitor.h | 7 +++++-- src/qemu/qemu_monitor_json.c | 37 +++++++++++++++++++++++++++---------- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 4 ++-- 6 files changed, 65 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9d1c33b..662937b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9006,7 +9006,12 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1;
Count me as one of those that would prefer to see:
bool fast; ... fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, QEMU_CAPS_QUERY_CPUS_FAST); ...
- rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug); + rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), + &info, + maxvcpus, + hotplug, + virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST));
rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug, fast);
if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -9025,7 +9030,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, * thread, but it runs every vCPU in that same thread. So it * is impossible to setup different affinity per thread. * - * What's more the 'query-cpus' command returns bizarre + * What's more the 'query-cpus[-fast]' command returns bizarre * data for the threads. It gives the TCG thread for the * vCPU 0, but for vCPUs 1-> N, it actually replies with * the main process thread ID. @@ -9126,7 +9131,10 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1;
- haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);
+ haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), + maxvcpus, + virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, + QEMU_CAPS_QUERY_CPUS_FAST));
Likewise,
bool fast; ...
fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps, QEMU_CAPS_QUERY_CPUS_FAST); haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus, fast);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap) goto cleanup;
[...]
John
If you're OK with this I can make the alterations... What you have works - it's just one of those preferential things related to seeing and/or relying on function calls within function calls.
Sure. On another day I might have used a local variable as well. Appreciating and accepting your offer to squash in this changes. -- Regards, Viktor Mihajlovski
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. Also extend the test program to recognize the halted property. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 ++ ...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 5 + ...qemumonitorjson-cpuinfo-x86-full-fast-cpus.json | 126 +++++++++++++++++++++ ...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 | 7 ++ tests/qemumonitortestutils.h | 1 + 9 files changed, 468 insertions(+), 26 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-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-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-full-fast-cpus.json b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json new file mode 100644 index 0000000..b8c2553 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json @@ -0,0 +1,126 @@ +{ + "return": [ + { + "arch": "x86", + "cpu-index": 0, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 0 + }, + "qom-path": "/machine/unattached/device[0]", + "thread-id": 895040 + }, + { + "arch": "x86", + "cpu-index": 1, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 1 + }, + "qom-path": "/machine/peripheral/vcpu1", + "thread-id": 895056 + }, + { + "arch": "x86", + "cpu-index": 2, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 2 + }, + "qom-path": "/machine/peripheral/vcpu2", + "thread-id": 895057 + }, + { + "arch": "x86", + "cpu-index": 3, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 3 + }, + "qom-path": "/machine/peripheral/vcpu3", + "thread-id": 895058 + }, + { + "arch": "x86", + "cpu-index": 4, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 4 + }, + "qom-path": "/machine/peripheral/vcpu4", + "thread-id": 895059 + }, + { + "arch": "x86", + "cpu-index": 5, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 5 + }, + "qom-path": "/machine/peripheral/vcpu5", + "thread-id": 895060 + }, + { + "arch": "x86", + "cpu-index": 6, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 6 + }, + "qom-path": "/machine/peripheral/vcpu6", + "thread-id": 895061 + }, + { + "arch": "x86", + "cpu-index": 7, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 7 + }, + "qom-path": "/machine/peripheral/vcpu7", + "thread-id": 895062 + }, + { + "arch": "x86", + "cpu-index": 8, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 8 + }, + "qom-path": "/machine/peripheral/vcpu8", + "thread-id": 895063 + }, + { + "arch": "x86", + "cpu-index": 9, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 9 + }, + "qom-path": "/machine/peripheral/vcpu9", + "thread-id": 895064 + }, + { + "arch": "x86", + "cpu-index": 10, + "props": { + "core-id": 0, + "thread-id": 0, + "socket-id": 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/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 0add50a..5daa2c9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1363,6 +1363,42 @@ 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) @@ -1370,15 +1406,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; @@ -1422,29 +1459,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; } @@ -2615,6 +2660,7 @@ struct testCPUInfoData { const char *name; size_t maxvcpus; virDomainXMLOptionPtr xmlopt; + bool fast; }; @@ -2669,6 +2715,9 @@ testQemuMonitorCPUInfoFormat(qemuMonitorCPUInfoPtr vcpus, virBufferAddLit(&buf, "\n"); } + if (vcpu->halted) + virBufferAddLit(&buf, "halted\n"); + virBufferAdjustIndent(&buf, -4); } @@ -2681,12 +2730,14 @@ 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; char *queryCpusStr = NULL; char *queryHotpluggableStr = NULL; char *actual = NULL; + const char *queryCpusFunction; qemuMonitorCPUInfoPtr vcpus = NULL; int rc; int ret = -1; @@ -2715,11 +2766,20 @@ testQemuMonitorCPUInfo(const void *opaque) queryHotpluggableStr) < 0) goto cleanup; - if (qemuMonitorTestAddItem(test, "query-cpus", queryCpusStr) < 0) + if (data->fast) + queryCpusFunction = "query-cpus-fast"; + else + queryCpusFunction = "query-cpus"; + + if (qemuMonitorTestAddItem(test, queryCpusFunction, queryCpusStr) < 0) goto cleanup; + vm = qemuMonitorTestGetDomainObj(test); + if (!vm) + return -1; + rc = qemuMonitorGetCPUInfo(qemuMonitorTestGetMonitor(test), - &vcpus, data->maxvcpus, true, false); + &vcpus, data->maxvcpus, true, data->fast); if (rc < 0) goto cleanup; @@ -2930,7 +2990,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; \ @@ -3014,6 +3082,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 66bccac..62f68ee 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1518,3 +1518,10 @@ qemuMonitorTestGetAgent(qemuMonitorTestPtr test) { return test->agent; } + + +virDomainObjPtr +qemuMonitorTestGetDomainObj(qemuMonitorTestPtr test) +{ + return test->vm; +} diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h index d3dc029..855e625 100644 --- a/tests/qemumonitortestutils.h +++ b/tests/qemumonitortestutils.h @@ -99,5 +99,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
On 04/04/2018 10:45 AM, Viktor Mihajlovski wrote:
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. Also extend the test program to recognize the halted property.
This last sentence involves code that probably should be separated into its own patch since it's unrelated to the new data. If it's moved this patch into one previously, then this patch/adjustment "proves" nothing changes (which is good). I can do that for you so you don't have to post another series. I would make you the author and add your S-o-b... Of course I need you to say OK for that too! The rest of this shows what I'd extract...
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 ++ ...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 5 + ...qemumonitorjson-cpuinfo-x86-full-fast-cpus.json | 126 +++++++++++++++++++++ ...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 | 7 ++ tests/qemumonitortestutils.h | 1 + 9 files changed, 468 insertions(+), 26 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-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-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 0add50a..5daa2c9 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c
[...]
@@ -2669,6 +2715,9 @@ testQemuMonitorCPUInfoFormat(qemuMonitorCPUInfoPtr vcpus, virBufferAddLit(&buf, "\n"); }
+ if (vcpu->halted) + virBufferAddLit(&buf, "halted\n"); + virBufferAdjustIndent(&buf, -4); }
@@ -2681,12 +2730,14 @@ testQemuMonitorCPUInfo(const void *opaque)
[...]
On 12.04.2018 17:52, John Ferlan wrote:
On 04/04/2018 10:45 AM, Viktor Mihajlovski wrote:
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. Also extend the test program to recognize the halted property.
This last sentence involves code that probably should be separated into its own patch since it's unrelated to the new data. If it's moved this patch into one previously, then this patch/adjustment "proves" nothing changes (which is good).
I can do that for you so you don't have to post another series. I would make you the author and add your S-o-b... Of course I need you to say OK for that too!
The rest of this shows what I'd extract... Good point. I will gladly accept your offer to split out the 'halted' part.
[...] -- Regards, Viktor Mihajlovski
Extract architecture specific data from query-cpus[-fast] if available. A new function qemuMonitorJSONExtractCPUArchInfo() can then 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> --- src/qemu/qemu_monitor.c | 4 +- src/qemu/qemu_monitor_json.c | 96 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2b4b7c7..5c4581a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2076,7 +2076,7 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, virBitmapPtr qemuMonitorGetCpuHalted(qemuMonitorPtr mon, size_t maxvcpus, - bool fast ATTRIBUTE_UNUSED) + bool fast) { struct qemuMonitorQueryCpusEntry *cpuentries = NULL; size_t ncpuentries = 0; @@ -2088,7 +2088,7 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon, 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 4f2018d..a46e6cf 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1517,15 +1517,104 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon) } -/* +/** + * 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" } + * + */ +static void +qemuMonitorJSONExtractCPUS390Info(virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu) +{ + const char *cpu_state = virJSONValueObjectGetString(jsoncpu, "cpu-state"); + + if (STREQ_NULLABLE(cpu_state, "operating") || + STREQ_NULLABLE(cpu_state, "load")) + cpu->halted = false; + else if (STREQ_NULLABLE(cpu_state, "stopped") || + STREQ_NULLABLE(cpu_state, "check-stop")) + cpu->halted = true; +} + + +/** + * qemuMonitorJSONExtractCPUArchInfo: + * @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 + * function. + * + */ +static void +qemuMonitorJSONExtractCPUArchInfo(virJSONValuePtr jsoncpu, + struct qemuMonitorQueryCpusEntry *cpu) +{ + const char *arch = virJSONValueObjectGetString(jsoncpu, "arch"); + + if (STREQ_NULLABLE(arch, "s390")) + qemuMonitorJSONExtractCPUS390Info(jsoncpu, cpu); +} + + +/** + * qemuMonitorJSONExtractCPUInfo: + * @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, + * "props": { + * "core-id": 0, + * "thread-id": 0, + * "socket-id": 0 + * }, + * "qom-path": "/machine/unattached/device[0]", + * "thread-id": 2631237, + * ...}, + * {...} + * ] + * or for s390 + * [{ "arch": "s390", + * "cpu-index": 0, + * "props": { + * "core-id": 0 + * }, + * "qom-path": "/machine/unattached/device[0]", + * "thread-id": 1237, + * "cpu-state": "operating", + * ...}, * {...} * ] */ @@ -1577,6 +1666,9 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, cpus[i].halted = halted; if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0) goto cleanup; + + /* process optional architecture-specific data */ + qemuMonitorJSONExtractCPUArchInfo(entry, cpus + i); } VIR_STEAL_PTR(*entries, cpus); -- 1.9.1
The s390 testcase verifies that the s390-specific cpu-state field is correctly mapped to the halted property. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- .../qemumonitorjson-cpuinfo-s390-fast-cpus.json | 25 ++++++++++++++++++++++ .../qemumonitorjson-cpuinfo-s390-fast-hotplug.json | 21 ++++++++++++++++++ .../qemumonitorjson-cpuinfo-s390-fast.data | 19 ++++++++++++++++ tests/qemumonitorjsontest.c | 2 ++ 4 files changed, 67 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-s390-fast-cpus.json b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json new file mode 100644 index 0000000..ef9c975 --- /dev/null +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json @@ -0,0 +1,25 @@ +{ + "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/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 5daa2c9..b468b04 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -3090,6 +3090,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 662937b..0331af7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9124,8 +9124,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
On 04/04/2018 10:45 AM, Viktor Mihajlovski wrote:
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.
v2 -> v3: ========= Patch changes: 1/6: - formatting changes as requested by John Ferlan - updated the qemucapabilitiestest XML files - dropped A/R-bys (due to change above)
2/6: - fixed comment to also account for query-cpus-fast
3/6: - updated json monitor tests to account for props returned by query-cpus and query-cpus-fast - updated json monitor tests to handle the halted property (moved here from patch 5)
4/6: - removed stale text (about callbacks) from commit message - simplified qemuMonitorJSONExtractCPUArchInfo call - enhanced sample JSON response in comment - fixed inter-function spacing
5/6: - kept s390-specific tests and moved the rest to patch 3 - reformatted the JSON responses (indented "props") - dropped R-b
v1 -> v2: ========= Patch changes: 1/6: - add Acked-by: Peter Krempa
2/6: - evaluate capability outside of monitor code (changes signatures of qemuMonitorGetCPUInfo and qemuMonitorGetCpuHalted) - remove ternary expressions as requested by review - remove Reviewed-bys due to code changes
3/6 - adapt test cases to changes above - remove Reviewed-bys due to code changes
4/6 - replace callbacks for architecture data extraction with direct function calls - remove Reviewed-bys due to code changes
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 | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 23 +++- src/qemu/qemu_monitor.c | 30 +++-- src/qemu/qemu_monitor.h | 7 +- src/qemu/qemu_monitor_json.c | 133 +++++++++++++++++++-- src/qemu/qemu_monitor_json.h | 3 +- tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 ++ .../qemumonitorjson-cpuinfo-s390-fast-cpus.json | 25 ++++ .../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 | 126 +++++++++++++++++++ ...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 | 123 +++++++++++++++---- tests/qemumonitortestutils.c | 7 ++ tests/qemumonitortestutils.h | 1 + 23 files changed, 706 insertions(+), 58 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
With a merge for patch1 to top of tree and a couple of caveats from patch2 and patch3 (with any luck the patches will make it to the list as well as this one ;-)) Reviewed-by: John Ferlan <jferlan@redhat.com> (series) As long as you say OK to the adjustments I can push... Perhaps tomorrow giving anyone else one last chance to provide their feedback! I also merged the qemu_capabilities adjustments. John
participants (3)
-
John Ferlan -
Viktor Mihajlovski -
Viktor VM Mihajlovski