On 09/15/14 10:48, Francesco Romani wrote: I'd rather "continue;" here as a vanishing domain shouldn't be a problem for this code. ACK with the nit above fixed. (I made the change locally, thus no need to re-send this one). Peter

This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL group of statistics. Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 7 +++++++ src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c2f9d26..eb62f96 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2513,6 +2513,7 @@ struct _virDomainStatsRecord { typedef enum { VIR_DOMAIN_STATS_STATE = (1 << 0), /* return domain state */ + VIR_DOMAIN_STATS_CPU_TOTAL = (1 << 1), /* return domain CPU info */ } virDomainStatsTypes; typedef enum { diff --git a/src/libvirt.c b/src/libvirt.c index f7e5a37..dfbd5c7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21596,6 +21596,13 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "state.reason" - reason for entering given state, returned as int from * virDomain*Reason enum corresponding to given state. * + * VIR_DOMAIN_STATS_CPU_TOTAL: Return CPU statistics and usage information. + * The typed parameter keys are in this format: + * "cpu.time" - total cpu time spent for this domain as unsigned long long. + * "cpu.user" - user cpu time spent as unsigned long long. + * "cpu.system" - system cpu time spent as unsigned long long. + * + * * Using 0 for @stats returns all stats groups supported by the given * hypervisor. * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 39e9d27..d0fad61 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -96,6 +96,7 @@ #include "storage/storage_driver.h" #include "virhostdev.h" #include "domain_capabilities.h" +#include "vircgroup.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -17388,6 +17389,45 @@ typedef enum { #define HAVE_MONITOR(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_MONITOR) +static int +qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv = dom->privateData; + unsigned long long cpu_time = 0; + unsigned long long user_time = 0; + unsigned long long sys_time = 0; + int err = 0; + + err = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time); + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.time", + cpu_time) < 0) + return -1; + + err = virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time); + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.user", + user_time) < 0) + return -1; + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.system", + sys_time) < 0) + return -1; + + return 0; +} + + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -17403,6 +17443,7 @@ struct qemuDomainGetStatsWorker { static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false }, + { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false }, { NULL, 0, false } }; -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote: This code doesn't check if priv->cgroup isn't NULL and dereferences it unconditionally. This would crash with shutoff machines. I'll add an if (priv->group) return 0; right at the beginning. Peter

This patch implements the VIR_DOMAIN_STATS_BALLOON group of statistics. Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 6 ++++++ src/qemu/qemu_driver.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index eb62f96..a5033ed 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2514,6 +2514,7 @@ struct _virDomainStatsRecord { typedef enum { VIR_DOMAIN_STATS_STATE = (1 << 0), /* return domain state */ VIR_DOMAIN_STATS_CPU_TOTAL = (1 << 1), /* return domain CPU info */ + VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */ } virDomainStatsTypes; typedef enum { diff --git a/src/libvirt.c b/src/libvirt.c index dfbd5c7..b3b71a0 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21602,6 +21602,12 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "cpu.user" - user cpu time spent as unsigned long long. * "cpu.system" - system cpu time spent as unsigned long long. * + * VIR_DOMAIN_STATS_BALLOON: Return memory balloon device information. + * The typed parameter keys are in this format: + * "balloon.current" - the memory in kiB currently used + * as unsigned long long. + * "balloon.maximum" - the maximum memory in kiB allowed + * as unsigned long long. * * Using 0 for @stats returns all stats groups supported by the given * hypervisor. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d0fad61..745b4f1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2525,6 +2525,7 @@ static int qemuDomainSendKey(virDomainPtr domain, return ret; } + static int qemuDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { @@ -17427,6 +17428,42 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, return 0; } +static int +qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + qemuDomainObjPrivatePtr priv = dom->privateData; + unsigned long long cur_balloon = 0; + int err = 0; + + if (dom->def->memballoon && + dom->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) { + cur_balloon = dom->def->mem.max_balloon; + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) { + cur_balloon = dom->def->mem.cur_balloon; + } else { + err = -1; + } + + if (!err && virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "balloon.current", + cur_balloon) < 0) + return -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "balloon.maximum", + dom->def->mem.max_balloon) < 0) + return -1; + + return 0; +} typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, @@ -17444,6 +17481,7 @@ struct qemuDomainGetStatsWorker { static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false }, { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false }, + { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true }, { NULL, 0, false } }; -- 1.9.3

This patch implements the VIR_DOMAIN_STATS_VCPU group of statistics. To do so, this patch also extracts a helper to gather the VCpu information. Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 12 +++ src/qemu/qemu_driver.c | 201 +++++++++++++++++++++++++++++-------------- 3 files changed, 150 insertions(+), 64 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a5033ed..4b851a5 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2515,6 +2515,7 @@ typedef enum { VIR_DOMAIN_STATS_STATE = (1 << 0), /* return domain state */ VIR_DOMAIN_STATS_CPU_TOTAL = (1 << 1), /* return domain CPU info */ VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */ + VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */ } virDomainStatsTypes; typedef enum { diff --git a/src/libvirt.c b/src/libvirt.c index b3b71a0..1d91c99 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21609,6 +21609,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "balloon.maximum" - the maximum memory in kiB allowed * as unsigned long long. * + * VIR_DOMAIN_STATS_VCPU: Return virtual CPU statistics. + * Due to VCPU hotplug, the vcpu.<num>.* array could be sparse. + * The actual size of the array correspond to "vcpu.current". + * The array size will never exceed "vcpu.maximum". + * The typed parameter keys are in this format: + * "vcpu.current" - current number of online virtual CPUs as unsigned int. + * "vcpu.maximum" - maximum number of online virtual CPUs as unsigned int. + * "vcpu.<num>.state" - state of the virtual CPU <num>, as int + * from virVcpuState enum. + * "vcpu.<num>.time" - virtual cpu time spent by virtual CPU <num> + * as unsigned long long. + * * Using 0 for @stats returns all stats groups supported by the given * hypervisor. * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 745b4f1..4a92f58 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1380,6 +1380,76 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, } +static int +qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, + unsigned char *cpumaps, int maplen) +{ + int maxcpu, hostcpus; + size_t i, v; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if ((hostcpus = nodeGetCPUCount()) < 0) + return -1; + + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + /* Clamp to actual number of vcpus */ + if (maxinfo > priv->nvcpupids) + maxinfo = priv->nvcpupids; + + if (maxinfo >= 1) { + if (info != NULL) { + memset(info, 0, sizeof(*info) * maxinfo); + for (i = 0; i < maxinfo; i++) { + info[i].number = i; + info[i].state = VIR_VCPU_RUNNING; + + if (priv->vcpupids != NULL && + qemuGetProcessInfo(&(info[i].cpuTime), + &(info[i].cpu), + NULL, + vm->pid, + priv->vcpupids[i]) < 0) { + virReportSystemError(errno, "%s", + _("cannot get vCPU placement & pCPU time")); + return -1; + } + } + } + + if (cpumaps != NULL) { + memset(cpumaps, 0, maplen * maxinfo); + if (priv->vcpupids != NULL) { + for (v = 0; v < maxinfo; v++) { + unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); + virBitmapPtr map = NULL; + unsigned char *tmpmap = NULL; + int tmpmapLen = 0; + + if (virProcessGetAffinity(priv->vcpupids[v], + &map, maxcpu) < 0) + return -1; + virBitmapToData(map, &tmpmap, &tmpmapLen); + if (tmpmapLen > maplen) + tmpmapLen = maplen; + memcpy(cpumap, tmpmap, tmpmapLen); + + VIR_FREE(tmpmap); + virBitmapFree(map); + } + } else { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not available")); + return -1; + } + } + } + return maxinfo; +} + + static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, int id) { @@ -4960,10 +5030,7 @@ qemuDomainGetVcpus(virDomainPtr dom, int maplen) { virDomainObjPtr vm; - size_t i; - int v, maxcpu, hostcpus; int ret = -1; - qemuDomainObjPrivatePtr priv; if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -4978,67 +5045,7 @@ qemuDomainGetVcpus(virDomainPtr dom, goto cleanup; } - priv = vm->privateData; - - if ((hostcpus = nodeGetCPUCount()) < 0) - goto cleanup; - - maxcpu = maplen * 8; - if (maxcpu > hostcpus) - maxcpu = hostcpus; - - /* Clamp to actual number of vcpus */ - if (maxinfo > priv->nvcpupids) - maxinfo = priv->nvcpupids; - - if (maxinfo >= 1) { - if (info != NULL) { - memset(info, 0, sizeof(*info) * maxinfo); - for (i = 0; i < maxinfo; i++) { - info[i].number = i; - info[i].state = VIR_VCPU_RUNNING; - - if (priv->vcpupids != NULL && - qemuGetProcessInfo(&(info[i].cpuTime), - &(info[i].cpu), - NULL, - vm->pid, - priv->vcpupids[i]) < 0) { - virReportSystemError(errno, "%s", - _("cannot get vCPU placement & pCPU time")); - goto cleanup; - } - } - } - - if (cpumaps != NULL) { - memset(cpumaps, 0, maplen * maxinfo); - if (priv->vcpupids != NULL) { - for (v = 0; v < maxinfo; v++) { - unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); - virBitmapPtr map = NULL; - unsigned char *tmpmap = NULL; - int tmpmapLen = 0; - - if (virProcessGetAffinity(priv->vcpupids[v], - &map, maxcpu) < 0) - goto cleanup; - virBitmapToData(map, &tmpmap, &tmpmapLen); - if (tmpmapLen > maplen) - tmpmapLen = maplen; - memcpy(cpumap, tmpmap, tmpmapLen); - - VIR_FREE(tmpmap); - virBitmapFree(map); - } - } else { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cpu affinity is not available")); - goto cleanup; - } - } - } - ret = maxinfo; + ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen); cleanup: if (vm) @@ -17465,6 +17472,71 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, return 0; } + +static int +qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + int ret = -1; + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + virVcpuInfoPtr cpuinfo = NULL; + + if (virTypedParamsAddUInt(&record->params, + &record->nparams, + maxparams, + "vcpu.current", + (unsigned) dom->def->vcpus) < 0) + return -1; + + if (virTypedParamsAddUInt(&record->params, + &record->nparams, + maxparams, + "vcpu.maximum", + (unsigned) dom->def->maxvcpus) < 0) + return -1; + + if (VIR_ALLOC_N(cpuinfo, dom->def->vcpus) < 0) + return -1; + + if (qemuDomainHelperGetVcpus(dom, cpuinfo, dom->def->vcpus, + NULL, 0) < 0) { + virResetLastError(); + ret = 0; /* it's ok to be silent and go ahead */ + goto cleanup; + } + + for (i = 0; i < dom->def->vcpus; i++) { + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "vcpu.%u.state", cpuinfo[i].number); + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + param_name, + cpuinfo[i].state) < 0) + goto cleanup; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "vcpu.%u.time", cpuinfo[i].number); + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + param_name, + cpuinfo[i].cpuTime) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(cpuinfo); + return ret; +} + + typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -17482,6 +17554,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false }, { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false }, { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true }, + { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { NULL, 0, false } }; -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote: nvcpupids is 0 if the VM is offline, thus: You don't initialize the returned CPU numbers to sane values (they stay "0") .. and this then tries to assign the "vcpu.0.state" parameter multiple times when a VM has multiple CPUs and is offline. I'll add a check that will skip this if the domain is offline. Peter

This patch implements the VIR_DOMAIN_STATS_BLOCK group of statistics. To do so, an helper function to get the block stats of all the disks of a domain is added. Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 20 +++++++ src/qemu/qemu_driver.c | 81 ++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 26 +++++++++ src/qemu/qemu_monitor.h | 20 +++++++ src/qemu/qemu_monitor_json.c | 136 +++++++++++++++++++++++++++++-------------- src/qemu/qemu_monitor_json.h | 4 ++ 7 files changed, 245 insertions(+), 43 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 17b1b43..724314e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2517,6 +2517,7 @@ typedef enum { VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */ VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */ VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */ + VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */ } virDomainStatsTypes; typedef enum { diff --git a/src/libvirt.c b/src/libvirt.c index 5534b2f..ab10a3a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21635,6 +21635,26 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "net.<num>.tx.errs" - transmission errors as unsigned long long. * "net.<num>.tx.drop" - transmit packets dropped as unsigned long long. * + * VIR_DOMAIN_STATS_BLOCK: Return block devices statistics. + * The typed parameter keys are in this format: + * "block.count" - number of block devices on this domain + * as unsigned int. + * "block.<num>.name" - name of the block device <num> as string. + * matches the name of the block device. + * "block.<num>.rd.reqs" - number of read requests as unsigned long long. + * "block.<num>.rd.bytes" - number of read bytes as unsigned long long. + * "block.<num>.rd.times" - total time (ns) spent on reads as + * unsigned long long. + * "block.<num>.wr.reqs" - number of write requests as unsigned long long. + * "block.<num>.wr.bytes" - number of written bytes as unsigned long long. + * "block.<num>.wr.times" - total time (ns) spent on writes as + * unsigned long long. + * "block.<num>.fl.reqs" - total flush requests as unsigned long long. + * "block.<num>.fl.times" - total time (ns) spent on cache flushing as + * unsigned long long. + * "block.<num>.errors" - Xen only: the 'oo_req' value as + * unsigned long long. + * * Using 0 for @stats returns all stats groups supported by the given * hypervisor. * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 016499d..446b04b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9664,6 +9664,7 @@ qemuDomainBlockStats(virDomainPtr dom, return ret; } + static int qemuDomainBlockStatsFlags(virDomainPtr dom, const char *path, @@ -17621,6 +17622,85 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, #undef QEMU_ADD_NET_PARAM +/* expects a LL, but typed parameter must be ULL */ +#define QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, num, name, value) \ +do { \ + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ + "block.%zu.%s", num, name); \ + if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + value) < 0) \ + goto cleanup; \ +} while (0) + +static int +qemuDomainGetStatsBlock(virQEMUDriverPtr driver, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags) +{ + size_t i; + int ret = -1; + int nstats = 0; + qemuBlockStatsPtr stats = NULL; + qemuDomainObjPrivatePtr priv = dom->privateData; + + if (!HAVE_MONITOR(privflags) || !virDomainObjIsActive(dom)) + return 0; /* it's ok, just go ahead silently */ + + if (VIR_ALLOC_N(stats, dom->def->ndisks) < 0) + return -1; + + qemuDomainObjEnterMonitor(driver, dom); + + nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL, + stats, nstats); + + qemuDomainObjExitMonitor(driver, dom); + + if (nstats < 0) { + virResetLastError(); + ret = 0; /* still ok, again go ahead silently */ + goto cleanup; + } + + QEMU_ADD_COUNT_PARAM(record, maxparams, "block", dom->def->ndisks); + + for (i = 0; i < nstats; i++) { + QEMU_ADD_NAME_PARAM(record, maxparams, + "block", i, dom->def->disks[i]->dst); + + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "rd.reqs", stats[i].rd_req); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "rd.bytes", stats[i].rd_bytes); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "rd.times", stats[i].rd_total_times); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "wr.reqs", stats[i].wr_req); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "wr.bytes", stats[i].wr_bytes); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "wr.times", stats[i].wr_total_times); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "fl.reqs", stats[i].flush_req); + QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, + "fl.times", stats[i].flush_total_times); + } + + ret = 0; + + cleanup: + VIR_FREE(stats); + return ret; +} + +#undef QEMU_ADD_BLOCK_PARAM_LL + #undef QEMU_ADD_NAME_PARAM #undef QEMU_ADD_COUNT_PARAM @@ -17644,6 +17724,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true }, { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, + { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true }, { NULL, 0, false } }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6059133..1ca8d55 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1760,6 +1760,32 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, return ret; } +/* Fills the first 'nstats' block stats. 'stats' must be an array. + * Returns <0 on error, otherwise the number of block stats retrieved. + * if 'dev_name' is != NULL, look for this device only and skip + * any other. In that case return value cannot be greater than 1. + */ +int +qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, + const char *dev_name, + qemuBlockStatsPtr stats, + int nstats) +{ + int ret; + VIR_DEBUG("mon=%p dev=%s", mon, dev_name); + + if (mon->json) { + ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, + stats, nstats); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to query all block stats with this QEMU")); + return -1; + } + + return ret; +} + /* Return 0 and update @nparams with the number of block stats * QEMU supports if success. Return -1 if failure. */ diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c32001d..6eb0036 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -346,6 +346,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, long long *flush_req, long long *flush_total_times, long long *errs); + +typedef struct _qemuBlockStats qemuBlockStats; +typedef qemuBlockStats *qemuBlockStatsPtr; +struct _qemuBlockStats { + long long rd_req; + long long rd_bytes; + long long wr_req; + long long wr_bytes; + long long rd_total_times; + long long wr_total_times; + long long flush_req; + long long flush_total_times; +}; + +int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, + const char *dev_name, + qemuBlockStatsPtr stats, + int nstats) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0c4832a..d45c41f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1734,13 +1734,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *flush_total_times, long long *errs) { - int ret; - size_t i; - bool found = false; - virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", - NULL); - virJSONValuePtr reply = NULL; - virJSONValuePtr devices; + qemuBlockStats stats; + int ret = -1; *rd_req = *rd_bytes = -1; *wr_req = *wr_bytes = *errs = -1; @@ -1754,9 +1749,49 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, if (flush_total_times) *flush_total_times = -1; + if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, &stats, 1) != 1) + goto cleanup; + + *rd_req = stats.rd_req; + *rd_bytes = stats.rd_bytes; + *wr_req = stats.wr_req; + *wr_bytes = stats.wr_bytes; + *errs = -1; /* QEMU does not have this */ + + if (rd_total_times) + *rd_total_times = stats.rd_total_times; + if (wr_total_times) + *wr_total_times = stats.wr_total_times; + if (flush_req) + *flush_req = stats.flush_req; + if (flush_total_times) + *flush_total_times = stats.flush_total_times; + + ret = 0; + + cleanup: + return ret; +} + + +int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, + const char *dev_name, + qemuBlockStatsPtr bstats, + int nstats) +{ + int ret, count; + size_t i; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices; + if (!cmd) return -1; + if (!bstats || nstats <= 0) + return -1; + ret = qemuMonitorJSONCommand(mon, cmd, &reply); if (ret == 0) @@ -1772,108 +1807,123 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } - for (i = 0; i < virJSONValueArraySize(devices); i++) { + count = 0; + for (i = 0; i < virJSONValueArraySize(devices) && count < nstats; i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); virJSONValuePtr stats; - const char *thisdev; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not in expected format")); - goto cleanup; - } - - if ((thisdev = virJSONValueObjectGetString(dev, "device")) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not in expected format")); + _("blockstats device entry was not " + "in expected format")); goto cleanup; } - /* New QEMU has separate names for host & guest side of the disk - * and libvirt gives the host side a 'drive-' prefix. The passed - * in dev_name is the guest side though + /* If dev_name is specified, we are looking for a specific device, + * so we must be stricter. */ - if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) - thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); + if (dev_name) { + const char *thisdev = virJSONValueObjectGetString(dev, "device"); + if (!thisdev) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not " + "in expected format")); + goto cleanup; + } - if (STRNEQ(thisdev, dev_name)) - continue; + /* New QEMU has separate names for host & guest side of the disk + * and libvirt gives the host side a 'drive-' prefix. The passed + * in dev_name is the guest side though + */ + if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) + thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STRNEQ(thisdev, dev_name)) + continue; + } - found = true; if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || stats->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not in expected format")); + _("blockstats stats entry was not " + "in expected format")); goto cleanup; } - if (virJSONValueObjectGetNumberLong(stats, "rd_bytes", rd_bytes) < 0) { + if (virJSONValueObjectGetNumberLong(stats, "rd_bytes", + &bstats->rd_bytes) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "rd_bytes"); goto cleanup; } - if (virJSONValueObjectGetNumberLong(stats, "rd_operations", rd_req) < 0) { + if (virJSONValueObjectGetNumberLong(stats, "rd_operations", + &bstats->rd_req) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "rd_operations"); goto cleanup; } - if (rd_total_times && - virJSONValueObjectHasKey(stats, "rd_total_time_ns") && + if (virJSONValueObjectHasKey(stats, "rd_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "rd_total_time_ns", - rd_total_times) < 0)) { + &bstats->rd_total_times) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "rd_total_time_ns"); goto cleanup; } - if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", wr_bytes) < 0) { + if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", + &bstats->wr_bytes) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "wr_bytes"); goto cleanup; } - if (virJSONValueObjectGetNumberLong(stats, "wr_operations", wr_req) < 0) { + if (virJSONValueObjectGetNumberLong(stats, "wr_operations", + &bstats->wr_req) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "wr_operations"); goto cleanup; } - if (wr_total_times && - virJSONValueObjectHasKey(stats, "wr_total_time_ns") && + if (virJSONValueObjectHasKey(stats, "wr_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "wr_total_time_ns", - wr_total_times) < 0)) { + &bstats->wr_total_times) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "wr_total_time_ns"); goto cleanup; } - if (flush_req && - virJSONValueObjectHasKey(stats, "flush_operations") && + if (virJSONValueObjectHasKey(stats, "flush_operations") && (virJSONValueObjectGetNumberLong(stats, "flush_operations", - flush_req) < 0)) { + &bstats->flush_req) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "flush_operations"); goto cleanup; } - if (flush_total_times && - virJSONValueObjectHasKey(stats, "flush_total_time_ns") && + if (virJSONValueObjectHasKey(stats, "flush_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "flush_total_time_ns", - flush_total_times) < 0)) { + &bstats->flush_total_times) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s statistic"), "flush_total_time_ns"); goto cleanup; } + + count++; + bstats++; + + if (dev_name && count) + break; } - if (!found) { + if (dev_name && !count) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find statistics for device '%s'"), dev_name); goto cleanup; } - ret = 0; + + ret = count; cleanup: virJSONValueFree(cmd); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index b5c61ca..b6d46d4 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -79,6 +79,10 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, long long *flush_req, long long *flush_total_times, long long *errs); +int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, + const char *dev_name, + qemuBlockStatsPtr stats, + int nstats); int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote: I've just noticed this spurious newline. nstats is here initialized to 0. and not touched, thus it's still 0 when called here, which ... .. returns failure here. nstats needs to be set to the disk count, otherwise this stats group won't work. Peter