[libvirt] qemu: lifecycle: reboot + shutdown, unexpected vm status.
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. */
----------------------------------------------------------------------------------
Discussion:
Although the solution above seems to have solved the problem, but it seems a little
awkward to have sheets like this.
Maybe we should let sheet2 consistent with sheet1, rather than letting sheet1
consistent to sheet2. But It's too difficult to realize that, seems impossible
So, is there a better way to solve this problem? or, shall I commit this patch?