From: Eric Blake <eblake@redhat.com>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: aliguori@us.ibm.com, mdroth@linux.vnet.ibm.com,
"libvir-list@redhat.com" <libvir-list@redhat.com>,
armbru@redhat.com, qemu-devel@nongnu.org, agl@us.ibm.com,
amit.shah@redhat.com
Subject: Re: [Qemu-devel] [RFC 0/5]: QMP: add balloon-get-memory-stats command
Date: Thu, 19 Jan 2012 10:15:55 -0700 [thread overview]
Message-ID: <4F184FCB.2010505@redhat.com> (raw)
In-Reply-To: <1326988591-27241-1-git-send-email-lcapitulino@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3424 bytes --]
On 01/19/2012 08:56 AM, 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).
[I went ahead and cc'd the libvirt list]
Yes, libvirt can live with this approach. And having this in parallel
to a qemu-ga verb is nice, since, as it was pointed out, this would
allow interaction with guests that have a balloon device but not a guest
agent.
You may want to read this thread [1], for thoughts on the impact of
making another existing blocking command be extended into one that
starts an async event and ends when an event is raised; libvirt can
expose both a blocking and an asynchronous implementation to the user on
top of the qemu model being just asynchronous.
[1] https://www.redhat.com/archives/libvir-list/2012-January/msg00562.html
Thinking aloud - do we need a means to poll the state of the
balloon-stat query? On the one hand, if libvirtd issues the start
command, then gets stopped, then the event occurs, then libvirtd is
restarted, then libvirt won't know that the event was missed. On the
other hand, since this involves guest interaction, libvirt already has
to assume that the guest may be malicious and refuse to report stats
and/or report invalid stats, so libvirt would already have to be
prepared to give up if no event has arrived in a fixed amount of time,
and that also means that restarting libvirtd can just ignore any balloon
query that was in flight before the restart.
So I guess I'm okay with just a start and an event, with no poll of the
last-known guest response. But it does mean that qemu has to gracefully
handle if libvirt makes two start requests in a row without any
intervening events, and conversely that libvirt has to be prepared for
an event that happens even when libvirt doesn't remember triggering the
start command.
> 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...
Most likely, we would want to teach libvirt to use both methods, and
give the choice to the user on which approach to use when the guest
supports both.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
next prev parent reply other threads:[~2012-01-19 17:16 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-19 15:56 [Qemu-devel] [RFC 0/5]: QMP: add balloon-get-memory-stats command Luiz Capitulino
2012-01-19 15:56 ` [Qemu-devel] [PATCH 1/5] balloon: qmp_balloon(): Use error_set() Luiz Capitulino
2012-01-19 15:56 ` [Qemu-devel] [PATCH 2/5] balloon: Drop unused include Luiz Capitulino
2012-01-19 15:56 ` [Qemu-devel] [PATCH 3/5] balloon: Drop old stats interface Luiz Capitulino
2012-01-19 15:56 ` [Qemu-devel] [PATCH 4/5] balloon: Rename QEMUBalloonStatus to QEMUBalloonInfo Luiz Capitulino
2012-01-19 15:56 ` [Qemu-devel] [PATCH 5/5] qmp: add balloon-get-memory-stats & event Luiz Capitulino
2012-01-19 16:43 ` [Qemu-devel] [RFC 0/5]: QMP: add balloon-get-memory-stats command Michael Roth
2012-01-19 16:58 ` Luiz Capitulino
2012-01-19 17:15 ` Eric Blake [this message]
2012-01-20 12:02 ` Luiz Capitulino
2012-01-19 21:50 ` Adam Litke
2012-01-20 12:06 ` Luiz Capitulino
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F184FCB.2010505@redhat.com \
--to=eblake@redhat.com \
--cc=agl@us.ibm.com \
--cc=aliguori@us.ibm.com \
--cc=amit.shah@redhat.com \
--cc=armbru@redhat.com \
--cc=lcapitulino@redhat.com \
--cc=libvir-list@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).