From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id B665A1FF0C1 for ; Wed, 26 Aug 2026 15:30:50 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 57D6921338; Wed, 26 Aug 2026 15:30:50 +0200 (CEST) Message-ID: Date: Wed, 26 Aug 2026 15:30:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH pve-qemu] savevm-async: fix stuck paused vm after snapshot To: Erik Fastermann , pve-devel@lists.proxmox.com References: <20260612075345.110118-1-e.fastermann@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260612075345.110118-1-e.fastermann@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787751038462 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.790 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: LWY55OCAQBEO4GFJQAI55FSK3TIMK3S7 X-Message-ID-Hash: LWY55OCAQBEO4GFJQAI55FSK3TIMK3S7 X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 12.06.26 um 9:53 AM schrieb Erik Fastermann: > When creating a snapshot of a paused vm, the vm hung indefinitely in the > state "paused (finish-migrate)" and had to be restarted. This patch > fixes this by only transitioning the vm run state if the vm was > currently running. > > Signed-off-by: Erik Fastermann > --- > ...c-fix-stuck-paused-vm-after-snapshot.patch | 48 +++++++++++++++++++ > debian/patches/series | 1 + > 2 files changed, 49 insertions(+) > create mode 100644 debian/patches/pve/0047-savevm-async-fix-stuck-paused-vm-after-snapshot.patch > > diff --git a/debian/patches/pve/0047-savevm-async-fix-stuck-paused-vm-after-snapshot.patch b/debian/patches/pve/0047-savevm-async-fix-stuck-paused-vm-after-snapshot.patch > new file mode 100644 > index 0000000..46f0422 > --- /dev/null > +++ b/debian/patches/pve/0047-savevm-async-fix-stuck-paused-vm-after-snapshot.patch > @@ -0,0 +1,48 @@ > +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 > +From: Erik Fastermann > +Date: Thu, 11 Jun 2026 13:42:05 +0200 > +Subject: [PATCH] savevm-async: fix stuck paused vm after snapshot > + > +When creating a snapshot of a paused vm, the vm hung indefinitely in the > +state "paused (finish-migrate)" and had to be restarted. This patch > +fixes this by only transitioning the vm run state if the vm was > +currently running. > + > +Signed-off-by: Erik Fastermann > +--- > + migration/savevm-async.c | 12 ++++++++---- > + 1 file changed, 8 insertions(+), 4 deletions(-) > + > +diff --git a/migration/savevm-async.c b/migration/savevm-async.c > +index acd1a4de6e..153433f539 100644 > +--- a/migration/savevm-async.c > ++++ b/migration/savevm-async.c > +@@ -181,9 +181,11 @@ static void process_savevm_finalize(void *opaque) > + blk_set_aio_context(snap_state.target, qemu_get_aio_context(), NULL); > + > + snap_state.vm_needs_start = runstate_is_running(); > +- ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); > +- if (ret < 0) { > +- save_snapshot_error("vm_stop_force_state error %d", ret); > ++ if (snap_state.vm_needs_start) { > ++ ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); It feels wrong to me to avoid going into the finish-migrate state, since we do things like qemu_savevm_state_complete_precopy(), qemu_savevm_state_cleanup() and save_snapshot_cleanup() below. In migration/migration.c there is if (runstate_is_live(s->vm_old_state)) { if (!runstate_check(RUN_STATE_SHUTDOWN)) { vm_start(); } } else { if (runstate_check(RUN_STATE_FINISH_MIGRATE)) { runstate_set(s->vm_old_state); } } The old state is saved via runstate_get() in migration_stop_vm() before stopping the VM. I think we should do the same. > ++ if (ret < 0) { > ++ save_snapshot_error("vm_stop_force_state error %d", ret); > ++ } > + } > + > + if (!aborted) { > +@@ -370,7 +372,9 @@ void qmp_savevm_start(const char *statefile, Error **errp) > + > + if (!statefile) { > + snap_state.vm_needs_start = runstate_is_running(); And for us, the place to store the old state is here. > +- vm_stop(RUN_STATE_SAVE_VM); > ++ if (snap_state.vm_needs_start) { > ++ vm_stop(RUN_STATE_SAVE_VM); > ++ } > + snap_state.state = SAVE_STATE_COMPLETED; > + return; > + } > +-- > +2.47.3 > + > diff --git a/debian/patches/series b/debian/patches/series > index 84c0664..6463a4a 100644 > --- a/debian/patches/series > +++ b/debian/patches/series > @@ -70,3 +70,4 @@ pve/0043-PVE-backup-get-device-info-allow-caller-to-specify-f.patch > pve/0044-PVE-backup-implement-backup-access-setup-and-teardow.patch > pve/0045-PVE-backup-prepare-for-the-switch-to-using-blockdev-.patch > pve/0046-savevm-async-reuse-migration-blocker-check-for-snaps.patch > +pve/0047-savevm-async-fix-stuck-paused-vm-after-snapshot.patch