[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] Add monitor command for system_reboot
From: |
Ryan Harper |
Subject: |
[Qemu-devel] Re: [PATCH] Add monitor command for system_reboot |
Date: |
Fri, 8 May 2009 12:13:05 -0500 |
User-agent: |
Mutt/1.5.6+20040907i |
* Anthony Liguori <address@hidden> [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 <address@hidden>
> >
> >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
address@hidden