From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50251) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RqYDd-0000L6-W6 for qemu-devel@nongnu.org; Thu, 26 Jan 2012 17:55:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RqYDc-0005zx-JG for qemu-devel@nongnu.org; Thu, 26 Jan 2012 17:55:01 -0500 Received: from mail-pw0-f45.google.com ([209.85.160.45]:59661) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RqYDb-0005zl-NJ for qemu-devel@nongnu.org; Thu, 26 Jan 2012 17:55:00 -0500 Received: by pbdd2 with SMTP id d2so1365927pbd.4 for ; Thu, 26 Jan 2012 14:54:58 -0800 (PST) Message-ID: <4F21D9BE.8030805@codemonkey.ws> Date: Thu, 26 Jan 2012 16:54:54 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <20120126144632.GM21211@redhat.com> <4F216EAB.2000707@redhat.com> In-Reply-To: <4F216EAB.2000707@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: libvir-list@redhat.com, Michal Privoznik , QEMU Developers On 01/26/2012 09:18 AM, Eric Blake wrote: > [adding qemu-devel] > > On 01/26/2012 07:46 AM, Daniel P. Berrange wrote: >>> One thing, that you'll probably notice is this >>> 'set-support-level' command. Basically, it tells GA what qemu version >>> is it running on. Ideally, this should be done as soon as >>> GA starts up. However, that cannot be determined from outside >>> world as GA doesn't emit any events yet. >>> Ideally^2 this command should be left out as it should be qemu >>> who tells its own agent this kind of information. >>> Anyway, I was going to call this command in qemuProcess{Startup, >>> Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs >>> so guest can boot and start GA, but that implies returning from qemuProcess*. >>> >>> So I am setting this just before 'guest-suspend' command, as >>> there is one more thing about GA. It is unable to remember anything >>> upon its restart (GA process). Which has BTW show flaw >>> in our current code with FS freeze& thaw. If we freeze guest >>> FS, and somebody restart GA, the simple FS Thaw will not succeed as >>> GA thinks FS are not frozen. But that's a different cup of tea. >>> >>> Because of what written above, we need to call set-level >>> on every suspend. >> >> >> IMHO all this says that the 'set-level' command is a conceptually >> unfixably broken design& should be killed in QEMU before it turns >> into an even bigger mess. >> >> Once we're in a situation where we need to call 'set-level' prior >> to every single invocation, you might as well just allow the QEMU >> version number to be passed in directly as an arg to the command >> you are running directly thus avoiding this horrificness. > > Qemu folks, would you care to chime in on this? Big Ack on my part. I told Mike this afternoon that I wasn't going to take the pull request with this command in it. The fundamental problem here is simple--untested code is broken code. Until we introduce a resume from suspend command (such that we can test the guest agent suspend command), we shouldn't be implementing a guest-agent suspend command. As far as I can tell, the only reason we're introducing it is because we're trying to add a multiplexed command that does suspend to ram and suspend to disk. Since it's multiplexed, it's an all-or-nothing introduction. We're then adding a side-interface to attempt to deal work around that. Let's not introduce a multiplexed command in the first place. Here's what I suggest: 1) Throw away set-level interface. 2) Introduce a suspend-to-disk command. 3) Plan to introduce a suspend-to-ram command, but I won't pull it until we have the ability to test it successfully (which means we probably need a resume-from-ram command for QMP). 4) libvirt can probe the existence of suspend-to-disk in the guest agent and act accordingly. 5) To implement virDomainSuspendToRam (or whatever it will be called), libvirt should: a) check if the guest agent command 'suspend-to-ram' exists b) check if the QMP command 'resume-from-ram' exists 6) The recommendation of (5) should be prominently documented in qapi-schema.json 7) In order for libvirt to start implementing (5), we should stub out (3) in qapi-schema-guest.json but set gen=False. That commits us to the interface without actually introducing the command. Regards, Anthony Liguori