From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M2TdI-0002fX-Ih for qemu-devel@nongnu.org; Fri, 08 May 2009 13:13:12 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M2TdE-0002cf-4r for qemu-devel@nongnu.org; Fri, 08 May 2009 13:13:12 -0400 Received: from [199.232.76.173] (port=56547 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M2TdD-0002cc-Uc for qemu-devel@nongnu.org; Fri, 08 May 2009 13:13:07 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:33333) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M2TdD-0007kG-JP for qemu-devel@nongnu.org; Fri, 08 May 2009 13:13:07 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e4.ny.us.ibm.com (8.13.1/8.13.1) with ESMTP id n48H91XU004149 for ; Fri, 8 May 2009 13:09:01 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n48HD6lm149736 for ; Fri, 8 May 2009 13:13:06 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n48HBBIO012557 for ; Fri, 8 May 2009 13:11:11 -0400 Date: Fri, 8 May 2009 12:13:05 -0500 From: Ryan Harper Message-ID: <20090508171305.GC3233@us.ibm.com> References: <1241801561-11441-1-git-send-email-ryanh@us.ibm.com> <4A046503.8050209@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A046503.8050209@us.ibm.com> Subject: [Qemu-devel] Re: [PATCH] Add monitor command for system_reboot List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Ryan Harper , "qemu-devel@nongnu.org" * Anthony Liguori [2009-05-08 12:00]: > Ryan Harper wrote: > >Add a new monitor command (system_reboot) for a soft reboot which uses > >system_powerdown to trigger ACPI shutdown in the guest and once shutdown is > >complete, trigger a reset instead of exiting qemu. > > > >Depends on commit a6d6552426dcbf726e5549f08b70c9318d6be14b which enabled > >ACPI > >power button support. > > > >Tested with Ubuntu 9.04 64-bit guest. > > > >Signed-off-by: Ryan Harper > > > >diff --git a/hw/acpi.c b/hw/acpi.c > >index dbaf18a..346d100 100644 > >--- a/hw/acpi.c > >+++ b/hw/acpi.c > >@@ -151,7 +151,13 @@ static void pm_ioport_writew(void *opaque, uint32_t > >addr, uint32_t val) > > sus_typ = (val >> 10) & 7; > > switch(sus_typ) { > > case 0: /* soft power off */ > >- qemu_system_shutdown_request(); > >+ /* after powerdown, if on system_reboot path, call > >reset + instead of shutdown */ > >+ if (qemu_reboot_requested()) { > >+ qemu_system_reset_request(); > >+ } else { > >+ qemu_system_shutdown_request(); > >+ } > > break; > > > > If qemu_shutdown_requested(), then we'll immediately shutdown the system. Right, but this request happens after we've sent the ACPI powerdown event. After we've done the powerdown (acpi aware guests can do a shutdown), instead of then calling shutdown, which exits, we call reset. > > qemu_reboot_requested() has different semantics, it's really a soft > request. I think the name needs to reflect that. Not sure what you mean here. > > Also, the soft reset flag ought to get reset whenever a VM changes it's > state. That is, if you do a system_reboot, then a system_reset (imagine > that the reboot fails), then you do a normal powerdown in the guest, > instead of shutting off like the user would expect, we'll reboot because > the qemu_reboot_requested() is still true. > > A good way to do that would be by registering a reset handler in > hw/acpi.c. In fact, I think that the whole functionality probably > should live in hw/acpi.c. OK > > And I think this also needs to be stored as part of the savevm state for > hw/acpi.c. If you do a system_reboot followed by an immediate live > migration, without the savevm handler, the VM will shutdown completely > after the migration instead of rebooting as expected. Would it? I don't see that we are saving powerdown|shutdown|reset request flags? Sounds like all of those flags need to be in the save state, and separate patch IMHO. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ryanh@us.ibm.com