* [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats @ 2012-12-14 15:49 Luiz Capitulino 2012-12-14 15:49 ` [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API Luiz Capitulino ` (3 more replies) 0 siblings, 4 replies; 23+ messages in thread From: Luiz Capitulino @ 2012-12-14 15:49 UTC (permalink / raw) To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl This new try to re-enable the virtio-balloon driver stats 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 v2 - drop qmp & hmp interface of old stats code - allow polling callback to run even if the guest hasn't negotiated stats feature bit - 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 | 91 +++++++++++++++++++ hmp.c | 24 +---- hw/virtio-balloon.c | 200 +++++++++++++++++++++++++++++++++++++----- qapi-schema.json | 20 +---- qmp-commands.hx | 13 --- 5 files changed, 269 insertions(+), 79 deletions(-) create mode 100644 docs/virtio-balloon-stats.txt -- 1.8.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [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 2012-12-14 15:49 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino ` (2 subsequent siblings) 3 siblings, 1 reply; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ messages in thread
* [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats 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-14 15:49 ` Luiz Capitulino 2012-12-14 15:49 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino 2012-12-15 7:19 ` [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Dietmar Maurer 3 siblings, 0 replies; 23+ messages in thread From: Luiz Capitulino @ 2012-12-14 15:49 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 each stat individually. The following control properties are introduced: o stats-polling-interval: a value greater than zero enables polling in the specified interval (in seconds). When value equals zero, polling is disabled. If polling is already enabled and a value greater than zero is written, the polling interval time is changed o stats-last-update: last stats update timestamp, in seconds. The following stats properties are introduced, all values are in bytes: o stat-swap-in o stat-swap-out o stat-major-faults o stat-minor-faults o stat-free-memory o stat-total-memory Please, refer to the documentation introduced by the next commit for more information and examples. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- hw/virtio-balloon.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 176 insertions(+), 2 deletions(-) diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index 4398025..47e35b1 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -22,6 +22,8 @@ #include "virtio-balloon.h" #include "kvm.h" #include "exec-memory.h" +#include "qemu-timer.h" +#include "qapi/qapi-visit-core.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,138 @@ 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_last_update(Object *obj, struct Visitor *v, + void *opaque, const char *name, + Error **errp) +{ + VirtIOBalloon *s = opaque; + visit_type_int(v, &s->stats_last_update, name, errp); +} + +static void balloon_stats_get_stat(Object *obj, struct Visitor *v, + void *opaque, const char *name, Error **errp) +{ + VirtIOBalloon *s = opaque; + int i; + + for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) { + if (!strcmp(balloon_stat_names[i], name)) { + break; + } + } + + if (i == VIRTIO_BALLOON_S_NR) { + error_setg(errp, "invalid stat name '%s'", name); + return; + } + + if (s->stats[i] == -1) { + /* + * Possible reasons for this error: + * + * - The timer hasn't been enabled + * - The guest hasn't loaded its balloon driver + * - The guest's balloon driver doesn't support memory stats + * - The guest's balloon driver doesn't support this stat + * - The guest's balloon driver didn't send this stat for + * whatever reason + */ + error_setg(errp, + "guest didn't update '%s' (does the guest support it?)", name); + return; + } + + visit_type_int(v, (int64_t *) &s->stats[i], name, 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 positive"); + 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 +254,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 +276,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) @@ -212,7 +372,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id) VirtIODevice *virtio_balloon_init(DeviceState *dev) { VirtIOBalloon *s; - int ret; + int i, ret; s = (VirtIOBalloon *)virtio_common_init("virtio-balloon", VIRTIO_ID_BALLOON, @@ -239,6 +399,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev) register_savevm(dev, "virtio-balloon", -1, 1, virtio_balloon_save, virtio_balloon_load, s); + for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) { + object_property_add(OBJECT(dev), balloon_stat_names[i], "int", + balloon_stats_get_stat, NULL, NULL, s, NULL); + } + + object_property_add(OBJECT(dev), "stats-last-update", "int", + balloon_stats_get_last_update, NULL, NULL, s, NULL); + + object_property_add(OBJECT(dev), "stats-polling-interval", "int", + balloon_stats_get_poll_interval, + balloon_stats_set_poll_interval, + NULL, s, NULL); + return &s->vdev; } @@ -246,6 +419,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.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats 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-14 15:49 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino @ 2012-12-14 15:49 ` Luiz Capitulino 2012-12-15 7:19 ` [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Dietmar Maurer 3 siblings, 0 replies; 23+ messages in thread From: Luiz Capitulino @ 2012-12-14 15:49 UTC (permalink / raw) To: qemu-devel; +Cc: aliguori, mdroth, dietmar, agl Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- docs/virtio-balloon-stats.txt | 91 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 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..990e746 --- /dev/null +++ b/docs/virtio-balloon-stats.txt @@ -0,0 +1,91 @@ +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. + +Basically, clients first have to enable polling, then they can query the +available statistics. + +There are two control properties and six memory statistics properties. + +The control properties are: + + o stats-polling-interval: polling time interval in seconds, it can be: + + > 0 enables polling in the specified interval. If polling is already + enabled, the polling time interval will be changed to the new value + + 0 disables polling. Previous polled statistics are still valid and + can be queried. + + o stats-last-update: last stats update timestamp, in seconds + +The following statistics are available, all values are in bytes: + + o stat-swap-in + o stat-swap-out + o stat-major-faults + o stat-minor-faults + o stat-free-memory + o stat-total-memory + +Also, please note the following: + + - If a statistic is queried before the timer is enabled or if the guest + doesn't support a particular statistic, an error will be returned + + - Previously polled statistics remain available even if the timer is + later disabled + + - Polling can be enabled even if the guest doesn't support memory + statistics or its balloon driver hasn't been loaded. Applications + can check this condition by checking that stats-last-update doesn't + change + + - 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": "stats-polling-interval", "value": 2 } } + +{ "return": {} } + +Change polling to 10 seconds: + +{ "execute": "qom-set", + "arguments": { "path": "/machine/peripheral-anon/device[1]", + "property": "stats-polling-interval", "value": 10 } } + +{ "return": {} } + +Get last update timestamp and free memory stat: + +{ "execute": "qom-get", + "arguments": { "path": "/machine/peripheral-anon/device[1]", + "property": "stats-last-update" } } + +{ "return": 1354629634 } + +{ "execute": "qom-get", + "arguments": { "path": "/machine/peripheral-anon/device[1]", + "property": "stat-free-memory" } } + +{ "return": 845115392 } + +Disable polling: + +{ "execute": "qom-set", + "arguments": { "path": "/machine/peripheral-anon/device[1]", + "property": "stats-polling-interval", "value": 0 } } + +{ "return": {} } -- 1.8.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-14 15:49 [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Luiz Capitulino ` (2 preceding siblings ...) 2012-12-14 15:49 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino @ 2012-12-15 7:19 ` Dietmar Maurer 2012-12-17 11:52 ` Luiz Capitulino 3 siblings, 1 reply; 23+ messages in thread From: Dietmar Maurer @ 2012-12-15 7:19 UTC (permalink / raw) To: Luiz Capitulino, qemu-devel@nongnu.org Cc: aliguori@us.ibm.com, mdroth@linux.vnet.ibm.com, agl@us.ibm.com > - drop qmp & hmp interface of old stats code I think the old interface is not that bad (the new one is clumsy, because we need 6 qmp call instead of one to get all stats). Can't we try to make it functional and keep it? Seriously, I think 'interval' should be a property we can pass at qemu startup. Then we can simply use the old interface to get cached values. I also do not understand you argument that you can't use the old interface because it was synchrounous - removing the old interface is also not compatible ;-) ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-15 7:19 ` [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Dietmar Maurer @ 2012-12-17 11:52 ` Luiz Capitulino 2012-12-17 12:17 ` Dietmar Maurer 0 siblings, 1 reply; 23+ messages in thread From: Luiz Capitulino @ 2012-12-17 11:52 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 Sat, 15 Dec 2012 07:19:13 +0000 Dietmar Maurer <dietmar@proxmox.com> wrote: > > - drop qmp & hmp interface of old stats code > > I think the old interface is not that bad (the new one is clumsy, because > we need 6 qmp call instead of one to get all stats). Can't > we try to make it functional and keep it? No, because it breaks existing clients and that's why it's disabled since qemu 0.12 or so. Let me try once more to explain our options and why I think this is series is our best choice. Basically, we have try options: 1. Add stats to query-balloon This breaks existing clients, because query-balloon is a synchronous command that returns immediately. If we add stats to it (as we did) then it will have to wait for the guest to respond, which can take a long time or even doesn't happen at all if the guest is paused. This broke libvirt, as explained here: https://bugzilla.redhat.com/show_bug.cgi?id=623903 2. Add a new command that just request the stats to be sent (ie. it doesn't block), and then get the stats through an event This was my last proposal: http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg00983.html This sort of worked, but it has two problems. First, events can be lost. Second, most failures can't be synchronously reported. 3. Add stats through device properties (this series) While this isn't perfect, it does solve the issues with previous implementations. Besides, it has the advantage of avoiding adding new commands and it integrates nicely with the balloon driver, making some errors automatically reported (eg. if the balloon is not enabled). > Seriously, I think 'interval' should be a property we can pass at qemu > startup. Then we can simply use the old interface to get cached values. What if you want to change the interval or even set it case it wasn't set? You'll need a new command. Any different behavior we want to add will require a new command. Which is one of your own complaints against this series. > I also do not understand you argument that you can't use the old > interface because it was synchrounous - Explained above. > removing the old interface is also > not compatible ;-) It's not, for two reasons. First, all fields are optional. Second, this was never in a stable release (or was disabled right after the release). ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-17 11:52 ` Luiz Capitulino @ 2012-12-17 12:17 ` Dietmar Maurer 2012-12-17 12:22 ` Luiz Capitulino 0 siblings, 1 reply; 23+ messages in thread From: Dietmar Maurer @ 2012-12-17 12:17 UTC (permalink / raw) To: Luiz Capitulino Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, agl@us.ibm.com > Let me try once more to explain our options and why I think this is series is > our best choice. > > Basically, we have try options: > > 1. Add stats to query-balloon > > This breaks existing clients, because query-balloon is a synchronous > command that returns immediately. If we add stats to it (as we did) then it > will have to wait for the guest to respond, which can take a long time or even > doesn't happen at all if the guest is paused. No, there is no need to wait (see my patch). > This broke libvirt, as explained here: > > https://bugzilla.redhat.com/show_bug.cgi?id=623903 My new patch does not introduce such bug. > 2. Add a new command that just request the stats to be sent (ie. it doesn't > block), and then get the stats through an event > > This was my last proposal: > > http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg00983.html > > This sort of worked, but it has two problems. First, events can be lost. > Second, most failures can't be synchronously reported. > > 3. Add stats through device properties (this series) > > While this isn't perfect, it does solve the issues with previous > implementations. Besides, it has the advantage of avoiding adding new > commands and it integrates nicely with the balloon driver, making some > errors automatically reported (eg. if the balloon is not enabled). We can have both - see my new patch. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-17 12:17 ` Dietmar Maurer @ 2012-12-17 12:22 ` Luiz Capitulino 2012-12-17 12:26 ` Dietmar Maurer 0 siblings, 1 reply; 23+ messages in thread From: Luiz Capitulino @ 2012-12-17 12:22 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:17:28 +0000 Dietmar Maurer <dietmar@proxmox.com> wrote: > > Let me try once more to explain our options and why I think this is series is > > our best choice. > > > > Basically, we have try options: > > > > 1. Add stats to query-balloon > > > > This breaks existing clients, because query-balloon is a synchronous > > command that returns immediately. If we add stats to it (as we did) then it > > will have to wait for the guest to respond, which can take a long time or even > > doesn't happen at all if the guest is paused. > > No, there is no need to wait (see my patch). It has problems, replied to it. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-17 12:22 ` Luiz Capitulino @ 2012-12-17 12:26 ` Dietmar Maurer 2012-12-17 12:38 ` Luiz Capitulino 0 siblings, 1 reply; 23+ messages in thread From: Dietmar Maurer @ 2012-12-17 12:26 UTC (permalink / raw) To: Luiz Capitulino Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, agl@us.ibm.com > > No, there is no need to wait (see my patch). > > It has problems, replied to it. Please can you post a test case which show those problems? I simply can't reproduce any problem you mention. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-17 12:26 ` Dietmar Maurer @ 2012-12-17 12:38 ` Luiz Capitulino 2012-12-17 12:39 ` Dietmar Maurer 0 siblings, 1 reply; 23+ messages in thread From: Luiz Capitulino @ 2012-12-17 12:38 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:26:34 +0000 Dietmar Maurer <dietmar@proxmox.com> wrote: > > > No, there is no need to wait (see my patch). > > > > It has problems, replied to it. > > Please can you post a test case which show those problems? I simply can't > reproduce any problem you mention. Which problems are you referring to? Also, I think it's time for you to search for answers and make some effort at understanding my arguments. Since the beginning of this thread I've tried to respond you and provide you all information I have, but all I heard from you is "show me", "I don't see it" and "I want it implemented my way, period". That's by far not productive. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats 2012-12-17 12:38 ` Luiz Capitulino @ 2012-12-17 12:39 ` Dietmar Maurer 0 siblings, 0 replies; 23+ messages in thread From: Dietmar Maurer @ 2012-12-17 12:39 UTC (permalink / raw) To: Luiz Capitulino Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, agl@us.ibm.com > Since the beginning of this thread I've tried to respond you and provide you > all information I have, but all I heard from you is "show me", "I don't see it" > and "I want it implemented my way, period". > > That's by far not productive. I event sent you a working patch! Anyways - I also give up now. ^ permalink raw reply [flat|nested] 23+ 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 0 siblings, 1 reply; 23+ 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] 23+ messages in thread
* [Qemu-devel] [PATCH 1/3] balloon: drop old stats code & API 2013-01-18 19:29 [Qemu-devel] [PATCH v3 " Luiz Capitulino @ 2013-01-18 19:29 ` Luiz Capitulino 2013-01-18 20:01 ` Eric Blake 0 siblings, 1 reply; 23+ 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] 23+ 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; 23+ 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] 23+ messages in thread
end of thread, other threads:[~2013-01-18 20:53 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2012-12-14 15:49 ` [Qemu-devel] [PATCH 2/3] balloon: re-enable balloon stats Luiz Capitulino 2012-12-14 15:49 ` [Qemu-devel] [PATCH 3/3] docs: document virtio-balloon stats Luiz Capitulino 2012-12-15 7:19 ` [Qemu-devel] [PATCH v2 0/3] re-enable balloon stats Dietmar Maurer 2012-12-17 11:52 ` Luiz Capitulino 2012-12-17 12:17 ` Dietmar Maurer 2012-12-17 12:22 ` Luiz Capitulino 2012-12-17 12:26 ` Dietmar Maurer 2012-12-17 12:38 ` Luiz Capitulino 2012-12-17 12:39 ` Dietmar Maurer -- strict thread matches above, loose matches on Subject: below -- 2013-01-18 19:29 [Qemu-devel] [PATCH v3 " 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
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).