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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list