5:44 a.m.
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.
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 | 4 +-
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_domain.c | 21 +++-
src/qemu/qemu_monitor.c | 30 +++--
src/qemu/qemu_monitor.h | 7 +-
src/qemu/qemu_monitor_json.c | 116 +++++++++++++++++--
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 | 123 ++++++++++++++++-----
tests/qemumonitortestutils.c | 6 +
tests/qemumonitortestutils.h | 1 +
19 files changed, 625 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
5:44 a.m.
New subject: [libvirt] [PATCHv2 1/6] qemu: add capability detection for query-cpus-fast
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Acked-by: Peter Krempa <pkrempa(a)redhat.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
11:03 a.m.
New subject: [libvirt] [PATCHv2 1/6] qemu: add capability detection for query-cpus-fast
On 03/05/2018 06:44 AM, 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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Acked-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 4 +++-
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)
Now that the 2.12 caps update has been pushed... It's "open season" on
the qemu_capabilities.{c,h}. Since this series has been on list the
longest, I'll start here...
This particular patch will need an update to add the new flag to the
various caps_2.12*.xml files (VIR_TEST_REGENERATE_OUTPUT=1
tests/qemucapabilitiestest does the trick).
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}
I know this has been ACK'd; however, I recently pushed a small fixup in
this area that you'll have a merge with, see commit id '1706bef6' (I saw
that while working on something else, but noted this patch is affected).
Still for this new entry, it'd be better to use the format :
{ "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST },
John
};
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;
3:16 a.m.
New subject: [libvirt] [PATCHv2 1/6] qemu: add capability detection for query-cpus-fast
On 23.03.2018 17:03, John Ferlan wrote:
On 03/05/2018 06:44 AM, 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(a)linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
> Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
> Acked-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_capabilities.c | 4 +++-
> src/qemu/qemu_capabilities.h | 1 +
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
Thanks for the thorough review of the series.
Now that the 2.12 caps update has been pushed... It's "open season" on
the qemu_capabilities.{c,h}. Since this series has been on list the
longest, I'll start here...
This particular patch will need an update to add the new flag to the
various caps_2.12*.xml files (VIR_TEST_REGENERATE_OUTPUT=1
tests/qemucapabilitiestest does the trick).
OK.
> 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}
I know this has been ACK'd; however, I recently pushed a small fixup in
this area that you'll have a merge with, see commit id '1706bef6' (I saw
that while working on something else, but noted this patch is affected).
Still for this new entry, it'd be better to use the format :
{ "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST },
I agree, will change.
John
> };
>
> 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;
>
--
Regards,
Viktor Mihajlovski
5:44 a.m.
New subject: [libvirt] [PATCHv2 2/6] qemu: use query-cpus-fast in JSON monitor
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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_domain.c | 12 ++++++++++--
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, 64 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b4efc8..4079fb3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8683,7 +8683,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;
@@ -8803,7 +8808,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 ad5c572..22b2091 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 954ae88..7b92d41 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -530,8 +530,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 a09e93e..6a5fb12 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,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;
@@ -1520,10 +1529,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,13 +1543,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;
@@ -1553,7 +1570,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..2e685ce 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) {
@@ -2716,7 +2716,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
11:05 a.m.
New subject: [libvirt] [PATCHv2 2/6] qemu: use query-cpus-fast in JSON monitor
On 03/05/2018 06:44 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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_domain.c | 12 ++++++++++--
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, 64 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b4efc8..4079fb3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8683,7 +8683,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));
Perhaps we should create a qemuMonitorGetCPUInfoFast API since we should
also be able to return the @props {core-id, thread-id, socket-id} and
report them. I'm not steadfast on this suggestion - maybe Peter has a
stronger opinion one way or the other. Still it allows future
adjustments and additions to -fast to be more easily "handled".
It would seem those values could be useful although I do see there could
be some "impact" with how hotplug works and the default setting to -1 of
the values by qemuMonitorCPUInfoClear.
FWIW: Below this point as the returned @info structure is parsed,
there's a comment about query-cpus that would then of course not
necessarily be true or at the very least "adjusted properly" for this
new -fast model.
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
@@ -8803,7 +8808,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));
Maybe this one should pass 'false' for now until patch 4 comes along to
add the code that fetches the cpu-state and alters halted for the -fast
model. Depending on how one feels about larger patches vs having a 2
patch gap to not have the support for -fast... I dunno, I could see
reason to merge patch 4 here too in order to be in a patch feature for
feature compatible.
if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap)
goto cleanup;
[...]
diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
index a09e93e..6a5fb12 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,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");
+ }
So query-fast doesn't report "halted" which means we default to false
for every arch other than s390 which gets fixed in 2 patches. IIRC other
arches weren't ever reporting halted = true for the not fast and only
our test environment had the halted set. Although I could be wrong -
Peter was much more involved in that code, so I assume he'd have a more
definitive answer.
Anyway, from the 0/6 cover, I see:
"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."
That's not really the case here since halted defaults to false, but
figured I'd ask just to be sure. If not reporting it was the desire,
then "bool halted" would need to to turn into a virTristateBool in
qemuMonitorQueryCpusEntry and _qemuMonitorCPUInfo. That way it would be
either properly reported as halted or not and if not provided, it would
not be printed. But that may cause issues for other code that uses
halted... Of course I'm not sure
cpus[i].qemu_id = cpuid;
cpus[i].tid = thread;
John
3:12 a.m.
New subject: [libvirt] [PATCHv2 2/6] qemu: use query-cpus-fast in JSON monitor
On 23.03.2018 17:05, John Ferlan wrote:
[...]
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8b4efc8..4079fb3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8683,7 +8683,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));
Perhaps we should create a qemuMonitorGetCPUInfoFast API since we should
also be able to return the @props {core-id, thread-id, socket-id} and
report them. I'm not steadfast on this suggestion - maybe Peter has a
stronger opinion one way or the other. Still it allows future
adjustments and additions to -fast to be more easily "handled".
It would seem those values could be useful although I do see there could
be some "impact" with how hotplug works and the default setting to -1 of
the values by qemuMonitorCPUInfoClear.
Actually, query-cpus[-fast] is not reporting
the topology (socket, core,
thread). The reported thread id is referring to the QEMU CPU thread.
FWIW: Below this point as the returned @info structure is parsed,
there's a comment about query-cpus that would then of course not
necessarily be true or at the very least "adjusted properly" for this
new -fast model.
Agreed, this one escaped me. Will change.
>
> if (qemuDomainObjExitMonitor(driver, vm) < 0)
> goto cleanup;
> @@ -8803,7 +8808,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));
Maybe this one should pass 'false' for now until patch 4 comes along to
add the code that fetches the cpu-state and alters halted for the -fast
model. Depending on how one feels about larger patches vs having a 2
patch gap to not have the support for -fast... I dunno, I could see
reason to merge patch 4 here too in order to be in a patch feature for
feature compatible.>
FWIW, I intended to make the patches deprecation-safe, i.e.
prevent
calling query-cpus on a QEMU not supporting it anymore. I see your point
though and would have no principal objections to merging patches 2 and 4
(plus the testcase patches 3 and 5). Would be great to hear more opinions...
>
> if (qemuDomainObjExitMonitor(driver, vm) < 0 || !haltedmap)
> goto cleanup;
[...]
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a09e93e..6a5fb12 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,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");
> + }
So query-fast doesn't report "halted" which means we default to false
for every arch other than s390 which gets fixed in 2 patches. IIRC other
arches weren't ever reporting halted = true for the not fast and only
our test environment had the halted set. Although I could be wrong -
Peter was much more involved in that code, so I assume he'd have a more
definitive answer.
Halted was reported at least for x86, and a value of true was
rather
common ony multi-core systems. However, defaulting to false doesn't hurt
(beyond the temporary impact on s390), because the user visible halted
value is a tristate for a while now.
Anyway, from the 0/6 cover, I see:
"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."
That's not really the case here since halted defaults to false, but
figured I'd ask just to be sure. If not reporting it was the desire,
then "bool halted" would need to to turn into a virTristateBool in
qemuMonitorQueryCpusEntry and _qemuMonitorCPUInfo. That way it would be
either properly reported as halted or not and if not provided, it would
not be printed. But that may cause issues for other code that uses
halted... Of course I'm not sure
>
> cpus[i].qemu_id = cpuid;
> cpus[i].tid = thread;
John
--
Regards,
Viktor Mihajlovski
6:28 a.m.
New subject: [libvirt] [PATCHv2 2/6] qemu: use query-cpus-fast in JSON monitor
On 26.03.2018 10:12, Viktor Mihajlovski wrote:
On 23.03.2018 17:05, John Ferlan wrote:
[...]
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 8b4efc8..4079fb3 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -8683,7 +8683,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));
>
> Perhaps we should create a qemuMonitorGetCPUInfoFast API since we should
> also be able to return the @props {core-id, thread-id, socket-id} and
> report them. I'm not steadfast on this suggestion - maybe Peter has a
> stronger opinion one way or the other. Still it allows future
> adjustments and additions to -fast to be more easily "handled".
>
> It would seem those values could be useful although I do see there could
> be some "impact" with how hotplug works and the default setting to -1 of
> the values by qemuMonitorCPUInfoClear.
Actually, query-cpus[-fast] is not reporting the topology (socket, core,
thread). The reported thread id is referring to the QEMU CPU thread.
Well, I have
to take that back as I ignored that QEMU is providing the
props since 2.10 with query-cpus (and now with query-cpus-fast). If we
decide to parse the props, we could do it with the pre-existing
qemuMonitorGetCPUInfo function, but I would prefer this to be done in
separate patch.
[...]
--
Regards,
Viktor Mihajlovski
8:14 a.m.
New subject: [libvirt] [PATCHv2 2/6] qemu: use query-cpus-fast in JSON monitor
On 03/26/2018 07:28 AM, Viktor Mihajlovski wrote:
On 26.03.2018 10:12, Viktor Mihajlovski wrote:
> On 23.03.2018 17:05, John Ferlan wrote:
> [...]
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 8b4efc8..4079fb3 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -8683,7 +8683,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));
>>
>> Perhaps we should create a qemuMonitorGetCPUInfoFast API since we should
>> also be able to return the @props {core-id, thread-id, socket-id} and
>> report them. I'm not steadfast on this suggestion - maybe Peter has a
>> stronger opinion one way or the other. Still it allows future
>> adjustments and additions to -fast to be more easily "handled".
>>
>> It would seem those values could be useful although I do see there could
>> be some "impact" with how hotplug works and the default setting to -1
of
>> the values by qemuMonitorCPUInfoClear.
> Actually, query-cpus[-fast] is not reporting the topology (socket, core,
> thread). The reported thread id is referring to the QEMU CPU thread.
Well, I have to take that back as I ignored that QEMU is providing the
props since 2.10 with query-cpus (and now with query-cpus-fast). If we
decide to parse the props, we could do it with the pre-existing
qemuMonitorGetCPUInfo function, but I would prefer this to be done in
separate patch.
[...]
I didn't even look at the query-cpus and @props - I was just considering
the -fast code. And yes, looking at @props for query-cpus would have to
be a separate patch with it's own capability - if of course it was even
deemed worthwhile to do...
John
5:44 a.m.
New subject: [libvirt] [PATCHv2 3/6] tests: add qemumonitorjson tests for query-cpus-fast
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(a)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 | 118 ++++++++++++++++-----
tests/qemumonitortestutils.c | 6 ++
tests/qemumonitortestutils.h | 1 +
6 files changed, 394 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-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 2e685ce..a932e39 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1360,6 +1360,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)
@@ -1367,15 +1403,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 +1456,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 +2657,7 @@ struct testCPUInfoData {
const char *name;
size_t maxvcpus;
virDomainXMLOptionPtr xmlopt;
+ bool fast;
};
@@ -2678,12 +2724,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;
@@ -2712,11 +2760,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;
@@ -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
11:07 a.m.
New subject: [libvirt] [PATCHv2 3/6] tests: add qemumonitorjson tests for query-cpus-fast
On 03/05/2018 06:44 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.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)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 | 118 ++++++++++++++++-----
tests/qemumonitortestutils.c | 6 ++
tests/qemumonitortestutils.h | 1 +
6 files changed, 394 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-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
+ },
Each of these seems to be missing the "props" structure even though the
hotplug does have it. As noted in my 2/6 review - that could be useful
data to parse (and return and save for printing in the stats output). In
fact I recall those values being the most painful when dealing with this
the last time I was reviewing in here...
+ {
+ "arch": "x86",
+ "cpu-index": 1,
+ "qom-path": "/machine/peripheral/vcpu1",
+ "thread-id": 895056
+ },
[...]
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"
+ },
See you do show the props here...
+ {
+ "props": {
+ "core-id": 0,
+ "thread-id": 0,
+ "socket-id": 9
+ },
+ "vcpus-count": 1,
+ "qom-path": "/machine/peripheral/vcpu9",
+ "type": "Broadwell-x86_64-cpu"
+ },
[...]
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;
}
Format here is 2 blank lines between methods
John
+
+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__ */
6:19 a.m.
New subject: [libvirt] [PATCHv2 3/6] tests: add qemumonitorjson tests for query-cpus-fast
On 23.03.2018 17:07, John Ferlan wrote:
On 03/05/2018 06:44 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.
>
> Signed-off-by: Viktor Mihajlovski <mihajlov(a)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 | 118 ++++++++++++++++-----
> tests/qemumonitortestutils.c | 6 ++
> tests/qemumonitortestutils.h | 1 +
> 6 files changed, 394 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-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
> + },
Each of these seems to be missing the "props" structure even though the
hotplug does have it. As noted in my 2/6 review - that could be useful
data to parse (and return and save for printing in the stats output). In
fact I recall those values being the most painful when dealing with this
the last time I was reviewing in here...
Hm ... you are right, QEMUs implementing query-cpus-fast will have the
props structure filled, so we should account for it.
> + {
> + "arch": "x86",
> + "cpu-index": 1,
> + "qom-path": "/machine/peripheral/vcpu1",
> + "thread-id": 895056
> + },
[...]
> 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"
> + },
See you do show the props here...
> + {
> + "props": {
> + "core-id": 0,
> + "thread-id": 0,
> + "socket-id": 9
> + },
> + "vcpus-count": 1,
> + "qom-path": "/machine/peripheral/vcpu9",
> + "type": "Broadwell-x86_64-cpu"
> + },
[...]
> 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;
> }
Format here is 2 blank lines between methods
OK
John
> +
> +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__ */
>
--
Regards,
Viktor Mihajlovski
5:44 a.m.
New subject: [libvirt] [PATCHv2 4/6] qemu: add architecture-specific CPU info handling
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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_monitor.c | 4 +--
src/qemu/qemu_monitor_json.c | 79 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 22b2091..5840e25 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 6a5fb12..1924cfe 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1451,15 +1451,85 @@ 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:
+ * @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
+ * function.
+ *
+ */
+static void
+qemuMonitorJSONExtractCPUArchInfo(const char *arch,
+ virJSONValuePtr jsoncpu,
+ struct qemuMonitorQueryCpusEntry *cpu)
+{
+ if (STREQ_NULLABLE(arch, "s390"))
+ qemuMonitorJSONExtractCPUS390Info(jsoncpu, cpu);
+}
+
+/**
+ * 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 +1556,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
int thread = 0;
bool halted = false;
const char *qom_path;
+ const char *arch;
if (!entry) {
ret = -2;
goto cleanup;
@@ -1511,6 +1582,10 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
cpus[i].halted = halted;
if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0)
goto cleanup;
+
+ /* process optional architecture-specific data */
+ arch = virJSONValueObjectGetString(entry, "arch");
+ qemuMonitorJSONExtractCPUArchInfo(arch, entry, cpus + i);
}
VIR_STEAL_PTR(*entries, cpus);
--
1.9.1
11:08 a.m.
New subject: [libvirt] [PATCHv2 4/6] qemu: add architecture-specific CPU info handling
On 03/05/2018 06:44 AM, 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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_monitor.c | 4 +--
src/qemu/qemu_monitor_json.c | 79 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 22b2091..5840e25 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 6a5fb12..1924cfe 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1451,15 +1451,85 @@ 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;
Realistically, since false is the default, then only "stopped" and
"check-stop" need to be checked... Even 'uninitialized' would show up
as
false since it's not checked.
Perhaps you should create and carry up the calling stack a copy of the
cpu-state that way eventually it could be printed in a 'stats' output as
well...
I suppose another concern is that some future halted state is created
and this code does account for that leading to incorrect reporting and
well some other issues since @halted is used for various decisions.
+}
+
+/**
+ * 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
+ * function.
+ *
+ */
+static void
+qemuMonitorJSONExtractCPUArchInfo(const char *arch,
+ virJSONValuePtr jsoncpu,
+ struct qemuMonitorQueryCpusEntry *cpu)
+{
+ if (STREQ_NULLABLE(arch, "s390"))
+ qemuMonitorJSONExtractCPUS390Info(jsoncpu, cpu);
+}
+
+/**
+ * qemuMonitorJSONExtractCPUArchInfo:
^^ This is still 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,
+ * "qom-path": "/machine/unattached/device[0]",
+ * "thread_id": 2631237,
+ * ...},
May as well show the whole example and even provide a s390 example so
that it's more obvious...
* {...}
* ]
*/
@@ -1486,6 +1556,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
int thread = 0;
bool halted = false;
const char *qom_path;
+ const char *arch;
if (!entry) {
ret = -2;
goto cleanup;
@@ -1511,6 +1582,10 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
cpus[i].halted = halted;
if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0)
goto cleanup;
+
+ /* process optional architecture-specific data */
+ arch = virJSONValueObjectGetString(entry, "arch");
+ qemuMonitorJSONExtractCPUArchInfo(arch, entry, cpus + i);
Since you're passing @entry anyway, you could fetch @arch in
qemuMonitorJSONExtractCPUArchInfo and since it's only valid for "fast ==
true", calling should be gated on that; otherwise, this would be called
for both fast options.
BTW: Rather than "cpus[i]" and "cpus + i", could we just create a
local/stack "cpu" and use it?
John
}
VIR_STEAL_PTR(*entries, cpus);
8:10 a.m.
New subject: [libvirt] [PATCHv2 4/6] qemu: add architecture-specific CPU info handling
On 23.03.2018 17:08, John Ferlan wrote:
[...]
> +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;
Realistically, since false is the default, then only "stopped" and
"check-stop" need to be checked... Even 'uninitialized' would show up
as
false since it's not checked.
As you say, it's not necessary to handle the false case explicitly.
Still, I would like to be explicit here.
Perhaps you should create and carry up the calling stack a copy of
the
cpu-state that way eventually it could be printed in a 'stats' output as
well...
I suppose another concern is that some future halted state is created
and this code does account for that leading to incorrect reporting and
well some other issues since @halted is used for various decisions.
At this point in time I'm mainly concerned about providing the same
client behaviour with both query-cpus and query-cpus-fast.
In the long run it might make sense to provide architecture specific CPU
information, but this will require more thought and discussion.
> +}
> +
> +/**
> + * 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
> + * function.
> + *
> + */
> +static void
> +qemuMonitorJSONExtractCPUArchInfo(const char *arch,
> + virJSONValuePtr jsoncpu,
> + struct qemuMonitorQueryCpusEntry *cpu)
> +{
> + if (STREQ_NULLABLE(arch, "s390"))
> + qemuMonitorJSONExtractCPUS390Info(jsoncpu, cpu);
> +}
> +
> +/**
> + * qemuMonitorJSONExtractCPUArchInfo:
^^ This is still qemuMonitorJSONExtractCPUInfo
oops
> + * @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,
> + * ...},
May as well show the whole example and even provide a s390 example so
that it's more obvious...
can do
> * {...}
> * ]
> */
> @@ -1486,6 +1556,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
> int thread = 0;
> bool halted = false;
> const char *qom_path;
> + const char *arch;
> if (!entry) {
> ret = -2;
> goto cleanup;
> @@ -1511,6 +1582,10 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
> cpus[i].halted = halted;
> if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0)
> goto cleanup;
> +
> + /* process optional architecture-specific data */
> + arch = virJSONValueObjectGetString(entry, "arch");
> + qemuMonitorJSONExtractCPUArchInfo(arch, entry, cpus + i);
Since you're passing @entry anyway, you could fetch @arch in
qemuMonitorJSONExtractCPUArchInfo and since it's only valid for "fast ==
true", calling should be gated on that; otherwise, this would be called
for both fast options.
I can push the architecture extraction down the stack, but I
wouldn't
make the call optional. While not used, there's still
architecture-specific information returned in query-cpus.
BTW: Rather than "cpus[i]" and "cpus + i", could we just create a
local/stack "cpu" and use it?
I'll have a look.
John
> }
>
> VIR_STEAL_PTR(*entries, cpus);
>
--
Regards,
Viktor Mihajlovski
5:44 a.m.
New subject: [libvirt] [PATCHv2 5/6] tests: add testcase for s390 query-cpus-fast
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)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 a932e39..c642166 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2712,6 +2712,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
11:08 a.m.
New subject: [libvirt] [PATCHv2 5/6] tests: add testcase for s390 query-cpus-fast
On 03/05/2018 06:44 AM, Viktor Mihajlovski wrote:
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)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
I think you could "split" this out print the halted earlier (perhaps
before any of these patches) just to prove/disprove what (if anything)
changes when the -fast command is added.
Then the s390 specific change gets included with patch 4 (or merged with
patch 2 depending on how things go).
John
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 a932e39..c642166 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2712,6 +2712,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 ")", \
5:44 a.m.
New subject: [libvirt] [PATCHv2 6/6] qemu: refresh vcpu halted state only via query-cpus-fast
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay(a)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 4079fb3..a5fcf22 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8801,8 +8801,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
11:09 a.m.
New subject: [libvirt] [PATCHv2 6/6] qemu: refresh vcpu halted state only via query-cpus-fast
On 03/05/2018 06:44 AM, Viktor Mihajlovski wrote:
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
---
src/qemu/qemu_domain.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
BTW: You'll need a patch to docs/news.xml as this is something
newsworthy I would think. Although calling it "-fast" there may make
people wonder about the "slowness" aspect of the current algorithm.
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4079fb3..a5fcf22 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8801,8 +8801,13 @@ qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver,
return 0;
/* The halted state is interresting only on s390(x). On other platforms
"interesting"
- * 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))
So IOW systems that reported this previously that don't get the updated
QEMU bits (e.g. > 2.12) would now stop reporting halted only because of
the adverse performance impact.
But that is a data change and "policy" we'd be inflicting upon them
which maybe they don't care about. Perhaps in that case we should have a
"flag" that handles whether the caller cares or not.
I understand the motivation and reasoning; however, based on how such
things have been historically acked or nacked, I'm leaning towards
saying this patch shouldn't go in.
John
return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
9:32 a.m.
On 05.03.2018 12:44, 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.
[...]
ping (as QEMU has entered soft freeze for 2.12 this week), see also
https://wiki.qemu.org/ChangeLog/2.12#Monitor
--
Regards,
Viktor Mihajlovski
2433
days inactive
2454
days old
19 comments
2 participants
participants (2)
-
John Ferlan -
Viktor Mihajlovski