[libvirt] qemu: lifecycle: reboot + shutdown, unexpected vm status.

[Michal Privoznik]

On 16.04.2015 08:02, zhang bo wrote:
> Steps:
>     1 virsh reboot guest1 --mode=acpi
>     2 virsh shutdown guest1 --mode=agent
> 
> 
> Expected result:
>     As the SHUTDOWN job is after REBOOT, we expected the guest to be *shutoff*. (Do you think so?)
> 
> 
> Exacted result:
>     After the 2 steps above, the guest got *rebooted*.
> 
> 
> The reason to this problem:
>     1 in qemuDomainReboot(mode acpi), it sets priv->fakeReboot to 1.
>     2 after shutdown/reboot, qemu monitor IO trigged qemuProcessHandleShutdown(), which finds that priv->fakeReboot is 1, and reboot the guest.
> 
> 
> Root Cause of the problem:
>     After further look into  the problem, We found that the design of acpi/agent shutdown/reboot seems a little chaotic.
>     -----------------------------------
>     sheet1 who sets fakeReboot
>     -----------------------------------
>              shutdown           reboot
>     acpi        Y(0)              Y(1)
>     agent       N                 N
>     It's apparently, *acpi-mode* jobs set fakeReboot.
>     -----------------------------------
>     sheet2 who needs to check fakeReboot(qemuProcessHandleShutdown())
>     -----------------------------------
>              shutdown           reboot
>     acpi        Y                 Y
>     agent       *Y*                 *N*
>     Things become a little odd here. only agent-mode reboot doesn't check fakeReboot.
> 
>     We can tell from the above 2 sheets, that they're not consistent.
>     *Agent-mode shutdown needs to check fakeReboot(trigger by SHUTDOWN monitorIO event), while it didn't set it before.*
> 
>     The chaos is not caused by libvirtd, it's a systematic problem, including guest os and qemu. (guest os writes ACPI, triggers qemu, qemu then triggers libvirtd)
> 
> 
> My Solution:
>     A simple solution is to make the 1st sheet consistent to the 2nd sheet, which is:
>     -----------------------------------
>     sheet3 who should set fakeReboot:
>     -----------------------------------
>              shutdown           reboot
>     acpi        Y(0)              Y(1)
>     agent       *Y(0)*               N
>     -----------------------------------
>     we let agent-mode shutdown set fakeReboot to 0.
> --------------------------------------------------------------------------------
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7eb5a7d..c751dcf 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1959,6 +1959,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
>          goto endjob;
>      }
> 
> +    qemuDomainSetFakeReboot(driver, vm, isReboot);
> +
>      if (useAgent) {
>          qemuDomainObjEnterAgent(vm);
>          ret = qemuAgentShutdown(priv->agent, agentFlag);
> @@ -1970,7 +1972,6 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags)
>       */
>      if (!useAgent ||
>          (ret < 0 && (acpiRequested || !flags))) {
> -        qemuDomainSetFakeReboot(driver, vm, isReboot);
> 
>          /* Even if agent failed, we have to check if guest went away
>           * by itself while our locks were down.  */

Yes, the analysis is correct and the patch looks right. Can you please
post it among with commit message so that I can push it?

Michal