From 6f0627d4bd007808b6df4bffaae84c8bc081eded Mon Sep 17 00:00:00 2001 From: Filip Schauer Date: Fri, 13 Oct 2023 15:50:06 +0200 Subject: [PATCH] backup, migrate: fix races with suspended VMs that can wake up Fix races with ACPI-suspended VMs which could wake up during migration or during a suspend-mode backup. Revert prevention, of ACPI-suspended VMs automatically resuming after migration, introduced by 7ba974a6828d. The commit introduced a potential problem that causes a suspended VM that wakes up during migration to remain paused after the migration finishes. This can be fixed once QEMU preserves the 'suspended' runstate during migration (current patch on the qemu-devel list [0]) by checking for the 'suspended' runstate on the target after migration. Furthermore the commit increased the race window during the preparation of a suspend-mode backup, when a suspended VM wakes up between the vm_is_paused check in PVE::VZDump::QemuServer::prepare and PVE::VZDump::QemuServer::qga_fs_freeze. This causes the code to skip fs-freeze even if the VM has woken up, potentially leaving the file system in an inconsistent state. To prevent this, do not treat the suspended runstate as paused when migrating or archiving a VM. [0]: https://lists.nongnu.org/archive/html/qemu-devel/2023-08/msg05260.html Signed-off-by: Filip Schauer Reviewed-by: Fiona Ebner [ TL: massage in Fiona's extra info into commit message ] Signed-off-by: Thomas Lamprecht --- PVE/API2/Qemu.pm | 4 ++-- PVE/QemuMigrate.pm | 4 +++- PVE/QemuServer.pm | 6 +++--- PVE/VZDump/QemuServer.pm | 4 +++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index 9877ce24..38bdaabd 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -3084,7 +3084,7 @@ __PACKAGE__->register_method({ # sending a graceful shutdown command to paused VMs runs into timeouts, and even worse, when # the VM gets resumed later, it still gets the request delivered and powers off - if (PVE::QemuServer::vm_is_paused($vmid)) { + if (PVE::QemuServer::vm_is_paused($vmid, 1)) { if ($param->{forceStop}) { warn "VM is paused - stop instead of shutdown\n"; $shutdown = 0; @@ -3160,7 +3160,7 @@ __PACKAGE__->register_method({ my $node = extract_param($param, 'node'); my $vmid = extract_param($param, 'vmid'); - die "VM is paused - cannot shutdown\n" if PVE::QemuServer::vm_is_paused($vmid); + die "VM is paused - cannot shutdown\n" if PVE::QemuServer::vm_is_paused($vmid, 1); die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid); diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm index f41c61f7..111eeb07 100644 --- a/PVE/QemuMigrate.pm +++ b/PVE/QemuMigrate.pm @@ -224,7 +224,9 @@ sub prepare { } } - $self->{vm_was_paused} = 1 if PVE::QemuServer::vm_is_paused($vmid); + # Do not treat a suspended VM as paused, as it might wake up + # during migration and remain paused after migration finishes. + $self->{vm_was_paused} = 1 if PVE::QemuServer::vm_is_paused($vmid, 0); } my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, 1); diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 28956755..2cd89485 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -8497,7 +8497,7 @@ sub complete_migration_storage { } sub vm_is_paused { - my ($vmid) = @_; + my ($vmid, $include_suspended) = @_; my $qmpstatus = eval { PVE::QemuConfig::assert_config_exists_on_node($vmid); mon_cmd($vmid, "query-status"); @@ -8505,8 +8505,8 @@ sub vm_is_paused { warn "$@\n" if $@; return $qmpstatus && ( $qmpstatus->{status} eq "paused" || - $qmpstatus->{status} eq "suspended" || - $qmpstatus->{status} eq "prelaunch" + $qmpstatus->{status} eq "prelaunch" || + ($include_suspended && $qmpstatus->{status} eq "suspended") ); } diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm index b38d7e74..f811cbf2 100644 --- a/PVE/VZDump/QemuServer.pm +++ b/PVE/VZDump/QemuServer.pm @@ -67,7 +67,9 @@ sub prepare { $self->{vm_was_paused} = 0; if (!PVE::QemuServer::check_running($vmid)) { $self->{vm_was_running} = 0; - } elsif (PVE::QemuServer::vm_is_paused($vmid)) { + } elsif (PVE::QemuServer::vm_is_paused($vmid, 0)) { + # Do not treat a suspended VM as paused, as it would cause us to skip + # fs-freeze even if the VM wakes up before we reach qga_fs_freeze. $self->{vm_was_paused} = 1; }