[PATCH RESEND] qemu: disarm fake reboot flag on reset

Nikolay Shirokovskiy posted 1 patch 2 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20220405115259.665139-1-nikolay.shirokovskiy@openvz.org
Test syntax-check passed
src/qemu/qemu_process.c | 1 +
1 file changed, 1 insertion(+)
[PATCH RESEND] qemu: disarm fake reboot flag on reset
Posted by Nikolay Shirokovskiy 2 years ago
From: Maxim Nestratov <mnestratov@virtuozzo.com>

This is a quite an old (created at 2016) patch fixing an issue for at
that time contemporary Fedora 23. virsh reboot returns success (yet
after hanging for a while), VM is rebooted sucessfully too but then
shutdown from inside guest causes reboot and not shutdown.

VM has agent installed. So virsh reboot first tries to reboot VM thru
the agent. The agent calls 'shutdown -r' command. Typically it returns
instantly but on this distro for some reason it takes time. I did not
investigate the cause but the command waits in dbus client code,
probably waits for reply. The libvirt waits 60s for agent command to
execute and then errors out. Next reboot API falls back to ACPI shutdown
which returns successfully thus the reboot command return success too.

Yet shutdown command in guest eventually successfull and guest is truly
rebooted. So libvirt does not receive SHUTDOWN event and fake reboot
flag which is armed on fallback path stays armed. Thus next shutdown
from guest leads to reboot.

The issue has 100% repro on Fedora 23. On modern distros I can't
reproduce it at all. Shutdown command is asynchronous and returns
immediately even if I start some service that ignores TERM signal and
thus shutdown procedure waits for 90s (if I not mistaken) before sending
KILL.

Yet I guess it is nice to have this patch to be more robust.

Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org>
---
 src/qemu/qemu_process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f63fc3389d..d81ed9391a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -435,6 +435,7 @@ qemuProcessHandleReset(qemuMonitor *mon G_GNUC_UNUSED,
     if (priv->agent)
         qemuAgentNotifyEvent(priv->agent, QEMU_AGENT_EVENT_RESET);
 
+    qemuDomainSetFakeReboot(vm, false);
     qemuDomainSaveStatus(vm);
 
  unlock:
-- 
2.35.1
Re: [PATCH RESEND] qemu: disarm fake reboot flag on reset
Posted by Daniel P. Berrangé 2 years ago
On Tue, Apr 05, 2022 at 02:52:59PM +0300, Nikolay Shirokovskiy wrote:
> From: Maxim Nestratov <mnestratov@virtuozzo.com>
> 
> This is a quite an old (created at 2016) patch fixing an issue for at
> that time contemporary Fedora 23. virsh reboot returns success (yet
> after hanging for a while), VM is rebooted sucessfully too but then
> shutdown from inside guest causes reboot and not shutdown.
> 
> VM has agent installed. So virsh reboot first tries to reboot VM thru
> the agent. The agent calls 'shutdown -r' command. Typically it returns
> instantly but on this distro for some reason it takes time. I did not
> investigate the cause but the command waits in dbus client code,
> probably waits for reply. The libvirt waits 60s for agent command to
> execute and then errors out. Next reboot API falls back to ACPI shutdown
> which returns successfully thus the reboot command return success too.
> 
> Yet shutdown command in guest eventually successfull and guest is truly
> rebooted. So libvirt does not receive SHUTDOWN event and fake reboot
> flag which is armed on fallback path stays armed. Thus next shutdown
> from guest leads to reboot.
> 
> The issue has 100% repro on Fedora 23. On modern distros I can't
> reproduce it at all. Shutdown command is asynchronous and returns
> immediately even if I start some service that ignores TERM signal and
> thus shutdown procedure waits for 90s (if I not mistaken) before sending
> KILL.
> 
> Yet I guess it is nice to have this patch to be more robust.
> 
> Signed-off-by: Nikolay Shirokovskiy <nikolay.shirokovskiy@openvz.org>
> ---
>  src/qemu/qemu_process.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

and pushed


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|