On Wed, Mar 16, 2016 at 01:33:31PM +0300, Denis V. Lunev wrote: > On 03/04/2016 10:49 AM, Michael S. Tsirkin wrote: > >From: Igor Redko <red...@virtuozzo.com> > > > >We are making experiments with different autoballooning strategies > >based on the guest behavior. Thus we need to experiment with different > >guest statistics. For now every counter change requires QEMU recompilation > >and dances with Libvirt. > > > >This patch introduces transport for unrecognized counters in virtio-balloon. > >This transport can be used for measuring benefits from using new > >balloon counters, before submitting any patches. Current alternative > >is 'guest-exec' transport which isn't made for such delicate matters > >and can influence test results. > > > >Originally all counters with tag >= VIRTIO_BALLOON_S_NR were ignored. > >Instead of this we keep first (VIRTIO_BALLOON_S_NR + 32) counters from the > >queue and pass unrecognized ones with the following names: 'x-stat-XXXX', > >where XXXX is a tag number in hex. Defined counters are reported with their > >regular names. > > > >Signed-off-by: Igor Redko <red...@virtuozzo.com> > >Signed-off-by: Denis V. Lunev <d...@openvz.org> > >CC: Michael S. Tsirkin <m...@redhat.com> > >Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > >Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > >--- > > configure | 12 ++++++++++++ > > include/hw/virtio/virtio-balloon.h | 3 ++- > > hw/virtio/virtio-balloon.c | 32 ++++++++++++++++++++++++++------ > > 3 files changed, 40 insertions(+), 7 deletions(-) > > > >diff --git a/configure b/configure > >index 0c0472a..767d96e 100755 > >--- a/configure > >+++ b/configure > >@@ -315,6 +315,7 @@ vhdx="" > > numa="" > > tcmalloc="no" > > jemalloc="no" > >+unknown_balloon_stats="no" > > # parse CC options first > > for opt do > >@@ -1142,6 +1143,10 @@ for opt do > > ;; > > --enable-jemalloc) jemalloc="yes" > > ;; > >+ --enable-unknown-balloon-stats) unknown_balloon_stats="yes" > >+ ;; > >+ --disable-unknown-balloon-stats) unknown_balloon_stats="no" > >+ ;; > > *) > > echo "ERROR: unknown option $opt" > > echo "Try '$0 --help' for more information" > >@@ -1364,6 +1369,8 @@ disabled with --disable-FEATURE, default is enabled if > >available: > > numa libnuma support > > tcmalloc tcmalloc support > > jemalloc jemalloc support > >+ unknown-balloon-stats report unknown balloon statistics counters > >+ ;; > > NOTE: The object files are built at the place where configure is launched > > EOF > >@@ -4790,6 +4797,7 @@ echo "bzip2 support $bzip2" > > echo "NUMA host support $numa" > > echo "tcmalloc support $tcmalloc" > > echo "jemalloc support $jemalloc" > >+echo "unknown balloon stat counters support $unknown_balloon_stats" > > if test "$sdl_too_old" = "yes"; then > > echo "-> Your SDL version is too old - please upgrade to have SDL support" > >@@ -5342,6 +5350,10 @@ if test "$rdma" = "yes" ; then > > echo "CONFIG_RDMA=y" >> $config_host_mak > > fi > >+if test "$unknown_balloon_stats" = "yes" ; then > >+ echo "CONFIG_UNKNOWN_BALLOON_STATS=y" >> $config_host_mak > >+fi > >+ > > # Hold two types of flag: > > # CONFIG_THREAD_SETNAME_BYTHREAD - we've got a way of setting the name > > on > > # a thread we have a handle to > >diff --git a/include/hw/virtio/virtio-balloon.h > >b/include/hw/virtio/virtio-balloon.h > >index 35f62ac..5c8730e 100644 > >--- a/include/hw/virtio/virtio-balloon.h > >+++ b/include/hw/virtio/virtio-balloon.h > >@@ -36,7 +36,8 @@ typedef struct VirtIOBalloon { > > VirtQueue *ivq, *dvq, *svq; > > uint32_t num_pages; > > uint32_t actual; > >- uint64_t stats[VIRTIO_BALLOON_S_NR]; > >+ VirtIOBalloonStatModern stats[VIRTIO_BALLOON_S_NR + 32]; > >+ uint16_t stats_cnt; > > VirtQueueElement *stats_vq_elem; > > size_t stats_vq_offset; > > QEMUTimer *stats_timer; > >diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > >index e97d403..64367ac 100644 > >--- a/hw/virtio/virtio-balloon.c > >+++ b/hw/virtio/virtio-balloon.c > >@@ -66,8 +66,7 @@ static const char *balloon_stat_names[] = { > > */ > > static inline void reset_stats(VirtIOBalloon *dev) > > { > >- int i; > >- for (i = 0; i < VIRTIO_BALLOON_S_NR; dev->stats[i++] = -1); > >+ dev->stats_cnt = 0; > > } > > static bool balloon_stats_supported(const VirtIOBalloon *s) > >@@ -133,12 +132,22 @@ static void balloon_stats_get_all(Object *obj, Visitor > >*v, const char *name, > > if (err) { > > goto out_end; > > } > >- for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) { > >- visit_type_uint64(v, balloon_stat_names[i], &s->stats[i], &err); > >+ for (i = 0; i < s->stats_cnt; i++) { > >+ if (s->stats[i].tag < VIRTIO_BALLOON_S_NR) { > >+ visit_type_uint64(v, balloon_stat_names[s->stats[i].tag], > >+ &s->stats[i].val, &err); > >+ } else { > >+#if defined(CONFIG_UNKNOWN_BALLOON_STATS) > >+ gchar *str = g_strdup_printf("x-stat-%04x", s->stats[i].tag); > >+ visit_type_uint64(v, str, &s->stats[i].val, &err); > >+ g_free(str); > >+#endif > >+ } > > if (err) { > > break; > > } > > } > >+ > > error_propagate(errp, err); > > err = NULL; > > visit_end_struct(v, &err); > >@@ -282,10 +291,21 @@ static void virtio_balloon_receive_stats(VirtIODevice > >*vdev, VirtQueue *vq) > > == sizeof(stat)) { > > uint16_t tag = virtio_tswap16(vdev, stat.tag); > > uint64_t val = virtio_tswap64(vdev, stat.val); > >+ int i; > > offset += sizeof(stat); > >- if (tag < VIRTIO_BALLOON_S_NR) > >- s->stats[tag] = val; > >+ for (i = 0; i < s->stats_cnt; i++) { > >+ if (s->stats[i].tag == tag) { > >+ break; > >+ } > >+ } > >+ if (i < ARRAY_SIZE(s->stats)) { > >+ s->stats[i].tag = tag; > >+ s->stats[i].val = val; > >+ if (s->stats_cnt <= i) { > >+ s->stats_cnt = i + 1; > >+ } > >+ } > > } > > s->stats_vq_offset = offset; > Michael, > > what has happened with this patch? > > I have seen pull request and this patch was coupled with > 'available' modification. 'available' code was resent 11.03 > and this one not. > > Den
Yes - I included it by mistake, and dropped in v2. Let's discuss after 2.6. -- MST