diff mbox

[PULL,08/16] virtio-balloon: export all balloon statistics

Message ID 1457077710-17678-9-git-send-email-mst@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin March 4, 2016, 7:49 a.m. UTC
From: Igor Redko <redkoi@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 <redkoi@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 configure                          | 12 ++++++++++++
 include/hw/virtio/virtio-balloon.h |  3 ++-
 hw/virtio/virtio-balloon.c         | 32 ++++++++++++++++++++++++++------
 3 files changed, 40 insertions(+), 7 deletions(-)

Comments

Denis V. Lunev March 16, 2016, 10:33 a.m. UTC | #1
On 03/04/2016 10:49 AM, Michael S. Tsirkin wrote:
> From: Igor Redko <redkoi@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 <redkoi@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@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
Michael S. Tsirkin March 16, 2016, 10:37 a.m. UTC | #2
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 <redkoi@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 <redkoi@virtuozzo.com>
> >Signed-off-by: Denis V. Lunev <den@openvz.org>
> >CC: Michael S. Tsirkin <mst@redhat.com>
> >Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >Signed-off-by: Michael S. Tsirkin <mst@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.
Denis V. Lunev May 14, 2016, 10:49 a.m. UTC | #3
On 03/16/2016 01:37 PM, Michael S. Tsirkin wrote:
> 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 <redkoi@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 <redkoi@virtuozzo.com>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: Michael S. Tsirkin <mst@redhat.com>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@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.
>
how could we start the discussion?
diff mbox

Patch

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;