[libvirt] [PATCHv5 0/8] bulk stats: QEMU implementation

This patchset enhances the QEMU support for the new bulk stats API to include equivalents of these APIs: virDomainBlockInfo virDomainGetInfo - for balloon stats virDomainGetCPUStats virDomainBlockStatsFlags virDomainInterfaceStats virDomainGetVcpusFlags virDomainGetVcpus This subset of API is the one oVirt relies on. Scale/stress test on an oVirt test environment is in progress. The patchset is organized as follows: - the first patch enhances the internal stats gathering API to accomodate the needs of the groups which extract information using QEMU monitor jobs. - the next five patches implement the bulk stats groups, extracting helpers where do refactoring to extract internal helpers every time it is feasible and convenient. - the seventh patch enhances the virsh domstats command with options to use the new bulk stats. - the last patch enhances the block stats group adding the wr_highest_offset information, needed by oVirt for thin provisioned disks. ChangeLog v5: address reviewer's comment - Eric pointed out a possible flaw in balloon stats if QEMU monitor needs to be queried. A proper fix require further discussion and API changes (possbily just a new flag); However, since the balloon event is available in QEMU >= 1.2, I just dropped the query and relied on the event instead. Support for older QEMUs will be reintroduced, if needed, with following patches. - fix: per-domain monitor check and reporting. (pointed out by Peter) - reset last error when fail silently. (pointed out by Peter) v4: address reviewer's comment - addressed reviewers comments (Peter, Wang Rui). - pushed domain check into group stats functions. This follows the strategy to gather and report as much data as possible, silently skipping errors along the way. - moved the block allocation patch to the end of the series. v3: more polishing and fixes after first review - addressed Eric's comments. - squashed patches which extracts helpers with patches which use them. - changed gathering strategy: now code tries to reap as much information as possible instead to give up and bail out with error. Only critical errors cause the bulk stats to fail. - moved away from the transfer semantics. I find it error-prone and not flexible enough, I'd like to avoid as much as possible. - rearranged helpers to have one single QEMU query job with many monitor jobs nested inside. - fixed docs. - implemented missing virsh domstats bits. in v2: polishing and optimizations. - incorporated feedback from Li Wei (thanks). - added documentation. - optimized block group to gather all the information with just one call to QEMU monitor. - stripped to bare bones merged the 'block info' group into the 'block' group - oVirt actually needs just one stat from there. - reorganized the keys to be more consistent and shorter. Francesco Romani (8): qemu: bulk stats: extend internal collection API 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 virsh: add options to query bulk stats group qemu: bulk stats: add block allocation information include/libvirt/libvirt.h.in | 5 + src/libvirt.c | 61 +++++ src/qemu/qemu_driver.c | 530 +++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 26 +++ src/qemu/qemu_monitor.h | 21 ++ src/qemu/qemu_monitor_json.c | 227 +++++++++++++----- src/qemu/qemu_monitor_json.h | 4 + tools/virsh-domain-monitor.c | 35 +++ tools/virsh.pod | 4 +- 9 files changed, 777 insertions(+), 136 deletions(-) -- 1.9.3

Future patches which will implement more bulk stats groups for QEMU will need to access the connection object. To accomodate that, a few changes are needed: * enrich internal prototype to pass qemu driver object. * add per-group flag to mark if one collector needs monitor access or not. * if at least one collector of the requested stats needs monitor access, thus we must start a query job for each domain. The specific collectors will run nested monitor jobs inside that. * although requested, monitor could be not available. pass a flag to workers to signal the availability of monitor, in order to gather as much data as is possible anyway. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 62 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 73edda3..39e9d27 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17356,7 +17356,8 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, static int -qemuDomainGetStatsState(virDomainObjPtr dom, +qemuDomainGetStatsState(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, unsigned int privflags ATTRIBUTE_UNUSED) @@ -17379,8 +17380,17 @@ qemuDomainGetStatsState(virDomainObjPtr dom, } +typedef enum { + QEMU_DOMAIN_STATS_HAVE_MONITOR = (1 << 0), /* QEMU monitor available */ +} qemuDomainStatsFlags; + + +#define HAVE_MONITOR(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_MONITOR) + + typedef int -(*qemuDomainGetStatsFunc)(virDomainObjPtr dom, +(*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, + virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, unsigned int flags); @@ -17388,11 +17398,12 @@ typedef int struct qemuDomainGetStatsWorker { qemuDomainGetStatsFunc func; unsigned int stats; + bool monitor; }; static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { - { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE}, - { NULL, 0 } + { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false }, + { NULL, 0, false } }; @@ -17424,6 +17435,20 @@ qemuDomainGetStatsCheckSupport(unsigned int *stats, } +static bool +qemuDomainGetStatsNeedMonitor(unsigned int stats) +{ + size_t i; + + for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) + if (stats & qemuDomainGetStatsWorkers[i].stats) + if (qemuDomainGetStatsWorkers[i].monitor) + return true; + + return false; +} + + static int qemuDomainGetStats(virConnectPtr conn, virDomainObjPtr dom, @@ -17441,8 +17466,8 @@ qemuDomainGetStats(virConnectPtr conn, for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) { if (stats & qemuDomainGetStatsWorkers[i].stats) { - if (qemuDomainGetStatsWorkers[i].func(dom, tmp, &maxparams, - flags) < 0) + if (qemuDomainGetStatsWorkers[i].func(conn->privateData, dom, tmp, + &maxparams, flags) < 0) goto cleanup; } } @@ -17481,6 +17506,8 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, int nstats = 0; size_t i; int ret = -1; + unsigned int privflags = 0; + unsigned int domflags = 0; if (ndoms) virCheckFlags(VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1); @@ -17515,7 +17542,11 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, if (VIR_ALLOC_N(tmpstats, ndoms + 1) < 0) goto cleanup; + if (qemuDomainGetStatsNeedMonitor(stats)) + privflags |= QEMU_DOMAIN_STATS_HAVE_MONITOR; + for (i = 0; i < ndoms; i++) { + domflags = privflags; virDomainStatsRecordPtr tmp = NULL; if (!(dom = qemuDomObjFromDomain(doms[i]))) @@ -17525,12 +17556,22 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) continue; - if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0) - goto cleanup; + if (HAVE_MONITOR(domflags) && + qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0) + /* As it was never requested. Gather as much as possible anyway. */ + domflags &= ~QEMU_DOMAIN_STATS_HAVE_MONITOR; + + if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0) + goto endjob; if (tmp) tmpstats[nstats++] = tmp; + if (HAVE_MONITOR(domflags) && !qemuDomainObjEndJob(driver, dom)) { + dom = NULL; + goto cleanup; + } + virObjectUnlock(dom); dom = NULL; } @@ -17540,6 +17581,11 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, ret = nstats; + endjob: + if (HAVE_MONITOR(domflags) && dom) + if (!qemuDomainObjEndJob(driver, dom)) + dom = NULL; + cleanup: if (dom) virObjectUnlock(dom); -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote:
Future patches which will implement more bulk stats groups for QEMU will need to access the connection object.
To accomodate that, a few changes are needed:
* enrich internal prototype to pass qemu driver object. * add per-group flag to mark if one collector needs monitor access or not. * if at least one collector of the requested stats needs monitor access, thus we must start a query job for each domain. The specific collectors will run nested monitor jobs inside that. * although requested, monitor could be not available. pass a flag to workers to signal the availability of monitor, in order to gather as much data as is possible anyway.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 62 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 73edda3..39e9d27 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -17525,12 +17556,22 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) continue;
- if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0) - goto cleanup; + if (HAVE_MONITOR(domflags) && + qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0) + /* As it was never requested. Gather as much as possible anyway. */ + domflags &= ~QEMU_DOMAIN_STATS_HAVE_MONITOR; + + if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0) + goto endjob;
if (tmp) tmpstats[nstats++] = tmp;
+ if (HAVE_MONITOR(domflags) && !qemuDomainObjEndJob(driver, dom)) { + dom = NULL; + goto cleanup;
I'd rather "continue;" here as a vanishing domain shouldn't be a problem for this code.
+ } + virObjectUnlock(dom); dom = NULL; }
ACK with the nit above fixed. (I made the change locally, thus no need to re-send this one). Peter

On 09/15/14 11:20, Peter Krempa wrote:
On 09/15/14 10:48, Francesco Romani wrote:
Future patches which will implement more bulk stats groups for QEMU will need to access the connection object.
To accomodate that, a few changes are needed:
* enrich internal prototype to pass qemu driver object. * add per-group flag to mark if one collector needs monitor access or not. * if at least one collector of the requested stats needs monitor access, thus we must start a query job for each domain. The specific collectors will run nested monitor jobs inside that. * although requested, monitor could be not available. pass a flag to workers to signal the availability of monitor, in order to gather as much data as is possible anyway.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 62 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 73edda3..39e9d27 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -17525,12 +17556,22 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) continue;
- if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0) - goto cleanup; + if (HAVE_MONITOR(domflags) && + qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0) + /* As it was never requested. Gather as much as possible anyway. */ + domflags &= ~QEMU_DOMAIN_STATS_HAVE_MONITOR; + + if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0) + goto endjob;
if (tmp) tmpstats[nstats++] = tmp;
+ if (HAVE_MONITOR(domflags) && !qemuDomainObjEndJob(driver, dom)) { + dom = NULL; + goto cleanup;
I'd rather "continue;" here as a vanishing domain shouldn't be a problem for this code.
+ } + virObjectUnlock(dom); dom = NULL; }
ACK with the nit above fixed. (I made the change locally, thus no need to re-send this one).
Additionally I'll change the QEMU_DOMAIN_STATS_HAVE_MONITOR flag to QEMU_DOMAIN_STATS_HAVE_JOB as the flag denotes exactly that fact. Peter

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/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 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/libvirt.c | 7 +++++++ src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+)
ACK from v4 still stands. Peter

On 09/15/14 10:48, Francesco Romani wrote:
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/libvirt.c | 7 +++++++ src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+)
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
@@ -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);
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.
+ 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,
Peter

----- Original Message -----
From: "Peter Krempa" <pkrempa@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Monday, September 15, 2014 2:49:50 PM Subject: Re: [libvirt] [PATCHv5 2/8] qemu: bulk stats: implement CPU stats group
+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);
This code doesn't check if priv->cgroup isn't NULL and dereferences it unconditionally. This would crash with shutoff machines.
Ouch. Right, of course. I tested on running VMs actually, and that explains why I haven't catched it :(
I'll add an if (priv->group) return 0;
right at the beginning.
You mean if (!priv->cgroup) return 0; I believe -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani

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/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

On 09/15/14 10:48, Francesco Romani wrote:
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/libvirt.c | 6 ++++++ src/qemu/qemu_driver.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+)
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; }
+
Spurious whitespace change.
static int qemuDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) {
ACK without the newline. Peter

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@redhat.com> --- 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:
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@redhat.com> --- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 12 +++ src/qemu/qemu_driver.c | 201 +++++++++++++++++++++++++++++-------------- 3 files changed, 150 insertions(+), 64 deletions(-)
ACK, Peter

On 09/15/14 10:48, Francesco Romani wrote:
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@redhat.com> --- 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;
nvcpupids is 0 if the VM is offline, thus:
+ + if (maxinfo >= 1) { + if (info != NULL) { + memset(info, 0, sizeof(*info) * maxinfo); + for (i = 0; i < maxinfo; i++) { + info[i].number = i;
You don't initialize the returned CPU numbers to sane values (they stay "0")
+ 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) {
@@ -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);
.. 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.
+ 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,
Peter

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/libvirt.c | 14 +++++++ src/qemu/qemu_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 4b851a5..17b1b43 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2516,6 +2516,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/libvirt.c b/src/libvirt.c index 1d91c99..5534b2f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21621,6 +21621,20 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "vcpu.<num>.time" - virtual cpu time spent by virtual CPU <num> * as unsigned long long. * + * VIR_DOMAIN_STATS_INTERFACE: Return network interface statistics. + * The typed parameter keys are in this format: + * "net.count" - number of network interfaces on this domain + * as unsigned int. + * "net.<num>.name" - name of the interface <num> as string. + * "net.<num>.rx.bytes" - bytes received as unsigned long long. + * "net.<num>.rx.pkts" - packets received as unsigned long long. + * "net.<num>.rx.errs" - receive errors as unsigned long long. + * "net.<num>.rx.drop" - receive packets dropped as unsigned long long. + * "net.<num>.tx.bytes" - bytes transmitted as unsigned long long. + * "net.<num>.tx.pkts" - packets transmitted as unsigned long long. + * "net.<num>.tx.errs" - transmission errors as unsigned long long. + * "net.<num>.tx.drop" - transmit packets dropped 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 4a92f58..016499d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17536,6 +17536,94 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, return ret; } +#define QEMU_ADD_COUNT_PARAM(record, maxparams, type, count) \ +do { \ + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "%s.count", type); \ + if (virTypedParamsAddUInt(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + count) < 0) \ + return -1; \ +} while (0) + +#define QEMU_ADD_NAME_PARAM(record, maxparams, type, num, name) \ +do { \ + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ + "%s.%zu.name", type, num); \ + if (virTypedParamsAddString(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + name) < 0) \ + return -1; \ +} while (0) + +#define QEMU_ADD_NET_PARAM(record, maxparams, num, name, value) \ +do { \ + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ + "net.%zu.%s", num, name); \ + if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + value) < 0) \ + return -1; \ +} while (0) + +static int +qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + struct _virDomainInterfaceStats tmp; + + QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets); + + /* 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) { + virResetLastError(); + continue; + } + + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", i, dom->def->nets[i]->ifname); + + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.bytes", tmp.rx_bytes); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.pkts", tmp.rx_packets); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.errs", tmp.rx_errs); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.drop", tmp.rx_drop); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.bytes", tmp.tx_bytes); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.pkts", tmp.tx_packets); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.errs", tmp.tx_errs); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.drop", tmp.tx_drop); + } + + return 0; +} + +#undef QEMU_ADD_NET_PARAM + +#undef QEMU_ADD_NAME_PARAM + +#undef QEMU_ADD_COUNT_PARAM typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, @@ -17555,6 +17643,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false }, { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true }, { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false }, + { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false }, { NULL, 0, false } }; -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote:
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/libvirt.c | 14 +++++++ src/qemu/qemu_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+)
+static int +qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + struct _virDomainInterfaceStats tmp; + + QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets); + + /* 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) { + virResetLastError(); + continue; + }
We could change the order of these two lines, so that we at least get a list of interfaces in case the stats are not available.
+ + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", i, dom->def->nets[i]->ifname); + + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.bytes", tmp.rx_bytes); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.pkts", tmp.rx_packets); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.errs", tmp.rx_errs); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "rx.drop", tmp.rx_drop); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.bytes", tmp.tx_bytes); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.pkts", tmp.tx_packets); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.errs", tmp.tx_errs); + QEMU_ADD_NET_PARAM(record, maxparams, i, + "tx.drop", tmp.tx_drop); + } + + return 0; +} + +#undef QEMU_ADD_NET_PARAM + +#undef QEMU_ADD_NAME_PARAM + +#undef QEMU_ADD_COUNT_PARAM
typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver,
ACK with the change, although we should add docs that state that stats groups that return "arrays" of stats may have individual elements missing in case stats are not available. Peter

On 09/15/14 10:48, Francesco Romani wrote:
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/libvirt.c | 14 +++++++ src/qemu/qemu_driver.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+)
+static int +qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, + virDomainStatsRecordPtr record, + int *maxparams, + unsigned int privflags ATTRIBUTE_UNUSED) +{ + size_t i; + struct _virDomainInterfaceStats tmp; + + QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets); + + /* 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) {
dom->def->nets[i]->ifname is NULL if the VM is inactive. I think we should skip this complete group if the VM is offline. I'll add a if (!virDomainObjIsActive(dom)) return 0; to the beginning of this function. If the vm isn't active, there isn't much to report.
+ virResetLastError(); + continue; + }
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@redhat.com> --- 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:
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@redhat.com> --- 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(-)
HAVE_MONITOR needs to be changed to HAVE_JOB due to my previous change. ACK, Peter

On 09/15/14 10:48, Francesco Romani wrote:
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@redhat.com> --- 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/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; }
+
I've just noticed this spurious newline.
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;
nstats is here initialized to 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);
and not touched, thus it's still 0 when called here, which ...
+ + 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
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
+ +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;
.. returns failure here.
+ ret = qemuMonitorJSONCommand(mon, cmd, &reply);
if (ret == 0)
nstats needs to be set to the disk count, otherwise this stats group won't work. Peter

Exports to the domstats commands the new bulk stats groups. Signed-off-by: Francesco Romani <fromani@redhat.com> --- tools/virsh-domain-monitor.c | 35 +++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 055d8d2..d013ca8 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1972,6 +1972,26 @@ static const vshCmdOptDef opts_domstats[] = { .type = VSH_OT_BOOL, .help = N_("report domain state"), }, + {.name = "cpu-total", + .type = VSH_OT_BOOL, + .help = N_("report domain physical cpu usage"), + }, + {.name = "balloon", + .type = VSH_OT_BOOL, + .help = N_("report domain balloon statistics"), + }, + {.name = "vcpu", + .type = VSH_OT_BOOL, + .help = N_("report domain virtual cpu information"), + }, + {.name = "interface", + .type = VSH_OT_BOOL, + .help = N_("report domain network interface information"), + }, + {.name = "block", + .type = VSH_OT_BOOL, + .help = N_("report domain block device statistics"), + }, {.name = "list-active", .type = VSH_OT_BOOL, .help = N_("list only active domains"), @@ -2063,6 +2083,21 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "state")) stats |= VIR_DOMAIN_STATS_STATE; + if (vshCommandOptBool(cmd, "cpu-total")) + stats |= VIR_DOMAIN_STATS_CPU_TOTAL; + + if (vshCommandOptBool(cmd, "balloon")) + stats |= VIR_DOMAIN_STATS_BALLOON; + + if (vshCommandOptBool(cmd, "vcpu")) + stats |= VIR_DOMAIN_STATS_VCPU; + + if (vshCommandOptBool(cmd, "interface")) + stats |= VIR_DOMAIN_STATS_INTERFACE; + + if (vshCommandOptBool(cmd, "block")) + stats |= VIR_DOMAIN_STATS_BLOCK; + if (vshCommandOptBool(cmd, "list-active")) flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE; diff --git a/tools/virsh.pod b/tools/virsh.pod index 5d4b12b..b929480 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -814,6 +814,7 @@ I<snapshot-create> for disk snapshots) will accept either target or unique source names printed by this command. =item B<domstats> [I<--raw>] [I<--enforce>] [I<--state>] +[I<--cpu-total>][I<--balloon>][I<--vcpu>][I<--interface>][I<--block>] [[I<--list-active>] [I<--list-inactive>] [I<--list-persistent>] [I<--list-transient>] [I<--list-running>] [I<--list-paused>] [I<--list-shutoff>] [I<--list-other>]] | [I<domain> ...] @@ -831,7 +832,8 @@ behavior use the I<--raw> flag. The individual statistics groups are selectable via specific flags. By default all supported statistics groups are returned. Supported -statistics groups flags are: I<--state>. +statistics groups flags are: I<--state>, I<--cpu-total>, I<--balloon>, +I<--vcpu>, I<--interface>, I<--block>. Selecting a specific statistics groups doesn't guarantee that the daemon supports the selected group of stats. Flag I<--enforce> -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote:
Exports to the domstats commands the new bulk stats groups.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- tools/virsh-domain-monitor.c | 35 +++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 +++- 2 files changed, 38 insertions(+), 1 deletion(-)
ACK, although the docs I've done for the first stats group could be better. But this can be done separately. Peter

Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats. This patch extend the block information in the bulk stats with the allocation information. To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information. Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/libvirt.c | 2 + src/qemu/qemu_driver.c | 18 +++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ab10a3a..a8892a2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21654,6 +21654,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * unsigned long long. * "block.<num>.errors" - Xen only: the 'oo_req' value as * unsigned long long. + * "block.<num>.allocation" - offset of the highest written sector + * 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 446b04b..a34fffd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17636,6 +17636,19 @@ do { \ goto cleanup; \ } while (0) +#define QEMU_ADD_BLOCK_PARAM_ULL(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 (virTypedParamsAddULLong(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + value) < 0) \ + goto cleanup; \ +} while (0) + static int qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -17690,6 +17703,9 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, "fl.reqs", stats[i].flush_req); QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i, "fl.times", stats[i].flush_total_times); + + QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, i, + "allocation", stats[i].wr_highest_offset); } ret = 0; @@ -17701,6 +17717,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, #undef QEMU_ADD_BLOCK_PARAM_LL +#undef QEMU_ADD_BLOCK_PARAM_ULL + #undef QEMU_ADD_NAME_PARAM #undef QEMU_ADD_COUNT_PARAM diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6eb0036..d0f0bf5 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -358,6 +358,7 @@ struct _qemuBlockStats { long long wr_total_times; long long flush_req; long long flush_total_times; + unsigned long long wr_highest_offset; }; int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d45c41f..1948b85 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1774,6 +1774,40 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, } +typedef enum { + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS, + QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET +} qemuMonitorBlockExtentError; + + +static int +qemuMonitorJSONDevGetBlockExtent(virJSONValuePtr dev, + unsigned long long *extent) +{ + virJSONValuePtr stats; + virJSONValuePtr parent; + + if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || + parent->type != VIR_JSON_TYPE_OBJECT) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT; + } + + if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || + stats->type != VIR_JSON_TYPE_OBJECT) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS; + } + + if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", + extent) < 0) { + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET; + } + + return QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK; +} + + int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, qemuBlockStatsPtr bstats, @@ -1910,6 +1944,9 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } + /* it's ok to not have this information here. Just skip silently. */ + qemuMonitorJSONDevGetBlockExtent(dev, &bstats->wr_highest_offset); + count++; bstats++; @@ -2005,6 +2042,36 @@ int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, return ret; } + +static int +qemuMonitorJSONReportBlockExtentError(qemuMonitorBlockExtentError error) +{ + switch (error) { + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats parent entry was not in " + "expected format")); + break; + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not in " + "expected format")); + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_highest_offset"); + break; + + case QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK: + return 0; + } + + return -1; +} + + int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, const char *dev_name, unsigned long long *extent) @@ -2039,9 +2106,8 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); - virJSONValuePtr stats; - virJSONValuePtr parent; const char *thisdev; + int err; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not in expected format")); @@ -2065,24 +2131,9 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, continue; found = true; - if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL || - parent->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats parent entry was not in expected format")); - goto cleanup; - } - - if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not in expected format")); - goto cleanup; - } - - if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset", extent) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_highest_offset"); + if ((err = qemuMonitorJSONDevGetBlockExtent(dev, extent)) != + QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK) { + qemuMonitorJSONReportBlockExtentError(err); goto cleanup; } } -- 1.9.3

On 09/15/14 10:48, Francesco Romani wrote:
Management software wants to be able to allocate disk space on demand. To support this they need keep track of the space occupation of the block device. This information is reported by qemu as part of block stats.
This patch extend the block information in the bulk stats with the allocation information.
To keep the same behaviour a helper is extracted from qemuMonitorJSONGetBlockExtent in order to get per-device allocation information.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/libvirt.c | 2 + src/qemu/qemu_driver.c | 18 +++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 91 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 92 insertions(+), 20 deletions(-)
ACK stands Peter

On 09/15/14 10:48, Francesco Romani wrote:
This patchset enhances the QEMU support for the new bulk stats API to include equivalents of these APIs:
virDomainBlockInfo virDomainGetInfo - for balloon stats virDomainGetCPUStats virDomainBlockStatsFlags virDomainInterfaceStats virDomainGetVcpusFlags virDomainGetVcpus
This subset of API is the one oVirt relies on. Scale/stress test on an oVirt test environment is in progress.
The patchset is organized as follows: - the first patch enhances the internal stats gathering API to accomodate the needs of the groups which extract information using QEMU monitor jobs. - the next five patches implement the bulk stats groups, extracting helpers where do refactoring to extract internal helpers every time it is feasible and convenient. - the seventh patch enhances the virsh domstats command with options to use the new bulk stats. - the last patch enhances the block stats group adding the wr_highest_offset information, needed by oVirt for thin provisioned disks.
ChangeLog
v5: address reviewer's comment - Eric pointed out a possible flaw in balloon stats if QEMU monitor needs to be queried. A proper fix require further discussion and API changes (possbily just a new flag); However, since the balloon event is available in QEMU >= 1.2, I just dropped the query and relied on the event instead. Support for older QEMUs will be reintroduced, if needed, with following patches. - fix: per-domain monitor check and reporting. (pointed out by Peter) - reset last error when fail silently. (pointed out by Peter)
The changes look good. I've done a few finishing touches and I'm going to give the series some testing before pushing. The series should be pushed by today after I finish. Thanks for your cooperation in finishing this. Peter

----- Original Message -----
From: "Peter Krempa" <pkrempa@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Monday, September 15, 2014 2:25:08 PM Subject: Re: [libvirt] [PATCHv5 0/8] bulk stats: QEMU implementation
[...]
ChangeLog
v5: address reviewer's comment - Eric pointed out a possible flaw in balloon stats if QEMU monitor needs to be queried. A proper fix require further discussion and API changes (possbily just a new flag); However, since the balloon event is available in QEMU >= 1.2, I just dropped the query and relied on the event instead. Support for older QEMUs will be reintroduced, if needed, with following patches. - fix: per-domain monitor check and reporting. (pointed out by Peter) - reset last error when fail silently. (pointed out by Peter)
The changes look good. I've done a few finishing touches and I'm going to give the series some testing before pushing. The series should be pushed by today after I finish.
Thanks for your cooperation in finishing this.
My pleasure. Thank you and the other reviewers for your work! Bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani

Richard, Since this series has been applied, could you please update virt-top to use these bulk APIs, this should fix the long update interval bug when domain has a lot of disks. Thanks. On 09/15/2014 04:48 PM, Francesco Romani wrote:
This patchset enhances the QEMU support for the new bulk stats API to include equivalents of these APIs:
virDomainBlockInfo virDomainGetInfo - for balloon stats virDomainGetCPUStats virDomainBlockStatsFlags virDomainInterfaceStats virDomainGetVcpusFlags virDomainGetVcpus
This subset of API is the one oVirt relies on. Scale/stress test on an oVirt test environment is in progress.
The patchset is organized as follows: - the first patch enhances the internal stats gathering API to accomodate the needs of the groups which extract information using QEMU monitor jobs. - the next five patches implement the bulk stats groups, extracting helpers where do refactoring to extract internal helpers every time it is feasible and convenient. - the seventh patch enhances the virsh domstats command with options to use the new bulk stats. - the last patch enhances the block stats group adding the wr_highest_offset information, needed by oVirt for thin provisioned disks.
ChangeLog
v5: address reviewer's comment - Eric pointed out a possible flaw in balloon stats if QEMU monitor needs to be queried. A proper fix require further discussion and API changes (possbily just a new flag); However, since the balloon event is available in QEMU >= 1.2, I just dropped the query and relied on the event instead. Support for older QEMUs will be reintroduced, if needed, with following patches. - fix: per-domain monitor check and reporting. (pointed out by Peter) - reset last error when fail silently. (pointed out by Peter)
v4: address reviewer's comment - addressed reviewers comments (Peter, Wang Rui). - pushed domain check into group stats functions. This follows the strategy to gather and report as much data as possible, silently skipping errors along the way. - moved the block allocation patch to the end of the series.
v3: more polishing and fixes after first review - addressed Eric's comments. - squashed patches which extracts helpers with patches which use them. - changed gathering strategy: now code tries to reap as much information as possible instead to give up and bail out with error. Only critical errors cause the bulk stats to fail. - moved away from the transfer semantics. I find it error-prone and not flexible enough, I'd like to avoid as much as possible. - rearranged helpers to have one single QEMU query job with many monitor jobs nested inside. - fixed docs. - implemented missing virsh domstats bits.
in v2: polishing and optimizations. - incorporated feedback from Li Wei (thanks). - added documentation. - optimized block group to gather all the information with just one call to QEMU monitor. - stripped to bare bones merged the 'block info' group into the 'block' group - oVirt actually needs just one stat from there. - reorganized the keys to be more consistent and shorter.
Francesco Romani (8): qemu: bulk stats: extend internal collection API 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 virsh: add options to query bulk stats group qemu: bulk stats: add block allocation information
include/libvirt/libvirt.h.in | 5 + src/libvirt.c | 61 +++++ src/qemu/qemu_driver.c | 530 +++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 26 +++ src/qemu/qemu_monitor.h | 21 ++ src/qemu/qemu_monitor_json.c | 227 +++++++++++++----- src/qemu/qemu_monitor_json.h | 4 + tools/virsh-domain-monitor.c | 35 +++ tools/virsh.pod | 4 +- 9 files changed, 777 insertions(+), 136 deletions(-)

On Thu, Sep 25, 2014 at 05:33:39PM +0800, Li Wei wrote:
Richard,
Since this series has been applied, could you please update virt-top to use these bulk APIs, this should fix the long update interval bug when domain has a lot of disks.
Yes, I'll be doing this once virt-v2v is out of the way. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
participants (4)
-
Francesco Romani
-
Li Wei
-
Peter Krempa
-
Richard W.M. Jones