[libvirt] [PATCH 0/4] Add VCPU halted to domain statistics
As a result of the discussions on https://www.redhat.com/archives/libvir-list/2016-October/msg00531.html: In order to be able to report the per VCPU halted condition as returned by the QEMU monitor this series extends the VCPU-specific domain statistics with a vcpu.n.halted value. This is done by - extending the monitor to pass back the halted condition for the vcpus - adding a new field to the private domain vcpu object reflecting the halted condition for the vcpu - modifying the driver code to report the new VCPU domain statistics value The halted condition is however not recorded in the internal XML format, since the state can change asynchronously (without notification). Patches 1/4 and 2/4 are taken without modifications from the original posting. In 4/4 I've only updated virsh.pod as I didn't want to touch libvirt-domain.c in this context just for documentation purposes. Viktor Mihajlovski (4): qemu: Add monitor support for CPU halted state qemu: Add domain support for VCPU halted state qemu: add vcpu.n.halted to vcpu domain stats doc: update virsh domstats documentation for vcpu statistics src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 ++++ src/qemu/qemu_driver.c | 34 ++++++++++++++++++++--- src/qemu/qemu_monitor.c | 6 +++- src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 3 ++ src/qemu/qemu_monitor_text.c | 8 +++++- tests/qemumonitorjsontest.c | 8 +++--- tools/virsh.pod | 7 +++++ 9 files changed, 130 insertions(+), 10 deletions(-) -- 1.9.1
Extended the qemuMonitorCPUInfo with a halted flag. Extract the halted flag for both text and JSON monitor. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_monitor.c | 6 +++++- src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 3 +++ src/qemu/qemu_monitor_text.c | 8 +++++++- tests/qemumonitorjsontest.c | 8 ++++---- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 5175f4e..d9f66a2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1677,6 +1677,7 @@ qemuMonitorCPUInfoClear(qemuMonitorCPUInfoPtr cpus, cpus[i].thread_id = -1; cpus[i].vcpus = 0; cpus[i].tid = 0; + cpus[i].halted = false; VIR_FREE(cpus[i].qom_path); VIR_FREE(cpus[i].alias); @@ -1725,8 +1726,10 @@ qemuMonitorGetCPUInfoLegacy(struct qemuMonitorQueryCpusEntry *cpuentries, size_t i; for (i = 0; i < maxvcpus; i++) { - if (i < ncpuentries) + if (i < ncpuentries) { vcpus[i].tid = cpuentries[i].tid; + vcpus[i].halted = cpuentries[i].halted; + } /* for legacy hotplug to work we need to fake the vcpu count added by * enabling a given vcpu */ @@ -1864,6 +1867,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl } vcpus[anyvcpu].tid = cpuentries[j].tid; + vcpus[anyvcpu].halted = cpuentries[j].halted; } return 0; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7d78e5b..cb27412 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -394,6 +394,7 @@ int qemuMonitorSystemPowerdown(qemuMonitorPtr mon); struct qemuMonitorQueryCpusEntry { pid_t tid; char *qom_path; + bool halted; }; void qemuMonitorQueryCpusFree(struct qemuMonitorQueryCpusEntry *entries, size_t nentries); @@ -441,6 +442,8 @@ struct _qemuMonitorCPUInfo { /* internal for use in the matching code */ char *qom_path; + + bool halted; }; typedef struct _qemuMonitorCPUInfo qemuMonitorCPUInfo; typedef qemuMonitorCPUInfo *qemuMonitorCPUInfoPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b93220b..dff6d42 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1349,6 +1349,7 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data, for (i = 0; i < ncpus; i++) { virJSONValuePtr entry = virJSONValueArrayGet(data, i); int thread = 0; + bool halted = false; const char *qom_path; if (!entry) { ret = -2; @@ -1358,9 +1359,11 @@ 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, "thread_id", &thread)); + ignore_value(virJSONValueObjectGetBoolean(entry, "halted", &halted)); qom_path = virJSONValueObjectGetString(entry, "qom_path"); cpus[i].tid = thread; + cpus[i].halted = halted; if (VIR_STRDUP(cpus[i].qom_path, qom_path) < 0) goto cleanup; } diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index ff7cc79..f975347 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -521,7 +521,7 @@ qemuMonitorTextQueryCPUs(qemuMonitorPtr mon, * (qemu) info cpus * * CPU #0: pc=0x00000000000f0c4a thread_id=30019 * CPU #1: pc=0x00000000fffffff0 thread_id=30020 - * CPU #2: pc=0x00000000fffffff0 thread_id=30021 + * CPU #2: pc=0x00000000fffffff0 (halted) thread_id=30021 * */ line = qemucpus; @@ -541,6 +541,12 @@ qemuMonitorTextQueryCPUs(qemuMonitorPtr mon, cpu.tid = tid; + /* Extract halted indicator */ + if ((offset = strstr(line, "(halted)")) != NULL) + cpu.halted = true; + else + cpu.halted = false; + if (VIR_APPEND_ELEMENT_COPY(cpus, ncpus, cpu) < 0) { ret = -1; goto cleanup; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 993a373..5e72a0f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1332,10 +1332,10 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data) int ret = -1; struct qemuMonitorQueryCpusEntry *cpudata = NULL; struct qemuMonitorQueryCpusEntry expect[] = { - {17622, (char *) "/machine/unattached/device[0]"}, - {17624, (char *) "/machine/unattached/device[1]"}, - {17626, (char *) "/machine/unattached/device[2]"}, - {17628, NULL}, + {17622, (char *) "/machine/unattached/device[0]", true}, + {17624, (char *) "/machine/unattached/device[1]", true}, + {17626, (char *) "/machine/unattached/device[2]", true}, + {17628, NULL, true}, }; size_t ncpudata = 0; size_t i; -- 1.9.1
Adding a field to the domain's private vcpu object to hold the halted state information. Adding two functions in support of the halted state: - qemuDomainGetVcpuHalted: retrieve the halted state from a private vcpu object - qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states via qemu monitor and store the results in the private vcpu objects Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Hao QingFeng <haoqf@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 ++++ 2 files changed, 71 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c9416f..beb2d20 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6045,6 +6045,72 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, return ret; } +/** + * qemuDomainGetVcpuHalted: + * @vm: domain object + * @vcpu: cpu id + * + * Returns the vCPU halted state. + */ +bool +qemuDomainGetVcpuHalted(virDomainObjPtr vm, + unsigned int vcpuid) +{ + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); + return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted; +} + +/** + * qemuDomainRefreshVcpuHalted: + * @driver: qemu driver data + * @vm: domain object + * @asyncJob: current asynchronous job type + * + * Updates vCPU halted state in the private data of @vm. + * + * Returns 0 on success and -1 on error + */ +int +qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob) +{ + virDomainVcpuDefPtr vcpu; + qemuMonitorCPUInfoPtr info = NULL; + size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); + size_t i; + bool hotplug; + int rc; + int ret = -1; + + /* Not supported currently for TCG, see qemuDomainRefreshVcpuInfo */ + if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU) + return 0; + + hotplug = qemuDomainSupportsNewVcpuHotplug(vm); + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus, hotplug); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + if (rc < 0) + goto cleanup; + + for (i = 0; i < maxvcpus; i++) { + vcpu = virDomainDefGetVcpu(vm->def, i); + QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = info[i].halted; + } + + ret = 0; + + cleanup: + qemuMonitorCPUInfoFree(info, maxvcpus); + return ret; +} bool qemuDomainSupportsNicdev(virDomainDefPtr def, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index dee5d93..ec20796 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -320,6 +320,7 @@ struct _qemuDomainVcpuPrivate { pid_t tid; /* vcpu thread id */ int enable_id; /* order in which the vcpus were enabled in qemu */ char *alias; + bool halted; /* information for hotpluggable cpus */ char *type; @@ -665,6 +666,10 @@ int qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, bool state); +bool qemuDomainGetVcpuHalted(virDomainObjPtr vm, unsigned int vcpu); +int qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob); bool qemuDomainSupportsNicdev(virDomainDefPtr def, virDomainNetDefPtr net); -- 1.9.1
Extended qemuDomainGetStatsVcpu to include the per vcpu halted indicator if reported by QEMU. The key for new boolean value has the format "vcpu.<n>.halted". Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8789c9d..b269112 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1454,7 +1454,8 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, unsigned long long *cpuwait, int maxinfo, unsigned char *cpumaps, - int maplen) + int maplen, + bool *cpuhalted) { size_t ncpuinfo = 0; size_t i; @@ -1474,6 +1475,9 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, if (cpumaps) memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo); + if (cpuhalted) + memset(cpuhalted, 0, sizeof(*cpuhalted) * maxinfo); + for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo < maxinfo; i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); pid_t vcpupid = qemuDomainGetVcpuPid(vm, i); @@ -1511,6 +1515,9 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, return -1; } + if (cpuhalted) + cpuhalted[ncpuinfo] = qemuDomainGetVcpuHalted(vm, ncpuinfo); + ncpuinfo++; } @@ -5455,7 +5462,8 @@ qemuDomainGetVcpus(virDomainPtr dom, goto cleanup; } - ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen); + ret = qemuDomainHelperGetVcpus(vm, info, NULL, maxinfo, cpumaps, maplen, + NULL); cleanup: virDomainObjEndAPI(&vm); @@ -18922,7 +18930,7 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver, static int -qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, @@ -18933,6 +18941,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; virVcpuInfoPtr cpuinfo = NULL; unsigned long long *cpuwait = NULL; + bool *cpuhalted = NULL; if (virTypedParamsAddUInt(&record->params, &record->nparams, @@ -18952,9 +18961,14 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0) goto cleanup; + if (qemuDomainRefreshVcpuHalted(driver, dom, + QEMU_ASYNC_JOB_NONE) == 0 && + VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0) + goto cleanup; + if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait, virDomainDefGetVcpus(dom->def), - NULL, 0) < 0) { + NULL, 0, cpuhalted) < 0) { virResetLastError(); ret = 0; /* it's ok to be silent and go ahead */ goto cleanup; @@ -18990,6 +19004,17 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, param_name, cpuwait[i]) < 0) goto cleanup; + + if (cpuhalted) { + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "vcpu.%u.halted", cpuinfo[i].number); + if (virTypedParamsAddBoolean(&record->params, + &record->nparams, + maxparams, + param_name, + cpuhalted[i]) < 0) + goto cleanup; + } } ret = 0; @@ -18997,6 +19022,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, cleanup: VIR_FREE(cpuinfo); VIR_FREE(cpuwait); + VIR_FREE(cpuhalted); return ret; } -- 1.9.1
Added description for new vcpu.<num>.halted statistics value. While there, also added a description for vcpu.<num>.wait and clarified the units displayed for time and wait. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- tools/virsh.pod | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/virsh.pod b/tools/virsh.pod index 1fe4269..9e3a248 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -887,6 +887,9 @@ default all supported statistics groups are returned. Supported statistics groups flags are: I<--state>, I<--cpu-total>, I<--balloon>, I<--vcpu>, I<--interface>, I<--block>, I<--perf>. +Note that - depending on the hypervisor type and version or the domain state +- not all of the following statistics may be returned. + When selecting the I<--state> group the following fields are returned: "state.state" - state of the VM, returned as number from virDomainState enum, "state.reason" - reason for entering given state, returned as int from @@ -917,6 +920,10 @@ I<--vcpu> returns: "vcpu.<num>.state" - state of the virtual CPU <num>, as number from virVcpuState enum, "vcpu.<num>.time" - virtual cpu time spent by virtual CPU <num> + (in microseconds), +"vcpu.<num>.wait" - virtual cpu time spent by virtual CPU <num> +waiting on I/O (in microseconds), +"vcpu.<num>.halted" - virtual CPU <num> is halted: yes or no I<--interface> returns: "net.count" - number of network interfaces on this domain, -- 1.9.1
On 10/13/2016 07:42 AM, Viktor Mihajlovski wrote:
As a result of the discussions on https://www.redhat.com/archives/libvir-list/2016-October/msg00531.html:
In order to be able to report the per VCPU halted condition as returned by the QEMU monitor this series extends the VCPU-specific domain statistics with a vcpu.n.halted value.
This is done by - extending the monitor to pass back the halted condition for the vcpus - adding a new field to the private domain vcpu object reflecting the halted condition for the vcpu - modifying the driver code to report the new VCPU domain statistics value
The halted condition is however not recorded in the internal XML format, since the state can change asynchronously (without notification).
Patches 1/4 and 2/4 are taken without modifications from the original posting. In 4/4 I've only updated virsh.pod as I didn't want to touch libvirt-domain.c in this context just for documentation purposes.
Viktor Mihajlovski (4): qemu: Add monitor support for CPU halted state qemu: Add domain support for VCPU halted state qemu: add vcpu.n.halted to vcpu domain stats doc: update virsh domstats documentation for vcpu statistics
src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 5 ++++ src/qemu/qemu_driver.c | 34 ++++++++++++++++++++--- src/qemu/qemu_monitor.c | 6 +++- src/qemu/qemu_monitor.h | 3 ++ src/qemu/qemu_monitor_json.c | 3 ++ src/qemu/qemu_monitor_text.c | 8 +++++- tests/qemumonitorjsontest.c | 8 +++--- tools/virsh.pod | 7 +++++ 9 files changed, 130 insertions(+), 10 deletions(-)
The series looks good to me - do you think it's worthwhile to mention anywhere in virsh.pod that the "halted" could mean "really halted" or "not scheduled" or "not actively processing" depending on architecture? (and how best to say it). I can tweak virsh.pod before pushing based on your or others feedback/thoughts. Tks - John
On 24.10.2016 16:29, John Ferlan wrote: [...]
The series looks good to me - do you think it's worthwhile to mention anywhere in virsh.pod that the "halted" could mean "really halted" or "not scheduled" or "not actively processing" depending on architecture? (and how best to say it).
As the other values' documentation is quite terse, I was tempted to stay in style by being brief as well. Maybe something along the lines of: ... halted: yes or no (may indicate the processor is idle or even disabled, depending on the architecture).
I can tweak virsh.pod before pushing based on your or others feedback/thoughts.
Surely no objections from my side. [...] -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 10/24/2016 11:24 AM, Viktor Mihajlovski wrote:
On 24.10.2016 16:29, John Ferlan wrote: [...]
The series looks good to me - do you think it's worthwhile to mention anywhere in virsh.pod that the "halted" could mean "really halted" or "not scheduled" or "not actively processing" depending on architecture? (and how best to say it).
As the other values' documentation is quite terse, I was tempted to stay in style by being brief as well. Maybe something along the lines of: ... halted: yes or no (may indicate the processor is idle or even disabled, depending on the architecture).
Perfect - I went with that.
I can tweak virsh.pod before pushing based on your or others feedback/thoughts.
Surely no objections from my side. [...]
Now pushed John
On 25.10.2016 00:56, John Ferlan wrote:
[...]
Now pushed
John
Thanks! -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
John Ferlan -
Viktor Mihajlovski