Re: [Qemu-devel] [PATCH 0/3] re-enable balloon stats

[Dietmar Maurer]

> > > > Why don't we raise the error when we query the values?

You already raise an error here, so that works out of the box:

    if (s->stats[i] == -1) {
        error_setg(errp,
            "timer hasn't been enabled or guest doesn't support '%s'", name);

> > >
> > > Hmm, that's a good idea.
> > >
> > > Only small nit is that, today old stats will remain available to be
> > > queried even if you disable the timer. If we do what you suggest,
> > > old stats won't be available if the module is unloaded (well, I
> > > *guess* the guest will unset the feature bit on module removal).
> >
> > What is the problem with that? It would be no problem to simulate the
> > old behavior, but why do you want that?
> 
> I actually just thought that it's a nice to have.

The following patch seems to work for me:

Index: new/hw/virtio-balloon.c
===================================================================
--- new.orig/hw/virtio-balloon.c        2012-12-12 14:05:56.000000000 +0100
+++ new/hw/virtio-balloon.c     2012-12-12 14:07:43.000000000 +0100
@@ -111,6 +111,10 @@
 {
     VirtIOBalloon *s = opaque;
 
+    if (!balloon_stats_supported(s) || !runstate_is_running()) {
+        return;
+    }
+
     virtqueue_push(s->svq, &s->stats_vq_elem, s->stats_vq_offset);
     virtio_notify(&s->vdev, s->svq);
 }
@@ -164,11 +168,6 @@
     VirtIOBalloon *s = opaque;
     int64_t value;
 
-    if (!balloon_stats_supported(s)) {
-        error_setg(errp, "guest doesn\'t support balloon stats");
-        return;
-    }
-
     visit_type_int(v, &value, name, errp);
     if (error_is_set(errp)) {
         return;