* [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-14 15:49 [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Luiz Capitulino
@ 2012-12-14 15:49 ` Luiz Capitulino
2012-12-17 10:13 ` Dietmar Maurer
0 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2012-12-14 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl
Next commit will re-enable balloon stats with a different interface, but
this old code conflicts with it. Let's drop it.
It's important to note that the QMP and HMP interfaces are also dropped
by this commit. That shouldn't be a problem though, because:
1. All QMP fields are optional
2. This has never been really used
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hmp.c | 24 +-----------------------
hw/virtio-balloon.c | 22 ----------------------
qapi-schema.json | 20 +-------------------
qmp-commands.hx | 13 -------------
4 files changed, 2 insertions(+), 77 deletions(-)
diff --git a/hmp.c b/hmp.c
index 180ba2b..f1cbbbe 100644
--- a/hmp.c
+++ b/hmp.c
@@ -464,29 +464,7 @@ void hmp_info_balloon(Monitor *mon)
return;
}
- monitor_printf(mon, "balloon: actual=%" PRId64, info->actual >> 20);
- if (info->has_mem_swapped_in) {
- monitor_printf(mon, " mem_swapped_in=%" PRId64, info->mem_swapped_in);
- }
- if (info->has_mem_swapped_out) {
- monitor_printf(mon, " mem_swapped_out=%" PRId64, info->mem_swapped_out);
- }
- if (info->has_major_page_faults) {
- monitor_printf(mon, " major_page_faults=%" PRId64,
- info->major_page_faults);
- }
- if (info->has_minor_page_faults) {
- monitor_printf(mon, " minor_page_faults=%" PRId64,
- info->minor_page_faults);
- }
- if (info->has_free_mem) {
- monitor_printf(mon, " free_mem=%" PRId64, info->free_mem);
- }
- if (info->has_total_mem) {
- monitor_printf(mon, " total_mem=%" PRId64, info->total_mem);
- }
-
- monitor_printf(mon, "\n");
+ monitor_printf(mon, "balloon: actual=%" PRId64 "\n", info->actual >> 20);
qapi_free_BalloonInfo(info);
}
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index dd1a650..4398025 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -164,28 +164,6 @@ static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
{
VirtIOBalloon *dev = opaque;
-
-#if 0
- /* Disable guest-provided stats for now. For more details please check:
- * https://bugzilla.redhat.com/show_bug.cgi?id=623903
- *
- * If you do enable it (which is probably not going to happen as we
- * need a new command for it), remember that you also need to fill the
- * appropriate members of the BalloonInfo structure so that the stats
- * are returned to the client.
- */
- if (dev->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ)) {
- virtqueue_push(dev->svq, &dev->stats_vq_elem, dev->stats_vq_offset);
- virtio_notify(&dev->vdev, dev->svq);
- return;
- }
-#endif
-
- /* Stats are not supported. Clear out any stale values that might
- * have been set by a more featureful guest kernel.
- */
- reset_stats(dev);
-
info->actual = ram_size - ((uint64_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT);
}
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..e2fdd40 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -977,28 +977,10 @@
#
# @actual: the number of bytes the balloon currently contains
#
-# @mem_swapped_in: #optional number of pages swapped in within the guest
-#
-# @mem_swapped_out: #optional number of pages swapped out within the guest
-#
-# @major_page_faults: #optional number of major page faults within the guest
-#
-# @minor_page_faults: #optional number of minor page faults within the guest
-#
-# @free_mem: #optional amount of memory (in bytes) free in the guest
-#
-# @total_mem: #optional amount of memory (in bytes) visible to the guest
-#
# Since: 0.14.0
#
-# Notes: all current versions of QEMU do not fill out optional information in
-# this structure.
##
-{ 'type': 'BalloonInfo',
- 'data': {'actual': 'int', '*mem_swapped_in': 'int',
- '*mem_swapped_out': 'int', '*major_page_faults': 'int',
- '*minor_page_faults': 'int', '*free_mem': 'int',
- '*total_mem': 'int'} }
+{ 'type': 'BalloonInfo', 'data': {'actual': 'int' } }
##
# @query-balloon:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c692d0..c05d336 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2549,13 +2549,6 @@ Make an asynchronous request for balloon info. When the request completes a
json-object will be returned containing the following data:
- "actual": current balloon value in bytes (json-int)
-- "mem_swapped_in": Amount of memory swapped in bytes (json-int, optional)
-- "mem_swapped_out": Amount of memory swapped out in bytes (json-int, optional)
-- "major_page_faults": Number of major faults (json-int, optional)
-- "minor_page_faults": Number of minor faults (json-int, optional)
-- "free_mem": Total amount of free and unused memory in
- bytes (json-int, optional)
-- "total_mem": Total amount of available memory in bytes (json-int, optional)
Example:
@@ -2563,12 +2556,6 @@ Example:
<- {
"return":{
"actual":1073741824,
- "mem_swapped_in":0,
- "mem_swapped_out":0,
- "major_page_faults":142,
- "minor_page_faults":239245,
- "free_mem":1014185984,
- "total_mem":1044668416
}
}
--
1.8.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-14 15:49 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
@ 2012-12-17 10:13 ` Dietmar Maurer
2012-12-17 11:57 ` Luiz Capitulino
2012-12-18 21:34 ` Eric Blake
0 siblings, 2 replies; 19+ messages in thread
From: Dietmar Maurer @ 2012-12-17 10:13 UTC (permalink / raw)
To: Luiz Capitulino, qemu-devel@nongnu.org
Cc: aliguori@us.ibm.com, mdroth@linux.vnet.ibm.com, agl@us.ibm.com
> Next commit will re-enable balloon stats with a different interface, but this
> old code conflicts with it. Let's drop it.
I don't really see any conflicts here?
> It's important to note that the QMP and HMP interfaces are also dropped by
> this commit. That shouldn't be a problem though, because:
>
> 1. All QMP fields are optional
> 2. This has never been really used
Why don't we simply implement the missing parts - it's just a few lines of code.
for example:
Index: new/hw/virtio-balloon.c
===================================================================
--- new.orig/hw/virtio-balloon.c 2012-12-17 07:55:34.000000000 +0100
+++ new/hw/virtio-balloon.c 2012-12-17 09:20:32.000000000 +0100
@@ -59,7 +59,7 @@
}
static const char *balloon_stat_names[] = {
- [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
+ [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
[VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
[VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
[VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
@@ -314,6 +314,30 @@
VirtIOBalloon *dev = opaque;
info->actual = ram_size - ((uint64_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT);
+
+ info->total_mem = ram_size;
+
+ if (!(balloon_stats_enabled(dev) && dev->stats_last_update)) {
+ return;
+ }
+
+ info->last_update = dev->stats_last_update;
+ info->has_last_update = true;
+
+ info->mem_swapped_in = dev->stats[VIRTIO_BALLOON_S_SWAP_IN];
+ info->has_mem_swapped_in = info->mem_swapped_in >= 0 ? true : false;
+
+ info->mem_swapped_out = dev->stats[VIRTIO_BALLOON_S_SWAP_OUT];
+ info->has_mem_swapped_out = info->mem_swapped_out >= 0 ? true : false;
+
+ info->major_page_faults = dev->stats[VIRTIO_BALLOON_S_MAJFLT];
+ info->has_major_page_faults = info->major_page_faults >= 0 ? true : false;
+
+ info->minor_page_faults = dev->stats[VIRTIO_BALLOON_S_MINFLT];
+ info->has_minor_page_faults = info->minor_page_faults >= 0 ? true : false;
+
+ info->free_mem = dev->stats[VIRTIO_BALLOON_S_MEMFREE];
+ info->has_free_mem = info->free_mem >= 0 ? true : false;
}
static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
Index: new/qapi-schema.json
===================================================================
--- new.orig/qapi-schema.json 2012-12-17 08:19:30.000000000 +0100
+++ new/qapi-schema.json 2012-12-17 08:35:55.000000000 +0100
@@ -1044,6 +1044,8 @@
#
# @actual: the number of bytes the balloon currently contains
#
+# @last_update: #optional time when stats got updated from guest
+#
# @mem_swapped_in: #optional number of pages swapped in within the guest
#
# @mem_swapped_out: #optional number of pages swapped out within the guest
@@ -1054,18 +1056,15 @@
#
# @free_mem: #optional amount of memory (in bytes) free in the guest
#
-# @total_mem: #optional amount of memory (in bytes) visible to the guest
+# @total_mem: amount of memory (in bytes) visible to the guest
#
# Since: 0.14.0
-#
-# Notes: all current versions of QEMU do not fill out optional information in
-# this structure.
##
{ 'type': 'BalloonInfo',
- 'data': {'actual': 'int', '*mem_swapped_in': 'int',
+ 'data': {'actual': 'int', '*last_update': 'int', '*mem_swapped_in': 'int',
'*mem_swapped_out': 'int', '*major_page_faults': 'int',
'*minor_page_faults': 'int', '*free_mem': 'int',
- '*total_mem': 'int'} }
+ 'total_mem': 'int'} }
##
# @query-balloon:
Index: new/hmp.c
===================================================================
--- new.orig/hmp.c 2012-12-17 08:36:51.000000000 +0100
+++ new/hmp.c 2012-12-17 09:06:15.000000000 +0100
@@ -497,6 +497,11 @@
}
monitor_printf(mon, "balloon: actual=%" PRId64, info->actual >> 20);
+ monitor_printf(mon, " total_mem=%" PRId64, info->total_mem >> 20);
+ if (info->has_free_mem) {
+ monitor_printf(mon, " free_mem=%" PRId64, info->free_mem >> 20);
+ }
+
if (info->has_mem_swapped_in) {
monitor_printf(mon, " mem_swapped_in=%" PRId64, info->mem_swapped_in);
}
@@ -511,11 +516,9 @@
monitor_printf(mon, " minor_page_faults=%" PRId64,
info->minor_page_faults);
}
- if (info->has_free_mem) {
- monitor_printf(mon, " free_mem=%" PRId64, info->free_mem);
- }
- if (info->has_total_mem) {
- monitor_printf(mon, " total_mem=%" PRId64, info->total_mem);
+ if (info->has_last_update) {
+ monitor_printf(mon, " last_update=%" PRId64,
+ info->last_update);
}
monitor_printf(mon, "\n");
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-17 10:13 ` Dietmar Maurer
@ 2012-12-17 11:57 ` Luiz Capitulino
2012-12-17 12:23 ` Dietmar Maurer
2012-12-18 21:34 ` Eric Blake
1 sibling, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2012-12-17 11:57 UTC (permalink / raw)
To: Dietmar Maurer
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com,
qemu-devel@nongnu.org, agl@us.ibm.com
On Mon, 17 Dec 2012 10:13:40 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:
> > Next commit will re-enable balloon stats with a different interface, but this
> > old code conflicts with it. Let's drop it.
>
> I don't really see any conflicts here?
query-balloon resets all stats to -1. So, if it's issued after the stats have
been polled all values are lost.
> > It's important to note that the QMP and HMP interfaces are also dropped by
> > this commit. That shouldn't be a problem though, because:
> >
> > 1. All QMP fields are optional
> > 2. This has never been really used
>
> Why don't we simply implement the missing parts - it's just a few lines of code.
For the reasons explained in my last email. We'd need at least two commands,
one for to enable polling/set time interval and also query-balloon-stats,
as adding this to query-balloon would mix semantics (ie. one field is always
there and the others have polling semantics).
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-17 11:57 ` Luiz Capitulino
@ 2012-12-17 12:23 ` Dietmar Maurer
2012-12-17 12:33 ` Luiz Capitulino
0 siblings, 1 reply; 19+ messages in thread
From: Dietmar Maurer @ 2012-12-17 12:23 UTC (permalink / raw)
To: Luiz Capitulino
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com,
qemu-devel@nongnu.org, agl@us.ibm.com
> > I don't really see any conflicts here?
>
> query-balloon resets all stats to -1. So, if it's issued after the stats have been
> polled all values are lost.
I can't see that in the code. I also tested with my patch, and it does not reset
any stats. So I can't see that conflict?
There is no code to reset stats inside virtio_balloon_stat()
> > > It's important to note that the QMP and HMP interfaces are also
> > > dropped by this commit. That shouldn't be a problem though, because:
> > >
> > > 1. All QMP fields are optional
> > > 2. This has never been really used
> >
> > Why don't we simply implement the missing parts - it's just a few lines of
> code.
>
> For the reasons explained in my last email. We'd need at least two
> commands, one for to enable polling/set time interval and also query-
> balloon-stats, as adding this to query-balloon would mix semantics (ie. one
> field is always there and the others have polling semantics).
Yes, we need 2 commands - and what is the problem?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-17 12:23 ` Dietmar Maurer
@ 2012-12-17 12:33 ` Luiz Capitulino
2012-12-17 12:36 ` Dietmar Maurer
0 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2012-12-17 12:33 UTC (permalink / raw)
To: Dietmar Maurer
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com,
qemu-devel@nongnu.org, agl@us.ibm.com
On Mon, 17 Dec 2012 12:23:59 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:
> > > I don't really see any conflicts here?
> >
> > query-balloon resets all stats to -1. So, if it's issued after the stats have been
> > polled all values are lost.
>
> I can't see that in the code. I also tested with my patch, and it does not reset
> any stats. So I can't see that conflict?
>
> There is no code to reset stats inside virtio_balloon_stat()
static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
{
VirtIOBalloon *dev = opaque;
#if 0
/* Disable guest-provided stats for now. For more details please check:
* https://bugzilla.redhat.com/show_bug.cgi?id=623903
*
* If you do enable it (which is probably not going to happen as we
* need a new command for it), remember that you also need to fill the
* appropriate members of the BalloonInfo structure so that the stats
* are returned to the client.
*/
if (dev->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ)) {
virtqueue_push(dev->svq, &dev->stats_vq_elem, dev->stats_vq_offset);
virtio_notify(&dev->vdev, dev->svq);
return;
}
#endif
/* Stats are not supported. Clear out any stale values that might
* have been set by a more featureful guest kernel.
*/
reset_stats(dev); <<<<========================================================
^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^
info->actual = ram_size - ((uint64_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT);
}
>
> > > > It's important to note that the QMP and HMP interfaces are also
> > > > dropped by this commit. That shouldn't be a problem though, because:
> > > >
> > > > 1. All QMP fields are optional
> > > > 2. This has never been really used
> > >
> > > Why don't we simply implement the missing parts - it's just a few lines of
> > code.
> >
> > For the reasons explained in my last email. We'd need at least two
> > commands, one for to enable polling/set time interval and also query-
> > balloon-stats, as adding this to query-balloon would mix semantics (ie. one
> > field is always there and the others have polling semantics).
>
> Yes, we need 2 commands - and what is the problem?
The problem is adding new commands when that's not required. With QOM
everything can be done with existing commands.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-17 12:33 ` Luiz Capitulino
@ 2012-12-17 12:36 ` Dietmar Maurer
2012-12-17 12:39 ` Luiz Capitulino
0 siblings, 1 reply; 19+ messages in thread
From: Dietmar Maurer @ 2012-12-17 12:36 UTC (permalink / raw)
To: Luiz Capitulino
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com,
qemu-devel@nongnu.org, agl@us.ibm.com
> > There is no code to reset stats inside virtio_balloon_stat()
>
> static void virtio_balloon_stat(void *opaque, BalloonInfo *info) {
> VirtIOBalloon *dev = opaque;
>
> #if 0
> /* Disable guest-provided stats for now. For more details please check:
> * https://bugzilla.redhat.com/show_bug.cgi?id=623903
My patch allies on top of your patch, and you already removed that code.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-17 12:36 ` Dietmar Maurer
@ 2012-12-17 12:39 ` Luiz Capitulino
0 siblings, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2012-12-17 12:39 UTC (permalink / raw)
To: Dietmar Maurer
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com,
qemu-devel@nongnu.org, agl@us.ibm.com
On Mon, 17 Dec 2012 12:36:59 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:
> > > There is no code to reset stats inside virtio_balloon_stat()
> >
> > static void virtio_balloon_stat(void *opaque, BalloonInfo *info) {
> > VirtIOBalloon *dev = opaque;
> >
> > #if 0
> > /* Disable guest-provided stats for now. For more details please check:
> > * https://bugzilla.redhat.com/show_bug.cgi?id=623903
>
> My patch allies on top of your patch, and you already removed that code.
This was in reply to your comment that said you didn't see any
conflict with the code being dropped and *my* new code.
Please, stop trolling.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-17 10:13 ` Dietmar Maurer
2012-12-17 11:57 ` Luiz Capitulino
@ 2012-12-18 21:34 ` Eric Blake
2012-12-19 5:18 ` Dietmar Maurer
2012-12-19 11:27 ` Luiz Capitulino
1 sibling, 2 replies; 19+ messages in thread
From: Eric Blake @ 2012-12-18 21:34 UTC (permalink / raw)
To: Dietmar Maurer
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, agl@us.ibm.com,
qemu-devel@nongnu.org, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]
On 12/17/2012 03:13 AM, Dietmar Maurer wrote:
>> Next commit will re-enable balloon stats with a different interface, but this
>> old code conflicts with it. Let's drop it.
>
> I don't really see any conflicts here?
>
>> It's important to note that the QMP and HMP interfaces are also dropped by
>> this commit. That shouldn't be a problem though, because:
>>
>> 1. All QMP fields are optional
>> 2. This has never been really used
Libvirt has been using it when available (although reluctantly, as it
risks hanging on an uncooperative guest); and while libvirt can be
patched to call 6 QOM commands in a row to query six different QOM
stats, I still think it would be nicer to add a command that provides
all the stats at once. In particular, when calling 6 commands in
series, you no longer have an atomic picture of the guest (the polling
interval could hit between two QOM queries, resulting in a combined set
of statistics that has no counterpart to the transition of states that
the guest actually went through). On the other hand, since the stats
are already polling-based, and since it requires cooperation from the
guest, not having a guarantee of an atomic set of stats is not really
much of a loss.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-18 21:34 ` Eric Blake
@ 2012-12-19 5:18 ` Dietmar Maurer
2012-12-19 11:27 ` Luiz Capitulino
1 sibling, 0 replies; 19+ messages in thread
From: Dietmar Maurer @ 2012-12-19 5:18 UTC (permalink / raw)
To: Eric Blake
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, agl@us.ibm.com,
qemu-devel@nongnu.org, Luiz Capitulino
> Libvirt has been using it when available (although reluctantly, as it risks
> hanging on an uncooperative guest); and while libvirt can be patched to call 6
> QOM commands in a row to query six different QOM stats, I still think it
> would be nicer to add a command that provides all the stats at once. In
> particular, when calling 6 commands in series, you no longer have an atomic
> picture of the guest (the polling interval could hit between two QOM queries,
> resulting in a combined set of statistics that has no counterpart to the
> transition of states that the guest actually went through). On the other hand,
> since the stats are already polling-based, and since it requires cooperation
> from the guest, not having a guarantee of an atomic set of stats is not really
> much of a loss.
I already implemented that and sent patches to this list. The patch is more or less
trivial - we simply read the cached values. It does not hang on an
uncooperative guest, and works without any problems.
But unfortunately, Luiz dislikes the patch (I do not understand why). So I stopped posting it.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-18 21:34 ` Eric Blake
2012-12-19 5:18 ` Dietmar Maurer
@ 2012-12-19 11:27 ` Luiz Capitulino
2012-12-20 18:36 ` Eric Blake
1 sibling, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2012-12-19 11:27 UTC (permalink / raw)
To: Eric Blake
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, agl@us.ibm.com,
Dietmar Maurer, qemu-devel@nongnu.org
On Tue, 18 Dec 2012 14:34:16 -0700
Eric Blake <eblake@redhat.com> wrote:
> On 12/17/2012 03:13 AM, Dietmar Maurer wrote:
> >> Next commit will re-enable balloon stats with a different interface, but this
> >> old code conflicts with it. Let's drop it.
> >
> > I don't really see any conflicts here?
> >
> >> It's important to note that the QMP and HMP interfaces are also dropped by
> >> this commit. That shouldn't be a problem though, because:
> >>
> >> 1. All QMP fields are optional
> >> 2. This has never been really used
>
> Libvirt has been using it when available (although reluctantly, as it
> risks hanging on an uncooperative guest);
This has always been disabled and qemu never returns the stats info.
I believe libvirt's code is rotting just like qemu's is.
> and while libvirt can be
> patched to call 6 QOM commands in a row to query six different QOM
> stats, I still think it would be nicer to add a command that provides
> all the stats at once. In particular, when calling 6 commands in
> series, you no longer have an atomic picture of the guest (the polling
> interval could hit between two QOM queries, resulting in a combined set
> of statistics that has no counterpart to the transition of states that
> the guest actually went through). On the other hand, since the stats
> are already polling-based, and since it requires cooperation from the
> guest, not having a guarantee of an atomic set of stats is not really
> much of a loss.
Something I have been wondering if whether it's possible to have only
one property (say balloon-statistics) and return all properties in a
dict. QOM properties return a visitor, so maybe that's possible.
I'll check that.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2012-12-19 11:27 ` Luiz Capitulino
@ 2012-12-20 18:36 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2012-12-20 18:36 UTC (permalink / raw)
To: Luiz Capitulino
Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, agl@us.ibm.com,
Dietmar Maurer, qemu-devel@nongnu.org
[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]
On 12/19/2012 04:27 AM, Luiz Capitulino wrote:
>> Libvirt has been using it when available (although reluctantly, as it
>> risks hanging on an uncooperative guest);
>
> This has always been disabled and qemu never returns the stats info.
> I believe libvirt's code is rotting just like qemu's is.
I agree that the QMP has never been providing it (since we pulled it out
of QMP precisely because of the potential for a hang), but if you go
back far enough to an old qemu that provided the stats via HMP, then
libvirt is indeed exposing those stats to the user.
There are two places in libvirt code that were collecting stats. One
for determining what to print for simple commands like 'virsh dumpxml',
where we only cared about balloon size and not the rest of the
statistics, and where we do NOT want to hang waiting on the guest; for
that case, the solution that Dan came up with was adding a balloon
event, and using the event instead of a query. The other usage is that
we have an API where the user can specifically request all the stats;
and there, we DO want to block on the guest to get the stats, and would
prefer getting the stats in a single command (but can tolerate getting
the stats via 6 separate commands, if that's what it takes). It is this
second usage that has been broken in libvirt ever since we crippled the
stats collection in qemu because of the undesirable hang in the first case.
> Something I have been wondering if whether it's possible to have only
> one property (say balloon-statistics) and return all properties in a
> dict. QOM properties return a visitor, so maybe that's possible.
>
> I'll check that.
Ah, that would be a nice trick - querying a single QOM property that
turns out to be a dict exposing multiple sub-properties at once. And if
you can get that to work, you are back to your goal of no new QMP command.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3 0/3] re-enable balloon stats
@ 2013-01-18 19:29 Luiz Capitulino
2013-01-18 19:29 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Luiz Capitulino @ 2013-01-18 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl
This series re-enables the virtio-balloon device stats feature. It now uses
QOM properties via a polling mechanism as suggested by Anthony here:
http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02390.html
o v3
- return all stats at once via the guest-stats property
(ie. kill individual querying of stats)
- rebase on top of latest master
- update doc
Luiz Capitulino (3):
balloon: drop old stats code & API
balloon: re-enable balloon stats
docs: document virtio-balloon stats
docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++
hmp.c | 24 +-----
hw/virtio-balloon.c | 175 ++++++++++++++++++++++++++++++++++++------
qapi-schema.json | 20 +----
qmp-commands.hx | 13 ----
5 files changed, 254 insertions(+), 80 deletions(-)
create mode 100644 docs/virtio-balloon-stats.txt
--
1.8.1.GIT
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2013-01-18 19:29 [Qemu-devel] [PATCH v3 0/3] re-enable balloon stats Luiz Capitulino
@ 2013-01-18 19:29 ` Luiz Capitulino
2013-01-18 20:01 ` Eric Blake
2013-01-18 19:29 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino
2013-01-18 19:29 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino
2 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2013-01-18 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl
Next commit will re-enable balloon stats with a different interface, but
this old code conflicts with it. Let's drop it.
It's important to note that the QMP and HMP interfaces are also dropped
by this commit. That shouldn't be a problem though, because:
1. All QMP fields are optional
2. This feature has always been disabled
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hmp.c | 24 +-----------------------
hw/virtio-balloon.c | 24 ------------------------
qapi-schema.json | 20 +-------------------
qmp-commands.hx | 13 -------------
4 files changed, 2 insertions(+), 79 deletions(-)
diff --git a/hmp.c b/hmp.c
index c7b6ba0..ae16916 100644
--- a/hmp.c
+++ b/hmp.c
@@ -465,29 +465,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
return;
}
- monitor_printf(mon, "balloon: actual=%" PRId64, info->actual >> 20);
- if (info->has_mem_swapped_in) {
- monitor_printf(mon, " mem_swapped_in=%" PRId64, info->mem_swapped_in);
- }
- if (info->has_mem_swapped_out) {
- monitor_printf(mon, " mem_swapped_out=%" PRId64, info->mem_swapped_out);
- }
- if (info->has_major_page_faults) {
- monitor_printf(mon, " major_page_faults=%" PRId64,
- info->major_page_faults);
- }
- if (info->has_minor_page_faults) {
- monitor_printf(mon, " minor_page_faults=%" PRId64,
- info->minor_page_faults);
- }
- if (info->has_free_mem) {
- monitor_printf(mon, " free_mem=%" PRId64, info->free_mem);
- }
- if (info->has_total_mem) {
- monitor_printf(mon, " total_mem=%" PRId64, info->total_mem);
- }
-
- monitor_printf(mon, "\n");
+ monitor_printf(mon, "balloon: actual=%" PRId64 "\n", info->actual >> 20);
qapi_free_BalloonInfo(info);
}
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 3040bc6..2520cba 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -164,28 +164,6 @@ static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
static void virtio_balloon_stat(void *opaque, BalloonInfo *info)
{
VirtIOBalloon *dev = opaque;
-
-#if 0
- /* Disable guest-provided stats for now. For more details please check:
- * https://bugzilla.redhat.com/show_bug.cgi?id=623903
- *
- * If you do enable it (which is probably not going to happen as we
- * need a new command for it), remember that you also need to fill the
- * appropriate members of the BalloonInfo structure so that the stats
- * are returned to the client.
- */
- if (dev->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ)) {
- virtqueue_push(dev->svq, &dev->stats_vq_elem, dev->stats_vq_offset);
- virtio_notify(&dev->vdev, dev->svq);
- return;
- }
-#endif
-
- /* Stats are not supported. Clear out any stale values that might
- * have been set by a more featureful guest kernel.
- */
- reset_stats(dev);
-
info->actual = ram_size - ((uint64_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT);
}
@@ -255,8 +233,6 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
- reset_stats(s);
-
s->qdev = dev;
register_savevm(dev, "virtio-balloon", -1, 1,
virtio_balloon_save, virtio_balloon_load, s);
diff --git a/qapi-schema.json b/qapi-schema.json
index 6d7252b..a4c6eca 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -977,28 +977,10 @@
#
# @actual: the number of bytes the balloon currently contains
#
-# @mem_swapped_in: #optional number of pages swapped in within the guest
-#
-# @mem_swapped_out: #optional number of pages swapped out within the guest
-#
-# @major_page_faults: #optional number of major page faults within the guest
-#
-# @minor_page_faults: #optional number of minor page faults within the guest
-#
-# @free_mem: #optional amount of memory (in bytes) free in the guest
-#
-# @total_mem: #optional amount of memory (in bytes) visible to the guest
-#
# Since: 0.14.0
#
-# Notes: all current versions of QEMU do not fill out optional information in
-# this structure.
##
-{ 'type': 'BalloonInfo',
- 'data': {'actual': 'int', '*mem_swapped_in': 'int',
- '*mem_swapped_out': 'int', '*major_page_faults': 'int',
- '*minor_page_faults': 'int', '*free_mem': 'int',
- '*total_mem': 'int'} }
+{ 'type': 'BalloonInfo', 'data': {'actual': 'int' } }
##
# @query-balloon:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cbf1280..3be5330 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2549,13 +2549,6 @@ Make an asynchronous request for balloon info. When the request completes a
json-object will be returned containing the following data:
- "actual": current balloon value in bytes (json-int)
-- "mem_swapped_in": Amount of memory swapped in bytes (json-int, optional)
-- "mem_swapped_out": Amount of memory swapped out in bytes (json-int, optional)
-- "major_page_faults": Number of major faults (json-int, optional)
-- "minor_page_faults": Number of minor faults (json-int, optional)
-- "free_mem": Total amount of free and unused memory in
- bytes (json-int, optional)
-- "total_mem": Total amount of available memory in bytes (json-int, optional)
Example:
@@ -2563,12 +2556,6 @@ Example:
<- {
"return":{
"actual":1073741824,
- "mem_swapped_in":0,
- "mem_swapped_out":0,
- "major_page_faults":142,
- "minor_page_faults":239245,
- "free_mem":1014185984,
- "total_mem":1044668416
}
}
--
1.8.1.GIT
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats
2013-01-18 19:29 [Qemu-devel] [PATCH v3 0/3] re-enable balloon stats Luiz Capitulino
2013-01-18 19:29 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
@ 2013-01-18 19:29 ` Luiz Capitulino
2013-01-18 20:04 ` Eric Blake
2013-01-18 19:29 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino
2 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2013-01-18 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl
The statistics are now available through device properties via a
polling mechanism. First a client has to enable polling, then it
can query available stats.
Polling is enabled by setting an update interval (in seconds)
to a property named guest-stats-polling-interval, like this:
{ "execute": "qom-set",
"arguments": { "path": "/machine/peripheral-anon/device[1]",
"property": "guest-stats-polling-interval", "value": 4 } }
Then the available stats can be retrieved by querying the
guest-stats property. The returned object is a dict containing
all available stats. Example:
{ "execute": "qom-get",
"arguments": { "path": "/machine/peripheral-anon/device[1]",
"property": "guest-stats" } }
{
"return": {
"stats": {
"stat-swap-out": 0,
"stat-free-memory": 844943360,
"stat-minor-faults": 219028,
"stat-major-faults": 235,
"stat-total-memory": 1044406272,
"stat-swap-in": 0
},
"last-update": 1358529861
}
}
Please, check the next commit for full documentation.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/virtio-balloon.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 150 insertions(+), 1 deletion(-)
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 2520cba..f4605fb 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -14,6 +14,7 @@
*/
#include "qemu/iov.h"
+#include "qemu/timer.h"
#include "qemu-common.h"
#include "virtio.h"
#include "pc.h"
@@ -22,6 +23,7 @@
#include "virtio-balloon.h"
#include "sysemu/kvm.h"
#include "exec/address-spaces.h"
+#include "qapi/visitor.h"
#if defined(__linux__)
#include <sys/mman.h>
@@ -36,6 +38,9 @@ typedef struct VirtIOBalloon
uint64_t stats[VIRTIO_BALLOON_S_NR];
VirtQueueElement stats_vq_elem;
size_t stats_vq_offset;
+ QEMUTimer *stats_timer;
+ int64_t stats_last_update;
+ int64_t stats_poll_interval;
DeviceState *qdev;
} VirtIOBalloon;
@@ -53,6 +58,16 @@ static void balloon_page(void *addr, int deflate)
#endif
}
+static const char *balloon_stat_names[] = {
+ [VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
+ [VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
+ [VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
+ [VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
+ [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
+ [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
+ [VIRTIO_BALLOON_S_NR] = NULL
+};
+
/*
* reset_stats - Mark all items in the stats array as unset
*
@@ -67,6 +82,118 @@ static inline void reset_stats(VirtIOBalloon *dev)
for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1);
}
+static bool balloon_stats_supported(const VirtIOBalloon *s)
+{
+ return s->vdev.guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ);
+}
+
+static bool balloon_stats_enabled(const VirtIOBalloon *s)
+{
+ return s->stats_poll_interval > 0;
+}
+
+static void balloon_stats_destroy_timer(VirtIOBalloon *s)
+{
+ if (balloon_stats_enabled(s)) {
+ qemu_del_timer(s->stats_timer);
+ qemu_free_timer(s->stats_timer);
+ s->stats_timer = NULL;
+ s->stats_poll_interval = 0;
+ }
+}
+
+static void balloon_stats_change_timer(VirtIOBalloon *s, int secs)
+{
+ qemu_mod_timer(s->stats_timer, qemu_get_clock_ms(vm_clock) + secs * 1000);
+}
+
+static void balloon_stats_poll_cb(void *opaque)
+{
+ VirtIOBalloon *s = opaque;
+
+ if (!balloon_stats_supported(s)) {
+ /* re-schedule */
+ balloon_stats_change_timer(s, s->stats_poll_interval);
+ return;
+ }
+
+ virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
+ virtio_notify(&s->vdev, s->svq);
+}
+
+static void balloon_stats_get_all(Object *obj, struct Visitor *v,
+ void *opaque, const char *name, Error **errp)
+{
+ VirtIOBalloon *s = opaque;
+ int i;
+
+ if (!s->stats_last_update) {
+ error_setg(errp, "guest hasn't updated any stats yet");
+ return;
+ }
+
+ visit_start_struct(v, NULL, "guest-stats", name, 0, errp);
+ visit_type_int(v, &s->stats_last_update, "last-update", errp);
+
+ visit_start_struct(v, NULL, NULL, "stats", 0, errp);
+ for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+ visit_type_int64(v, (int64_t *) &s->stats[i], balloon_stat_names[i],
+ errp);
+ }
+ visit_end_struct(v, errp);
+
+ visit_end_struct(v, errp);
+}
+
+static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ VirtIOBalloon *s = opaque;
+ visit_type_int(v, &s->stats_poll_interval, name, errp);
+}
+
+static void balloon_stats_set_poll_interval(Object *obj, struct Visitor *v,
+ void *opaque, const char *name,
+ Error **errp)
+{
+ VirtIOBalloon *s = opaque;
+ int64_t value;
+
+ visit_type_int(v, &value, name, errp);
+ if (error_is_set(errp)) {
+ return;
+ }
+
+ if (value < 0) {
+ error_setg(errp, "timer value must be greater than zero");
+ return;
+ }
+
+ if (value == s->stats_poll_interval) {
+ return;
+ }
+
+ if (value == 0) {
+ /* timer=0 disables the timer */
+ balloon_stats_destroy_timer(s);
+ return;
+ }
+
+ if (balloon_stats_enabled(s)) {
+ /* timer interval change */
+ s->stats_poll_interval = value;
+ balloon_stats_change_timer(s, value);
+ return;
+ }
+
+ /* create a new timer */
+ g_assert(s->stats_timer == NULL);
+ s->stats_timer = qemu_new_timer_ms(vm_clock, balloon_stats_poll_cb, s);
+ s->stats_poll_interval = value;
+ balloon_stats_change_timer(s, 0);
+}
+
static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOBalloon *s = to_virtio_balloon(vdev);
@@ -107,9 +234,10 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
VirtQueueElement *elem = &s->stats_vq_elem;
VirtIOBalloonStat stat;
size_t offset = 0;
+ qemu_timeval tv;
if (!virtqueue_pop(vq, elem)) {
- return;
+ goto out;
}
/* Initialize the stats to get rid of any stale values. This is only
@@ -128,6 +256,18 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
s->stats[tag] = val;
}
s->stats_vq_offset = offset;
+
+ if (qemu_gettimeofday(&tv) < 0) {
+ fprintf(stderr, "warning: %s: failed to get time of day\n", __func__);
+ goto out;
+ }
+
+ s->stats_last_update = tv.tv_sec;
+
+out:
+ if (balloon_stats_enabled(s)) {
+ balloon_stats_change_timer(s, s->stats_poll_interval);
+ }
}
static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
@@ -237,6 +377,14 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
register_savevm(dev, "virtio-balloon", -1, 1,
virtio_balloon_save, virtio_balloon_load, s);
+ object_property_add(OBJECT(dev), "guest-stats", "guest statistics",
+ balloon_stats_get_all, NULL, NULL, s, NULL);
+
+ object_property_add(OBJECT(dev), "guest-stats-polling-interval", "int",
+ balloon_stats_get_poll_interval,
+ balloon_stats_set_poll_interval,
+ NULL, s, NULL);
+
return &s->vdev;
}
@@ -244,6 +392,7 @@ void virtio_balloon_exit(VirtIODevice *vdev)
{
VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
+ balloon_stats_destroy_timer(s);
qemu_remove_balloon_handler(s);
unregister_savevm(s->qdev, "virtio-balloon", s);
virtio_cleanup(vdev);
--
1.8.1.GIT
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats
2013-01-18 19:29 [Qemu-devel] [PATCH v3 0/3] re-enable balloon stats Luiz Capitulino
2013-01-18 19:29 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
2013-01-18 19:29 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino
@ 2013-01-18 19:29 ` Luiz Capitulino
2013-01-18 20:00 ` Eric Blake
2 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2013-01-18 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
create mode 100644 docs/virtio-balloon-stats.txt
diff --git a/docs/virtio-balloon-stats.txt b/docs/virtio-balloon-stats.txt
new file mode 100644
index 0000000..621145e
--- /dev/null
+++ b/docs/virtio-balloon-stats.txt
@@ -0,0 +1,102 @@
+virtio balloon memory statistics
+================================
+
+The virtio balloon driver supports guest memory statistics reporting. These
+statistics are available to QEMU users as QOM (QEMU Object Model) device
+properties via a polling mechanism.
+
+Before querying the available stats, clients first have to enable polling.
+This is done by writing a time interval value (in seconds) to the
+guest-stats-polling-interval property. This value can be:
+
+ > 0 enables polling in the specified interval. If polling is already
+ enabled, the polling time interval is changed to the new value
+
+ 0 disables polling. Previous polled statistics are still valid and
+ can be queried.
+
+Once polling is enabled, the virtio-balloon device in QEMU will start
+polling the guest's balloon driver for new stats in the specified time
+interval.
+
+To retrieve those stats, clients have to query the guest-stats property,
+which will return a dictionary containing:
+
+ o A key named 'stats', containing all avaiable stats. If the guest
+ doesn't support a particular stat, its value will be -1. Currently,
+ the following stats are supported:
+
+ - stat-swap-in
+ - stat-swap-out
+ - stat-major-faults
+ - stat-minor-faults
+ - stat-free-memory
+ - stat-total-memory
+
+ o A key named last-update, which contains the last stats update
+ timestamp in seconds
+
+It's also important to note the following:
+
+ - Previously polled statistics remain available even if the polling is
+ later disabled
+
+ - As noted above, if a guest doesn't support a particular stat it
+ will always be -1. However, it's also possible that a guest couldn't
+ temporarily update one or even all stats. If this happens, just wait
+ for the next update
+
+ - If polling is enabled but the guest doesn't have stats support or
+ the balloon driver is not loaded, an error will be returned when
+ querying stats
+
+ - The polling timer is only re-armed when the guest responds to the
+ statistics request. This means that if a (buggy) guest doesn't ever
+ respond to the request the timer will never be re-armed, which has
+ the same effect as disabling polling
+
+Here are a few examples. The virtio-balloon device is assumed to be in the
+'/machine/peripheral-anon/device[1]' QOM path.
+
+Enable polling with 2 seconds interval:
+
+{ "execute": "qom-set",
+ "arguments": { "path": "/machine/peripheral-anon/device[1]",
+ "property": "guest-stats-polling-interval", "value": 2 } }
+
+{ "return": {} }
+
+Change polling to 10 seconds:
+
+{ "execute": "qom-set",
+ "arguments": { "path": "/machine/peripheral-anon/device[1]",
+ "property": "guest-stats-polling-interval", "value": 10 } }
+
+{ "return": {} }
+
+Get stats:
+
+{ "execute": "qom-get",
+ "arguments": { "path": "/machine/peripheral-anon/device[1]",
+ "property": "guest-stats" } }
+{
+ "return": {
+ "stats": {
+ "stat-swap-out": 0,
+ "stat-free-memory": 844943360,
+ "stat-minor-faults": 219028,
+ "stat-major-faults": 235,
+ "stat-total-memory": 1044406272,
+ "stat-swap-in": 0
+ },
+ "last-update": 1358529861
+ }
+}
+
+Disable polling:
+
+{ "execute": "qom-set",
+ "arguments": { "path": "/machine/peripheral-anon/device[1]",
+ "property": "stats-polling-interval", "value": 0 } }
+
+{ "return": {} }
--
1.8.1.GIT
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats
2013-01-18 19:29 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino
@ 2013-01-18 20:00 ` Eric Blake
2013-01-21 11:56 ` Luiz Capitulino
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2013-01-18 20:00 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, aliguori, qemu-devel, dietmar, agl
[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]
On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 102 insertions(+)
> create mode 100644 docs/virtio-balloon-stats.txt
>
> +
> + o A key named 'stats', containing all avaiable stats. If the guest
s/avaiable/available/
> + doesn't support a particular stat, its value will be -1. Currently,
> + the following stats are supported:
> +
> + - stat-swap-in
> + - stat-swap-out
> + - stat-major-faults
> + - stat-minor-faults
> + - stat-free-memory
> + - stat-total-memory
> +
> + o A key named last-update, which contains the last stats update
> + timestamp in seconds
Is it worth mentioning that this is a timestamp relative to the Unix
epoch? For that matter, does it even matter what the timestamp is
relative to, or just that it increases when a new poll completes? Is it
worth mentioning that the timestamp is computed by the host (that is, a
broken guest can't fake the timestamp, even if it can provide bogus data
for all the stats)?
> +
> + - As noted above, if a guest doesn't support a particular stat it
> + will always be -1. However, it's also possible that a guest couldn't
> + temporarily update one or even all stats. If this happens, just wait
s/couldn't temporarily/temporarily couldn't/
> +
> +Here are a few examples. The virtio-balloon device is assumed to be in the
> +'/machine/peripheral-anon/device[1]' QOM path.
Is this QOM path stable, or can it change depending on target
architecture and/or command-line arguments used to install the guest?
It might be worth showing which command line arguments set up this
particular QOM path.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API
2013-01-18 19:29 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
@ 2013-01-18 20:01 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2013-01-18 20:01 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, aliguori, qemu-devel, dietmar, agl
[-- Attachment #1: Type: text/plain, Size: 885 bytes --]
On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> Next commit will re-enable balloon stats with a different interface, but
> this old code conflicts with it. Let's drop it.
>
> It's important to note that the QMP and HMP interfaces are also dropped
> by this commit. That shouldn't be a problem though, because:
>
> 1. All QMP fields are optional
> 2. This feature has always been disabled
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> hmp.c | 24 +-----------------------
> hw/virtio-balloon.c | 24 ------------------------
> qapi-schema.json | 20 +-------------------
> qmp-commands.hx | 13 -------------
> 4 files changed, 2 insertions(+), 79 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats
2013-01-18 19:29 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino
@ 2013-01-18 20:04 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2013-01-18 20:04 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, aliguori, qemu-devel, dietmar, agl
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]
On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> The statistics are now available through device properties via a
> polling mechanism. First a client has to enable polling, then it
> can query available stats.
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats
2013-01-18 20:00 ` Eric Blake
@ 2013-01-21 11:56 ` Luiz Capitulino
0 siblings, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2013-01-21 11:56 UTC (permalink / raw)
To: Eric Blake; +Cc: mdroth, aliguori, qemu-devel, dietmar, agl
On Fri, 18 Jan 2013 13:00:28 -0700
Eric Blake <eblake@redhat.com> wrote:
> On 01/18/2013 12:29 PM, Luiz Capitulino wrote:
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > docs/virtio-balloon-stats.txt | 102 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 102 insertions(+)
> > create mode 100644 docs/virtio-balloon-stats.txt
> >
>
> > +
> > + o A key named 'stats', containing all avaiable stats. If the guest
>
> s/avaiable/available/
OK.
> > + doesn't support a particular stat, its value will be -1. Currently,
> > + the following stats are supported:
> > +
> > + - stat-swap-in
> > + - stat-swap-out
> > + - stat-major-faults
> > + - stat-minor-faults
> > + - stat-free-memory
> > + - stat-total-memory
> > +
> > + o A key named last-update, which contains the last stats update
> > + timestamp in seconds
>
> Is it worth mentioning that this is a timestamp relative to the Unix
> epoch? For that matter, does it even matter what the timestamp is
> relative to, or just that it increases when a new poll completes?
Yes, I think this field is only important to calculate the delta between
updates.
> Is it
> worth mentioning that the timestamp is computed by the host (that is, a
> broken guest can't fake the timestamp, even if it can provide bogus data
> for all the stats)?
I can mention that.
> > +
> > + - As noted above, if a guest doesn't support a particular stat it
> > + will always be -1. However, it's also possible that a guest couldn't
> > + temporarily update one or even all stats. If this happens, just wait
>
> s/couldn't temporarily/temporarily couldn't/
OK.
> > +
> > +Here are a few examples. The virtio-balloon device is assumed to be in the
> > +'/machine/peripheral-anon/device[1]' QOM path.
>
> Is this QOM path stable, or can it change depending on target
> architecture and/or command-line arguments used to install the guest?
I think it can change.
> It might be worth showing which command line arguments set up this
> particular QOM path.
Will do.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2013-01-21 11:56 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-18 19:29 [Qemu-devel] [PATCH v3 0/3] re-enable balloon stats Luiz Capitulino
2013-01-18 19:29 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
2013-01-18 20:01 ` Eric Blake
2013-01-18 19:29 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino
2013-01-18 20:04 ` Eric Blake
2013-01-18 19:29 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino
2013-01-18 20:00 ` Eric Blake
2013-01-21 11:56 ` Luiz Capitulino
-- strict thread matches above, loose matches on Subject: below --
2012-12-14 15:49 [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Luiz Capitulino
2012-12-14 15:49 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino
2012-12-17 10:13 ` Dietmar Maurer
2012-12-17 11:57 ` Luiz Capitulino
2012-12-17 12:23 ` Dietmar Maurer
2012-12-17 12:33 ` Luiz Capitulino
2012-12-17 12:36 ` Dietmar Maurer
2012-12-17 12:39 ` Luiz Capitulino
2012-12-18 21:34 ` Eric Blake
2012-12-19 5:18 ` Dietmar Maurer
2012-12-19 11:27 ` Luiz Capitulino
2012-12-20 18:36 ` Eric Blake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).