Message ID | 24E144B8C0207547AD09C467A8259F75578AC74C@lisa.maurer-it.com |
---|---|
State | New |
Headers | show |
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).
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;