8:05 a.m.
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.
changes in 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.
changes 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.
The patchset is organized as follows:
- the first patch enhance the internal stats gathering API
to accomodate the needs of the groups which extract information
using QEMU monitor jobs.
- the next 6 patches implement the bulk stats groups, extracting
helpers where do refactoring to extract internal helpers every time
it is feasible and convenient.
- the last patch enhance the virsh domstats command with options to
use the new bulk stats.
*** BLURB HERE ***
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
qemu: bulk stats: add block allocation information
virsh: add options to query bulk stats group
include/libvirt/libvirt.h.in | 5 +
src/libvirt.c | 61 +++++
src/qemu/qemu_driver.c | 577 +++++++++++++++++++++++++++++++++++++------
src/qemu/qemu_monitor.c | 22 ++
src/qemu/qemu_monitor.h | 21 ++
src/qemu/qemu_monitor_json.c | 211 +++++++++++-----
src/qemu/qemu_monitor_json.h | 4 +
src/qemu/qemu_monitor_text.c | 13 +
src/qemu/qemu_monitor_text.h | 4 +
tools/virsh-domain-monitor.c | 35 +++
tools/virsh.pod | 4 +-
11 files changed, 823 insertions(+), 134 deletions(-)
--
1.9.3
8:05 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
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 connection 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.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d724eeb..2950a4b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17314,7 +17314,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)
@@ -17338,7 +17339,8 @@ qemuDomainGetStatsState(virDomainObjPtr dom,
typedef int
-(*qemuDomainGetStatsFunc)(virDomainObjPtr dom,
+(*qemuDomainGetStatsFunc)(virConnectPtr conn,
+ virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
unsigned int flags);
@@ -17346,11 +17348,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 }
};
@@ -17382,6 +17385,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,
@@ -17399,7 +17416,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;
}
@@ -17435,6 +17452,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
virDomainObjPtr dom = NULL;
virDomainStatsRecordPtr *tmpstats = NULL;
bool enforce = !!(flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS);
+ bool needmon = false;
int ntempdoms;
int nstats = 0;
size_t i;
@@ -17473,6 +17491,8 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
if (VIR_ALLOC_N(tmpstats, ndoms + 1) < 0)
goto cleanup;
+ needmon = qemuDomainGetStatsNeedMonitor(stats);
+
for (i = 0; i < ndoms; i++) {
virDomainStatsRecordPtr tmp = NULL;
@@ -17483,11 +17503,21 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
!virConnectGetAllDomainStatsCheckACL(conn, dom->def))
continue;
- if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
+ if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) <
0)
goto cleanup;
- if (tmp)
- tmpstats[nstats++] = tmp;
+ if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
+ if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
+ goto endjob;
+
+ if (tmp)
+ tmpstats[nstats++] = tmp;
+ }
+
+ if (needmon && !qemuDomainObjEndJob(driver, dom)) {
+ dom = NULL;
+ goto cleanup;
+ }
virObjectUnlock(dom);
dom = NULL;
@@ -17498,6 +17528,11 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
ret = nstats;
+ endjob:
+ if (needmon && dom)
+ if (!qemuDomainObjEndJob(driver, dom))
+ dom = NULL;
+
cleanup:
if (dom)
virObjectUnlock(dom);
--
1.9.3
7:14 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
On 09/08/14 15:05, 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 connection 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.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d724eeb..2950a4b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17338,7 +17339,8 @@ qemuDomainGetStatsState(virDomainObjPtr dom,
typedef int
-(*qemuDomainGetStatsFunc)(virDomainObjPtr dom,
+(*qemuDomainGetStatsFunc)(virConnectPtr conn,
Looking through the rest of the series. Rather than the complete
connection object you need just the virQEMUDriverPtr for entering the
monitor, but I can live with this.
+ virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
unsigned int flags);
@@ -17483,11 +17503,21 @@ qemuConnectGetAllDomainStats(virConnectPtr
conn,
!virConnectGetAllDomainStatsCheckACL(conn, dom->def))
continue;
- if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
+ if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) <
0)
goto cleanup;
- if (tmp)
- tmpstats[nstats++] = tmp;
+ if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
Hmm this skips offline domains entirely if one of the stats groups needs
the monitor.
I think we should rather skip individual stats groups, or better stats
fields that we can't provide.
Any ideas?
+ if (qemuDomainGetStats(conn, dom, stats, &tmp,
flags) < 0)
+ goto endjob;
+
+ if (tmp)
+ tmpstats[nstats++] = tmp;
+ }
+
+ if (needmon && !qemuDomainObjEndJob(driver, dom)) {
+ dom = NULL;
+ goto cleanup;
+ }
virObjectUnlock(dom);
dom = NULL;
Peter
11:04 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 2:14:23 PM
Subject: Re: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
On 09/08/14 15:05, 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 connection 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.
>
> Signed-off-by: Francesco Romani <fromani(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 51
> ++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d724eeb..2950a4b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17338,7 +17339,8 @@ qemuDomainGetStatsState(virDomainObjPtr dom,
>
>
> typedef int
> -(*qemuDomainGetStatsFunc)(virDomainObjPtr dom,
> +(*qemuDomainGetStatsFunc)(virConnectPtr conn,
Looking through the rest of the series. Rather than the complete
connection object you need just the virQEMUDriverPtr for entering the
monitor, but I can live with this.
Since I need to resubmit to address your comments, I'll fix this
to pass just virQEMUDriverPtr.
>
> - if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
> + if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY)
> < 0)
> goto cleanup;
>
> - if (tmp)
> - tmpstats[nstats++] = tmp;
> + if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
Hmm this skips offline domains entirely if one of the stats groups needs
the monitor.
I think we should rather skip individual stats groups, or better stats
fields that we can't provide.
Any ideas?
What about this (pseudo-C):
unsigned int privflags = 0;
if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY)
needmon = false;
/* auto disable monitoring, the remainder of the function should be unchanged */
else
privflags |= MONITOR_AVAILABLE;
if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0)
/* pass monitor availability down the chain. Individual workers will
bail out immediately and silently if they need monitor but it is
not available
*/
goto endjob;
if (tmp)
tmpstats[nstats++] = tmp;
}
No other change should be needed to this patch, and with trivial changes
all the others can be fixed.
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
3:07 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
On 09/09/14 18:04, Francesco Romani wrote:
Peter
----- Original Message -----
> From: "Peter Krempa" <pkrempa(a)redhat.com>
> To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
> Sent: Tuesday, September 9, 2014 2:14:23 PM
> Subject: Re: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
>
> On 09/08/14 15:05, 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 connection 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.
>>
>> Signed-off-by: Francesco Romani <fromani(a)redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 51
>> ++++++++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index d724eeb..2950a4b 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>
>> @@ -17338,7 +17339,8 @@ qemuDomainGetStatsState(virDomainObjPtr dom,
>>
>>
>> typedef int
>> -(*qemuDomainGetStatsFunc)(virDomainObjPtr dom,
>> +(*qemuDomainGetStatsFunc)(virConnectPtr conn,
>
> Looking through the rest of the series. Rather than the complete
> connection object you need just the virQEMUDriverPtr for entering the
> monitor, but I can live with this.
Since I need to resubmit to address your comments, I'll fix this
to pass just virQEMUDriverPtr.
>>
>> - if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
>> + if (needmon && qemuDomainObjBeginJob(driver, dom,
QEMU_JOB_QUERY)
>> < 0)
>> goto cleanup;
>>
>> - if (tmp)
>> - tmpstats[nstats++] = tmp;
>> + if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
>
> Hmm this skips offline domains entirely if one of the stats groups needs
> the monitor.
>
> I think we should rather skip individual stats groups, or better stats
> fields that we can't provide.
>
> Any ideas?
What about this (pseudo-C):
unsigned int privflags = 0;
if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY)
needmon = false;
/* auto disable monitoring, the remainder of the function should be unchanged */
else
privflags |= MONITOR_AVAILABLE;
if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0)
/* pass monitor availability down the chain. Individual workers will
bail out immediately and silently if they need monitor but it is
not available
*/
goto endjob;
if (tmp)
tmpstats[nstats++] = tmp;
}
No other change should be needed to this patch, and with trivial changes
all the others can be fixed.
Also you can just grab the lock always and the workers will exit if the
VM is not alive. Having a domain job should be fine.
8:55 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Wednesday, September 10, 2014 10:07:33 AM
Subject: Re: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
[...]
>> Hmm this skips offline domains entirely if one of the stats
groups needs
>> the monitor.
>>
>> I think we should rather skip individual stats groups, or better stats
>> fields that we can't provide.
>>
>> Any ideas?
>
> What about this (pseudo-C):
>
> unsigned int privflags = 0;
>
> if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY)
> needmon = false;
> /* auto disable monitoring, the remainder of the function should be
> unchanged */
> else
> privflags |= MONITOR_AVAILABLE;
>
> if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
> if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0)
> /* pass monitor availability down the chain. Individual workers
> will
> bail out immediately and silently if they need monitor but
> it is
> not available
> */
> goto endjob;
>
> if (tmp)
> tmpstats[nstats++] = tmp;
> }
>
>
> No other change should be needed to this patch, and with trivial changes
> all the others can be fixed.
Also you can just grab the lock always and the workers will exit if the
VM is not alive. Having a domain job should be fine.
As you prefer. For oVirt, we do care of all the stats group implemented, and always all of
them,
(actually I may have missed some bits, e.g. the pininfo as you pointed out elsewhere -
going to fix),
so for our needs we'll always need to enter the monitor.
But other users of this API may beg to differ, and I believe is fair to assume that other
consumers
of this API could just use stats which doesn't require to enter the monitor: e.g.
CPU/VCPU/interface.
Hence, I was trying to be a good citizen and do not require monitor access unless it is
actually
needed; but if turns out it is OK to do a domain job anyway, I'll happily simplify my
code :)
Thanks and bests,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
9:11 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
On 09/10/14 15:55, Francesco Romani wrote:
----- Original Message -----
> From: "Peter Krempa" <pkrempa(a)redhat.com>
> To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
> Sent: Wednesday, September 10, 2014 10:07:33 AM
> Subject: Re: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
[...]
>>> Hmm this skips offline domains entirely if one of the stats groups needs
>>> the monitor.
>>>
>>> I think we should rather skip individual stats groups, or better stats
>>> fields that we can't provide.
>>>
>>> Any ideas?
>>
>> What about this (pseudo-C):
>>
>> unsigned int privflags = 0;
>>
>> if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY)
>> needmon = false;
>> /* auto disable monitoring, the remainder of the function should be
>> unchanged */
>> else
>> privflags |= MONITOR_AVAILABLE;
>>
>> if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
Drop this condition ...
>> if (qemuDomainGetStats(conn, dom, stats, &tmp,
privflags) < 0)
The individual stats groups will decide if they need the VM alive they
can skip it individually rather than omitting the vm completely.
>> /* pass monitor availability down the chain.
Individual workers
>> will
>> bail out immediately and silently if they need monitor but
>> it is
>> not available
>> */
>> goto endjob;
>>
>> if (tmp)
>> tmpstats[nstats++] = tmp;
>> }
>>
>>
>> No other change should be needed to this patch, and with trivial changes
>> all the others can be fixed.
>
> Also you can just grab the lock always and the workers will exit if the
> VM is not alive. Having a domain job should be fine.
As you prefer. For oVirt, we do care of all the stats group implemented, and always all
of them,
(actually I may have missed some bits, e.g. the pininfo as you pointed out elsewhere -
going to fix),
so for our needs we'll always need to enter the monitor.
But other users of this API may beg to differ, and I believe is fair to assume that other
consumers
of this API could just use stats which doesn't require to enter the monitor: e.g.
CPU/VCPU/interface.
In case you don't have any stat group that requires the monitor it is
imho fine not to get a job. If there is a group where we can enter it we
can just grab the monitor always ... see above.
Hence, I was trying to be a good citizen and do not require monitor access unless it is
actually
needed; but if turns out it is OK to do a domain job anyway, I'll happily simplify my
code :)
Thanks and bests,
Peter
3:08 a.m.
New subject: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
On 2014/9/8 21:05, Francesco Romani wrote:
+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;
+}
'stats' is not necessary in the loop.
How about this style?
static bool
qemuDomainGetStatsNeedMonitor(unsigned int stats)
{
size_t i;
if (!stats)
return false;
for (i = 0; qemuDomainGetStatsWorkers[i].func; i++)
if (qemuDomainGetStatsWorkers[i].stats &&
qemuDomainGetStatsWorkers[i].monitor)
return true;
return false;
}
8:05 a.m.
New subject: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 9 ++++++++
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index aced31c..e6ed803 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/libvirt.c b/src/libvirt.c
index 4806535..4d504ff 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21554,6 +21554,15 @@ 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.count" - number as unsigned int of physical cpus available to
+ * this domain.
+ * "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 2950a4b..cfc5941 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
@@ -17338,6 +17339,55 @@ qemuDomainGetStatsState(virConnectPtr conn ATTRIBUTE_UNUSED,
}
+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;
+ int err;
+
+ ncpus = nodeGetCPUCount();
+ if (ncpus > 0 &&
+ virTypedParamsAddUInt(&record->params,
+ &record->nparams,
+ maxparams,
+ "cpu.count",
+ (unsigned int)ncpus) < 0)
+ return -1;
+
+ 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)(virConnectPtr conn,
virDomainObjPtr dom,
@@ -17353,6 +17403,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
6:50 a.m.
New subject: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
On 09/08/14 15:05, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 9 ++++++++
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index aced31c..e6ed803 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/libvirt.c b/src/libvirt.c
index 4806535..4d504ff 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21554,6 +21554,15 @@ 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.count" - number as unsigned int of physical cpus available to
+ * this domain.
This is not really a VM property rather than a host property. I don't
think we should report this as it will be the same for all VMs on the host.
+ * "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 2950a4b..cfc5941 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
@@ -17338,6 +17339,55 @@ qemuDomainGetStatsState(virConnectPtr conn ATTRIBUTE_UNUSED,
}
+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;
+ int err;
+
+ ncpus = nodeGetCPUCount();
As said, this is a host-wide value.
+ if (ncpus > 0 &&
+ virTypedParamsAddUInt(&record->params,
+ &record->nparams,
+ maxparams,
+ "cpu.count",
+ (unsigned int)ncpus) < 0)
+ return -1;
Otherwise looks reasonable.
Peter
6:56 a.m.
New subject: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 1:50:25 PM
Subject: Re: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
On 09/08/14 15:05, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL
> group of statistics.
>
> Signed-off-by: Francesco Romani <fromani(a)redhat.com>
> ---
> include/libvirt/libvirt.h.in | 1 +
> src/libvirt.c | 9 ++++++++
> src/qemu/qemu_driver.c | 51
> ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index aced31c..e6ed803 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/libvirt.c b/src/libvirt.c
> index 4806535..4d504ff 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21554,6 +21554,15 @@ 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.count" - number as unsigned int of physical cpus available to
> + * this domain.
This is not really a VM property rather than a host property. I don't
think we should report this as it will be the same for all VMs on the host.
I'm OK with this. Just tried to mimic as closesly as possible the existing
behaviour, but you have a point here.
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
3:56 a.m.
New subject: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
On 2014/9/8 21:05, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 9 ++++++++
src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+)
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2950a4b..cfc5941 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"
Hi, Francesco.
I see the file including relationship. 'qemu_driver.c' includes
'qemu_cgroup.h' which
includes 'vircgroup.h'. There are other virCgroupGet* functions called in
qemu_driver.c
now. So I think here "include vircgroup.h" is not necessary.
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -17338,6 +17339,55 @@ qemuDomainGetStatsState(virConnectPtr conn ATTRIBUTE_UNUSED,
}
+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;
+ int err;
+
+ ncpus = nodeGetCPUCount();
+ if (ncpus > 0 &&
+ virTypedParamsAddUInt(&record->params,
+ &record->nparams,
+ maxparams,
+ "cpu.count",
+ (unsigned int)ncpus) < 0)
+ return -1;
+
+ 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;
1. If any of the 'err's is not zero, the function may returns 0 as success.
Is this the expected return? Or at least we can give a warning that we
miss some parameters.
2. I think it's better to report an error or warning log before return -1.
+ return 0;
+}
+
+
typedef int
(*qemuDomainGetStatsFunc)(virConnectPtr conn,
virDomainObjPtr dom,
@@ -17353,6 +17403,7 @@ struct qemuDomainGetStatsWorker {
static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = {
{ qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false },
+ { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
{ NULL, 0, false }
};
8:46 a.m.
New subject: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
----- Original Message -----
From: "Wang Rui" <moon.wangrui(a)huawei.com>
To: "Francesco Romani" <fromani(a)redhat.com>
Cc: libvir-list(a)redhat.com, pkrempa(a)redhat.com
Sent: Wednesday, September 10, 2014 10:56:47 AM
Subject: Re: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
On 2014/9/8 21:05, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL
> group of statistics.
>
> Signed-off-by: Francesco Romani <fromani(a)redhat.com>
> ---
> include/libvirt/libvirt.h.in | 1 +
> src/libvirt.c | 9 ++++++++
> src/qemu/qemu_driver.c | 51
> ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
[...]
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2950a4b..cfc5941 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"
Hi, Francesco.
I see the file including relationship. 'qemu_driver.c' includes
'qemu_cgroup.h' which
includes 'vircgroup.h'. There are other virCgroupGet* functions called in
qemu_driver.c
now. So I think here "include vircgroup.h" is not necessary.
Thanks for the research, I'll remove
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> @@ -17338,6 +17339,55 @@ qemuDomainGetStatsState(virConnectPtr conn
> ATTRIBUTE_UNUSED,
> }
>
>
> +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;
> + int err;
> +
> + ncpus = nodeGetCPUCount();
> + if (ncpus > 0 &&
> + virTypedParamsAddUInt(&record->params,
> + &record->nparams,
> + maxparams,
> + "cpu.count",
> + (unsigned int)ncpus) < 0)
> + return -1;
> +
> + 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;
1. If any of the 'err's is not zero, the function may returns 0 as success.
Is this the expected return? Or at least we can give a warning that we
miss some parameters.
2. I think it's better to report an error or warning log before return -1.
The idea here (well, at least my idea :) ) is to gather as much as data as possible,
and to silently skip failures here. The lack of expected output is a good enough
indicator that a domain is unresponsive.
-1 is reported for really unrecoverable error, like memory (re)allocation failure.
Otherwise, how could the client code be meaningfully informed that some group
failed, maybe partially? It is possible that different groups fail for different
domains: how could we convey this information?
I have no problems to go this route if there is consensus this is the preferred
way to go, but then we'll need to discuss how to convey a meaningful error
convention.
Bests,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
8:55 a.m.
New subject: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
On 09/10/14 15:46, Francesco Romani wrote:
----- Original Message -----
> From: "Wang Rui" <moon.wangrui(a)huawei.com>
> To: "Francesco Romani" <fromani(a)redhat.com>
> Cc: libvir-list(a)redhat.com, pkrempa(a)redhat.com
> Sent: Wednesday, September 10, 2014 10:56:47 AM
> Subject: Re: [libvirt] [PATCHv3 2/8] qemu: bulk stats: implement CPU stats group
>
> On 2014/9/8 21:05, Francesco Romani wrote:
>> This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL
>> group of statistics.
>>
>> Signed-off-by: Francesco Romani <fromani(a)redhat.com>
>> ---
>> include/libvirt/libvirt.h.in | 1 +
>> src/libvirt.c | 9 ++++++++
>> src/qemu/qemu_driver.c | 51
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 61 insertions(+)
> [...]
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 2950a4b..cfc5941 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"
>
> Hi, Francesco.
> I see the file including relationship. 'qemu_driver.c' includes
> 'qemu_cgroup.h' which
> includes 'vircgroup.h'. There are other virCgroupGet* functions called in
> qemu_driver.c
> now. So I think here "include vircgroup.h" is not necessary.
Thanks for the research, I'll remove
I don't think that's necessary. We guard includes by #ifdef-ing it and
also if something changes in the future we won't need to change the
includes. If you use the functions from vircgroup.h, then just include it.
>> #define VIR_FROM_THIS VIR_FROM_QEMU
>>
>> @@ -17338,6 +17339,55 @@ qemuDomainGetStatsState(virConnectPtr conn
>> ATTRIBUTE_UNUSED,
>> }
>>
>>
>> +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;
>> + int err;
>> +
>> + ncpus = nodeGetCPUCount();
>> + if (ncpus > 0 &&
>> + virTypedParamsAddUInt(&record->params,
>> + &record->nparams,
>> + maxparams,
>> + "cpu.count",
>> + (unsigned int)ncpus) < 0)
>> + return -1;
>> +
>> + 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;
>
> 1. If any of the 'err's is not zero, the function may returns 0 as success.
> Is this the expected return? Or at least we can give a warning that we
> miss some parameters.
> 2. I think it's better to report an error or warning log before return -1.
The idea here (well, at least my idea :) ) is to gather as much as data as possible,
and to silently skip failures here. The lack of expected output is a good enough
indicator that a domain is unresponsive.
-1 is reported for really unrecoverable error, like memory (re)allocation failure.
That makes sense. I'd implement it the same way.
Otherwise, how could the client code be meaningfully informed that some group
failed, maybe partially? It is possible that different groups fail for different
domains: how could we convey this information?
I think that skipping information snippets that can't be gathered makes
sense. (I hope that I've documented it that way) And if we'd like to
make failure mandatory we still can add a flag. I think that this
shouldn't be necessary though as long as the caller takes that into account.
I have no problems to go this route if there is consensus this is the preferred
way to go, but then we'll need to discuss how to convey a meaningful error
convention.
Bests,
Peter
8:05 a.m.
New subject: [libvirt] [PATCHv3 3/8] qemu: bulk stats: implement balloon group
This patch implements the VIR_DOMAIN_STATS_BALLOON
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 6 ++++
src/qemu/qemu_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 77 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e6ed803..1e4e428 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/libvirt.c b/src/libvirt.c
index 4d504ff..f21eb39 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21562,6 +21562,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 cfc5941..4f8ccac 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2520,6 +2520,47 @@ static int qemuDomainSendKey(virDomainPtr domain,
return ret;
}
+
+/*
+ * FIXME: this code is a stripped down version of what is done into
+ * qemuDomainGetInfo. Due to the different handling of jobs, it is not
+ * trivial to extract a common helper function.
+ */
+static void
+qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, virDomainObjPtr vm,
+ unsigned long *memory)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (vm->def->memballoon &&
+ 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 {
+ int err;
+ unsigned long long balloon;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
+ qemuDomainObjExitMonitor(driver, vm);
+
+ 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;
+ }
+ }
+}
+
+
static int qemuDomainGetInfo(virDomainPtr dom,
virDomainInfoPtr info)
{
@@ -17387,6 +17428,34 @@ 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;
+
+ qemuDomainGetBalloonMemory(driver, dom, &cur_balloon);
+
+ 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,
@@ -17404,6 +17473,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
6:45 a.m.
New subject: [libvirt] [PATCHv3 3/8] qemu: bulk stats: implement balloon group
On 09/08/14 15:05, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_BALLOON
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 6 ++++
src/qemu/qemu_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 77 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index e6ed803..1e4e428 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/libvirt.c b/src/libvirt.c
index 4d504ff..f21eb39 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21562,6 +21562,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 cfc5941..4f8ccac 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2520,6 +2520,47 @@ static int qemuDomainSendKey(virDomainPtr domain,
return ret;
}
+
+/*
+ * FIXME: this code is a stripped down version of what is done into
+ * qemuDomainGetInfo. Due to the different handling of jobs, it is not
+ * trivial to extract a common helper function.
+ */
+static void
+qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, virDomainObjPtr vm,
+ unsigned long *memory)
Use unsigned long long here. Unsigned long is 32 bit on 32bit systems.
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (vm->def->memballoon &&
+ 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 {
+ int err;
+ unsigned long long balloon;
Note this ...
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
+ qemuDomainObjExitMonitor(driver, vm);
+
+ 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 */
Should we in such case drop the balloon stat from the output?
+ *memory = vm->def->mem.max_balloon;
+ } else {
+ *memory = balloon;
Here it'd break.
+ }
+ }
+}
+
+
static int qemuDomainGetInfo(virDomainPtr dom,
virDomainInfoPtr info)
{
@@ -17387,6 +17428,34 @@ 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;
+
+ qemuDomainGetBalloonMemory(driver, dom, &cur_balloon);
+
+ 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,
@@ -17404,6 +17473,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 }
};
Peter
6:59 a.m.
New subject: [libvirt] [PATCHv3 3/8] qemu: bulk stats: implement balloon group
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 1:45:57 PM
Subject: Re: [libvirt] [PATCHv3 3/8] qemu: bulk stats: implement balloon group
On 09/08/14 15:05, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_BALLOON
> group of statistics.
>
> Signed-off-by: Francesco Romani <fromani(a)redhat.com>
> ---
> include/libvirt/libvirt.h.in | 1 +
> src/libvirt.c | 6 ++++
> src/qemu/qemu_driver.c | 70
> ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 77 insertions(+)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index e6ed803..1e4e428 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/libvirt.c b/src/libvirt.c
> index 4d504ff..f21eb39 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21562,6 +21562,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 cfc5941..4f8ccac 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2520,6 +2520,47 @@ static int qemuDomainSendKey(virDomainPtr domain,
> return ret;
> }
>
> +
> +/*
> + * FIXME: this code is a stripped down version of what is done into
> + * qemuDomainGetInfo. Due to the different handling of jobs, it is not
> + * trivial to extract a common helper function.
> + */
> +static void
> +qemuDomainGetBalloonMemory(virQEMUDriverPtr driver, virDomainObjPtr vm,
> + unsigned long *memory)
Use unsigned long long here. Unsigned long is 32 bit on 32bit systems.
Will fix
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> + if (vm->def->memballoon &&
> + 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 {
> + int err;
> + unsigned long long balloon;
Note this ...
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> + err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
> + qemuDomainObjExitMonitor(driver, vm);
> +
> + 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
> */
Should we in such case drop the balloon stat from the output?
That would be ok for me. Just let me know :)
> + *memory = vm->def->mem.max_balloon;
> + } else {
> + *memory = balloon;
Here it'd break.
Good catch, will fix.
Bests,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
8:05 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
This patch implements the VIR_DOMAIN_STATS_VCPU
group of statistics.
To do so, this patch also extracts a helper to gather the
VCpu information.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 13 +++
src/qemu/qemu_driver.c | 210 ++++++++++++++++++++++++++++++-------------
3 files changed, 160 insertions(+), 64 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 1e4e428..68573a0 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/libvirt.c b/src/libvirt.c
index f21eb39..0326847 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21569,6 +21569,19 @@ 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.
+ * "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU <num> as
int.
+ *
* 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 4f8ccac..6bcbfb5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1375,6 +1375,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 v, maxcpu, hostcpus;
+ size_t i;
+ 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)
{
@@ -4994,10 +5064,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;
@@ -5012,67 +5079,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)
@@ -17457,6 +17464,80 @@ 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[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 ((ret = qemuDomainHelperGetVcpus(dom,
+ cpuinfo,
+ dom->def->vcpus,
+ NULL,
+ 0)) < 0)
+ return 0;
+
+ 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;
+
+ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "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,
@@ -17474,6 +17555,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
6:56 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
On 09/08/14 15:05, 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(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 13 +++
src/qemu/qemu_driver.c | 210 ++++++++++++++++++++++++++++++-------------
3 files changed, 160 insertions(+), 64 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 1e4e428..68573a0 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/libvirt.c b/src/libvirt.c
index f21eb39..0326847 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21569,6 +21569,19 @@ 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.
+ * "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU <num> as
int.
This is not the CPU number the vCPU is pinned to but rather the current
CPU number where the vCPU is actually running. If you pin it to multiple
CPUs this may change in the range of the host CPUs the vCPU is pinned
to. Said this I don't think this is an useful stat.
Rather than this I'd like to see the mask of the host CPUs where this
vCPU is pinned to. (returned as a human readable bitmask string).
Any thoughts?
+ *
* 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 4f8ccac..6bcbfb5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
Otherwise looks good.
Peter
4:31 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
On 2014/9/9 19:56, Peter Krempa wrote:
> + * 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.
> > + * "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU
<num> as int.
This is not the CPU number the vCPU is pinned to but rather the current
CPU number where the vCPU is actually running. If you pin it to multiple
CPUs this may change in the range of the host CPUs the vCPU is pinned
to. Said this I don't think this is an useful stat.
Rather than this I'd like to see the mask of the host CPUs where this
vCPU is pinned to. (returned as a human readable bitmask string).
Any thoughts?
IMO giving the vcpu's pininfo is more useful than the current pcpu number on which
vcpu is running *now*. (I don't know the requirement of management very well.)
But if it means the pininfo, what about naming it "vcpu.<num>.pin" or
other
name which is less confusing?
10:54 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 1:56:09 PM
Subject: Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
> + * 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.
> + * "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU
<num> as int.
This is not the CPU number the vCPU is pinned to but rather the current
CPU number where the vCPU is actually running. If you pin it to multiple
CPUs this may change in the range of the host CPUs the vCPU is pinned
to. Said this I don't think this is an useful stat.
Right, my bad, I overlooked the docs (started to suspect when saw it changing too often
in my tests..).
I agree this is not very useful, I'll drop it.
Rather than this I'd like to see the mask of the host CPUs where
this
vCPU is pinned to. (returned as a human readable bitmask string).
Any thoughts?
Is that the data provided by
http://libvirt.org/html/libvirt-libvirt.html#virDomainGetVcpuPinInfo
it isn't? (I'm asking because docs aren't crystal clear for me).
I like this, but I'd also need to do a cross-check on my our code in oVirt.
Will be acceptable to drop the misleading vcpu.<num>.cpu info and to add
the pin info in a new followup patch, in this stats group?
Bests,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
11:07 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
On 09/11/14 17:54, Francesco Romani wrote:
----- Original Message -----
> From: "Peter Krempa" <pkrempa(a)redhat.com>
> To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
> Sent: Tuesday, September 9, 2014 1:56:09 PM
> Subject: Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
>> + * 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.
>> + * "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU
<num> as int.
>
> This is not the CPU number the vCPU is pinned to but rather the current
> CPU number where the vCPU is actually running. If you pin it to multiple
> CPUs this may change in the range of the host CPUs the vCPU is pinned
> to. Said this I don't think this is an useful stat.
Right, my bad, I overlooked the docs (started to suspect when saw it changing too often
in my tests..).
I agree this is not very useful, I'll drop it.
> Rather than this I'd like to see the mask of the host CPUs where this
> vCPU is pinned to. (returned as a human readable bitmask string).
>
> Any thoughts?
Is that the data provided by
http://libvirt.org/html/libvirt-libvirt.html#virDomainGetVcpuPinInfo
it isn't? (I'm asking because docs aren't crystal clear for me).
Yes that's exactly it. You should return it as a bitmap formatted to a
string. (see virBitmapFormat).
I like this, but I'd also need to do a cross-check on my our code in oVirt.
Will be acceptable to drop the misleading vcpu.<num>.cpu info and to add
the pin info in a new followup patch, in this stats group?
You definitely can add that later on. But you should drop .cpu from this
patch (not revert it later).
Bests,
Peter
11:51 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Thursday, September 11, 2014 6:07:48 PM
Subject: Re: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
> I like this, but I'd also need to do a cross-check on my our code in oVirt.
[...]
> Will be acceptable to drop the misleading vcpu.<num>.cpu
info and to add
> the pin info in a new followup patch, in this stats group?
You definitely can add that later on. But you should drop .cpu from this
patch (not revert it later).
Very good, next submission (v4) will drop the misleading 'cpu' item,
Hopefully I'll also be able to add the pin information;
otherwise I'll save for a followup patch.
Bests,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
5:29 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
On 2014/9/8 21:05, 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(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 13 +++
src/qemu/qemu_driver.c | 210 ++++++++++++++++++++++++++++++-------------
3 files changed, 160 insertions(+), 64 deletions(-)
[...]
+static int
+qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo,
+ unsigned char *cpumaps, int maplen)
+{
+ int v, maxcpu, hostcpus;
+ size_t i;
'v' can be size_t type ,too.(Although the code is moved here from
qemuDomainGetVcpus. :-) )
+ 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 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[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 ((ret = qemuDomainHelperGetVcpus(dom,
+ cpuinfo,
+ dom->def->vcpus,
+ NULL,
+ 0)) < 0)
+ return 0;
Memory of 'cpuinfo' will be leaked. Should we go to cleanup?
5:50 a.m.
New subject: [libvirt] [PATCHv3 4/8] qemu: bulk stats: implement VCPU group
> +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[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 ((ret = qemuDomainHelperGetVcpus(dom,
> + cpuinfo,
> + dom->def->vcpus,
> + NULL,
> + 0)) < 0)
> + return 0;
Memory of 'cpuinfo' will be leaked. Should we go to cleanup?
Ouch. Of course I do need to avoid this. Will fix.
Thanks,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
8:05 a.m.
New subject: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
This patch implements the VIR_DOMAIN_STATS_INTERFACE
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 14 +++++++
src/qemu/qemu_driver.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 68573a0..93aa1fb 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/libvirt.c b/src/libvirt.c
index 0326847..8aa6cb1 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21582,6 +21582,20 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* as unsigned long long.
* "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU <num> as
int.
*
+ * 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 long long.
+ * "net.<num>.rx.pkts" - packets received as long long.
+ * "net.<num>.rx.errs" - receive errors as long long.
+ * "net.<num>.rx.drop" - receive packets dropped as long long.
+ * "net.<num>.tx.bytes" - bytes transmitted as long long.
+ * "net.<num>.tx.pkts" - packets transmitted as long long.
+ * "net.<num>.tx.errs" - transmission errors as long long.
+ * "net.<num>.tx.drop" - transmit packets dropped as 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 6bcbfb5..989eb3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17537,6 +17537,92 @@ qemuDomainGetStatsVcpu(virConnectPtr conn 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.%lu.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.%lu.%s", num, 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;
+
+ 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)
+ 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)(virConnectPtr conn,
@@ -17556,6 +17642,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
6:42 a.m.
New subject: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
On 09/08/14 15:05, Francesco Romani wrote:
This patch implements the VIR_DOMAIN_STATS_INTERFACE
group of statistics.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 14 +++++++
src/qemu/qemu_driver.c | 87 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 68573a0..93aa1fb 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/libvirt.c b/src/libvirt.c
index 0326847..8aa6cb1 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21582,6 +21582,20 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* as unsigned long long.
* "vcpu.<num>.cpu" - physical CPU pinned to virtual CPU <num> as
int.
*
+ * 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 long long.
+ * "net.<num>.rx.pkts" - packets received as long long.
+ * "net.<num>.rx.errs" - receive errors as long long.
+ * "net.<num>.rx.drop" - receive packets dropped as long long.
+ * "net.<num>.tx.bytes" - bytes transmitted as long long.
+ * "net.<num>.tx.pkts" - packets transmitted as long long.
+ * "net.<num>.tx.errs" - transmission errors as long long.
+ * "net.<num>.tx.drop" - transmit packets dropped as long long.
Why are all of those represented as long long instead of unsigned long
long? I don't see how these could be negative. If we need to express
that the value is unsupported we can just drop it from here and not
waste half of the range here.
Any other opinions on this?
+ *
* 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 6bcbfb5..989eb3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17537,6 +17537,92 @@ qemuDomainGetStatsVcpu(virConnectPtr conn 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.%lu.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.%lu.%s", num, name); \
%lu? the count is unsigned int so you should be fine with %d
+ 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;
+
+ 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)
+ 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)(virConnectPtr conn,
@@ -17556,6 +17642,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 }
};
Peter
12:11 p.m.
New subject: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 1:42:15 PM
Subject: Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
> + * 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 long long.
> + * "net.<num>.rx.pkts" - packets received as long long.
> + * "net.<num>.rx.errs" - receive errors as long long.
> + * "net.<num>.rx.drop" - receive packets dropped as long long.
> + * "net.<num>.tx.bytes" - bytes transmitted as long long.
> + * "net.<num>.tx.pkts" - packets transmitted as long long.
> + * "net.<num>.tx.errs" - transmission errors as long long.
> + * "net.<num>.tx.drop" - transmit packets dropped as long long.
Why are all of those represented as long long instead of unsigned long
long? I don't see how these could be negative. If we need to express
that the value is unsupported we can just drop it from here and not
waste half of the range here.
Any other opinions on this?
I used long long because of this:
struct _virDomainInterfaceStats {
long long rx_bytes;
long long rx_packets;
long long rx_errs;
long long rx_drop;
long long tx_bytes;
long long tx_packets;
long long tx_errs;
long long tx_drop;
};
But I don't have any problem to cast them as unsigned, with something like:
#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.%u.%s", num, name); \
if (virTypedParamsAddULLong(&(record)->params, \
&(record)->nparams, \
maxparams, \
param_name, \
(unsigned long long)value) < 0) \
return -1; \
} while (0)
> + *
> * 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 6bcbfb5..989eb3e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17537,6 +17537,92 @@ qemuDomainGetStatsVcpu(virConnectPtr conn
> 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.%lu.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.%lu.%s", num, name); \
%lu? the count is unsigned int so you should be fine with %d
Yep but the cycle counter is size_t and then...
$ git diff
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9d53883..e90a8c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17487,7 +17487,7 @@ do { \
do { \
char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
- "net.%lu.%s", num, name); \
+ "net.%u.%s", num, name); \
if (virTypedParamsAddLLong(&(record)->params, \
&(record)->nparams, \
maxparams, \
$ make
[...]
make[1]: Entering directory `/home/fromani/Projects/libvirt/src'
CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo
qemu/qemu_driver.c: In function 'qemuDomainGetStatsInterface':
qemu/qemu_driver.c:17521:9: error: format '%u' expects argument of type
'unsigned int', but argument 4 has type 'size_t' [-Werror=format=]
QEMU_ADD_NET_PARAM(record, maxparams, i,
^
qemu/qemu_driver.c:17521:9: error: format '%u' expects argument of type
'unsigned int', but argument 4 has type 'size_t' [-Werror=format=]
$ gcc --version
gcc (GCC) 4.8.3 20140624 (Red Hat 4.8.3-1)
same story with '%d'. Keeping '%lu' for this reason.
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
12:16 p.m.
New subject: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
On 09/11/14 19:11, Francesco Romani wrote:
----- Original Message -----
> From: "Peter Krempa" <pkrempa(a)redhat.com>
> To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
> Sent: Tuesday, September 9, 2014 1:42:15 PM
> Subject: Re: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
>> + * 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 long long.
>> + * "net.<num>.rx.pkts" - packets received as long long.
>> + * "net.<num>.rx.errs" - receive errors as long long.
>> + * "net.<num>.rx.drop" - receive packets dropped as long long.
>> + * "net.<num>.tx.bytes" - bytes transmitted as long long.
>> + * "net.<num>.tx.pkts" - packets transmitted as long long.
>> + * "net.<num>.tx.errs" - transmission errors as long long.
>> + * "net.<num>.tx.drop" - transmit packets dropped as long long.
>
> Why are all of those represented as long long instead of unsigned long
> long? I don't see how these could be negative. If we need to express
> that the value is unsupported we can just drop it from here and not
> waste half of the range here.
>
> Any other opinions on this?
I used long long because of this:
struct _virDomainInterfaceStats {
long long rx_bytes;
long long rx_packets;
long long rx_errs;
long long rx_drop;
long long tx_bytes;
long long tx_packets;
long long tx_errs;
long long tx_drop;
};
But I don't have any problem to cast them as unsigned, with something like:
That will be fine with me as long as:
#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.%u.%s", num, name); \
if (virTypedParamsAddULLong(&(record)->params, \
... you don't add the param if value is < 0. Thus rather than expressing
the missing information via -1 just omit the parameter.
&(record)->nparams, \
maxparams, \
param_name, \
(unsigned long long)value) < 0) \
return -1; \
} while (0)
>
>> + *
>> * 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 6bcbfb5..989eb3e 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -17537,6 +17537,92 @@ qemuDomainGetStatsVcpu(virConnectPtr conn
>> 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.%lu.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.%lu.%s", num, name); \
>
> %lu? the count is unsigned int so you should be fine with %d
Yep but the cycle counter is size_t and then...
$ git diff
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9d53883..e90a8c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17487,7 +17487,7 @@ do { \
do { \
char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
- "net.%lu.%s", num, name); \
+ "net.%u.%s", num, name); \
if (virTypedParamsAddLLong(&(record)->params, \
&(record)->nparams, \
maxparams, \
$ make
[...]
make[1]: Entering directory `/home/fromani/Projects/libvirt/src'
CC qemu/libvirt_driver_qemu_impl_la-qemu_driver.lo
qemu/qemu_driver.c: In function 'qemuDomainGetStatsInterface':
qemu/qemu_driver.c:17521:9: error: format '%u' expects argument of type
'unsigned int', but argument 4 has type 'size_t' [-Werror=format=]
QEMU_ADD_NET_PARAM(record, maxparams, i,
^
qemu/qemu_driver.c:17521:9: error: format '%u' expects argument of type
'unsigned int', but argument 4 has type 'size_t' [-Werror=format=]
$ gcc --version
gcc (GCC) 4.8.3 20140624 (Red Hat 4.8.3-1)
same story with '%d'. Keeping '%lu' for this reason.
If it's size_t, then you should use %zu as a formatter.
Peter
1 p.m.
New subject: [libvirt] [PATCHv3 5/8] qemu: bulk stats: implement interface group
On 09/11/2014 11:16 AM, Peter Krempa wrote:
> struct _virDomainInterfaceStats {
> long long rx_bytes;
> long long rx_packets;
> long long rx_errs;
> long long rx_drop;
> long long tx_bytes;
> long long tx_packets;
> long long tx_errs;
> long long tx_drop;
> };
>
> But I don't have any problem to cast them as unsigned, with something like:
That will be fine with me as long as:
>
> #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.%u.%s", num, name); \
> if (virTypedParamsAddULLong(&(record)->params, \
... you don't add the param if value is < 0. Thus rather than expressing
the missing information via -1 just omit the parameter.
> &(record)->nparams, \
> maxparams, \
> param_name, \
> (unsigned long long)value) < 0) \
The cast is spurious; the C compiler does the right thing if you already
filter on value < 0 before calling this function to add the positive
values as unsigned long long.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:05 a.m.
New subject: [libvirt] [PATCHv3 6/8] qemu: bulk stats: implement block group
This patch implements the VIR_DOMAIN_STATS_BLOCK
group of statistics.
To do so, an helper function to get the block stats
of all the disks of a domain is added.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 17 ++++++
src/qemu/qemu_driver.c | 93 +++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 22 +++++++
src/qemu/qemu_monitor.h | 20 +++++++
src/qemu/qemu_monitor_json.c | 135 ++++++++++++++++++++++++++++++-------------
src/qemu/qemu_monitor_json.h | 4 ++
src/qemu/qemu_monitor_text.c | 13 +++++
src/qemu/qemu_monitor_text.h | 4 ++
9 files changed, 269 insertions(+), 40 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 93aa1fb..c8e0089 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/libvirt.c b/src/libvirt.c
index 8aa6cb1..e041fa2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21596,6 +21596,23 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "net.<num>.tx.errs" - transmission errors as long long.
* "net.<num>.tx.drop" - transmit packets dropped as 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 long long.
+ * "block.<num>.rd.bytes" - number of read bytes as long long.
+ * "block.<num>.rd.times" - total time (ns) spent on reads as long long.
+ * "block.<num>.wr.reqs" - number of write requests as long long.
+ * "block.<num>.wr.bytes" - number of written bytes as long long.
+ * "block.<num>.wr.times" - total time (ns) spent on writes as long
long.
+ * "block.<num>.fl.reqs" - total flush requests as long long.
+ * "block.<num>.fl.times" - total time (ns) spent on cache flushing
+ * as long long.
+ * "block.<num>.errors" - Xen only: the 'oo_req' value as 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 989eb3e..93afb7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9678,6 +9678,31 @@ qemuDomainBlockStats(virDomainPtr dom,
return ret;
}
+
+/*
+ * Returns at most the first `nstats' stats, then stops.
+ * Returns the number of stats filled.
+ */
+static int
+qemuDomainHelperGetBlockStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuBlockStatsPtr stats,
+ int nstats)
+{
+ int ret;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ ret = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL,
+ stats, nstats);
+
+ qemuDomainObjExitMonitor(driver, vm);
+
+ return ret;
+}
+
+
static int
qemuDomainBlockStatsFlags(virDomainPtr dom,
const char *path,
@@ -17620,6 +17645,73 @@ qemuDomainGetStatsInterface(virConnectPtr conn ATTRIBUTE_UNUSED,
#undef QEMU_ADD_NET_PARAM
+#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.%lu.%s", num, name); \
+ if (virTypedParamsAddLLong(&(record)->params, \
+ &(record)->nparams, \
+ maxparams, \
+ param_name, \
+ value) < 0) \
+ goto cleanup; \
+} while (0)
+
+static int
+qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams,
+ unsigned int privflags ATTRIBUTE_UNUSED)
+{
+ size_t i;
+ int ret = -1;
+ int nstats = 0;
+ qemuBlockStatsPtr stats = NULL;
+ virQEMUDriverPtr driver = conn->privateData;
+
+ if (VIR_ALLOC_N(stats, dom->def->ndisks) < 0)
+ return -1;
+
+ nstats = qemuDomainHelperGetBlockStats(driver, dom, stats,
+ dom->def->ndisks);
+ if (nstats < 0)
+ 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
+
#undef QEMU_ADD_NAME_PARAM
#undef QEMU_ADD_COUNT_PARAM
@@ -17643,6 +17735,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 702404a..d8ce875 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -29,6 +29,8 @@
#include <unistd.h>
#include <fcntl.h>
+#include "internal.h"
+
#include "qemu_monitor.h"
#include "qemu_monitor_text.h"
#include "qemu_monitor_json.h"
@@ -1760,6 +1762,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
return ret;
}
+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 {
+ ret = qemuMonitorTextGetAllBlockStatsInfo(mon, dev_name,
+ stats, nstats);
+ }
+
+ 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 ced198e..8e3fb44 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 4373ba2..aa95e71 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1734,13 +1734,9 @@ 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 nstats = 1;
+ int ret = -1;
*rd_req = *rd_bytes = -1;
*wr_req = *wr_bytes = *errs = -1;
@@ -1754,9 +1750,51 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
if (flush_total_times)
*flush_total_times = -1;
+ if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name,
+ &stats, nstats) != nstats)
+ 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;
+ size_t i;
+ bool found = false;
+ 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 +1810,125 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
goto cleanup;
}
+ ret = 0;
for (i = 0; i < virJSONValueArraySize(devices); 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"));
+ _("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"));
- goto cleanup;
+ if (ret > nstats) {
+ break;
}
- /* 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;
+ }
- 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;
}
+
+ ret++;
+ bstats++;
}
- if (!found) {
+ if (dev_name && !found) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find statistics for device '%s'"),
dev_name);
+ ret = 0;
goto cleanup;
}
- ret = 0;
cleanup:
virJSONValueFree(cmd);
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index a6d05b5..0744241 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,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 2bc8261..5cdbef3 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1048,6 +1048,19 @@ int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
return -1;
}
+
+int
+qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ const char *dev_name ATTRIBUTE_UNUSED,
+ qemuBlockStatsPtr blockstats ATTRIBUTE_UNUSED,
+ int nstats ATTRIBUTE_UNUSED)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unable to query all block stats with this QEMU"));
+ return -1;
+}
+
+
/* Return 0 on success, -1 on failure, or -2 if not supported. Size
* is in bytes. */
int qemuMonitorTextBlockResize(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index 49d4b88..9ebe8fb 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -71,6 +71,10 @@ int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon,
long long *flush_req,
long long *flush_total_times,
long long *errs);
+int qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon,
+ const char *dev_name,
+ qemuBlockStatsPtr blockstats,
+ int nstats);
int qemuMonitorTextGetBlockStatsParamsNumber(qemuMonitorPtr mon,
int *nparams);
int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon,
--
1.9.3
8:12 a.m.
New subject: [libvirt] [PATCHv3 6/8] qemu: bulk stats: implement block group
On 09/08/14 15:05, 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(a)redhat.com>
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 17 ++++++
src/qemu/qemu_driver.c | 93 +++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 22 +++++++
src/qemu/qemu_monitor.h | 20 +++++++
src/qemu/qemu_monitor_json.c | 135 ++++++++++++++++++++++++++++++-------------
src/qemu/qemu_monitor_json.h | 4 ++
src/qemu/qemu_monitor_text.c | 13 +++++
src/qemu/qemu_monitor_text.h | 4 ++
9 files changed, 269 insertions(+), 40 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 93aa1fb..c8e0089 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/libvirt.c b/src/libvirt.c
index 8aa6cb1..e041fa2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21596,6 +21596,23 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "net.<num>.tx.errs" - transmission errors as long long.
* "net.<num>.tx.drop" - transmit packets dropped as 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 long long.
+ * "block.<num>.rd.bytes" - number of read bytes as long long.
+ * "block.<num>.rd.times" - total time (ns) spent on reads as long
long.
+ * "block.<num>.wr.reqs" - number of write requests as long long.
+ * "block.<num>.wr.bytes" - number of written bytes as long long.
+ * "block.<num>.wr.times" - total time (ns) spent on writes as long
long.
+ * "block.<num>.fl.reqs" - total flush requests as long long.
+ * "block.<num>.fl.times" - total time (ns) spent on cache flushing
+ * as long long.
+ * "block.<num>.errors" - Xen only: the 'oo_req' value as long
long.
Same as for the interface stats. How about using unsigned long long and
dropping stats that are not available?
+ *
* 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 989eb3e..93afb7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9678,6 +9678,31 @@ qemuDomainBlockStats(virDomainPtr dom,
return ret;
}
+
+/*
+ * Returns at most the first `nstats' stats, then stops.
+ * Returns the number of stats filled.
+ */
+static int
+qemuDomainHelperGetBlockStats(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuBlockStatsPtr stats,
+ int nstats)
+{
+ int ret;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ ret = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL,
+ stats, nstats);
+
+ qemuDomainObjExitMonitor(driver, vm);
+
+ return ret;
+}
+
+
static int
qemuDomainBlockStatsFlags(virDomainPtr dom,
const char *path,
@@ -17620,6 +17645,73 @@ qemuDomainGetStatsInterface(virConnectPtr conn
ATTRIBUTE_UNUSED,
#undef QEMU_ADD_NET_PARAM
+#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.%lu.%s", num, name); \
+ if (virTypedParamsAddLLong(&(record)->params, \
+ &(record)->nparams, \
+ maxparams, \
+ param_name, \
+ value) < 0) \
+ goto cleanup; \
+} while (0)
+
+static int
+qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainObjPtr dom,
+ virDomainStatsRecordPtr record,
+ int *maxparams,
+ unsigned int privflags ATTRIBUTE_UNUSED)
+{
+ size_t i;
+ int ret = -1;
+ int nstats = 0;
+ qemuBlockStatsPtr stats = NULL;
+ virQEMUDriverPtr driver = conn->privateData;
+
+ if (VIR_ALLOC_N(stats, dom->def->ndisks) < 0)
+ return -1;
+
+ nstats = qemuDomainHelperGetBlockStats(driver, dom, stats,
+ dom->def->ndisks);
+ if (nstats < 0)
+ 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
+
#undef QEMU_ADD_NAME_PARAM
#undef QEMU_ADD_COUNT_PARAM
@@ -17643,6 +17735,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 702404a..d8ce875 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -29,6 +29,8 @@
#include <unistd.h>
#include <fcntl.h>
+#include "internal.h"
+
#include "qemu_monitor.h"
#include "qemu_monitor_text.h"
#include "qemu_monitor_json.h"
@@ -1760,6 +1762,26 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
return ret;
}
+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 {
+ ret = qemuMonitorTextGetAllBlockStatsInfo(mon, dev_name,
+ stats, nstats);
+ }
+
+ 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 ced198e..8e3fb44 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 4373ba2..aa95e71 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1734,13 +1734,9 @@ 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 nstats = 1;
+ int ret = -1;
*rd_req = *rd_bytes = -1;
*wr_req = *wr_bytes = *errs = -1;
@@ -1754,9 +1750,51 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
if (flush_total_times)
*flush_total_times = -1;
+ if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name,
+ &stats, nstats) != nstats)
Writing '1' instead of 'nstats' would make it a bit more readable as you
don't expect to get more or less stats.
+ 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;
+ size_t i;
+ bool found = false;
+ virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats",
+ NULL);
+ virJSONValuePtr reply = NULL;
+ virJSONValuePtr devices;
+
if (!cmd)
return -1;
+ if (!bstats || nstats <= 0)
+ return -1;
These are okay, they don't report an error, but also are for develoeper
errors only.
+
ret = qemuMonitorJSONCommand(mon, cmd, &reply);
if (ret == 0)
@@ -1772,108 +1810,125 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
goto cleanup;
}
+ ret = 0;
Here you set ret to 0.
for (i = 0; i < virJSONValueArraySize(devices); i++) {
After a few iterations it may be > 0. (Or 0 on the first)
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"));
+ _("blockstats device entry was not "
+ "in expected format"));
goto cleanup;
If you hit this error, you return ret, which is now in success mode so
no one will see it.
}
- if ((thisdev = virJSONValueObjectGetString(dev, "device")) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("blockstats device entry was not in expected
format"));
- goto cleanup;
+ if (ret > nstats) {
+ break;
}
- /* 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;
+ }
- 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;
}
+
+ ret++;
+ bstats++;
}
- if (!found) {
+ if (dev_name && !found) {
'found' here will be true if and only if ret == 1, so you don't really
need a separate variable
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find statistics for device '%s'"),
dev_name);
+ ret = 0;
I'd return -1 here as you've reported an error.
goto cleanup;
}
- ret = 0;
cleanup:
virJSONValueFree(cmd);
diff --git a/src/qemu/qemu_monitor_text.c
b/src/qemu/qemu_monitor_text.c
index 2bc8261..5cdbef3 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1048,6 +1048,19 @@ int qemuMonitorTextGetBlockExtent(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
return -1;
}
+
+int
+qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ const char *dev_name ATTRIBUTE_UNUSED,
+ qemuBlockStatsPtr blockstats ATTRIBUTE_UNUSED,
+ int nstats ATTRIBUTE_UNUSED)
+{
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("unable to query all block stats with this QEMU"));
You can save adding this function and fold it right into the monitor
wrapper function as we do now with commadns that don't have HMP
counterparts.
+ return -1;
+}
+
+
/* Return 0 on success, -1 on failure, or -2 if not supported. Size
* is in bytes. */
int qemuMonitorTextBlockResize(qemuMonitorPtr mon,
Peter
8:05 a.m.
New subject: [libvirt] [PATCHv3 7/8] qemu: bulk stats: add block allocation information
Management software, want 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, an helper is extracted from
qemuMonitorJSONGetBlockExtent in order to get per-device
allocation information.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
src/libvirt.c | 2 ++
src/qemu/qemu_driver.c | 15 +++++++++
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 76 ++++++++++++++++++++++++++++++++------------
4 files changed, 73 insertions(+), 21 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index e041fa2..ff8f891 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -21612,6 +21612,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* "block.<num>.fl.times" - total time (ns) spent on cache flushing
* as long long.
* "block.<num>.errors" - Xen only: the 'oo_req' value as 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 93afb7e..86e7893 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17658,6 +17658,18 @@ do { \
goto cleanup; \
} while (0)
+#define QEMU_ADD_BLOCK_PARAM_ULL(RECORD, MAXPARAMS, NUM, NAME, VALUE) \
+do { \
+ char param_name[NAME_MAX]; \
+ snprintf(param_name, NAME_MAX, "block.%lu.%s", NUM, NAME); \
+ if (virTypedParamsAddULLong(&RECORD->params, \
+ &RECORD->nparams, \
+ MAXPARAMS, \
+ param_name, \
+ VALUE) < 0) \
+ goto cleanup; \
+} while (0)
+
static int
qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainObjPtr dom,
@@ -17701,6 +17713,9 @@ qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
"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;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8e3fb44..97d7336 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 aa95e71..bc5616d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1776,6 +1776,55 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
}
+/* This helper function could be called in the
+ * qemuMonitorJSONGetAllBlockStatsInfo
+ * path - which is used also by the
+ * qemuMonitorJSONGetBlockStatsInfo
+ * path. In this case, we don't know in advance if the wr_highest_offset
+ * field is there, so it is OK to fail silently.
+ * However, we can get here by the
+ * qemuMonitorJSONGetBlockExtent
+ * path, and in that case we _must_ fail loudly.
+ */
+static int
+qemuMonitorJSONDevGetBlockExtent(virJSONValuePtr dev,
+ bool report_error,
+ unsigned long long *extent)
+{
+ virJSONValuePtr stats;
+ virJSONValuePtr parent;
+
+ if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL ||
+ parent->type != VIR_JSON_TYPE_OBJECT) {
+ if (report_error)
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("blockstats parent entry was not in "
+ "expected format"));
+ return -1;
+ }
+
+ if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL ||
+ stats->type != VIR_JSON_TYPE_OBJECT) {
+ if (report_error)
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("blockstats stats entry was not in "
+ "expected format"));
+ return -1;
+ }
+
+ if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset",
+ extent) < 0) {
+ if (report_error)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot read %s statistic"),
+ "wr_highest_offset");
+ return -1;
+ }
+
+ return 0;
+}
+
+
int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
const char *dev_name,
qemuBlockStatsPtr bstats,
@@ -1919,6 +1968,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
goto cleanup;
}
+ /* it's ok to not have this information here. Just skip silently. */
+ qemuMonitorJSONDevGetBlockExtent(dev, false,
+ &bstats->wr_highest_offset);
+
ret++;
bstats++;
}
@@ -2010,6 +2063,7 @@ int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon,
return ret;
}
+
int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon,
const char *dev_name,
unsigned long long *extent)
@@ -2044,8 +2098,6 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon,
for (i = 0; i < virJSONValueArraySize(devices); i++) {
virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
- virJSONValuePtr stats;
- virJSONValuePtr parent;
const char *thisdev;
if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -2070,26 +2122,8 @@ 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"));
+ if (qemuMonitorJSONDevGetBlockExtent(dev, true, extent) < 0)
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");
- goto cleanup;
- }
}
if (!found) {
--
1.9.3
8:19 a.m.
New subject: [libvirt] [PATCHv3 7/8] qemu: bulk stats: add block allocation information
On 09/08/14 15:05, Francesco Romani wrote:
Management software, want to be able to allocate disk space on
demand.
s/, want/ wants/
To support this, they need keep track of the space occupation
s/,//
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, an helper is extracted from
s/,// s/an/a/
qemuMonitorJSONGetBlockExtent in order to get per-device
allocation information.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
src/libvirt.c | 2 ++
src/qemu/qemu_driver.c | 15 +++++++++
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 76 ++++++++++++++++++++++++++++++++------------
4 files changed, 73 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
index aa95e71..bc5616d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1776,6 +1776,55 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
}
+/* This helper function could be called in the
+ * qemuMonitorJSONGetAllBlockStatsInfo
+ * path - which is used also by the
+ * qemuMonitorJSONGetBlockStatsInfo
+ * path. In this case, we don't know in advance if the wr_highest_offset
+ * field is there, so it is OK to fail silently.
+ * However, we can get here by the
+ * qemuMonitorJSONGetBlockExtent
+ * path, and in that case we _must_ fail loudly.
+ */
+static int
+qemuMonitorJSONDevGetBlockExtent(virJSONValuePtr dev,
+ bool report_error,
+ unsigned long long *extent)
+{
+ virJSONValuePtr stats;
+ virJSONValuePtr parent;
+
+ if ((parent = virJSONValueObjectGet(dev, "parent")) == NULL ||
+ parent->type != VIR_JSON_TYPE_OBJECT) {
+ if (report_error)
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("blockstats parent entry was not in "
+ "expected format"));
+ return -1;
+ }
+
+ if ((stats = virJSONValueObjectGet(parent, "stats")) == NULL ||
+ stats->type != VIR_JSON_TYPE_OBJECT) {
+ if (report_error)
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("blockstats stats entry was not in "
+ "expected format"));
+ return -1;
+ }
+
+ if (virJSONValueObjectGetNumberUlong(stats, "wr_highest_offset",
+ extent) < 0) {
+ if (report_error)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot read %s statistic"),
+ "wr_highest_offset");
+ return -1;
+ }
+
+ return 0;
+}
+
+
int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
const char *dev_name,
qemuBlockStatsPtr bstats,
@@ -1919,6 +1968,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
goto cleanup;
}
+ /* it's ok to not have this information here. Just skip silently. */
+ qemuMonitorJSONDevGetBlockExtent(dev, false,
+ &bstats->wr_highest_offset);
As you want to ignore errors, it would probably be better just to copy
the extraction code here without error reporting rather than extracting
it to a helper ... this isn't something that would be reused any more.
+
ret++;
bstats++;
}
@@ -2010,6 +2063,7 @@ int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon,
return ret;
}
+
int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon,
const char *dev_name,
unsigned long long *extent)
@@ -2044,8 +2098,6 @@ int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon,
for (i = 0; i < virJSONValueArraySize(devices); i++) {
virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
- virJSONValuePtr stats;
- virJSONValuePtr parent;
const char *thisdev;
if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -2070,26 +2122,8 @@ 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"));
+ if (qemuMonitorJSONDevGetBlockExtent(dev, true, extent) < 0)
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");
- goto cleanup;
- }
}
if (!found) {
Peter
2:55 a.m.
New subject: [libvirt] [PATCHv3 7/8] qemu: bulk stats: add block allocation information
----- Original Message -----
From: "Peter Krempa" <pkrempa(a)redhat.com>
To: "Francesco Romani" <fromani(a)redhat.com>, libvir-list(a)redhat.com
Sent: Tuesday, September 9, 2014 3:19:28 PM
Subject: Re: [libvirt] [PATCHv3 7/8] qemu: bulk stats: add block allocation information
On 09/08/14 15:05, Francesco Romani wrote:
> Management software, want to be able to allocate disk space on demand.
s/, want/ wants/
> To support this, they need keep track of the space occupation
s/,//
> 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, an helper is extracted from
s/,// s/an/a/
Thanks, will fix both.
[...]
> int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
> const char *dev_name,
> qemuBlockStatsPtr bstats,
> @@ -1919,6 +1968,10 @@ int
> qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
> goto cleanup;
> }
>
> + /* it's ok to not have this information here. Just skip silently.
> */
> + qemuMonitorJSONDevGetBlockExtent(dev, false,
> + &bstats->wr_highest_offset);
As you want to ignore errors, it would probably be better just to copy
the extraction code here without error reporting rather than extracting
it to a helper ... this isn't something that would be reused any more.
I definitely see your point.
But I'm not really OK to replicate code, even if it's "just" twice.
So, I'll work a bit more on that. I'll move this patch to the end
of the series to gain a bit more time.
If I can't come up with a better approach I'll go this way
and just copy the extraction code as suggested above.
Bests,
--
Francesco Romani
RedHat Engineering Virtualization R & D
Phone: 8261328
IRC: fromani
8:05 a.m.
New subject: [libvirt] [PATCH 8/8] virsh: add options to query bulk stats group
Exports to the domstats commands the new bulk stats groups.
Signed-off-by: Francesco Romani <fromani(a)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 4401d55..ec07913 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
7:16 a.m.
New subject: [libvirt] [PATCH 8/8] virsh: add options to query bulk stats group
On 09/08/14 15:05, Francesco Romani wrote:
Exports to the domstats commands the new bulk stats groups.
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
tools/virsh-domain-monitor.c | 35 +++++++++++++++++++++++++++++++++++
tools/virsh.pod | 4 +++-
2 files changed, 38 insertions(+), 1 deletion(-)
@@ -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>.
Maybe we should document this a bit more. But that is a pre-existing
problem.
Selecting a specific statistics groups doesn't guarantee that the
daemon supports the selected group of stats. Flag I<--enforce>
ACK,
Peter
3702
days inactive
3706
days old
36 comments
4 participants
participants (4)
-
Eric Blake -
Francesco Romani -
Peter Krempa -
Wang Rui