[0/3] re-enable balloon stats

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

On Wed, 12 Dec 2012 13:17:15 +0000
Dietmar Maurer <dietmar@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;
> 
> 
>

> > The following patch seems to work for me:
> 
> Yeah, that's what I'll do and the doc has to be updated too.

Btw, do you know a qmp call which returns the memory assigned to the VM
(the value qemu is started with (-m megs))?

On Wed, 12 Dec 2012 13:36:20 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:

> > > The following patch seems to work for me:
> > 
> > Yeah, that's what I'll do and the doc has to be updated too.
> 
> Btw, do you know a qmp call which returns the memory assigned to the VM
> (the value qemu is started with (-m megs))?

No, we only have info/query- balloon. I was also thinking about this the
other day.

> > Btw, do you know a qmp call which returns the memory assigned to the
> > VM (the value qemu is started with (-m megs))?
> 
> No, we only have info/query- balloon. I was also thinking about this the
> other day.

Can we add an additional property: "stat-max-memory"?

Or add 'max_mem' property to BalloonInfo (return with query-ballon)?

On Wed, 12 Dec 2012 13:45:54 +0000
Dietmar Maurer <dietmar@proxmox.com> wrote:

> > > Btw, do you know a qmp call which returns the memory assigned to the
> > > VM (the value qemu is started with (-m megs))?
> > 
> > No, we only have info/query- balloon. I was also thinking about this the
> > other day.
> 
> Can we add an additional property: "stat-max-memory"?
> 
> Or add 'max_mem' property to BalloonInfo (return with query-ballon)?

Yes, I prefer the latter. But let's have a more descriptive name, like
aximum-balloon-memory (or something like that).