From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M2U1i-0002kG-Om for qemu-devel@nongnu.org; Fri, 08 May 2009 13:38:26 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M2U1d-0002bK-Ou for qemu-devel@nongnu.org; Fri, 08 May 2009 13:38:26 -0400 Received: from [199.232.76.173] (port=48470 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M2U1d-0002bC-GC for qemu-devel@nongnu.org; Fri, 08 May 2009 13:38:21 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:55777) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1M2U1d-0004bq-2O for qemu-devel@nongnu.org; Fri, 08 May 2009 13:38:21 -0400 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by e34.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id n48HZi3P026169 for ; Fri, 8 May 2009 11:35:44 -0600 Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n48HcJMo060296 for ; Fri, 8 May 2009 11:38:19 -0600 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n48HcJNh005801 for ; Fri, 8 May 2009 11:38:19 -0600 Message-ID: <4A046E09.6080101@us.ibm.com> Date: Fri, 08 May 2009 12:38:17 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1241801561-11441-1-git-send-email-ryanh@us.ibm.com> <4A046503.8050209@us.ibm.com> <20090508171305.GC3233@us.ibm.com> In-Reply-To: <20090508171305.GC3233@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: Ryan Harper Cc: "qemu-devel@nongnu.org" Ryan Harper wrote: > * 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. > I'm saying, semantically, if you call 'qemu_shutdown_requested()', if it returns 1, it means, immediately shutdown the VM--regardless of where it's called. The semantics of qemu_reboot_requested() are, if it returns 1, then only when you see an ACPI soft power off, reset the VM. It's that difference in semantics that I think could lead to confusion. >> 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. > No, they don't. A qemu_powerdown_request() call happens from the monitor. This is to allow a graceful shutdown (as opposed to exiting from the monitor). This will trigger the TCG loop to immediately exit. The state doesn't need to be saved because you cannot do a migration in between when qemu_powerdown_request() is called and when the shutdown actually happens. -- Regards, Anthony Liguori