Re: [Qemu-devel] [RFC 0/5]: QMP: add balloon-get-memory-stats command

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 19 Jan 2012 15:50:06 -0600
Adam Litke <agl@us.ibm.com> wrote:

> On Thu, Jan 19, 2012 at 01:56:26PM -0200, Luiz Capitulino wrote:
> > Long ago, commit 625a5be added the guest provided memory statistics to
> > the query-balloon command. Unfortunately, it also introduced a severe
> > bug: query-balloon would hang if the guest didn't respond. This, in turn,
> > would also cause a hang in libvirt.
> > 
> > Because of that, we decided to disable the guest memory stats feature
> > (commit 11724ff).
> > 
> > As we decided to let commands implement ad-hoc async mechanisms until we
> > get a proper way to do it, I decided to try to re-enable that feature.
> > 
> > My idea is to have a command and an event. The command gets the process
> > started by sending a request to guest and returns. Later, when the guest
> > makes the memory stats info available, it's sent to the client by means
> > of an QMP event (please, take a look at patch 05/05 for full details).
> > 
> > I'm not sure if that approach is good for libvirt though, so it would be
> > very helpful to get their input (Eric, I'm CC'ing you here, but feel free
> > to route this to someone else).
> > 
> > Another interesting point is that, there's another way of doing this and
> > it's using qemu-ga instead. That's, qemu-ga could read that information
> > from proc and return it. This is easier & simpler, as it doesn't involve
> > guest communication. We also could return a lot more information if needed.
> > The only disadvantage I can see is the dependency on qemu-ga...
> > 
> >  QMP/qmp-events.txt  |   28 ++++++++++++++++++++++++++++
> >  balloon.c           |   47 +++++++++++++++++++++++++++++++++++++----------
> >  balloon.h           |    7 ++++---
> >  hmp.c               |   25 +------------------------
> >  hw/virtio-balloon.c |   39 +++++++++++++++++++++++++++------------
> >  monitor.c           |    3 +++
> >  monitor.h           |    1 +
> >  qapi-schema.json    |   42 ++++++++++++++++++++++--------------------
> >  qmp-commands.hx     |    6 ++++++
> >  9 files changed, 129 insertions(+), 69 deletions(-)
> > 
> 
> The patch series looks good.  Thanks for making the improvements.  Once it is
> upstream I can help out with the libvirt pieces.  I tested it with a Fedora-15
> VM and it worked out of the box :)  I just have one small nit.  Due to the way
> that the virtio stats queue works, the guest emits a stats event with bogus
> values when the balloon driver initializes (which gives the host control of the
> channel).  Your patches emit this initial event (which contains undefined
> values).  In my old code, we kept a boolean flag in the ballon device to record
> if stats have been requested and only if that was set would we raise the event.
> Without this, the guest can spam the host with an unlimited number of bogus
> events.

Yeah, I've seen this too. I'll fix it. We also need migration and HMP
support. Although the latter seems difficult as we don't have a way for
qemu subsystems to wait for events yet.

> Tested-by: Adam Litke <agl@us.ibm.com>

Thanks!

>