> > > > 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;