[libvirt] [PATCH 00/11] bulk stats: QEMU implementation
This patchset enhances the QEMU support for the new bulk stats API. What is added is the equivalent of these APIs: virDomainBlockInfo virDomainGetInfo - for balloon stats virDomainGetCPUStats virDomainBlockStatsFlags virDomainInterfaceStats virDomainGetVcpusFlags virDomainGetVcpus This subset of API is the one oVirt relies on. The patchset is organized as follows: - the first 4 patches do refactoring to extract internal helper functions to be used by the old API and by the new bulk one. For block stats on helper is actually added instead of extracted. - since some groups require access to the QEMU monitor, one patch extend the internal interface to easily accomodate that - finally, the last six patches implement the support for the bulk API. Francesco Romani (11): qemu: extract helper to get the current balloon qemu: extract helper to gather vcpu data qemu: add helper to get the block stats qemu: extract helper to get block info qemu: bulk stats: pass connection to workers qemu: bulk stats: implement CPU stats group qemu: bulk stats: implement balloon group qemu: bulk stats: implement VCPU group qemu: bulk stats: implement interface group qemu: bulk stats: implement block group qemu: bulk stats: implement blockinfo group include/libvirt/libvirt.h.in | 6 + src/qemu/qemu_driver.c | 558 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 502 insertions(+), 62 deletions(-) -- 1.9.3
Refactor the code to extract an helper method to get the current balloon settings. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 98 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 239a300..bbd16ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -168,6 +168,9 @@ static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid, const char *path, int oflags, bool *needUnlink, bool *bypassSecurityDriver); +static int qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned long *memory); virQEMUDriverPtr qemu_driver = NULL; @@ -2519,6 +2522,60 @@ static int qemuDomainSendKey(virDomainPtr domain, return ret; } +static int qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned long *memory) +{ + int ret = -1; + int err = 0; + qemuDomainObjPrivatePtr priv = vm->privateData; + + if ((vm->def->memballoon != NULL) && + (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { + *memory = vm->def->mem.max_balloon; + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) { + *memory = vm->def->mem.cur_balloon; + } else if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { + unsigned long long balloon; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) + err = 0; + else { + qemuDomainObjEnterMonitor(driver, vm); + err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); + qemuDomainObjExitMonitor(driver, vm); + } + if (!qemuDomainObjEndJob(driver, vm)) { + vm = NULL; + goto cleanup; + } + + if (err < 0) { + /* We couldn't get current memory allocation but that's not + * a show stopper; we wouldn't get it if there was a job + * active either + */ + *memory = vm->def->mem.cur_balloon; + } else if (err == 0) { + /* Balloon not supported, so maxmem is always the allocation */ + *memory = vm->def->mem.max_balloon; + } else { + *memory = balloon; + } + } else { + *memory = vm->def->mem.cur_balloon; + } + + ret = 0; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + static int qemuDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { @@ -2526,7 +2583,6 @@ static int qemuDomainGetInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; int err; - unsigned long long balloon; if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -2549,43 +2605,9 @@ static int qemuDomainGetInfo(virDomainPtr dom, info->maxMem = vm->def->mem.max_balloon; if (virDomainObjIsActive(vm)) { - qemuDomainObjPrivatePtr priv = vm->privateData; - - if ((vm->def->memballoon != NULL) && - (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { - info->memory = vm->def->mem.max_balloon; - } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) { - info->memory = vm->def->mem.cur_balloon; - } else if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) - goto cleanup; - if (!virDomainObjIsActive(vm)) - err = 0; - else { - qemuDomainObjEnterMonitor(driver, vm); - err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); - qemuDomainObjExitMonitor(driver, vm); - } - if (!qemuDomainObjEndJob(driver, vm)) { - vm = NULL; - goto cleanup; - } - - if (err < 0) { - /* We couldn't get current memory allocation but that's not - * a show stopper; we wouldn't get it if there was a job - * active either - */ - info->memory = vm->def->mem.cur_balloon; - } else if (err == 0) { - /* Balloon not supported, so maxmem is always the allocation */ - info->memory = vm->def->mem.max_balloon; - } else { - info->memory = balloon; - } - } else { - info->memory = vm->def->mem.cur_balloon; - } + err = qemuDomainGetBalloonMemory(driver, vm, &info->memory); + if (err) + return err; } else { info->memory = 0; } -- 1.9.3
Extracts an helper to gether the VCpu information. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bbd16ed..1842e60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -172,6 +172,12 @@ static int qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned long *memory); +static int qemuDomainHelperGetVcpus(virDomainObjPtr vm, + virVcpuInfoPtr info, + int maxinfo, + unsigned char *cpumaps, + int maplen); + virQEMUDriverPtr qemu_driver = NULL; @@ -4974,10 +4980,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; @@ -4992,7 +4995,25 @@ qemuDomainGetVcpus(virDomainPtr dom, goto cleanup; } - priv = vm->privateData; + ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen); + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int +qemuDomainHelperGetVcpus(virDomainObjPtr vm, + virVcpuInfoPtr info, + int maxinfo, + unsigned char *cpumaps, + int maplen) +{ + int ret = -1; + int v, maxcpu, hostcpus; + size_t i; + qemuDomainObjPrivatePtr priv = vm->privateData; if ((hostcpus = nodeGetCPUCount()) < 0) goto cleanup; -- 1.9.3
Add an helper function to get the block stats of a disk. This helper is meant to be used by the bulk stats API; future patches may want to refactor qemuDomainGetBlock* to make use of this function as well. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1842e60..e7dd5ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -136,6 +136,18 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_NB_BANDWIDTH_PARAM 6 +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; + long long errs; /* meaning less for QEMU */ +}; + static void processWatchdogEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, int action); @@ -178,6 +190,12 @@ static int qemuDomainHelperGetVcpus(virDomainObjPtr vm, unsigned char *cpumaps, int maplen); +static int qemuDiskGetBlockStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + struct qemuBlockStats *stats); + + virQEMUDriverPtr qemu_driver = NULL; @@ -9672,6 +9690,47 @@ qemuDomainBlockStats(virDomainPtr dom, return ret; } + + +static int +qemuDiskGetBlockStats(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + struct qemuBlockStats *stats) +{ + int ret = -1; + qemuDomainObjPrivatePtr priv; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + priv = vm->privateData; + + qemuDomainObjEnterMonitor(driver, vm); + + ret = qemuMonitorGetBlockStatsInfo(priv->mon, + disk->info.alias, + &stats->rd_req, + &stats->rd_bytes, + &stats->rd_total_times, + &stats->wr_req, + &stats->wr_bytes, + &stats->wr_total_times, + &stats->flush_req, + &stats->flush_total_times, + &stats->errs); + + qemuDomainObjExitMonitor(driver, vm); + + if (!qemuDomainObjEndJob(driver, vm)) + vm = NULL; + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + static int qemuDomainBlockStatsFlags(virDomainPtr dom, const char *path, -- 1.9.3
Extract qemuDiskGetBlockInfo helper. This way, the very same code will be used both by existing qemuDomainGetBlockInfo API and by the new bulk stats API. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e7dd5ed..ee0a576 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -195,6 +195,12 @@ static int qemuDiskGetBlockStats(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, struct qemuBlockStats *stats); +static int qemuDiskGetBlockInfo(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + const char *path, + virDomainBlockInfoPtr info); + virQEMUDriverPtr qemu_driver = NULL; @@ -10451,29 +10457,16 @@ qemuDomainGetBlockInfo(virDomainPtr dom, virDomainBlockInfoPtr info, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; - virDomainObjPtr vm; - int ret = -1; - int fd = -1; - off_t end; - virStorageSourcePtr meta = NULL; - virDomainDiskDefPtr disk = NULL; - struct stat sb; int idx; - int format; - int activeFail = false; - virQEMUDriverConfigPtr cfg = NULL; - char *alias = NULL; - char *buf = NULL; - ssize_t len; + int ret = -1; + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; virCheckFlags(0, -1); if (!(vm = qemuDomObjFromDomain(dom))) return -1; - cfg = virQEMUDriverGetConfig(driver); - if (virDomainGetBlockInfoEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -10489,7 +10482,34 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto cleanup; } - disk = vm->def->disks[idx]; + ret = qemuDiskGetBlockInfo(driver, vm, vm->def->disks[idx], path, info); + + cleanup: + virObjectUnlock(vm); + return ret; +} + + +static int +qemuDiskGetBlockInfo(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + const char *path, + virDomainBlockInfoPtr info) +{ + int ret = -1; + int fd = -1; + off_t end; + virQEMUDriverConfigPtr cfg = NULL; + virStorageSourcePtr meta = NULL; + struct stat sb; + int format; + int activeFail = false; + char *alias = NULL; + char *buf = NULL; + ssize_t len; + + cfg = virQEMUDriverGetConfig(driver); if (virStorageSourceIsLocalStorage(disk->src)) { if (!disk->src->path) { -- 1.9.3
Future patches which will implement more bulk stats groups for QEMU will need to access the connection object, so enrich the worker prototype. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ee0a576..d4eda06 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17320,7 +17320,8 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, static int -qemuDomainGetStatsState(virDomainObjPtr dom, +qemuDomainGetStatsState(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, unsigned int privflags ATTRIBUTE_UNUSED) @@ -17342,9 +17343,9 @@ qemuDomainGetStatsState(virDomainObjPtr dom, return 0; } - typedef int -(*qemuDomainGetStatsFunc)(virDomainObjPtr dom, +(*qemuDomainGetStatsFunc)(virConnectPtr conn, + virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, unsigned int flags); @@ -17405,7 +17406,7 @@ qemuDomainGetStats(virConnectPtr conn, for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) { if (stats & qemuDomainGetStatsWorkers[i].stats) { - if (qemuDomainGetStatsWorkers[i].func(dom, tmp, &maxparams, + if (qemuDomainGetStatsWorkers[i].func(conn, dom, tmp, &maxparams, flags) < 0) goto cleanup; } -- 1.9.3
This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL group of statistics. Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 9358314..992b124 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2511,6 +2511,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/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d4eda06..7ffd052 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 @@ -17343,6 +17344,60 @@ qemuDomainGetStatsState(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } + +static int +qemuDomainGetStatsCpu(virConnectPtr conn 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 ncpus = 0; + + ncpus = nodeGetCPUCount(); + + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + "cpu.count", + ncpus) < 0) + return -1; + + if (virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time) < 0) + return -1; + + if (virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time) < 0) + return -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.time", + cpu_time) < 0) + return -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.user", + user_time) < 0) + return -1; + + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + "cpu.system", + sys_time) < 0) + return -1; + + return 0; +} + + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, virDomainObjPtr dom, @@ -17357,6 +17412,7 @@ struct qemuDomainGetStatsWorker { static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE}, + { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL }, { NULL, 0 } }; -- 1.9.3
This patch implements the VIR_DOMAIN_STATS_BALLOON group of statistics. Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 992b124..78eb9b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2512,6 +2512,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/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7ffd052..9825f61 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17397,6 +17397,37 @@ qemuDomainGetStatsCpu(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int +qemuDomainGetStatsBalloon(virConnectPtr conn, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + virQEMUDriverPtr driver = conn->privateData; + unsigned long cur_balloon = 0; + int err = 0; + + err = qemuDomainGetBalloonMemory(driver, dom, &cur_balloon); + if (err) + return -1; + + if (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)(virConnectPtr conn, @@ -17413,6 +17444,7 @@ struct qemuDomainGetStatsWorker { static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE}, { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL }, + { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON }, { NULL, 0 } }; -- 1.9.3
This patch implements the VIR_DOMAIN_STATS_VCPU group of statistics. Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 72 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 78eb9b8..86ef18b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2513,6 +2513,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/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9825f61..527a6b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17429,6 +17429,77 @@ qemuDomainGetStatsBalloon(virConnectPtr conn, return 0; } + +static int +qemuDomainGetStatsVcpu(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + int ret = -1; + char param_name[NAME_MAX]; + virVcpuInfoPtr cpuinfo = NULL; + + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + "vcpu.current", + dom->def->vcpus) < 0) + return -1; + + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + "vcpu.maximum", + dom->def->maxvcpus) < 0) + return -1; + + if (VIR_ALLOC_N(cpuinfo, dom->def->vcpus) < 0) + return -1; + + if ((ret = qemuDomainHelperGetVcpus(dom, + cpuinfo, + dom->def->vcpus, + NULL, + 0)) < 0) + goto cleanup; + + for (i = 0; i < dom->def->vcpus; i++) { + snprintf(param_name, NAME_MAX, "vcpu.%u.state", cpuinfo[i].number); + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + param_name, + cpuinfo[i].state) < 0) + goto cleanup; + + snprintf(param_name, NAME_MAX, "vcpu.%u.time", cpuinfo[i].number); + if (virTypedParamsAddULLong(&record->params, + &record->nparams, + maxparams, + param_name, + cpuinfo[i].cpuTime) < 0) + goto cleanup; + + snprintf(param_name, NAME_MAX, "vcpu.%u.cpu", cpuinfo[i].number); + if (virTypedParamsAddInt(&record->params, + &record->nparams, + maxparams, + param_name, + cpuinfo[i].cpu) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(cpuinfo); + return ret; +} + + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, virDomainObjPtr dom, @@ -17445,6 +17516,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE}, { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL }, { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON }, + { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU }, { NULL, 0 } }; -- 1.9.3
This patch implements the VIR_DOMAIN_STATS_INTERFACE group of statistics. Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 86ef18b..8c15583 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2514,6 +2514,7 @@ typedef enum { 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 */ + VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */ } virDomainStatsTypes; typedef enum { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 527a6b4..818fcbc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17500,6 +17500,58 @@ qemuDomainGetStatsVcpu(virConnectPtr conn ATTRIBUTE_UNUSED, } +#define QEMU_ADD_NET_PARAM(RECORD, MAXPARAMS, IFNAME, NAME, VALUE) \ +do { \ + char param_name[NAME_MAX]; \ + snprintf(param_name, NAME_MAX, "net.%s.%s", IFNAME, NAME); \ + if (virTypedParamsAddLLong(&RECORD->params, \ + &RECORD->nparams, \ + MAXPARAMS, \ + param_name, \ + VALUE) < 0) \ + return -1; \ +} while (0) + +static int +qemuDomainGetStatsInterface(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + struct _virDomainInterfaceStats tmp; + + /* Check the path is one of the domain's network interfaces. */ + for (i = 0; i < dom->def->nnets; i++) { + memset(&tmp, 0, sizeof(tmp)); + + if (virNetInterfaceStats(dom->def->nets[i]->ifname, &tmp) < 0) + continue; + + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "rx.bytes", tmp.rx_bytes); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "rx.pkts", tmp.rx_packets); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "rx.errs", tmp.rx_errs); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "rx.drop", tmp.rx_drop); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "tx.bytes", tmp.tx_bytes); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "tx.pkts", tmp.tx_packets); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "tx.errs", tmp.tx_errs); + QEMU_ADD_NET_PARAM(record, maxparams, dom->def->nets[i]->ifname, + "tx.drop", tmp.tx_drop); + } + + return 0; +} + +#undef QEMU_ADD_NET_PARAM + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, virDomainObjPtr dom, @@ -17517,6 +17569,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL }, { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON }, { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU }, + { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE }, { NULL, 0 } }; -- 1.9.3
This patch implements the VIR_DOMAIN_STATS_BLOCK group of statistics. Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8c15583..372e098 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2515,6 +2515,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/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 818fcbc..344b02e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17552,6 +17552,59 @@ qemuDomainGetStatsInterface(virConnectPtr conn ATTRIBUTE_UNUSED, #undef QEMU_ADD_NET_PARAM +#define QEMU_ADD_BLOCK_PARAM(RECORD, MAXPARAMS, BLOCK, NAME, VALUE) \ +do { \ + char param_name[NAME_MAX]; \ + snprintf(param_name, NAME_MAX, "block.%s.%s", BLOCK, NAME); \ + if (virTypedParamsAddLLong(&RECORD->params, \ + &RECORD->nparams, \ + MAXPARAMS, \ + param_name, \ + VALUE) < 0) \ + return -1; \ +} while (0) + +static int +qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + virQEMUDriverPtr driver = conn->privateData; + struct qemuBlockStats stats; + size_t i; + + for (i = 0; i < dom->def->ndisks; i++) { + memset(&stats, 0, sizeof(stats)); + + if (qemuDiskGetBlockStats(driver, dom, dom->def->disks[i], &stats) < 0) + continue; + + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "rd.reqs", stats.rd_req); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "rd.bytes", stats.rd_bytes); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "rd.times", stats.rd_total_times); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "wr.reqs", stats.wr_req); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "wr.bytes", stats.wr_bytes); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "wr.times", stats.wr_total_times); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "fl.reqs", stats.flush_req); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "fl.times", stats.flush_total_times); + } + + return 0; +} + +#undef QEMU_ADD_BLOCK_PARAM + + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, virDomainObjPtr dom, @@ -17570,6 +17623,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON }, { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE }, + { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK }, { NULL, 0 } }; -- 1.9.3
Hi Francesco, I notice your patchset is much complete than mine which only focus on VIR_DOMAIN_STATS_BLOCK[1], but it seems your patch implement block stats query in a per-block style, this should be a bottleneck when there are a lot of block devices in a domain. Could you implement it in a bulk style? so we just need only one qmp-command for each domain. [1]: https://www.redhat.com/archives/libvir-list/2014-August/msg01497.html Thanks, Li Wei On 08/29/2014 03:15 PM, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_BLOCK group of statistics.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8c15583..372e098 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2515,6 +2515,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/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 818fcbc..344b02e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17552,6 +17552,59 @@ qemuDomainGetStatsInterface(virConnectPtr conn ATTRIBUTE_UNUSED,
#undef QEMU_ADD_NET_PARAM
+#define QEMU_ADD_BLOCK_PARAM(RECORD, MAXPARAMS, BLOCK, NAME, VALUE) \ +do { \ + char param_name[NAME_MAX]; \ + snprintf(param_name, NAME_MAX, "block.%s.%s", BLOCK, NAME); \ + if (virTypedParamsAddLLong(&RECORD->params, \ + &RECORD->nparams, \ + MAXPARAMS, \ + param_name, \ + VALUE) < 0) \ + return -1; \ +} while (0) + +static int +qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + virQEMUDriverPtr driver = conn->privateData; + struct qemuBlockStats stats; + size_t i; + + for (i = 0; i < dom->def->ndisks; i++) { + memset(&stats, 0, sizeof(stats)); + + if (qemuDiskGetBlockStats(driver, dom, dom->def->disks[i], &stats) < 0) + continue; + + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "rd.reqs", stats.rd_req); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "rd.bytes", stats.rd_bytes); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "rd.times", stats.rd_total_times); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "wr.reqs", stats.wr_req); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "wr.bytes", stats.wr_bytes); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "wr.times", stats.wr_total_times); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "fl.reqs", stats.flush_req); + QEMU_ADD_BLOCK_PARAM(record, maxparams, dom->def->disks[i]->dst, + "fl.times", stats.flush_total_times); + } + + return 0; +} + +#undef QEMU_ADD_BLOCK_PARAM + + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, virDomainObjPtr dom, @@ -17570,6 +17623,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON }, { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE }, + { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK }, { NULL, 0 } };
----- Original Message -----
From: "Li Wei" <lw@cn.fujitsu.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Monday, September 1, 2014 7:32:37 AM Subject: Re: [libvirt] [PATCH 10/11] qemu: bulk stats: implement block group
Hi Francesco,
I notice your patchset is much complete than mine which only focus on VIR_DOMAIN_STATS_BLOCK[1], but it seems your patch implement block stats query in a per-block style, this should be a bottleneck when there are a lot of block devices in a domain.
Could you implement it in a bulk style? so we just need only one qmp-command for each domain.
[1]: https://www.redhat.com/archives/libvir-list/2014-August/msg01497.html
Hi Li Wei, Thanks for pointing this out. Performance is surely a major concern of mine, then I'll improve my patch in the direction you outlined. I read your patch (actually I like some parts of your patch more than mine!) but I must somehow missed that. I'll wait for more reviews before to resubmit. Thanks and bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
This patch implements the VIR_DOMAIN_STATS_BLOCK_INFO group of statistics. This is different from the VIR_DOMAIN_STATS_BLOCK group because represents the information about the block device. Most notably, this group export the allocation information which is used by monitoring applications to detect the space exaustion on the block devices. Signed-off-by: Francesco Romani <fromani@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/qemu/qemu_driver.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 372e098..c0b695d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2516,6 +2516,7 @@ typedef enum { 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 */ + VIR_DOMAIN_STATS_BLOCK_INFO = (1 << 6), /* return domain block layout */ } virDomainStatsTypes; typedef enum { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 344b02e..564f1e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17604,6 +17604,49 @@ qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED, #undef QEMU_ADD_BLOCK_PARAM +#define QEMU_ADD_BLOCK_INFO_PARAM(RECORD, MAXPARAMS, BLOCK, NAME, VALUE) \ +do { \ + char param_name[NAME_MAX]; \ + snprintf(param_name, NAME_MAX, "block.%s.%s", BLOCK, NAME); \ + if (virTypedParamsAddULLong(&RECORD->params, \ + &RECORD->nparams, \ + MAXPARAMS, \ + param_name, \ + VALUE) < 0) \ + return -1; \ +} while (0) + +static int +qemuDomainGetStatsBlockInfo(virConnectPtr conn, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + virQEMUDriverPtr driver = conn->privateData; + virDomainBlockInfo info; + size_t i; + + for (i = 0; i < dom->def->ndisks; i++) { + memset(&info, 0, sizeof(info)); + + if (qemuDiskGetBlockInfo(driver, dom, dom->def->disks[i], + dom->def->disks[i]->dst, &info) < 0) + continue; + + QEMU_ADD_BLOCK_INFO_PARAM(record, maxparams, dom->def->disks[i]->dst, + "capacity", info.capacity); + QEMU_ADD_BLOCK_INFO_PARAM(record, maxparams, dom->def->disks[i]->dst, + "allocation", info.allocation); + QEMU_ADD_BLOCK_INFO_PARAM(record, maxparams, dom->def->disks[i]->dst, + "physical", info.physical); + } + + return 0; +} + +#undef QEMU_ADD_BLOCK_PARAM + typedef int (*qemuDomainGetStatsFunc)(virConnectPtr conn, @@ -17624,6 +17667,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU }, { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE }, { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK }, + { qemuDomainGetStatsBlockInfo, VIR_DOMAIN_STATS_BLOCK_INFO }, { NULL, 0 } }; -- 1.9.3
participants (2)
-
Francesco Romani -
Li Wei