From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43745) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnzsK-0000ob-Gl for qemu-devel@nongnu.org; Thu, 19 Jan 2012 16:50:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RnzsI-00031d-O0 for qemu-devel@nongnu.org; Thu, 19 Jan 2012 16:50:28 -0500 Received: from e38.co.us.ibm.com ([32.97.110.159]:56900) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnzsI-00031I-Ia for qemu-devel@nongnu.org; Thu, 19 Jan 2012 16:50:26 -0500 Received: from /spool/local by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 19 Jan 2012 14:50:23 -0700 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 2D09EC900D5 for ; Thu, 19 Jan 2012 16:50:11 -0500 (EST) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q0JLoAgn310636 for ; Thu, 19 Jan 2012 16:50:10 -0500 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q0JLo9VY012286 for ; Thu, 19 Jan 2012 16:50:10 -0500 Date: Thu, 19 Jan 2012 15:50:06 -0600 From: Adam Litke Message-ID: <20120119215006.GC6799@us.ibm.com> References: <1326988591-27241-1-git-send-email-lcapitulino@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1326988591-27241-1-git-send-email-lcapitulino@redhat.com> Subject: Re: [Qemu-devel] [RFC 0/5]: QMP: add balloon-get-memory-stats command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: aliguori@us.ibm.com, mdroth@linux.vnet.ibm.com, armbru@redhat.com, qemu-devel@nongnu.org, amit.shah@redhat.com, eblake@redhat.com 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. Tested-by: Adam Litke -- Adam Litke IBM Linux Technology Center