On Wed, 12 Dec 2012 13:17:15 +0000
Dietmar Maurer <diet...@proxmox.com> wrote:
> > > > > 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:
Yeah, that's what I'll do and the doc has to be updated too.
>
> 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;
>
>
>
