From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 4B9BA1FF09B for ; Mon, 14 Sep 2026 12:47:10 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B78F2215FA; Mon, 14 Sep 2026 12:46:50 +0200 (CEST) From: Erik Fastermann To: pve-devel@lists.proxmox.com Subject: [PATCH qemu v3 2/4] savevm-async: release resources on start failure path Date: Mon, 14 Sep 2026 12:46:40 +0200 Message-ID: <20260914104642.145478-3-e.fastermann@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260914104642.145478-1-e.fastermann@proxmox.com> References: <20260914104642.145478-1-e.fastermann@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.422 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) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: 2YR4PIL2JBNTUPWGEFENA6ICSSAH4CWL X-Message-ID-Hash: 2YR4PIL2JBNTUPWGEFENA6ICSSAH4CWL X-MailFrom: efastermann@ruth.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 CC: Erik Fastermann X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The fail label only set the save state and destroyed the iothread, leaving the finalize bottom half, the blocker, the file, and the target behind. Also free the local error of the failing call, of which only the message is copied into errp, here and in load_snapshot_from_blockdev(). Signed-off-by: Erik Fastermann --- ...async-for-background-state-snapshots.patch | 35 ++++++++++++++++--- ...add-optional-buffer-size-to-QEMUFile.patch | 4 +-- ...se-migration-blocker-check-for-snaps.patch | 2 +- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch b/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch index d4c3bb5..a4571d8 100644 --- a/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch +++ b/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch @@ -38,7 +38,8 @@ Signed-off-by: Stefan Reiter rebase for 11.0.0] Signed-off-by: Fiona Ebner [EF: drop QIOChannel reference - free migration vmdesc] + free migration vmdesc + release resources on start failure path] Signed-off-by: Erik Fastermann --- hmp-commands-info.hx | 13 + @@ -46,13 +47,13 @@ Signed-off-by: Erik Fastermann include/migration/snapshot.h | 2 + include/monitor/hmp.h | 3 + migration/meson.build | 1 + - migration/savevm-async.c | 612 +++++++++++++++++++++++++++++++++++ + migration/savevm-async.c | 636 +++++++++++++++++++++++++++++++++++ monitor/hmp-cmds.c | 38 +++ qapi/migration.json | 34 ++ qapi/misc.json | 25 ++ qemu-options.hx | 12 + system/vl.c | 10 + - 11 files changed, 767 insertions(+) + 11 files changed, 791 insertions(+) create mode 100644 migration/savevm-async.c diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx @@ -150,10 +151,10 @@ index 0222d5ea6e..90d62d5723 100644 ), gnutls, zlib) diff --git a/migration/savevm-async.c b/migration/savevm-async.c new file mode 100644 -index 0000000000..a0525d62dd +index 0000000000..ef014e93e8 --- /dev/null +++ b/migration/savevm-async.c -@@ -0,0 +1,612 @@ +@@ -0,0 +1,636 @@ +#include "qemu/osdep.h" +#include "migration/channel-savevm-async.h" +#include "migration/migration.h" @@ -627,11 +628,33 @@ index 0000000000..a0525d62dd + return; + +fail: ++ if (snap_state.finalize_bh) { ++ qemu_bh_delete(snap_state.finalize_bh); ++ snap_state.finalize_bh = NULL; ++ } ++ ++ if (snap_state.blocker) { ++ bdrv_op_unblock_all(target_bs, snap_state.blocker); ++ error_free(snap_state.blocker); ++ snap_state.blocker = NULL; ++ } ++ + if (migration_initialized) { + migration_state_cleanup(ms, true); + } + ++ if (snap_state.file) { ++ qemu_fclose(snap_state.file); ++ snap_state.file = NULL; ++ } ++ ++ if (snap_state.target) { ++ blk_unref(snap_state.target); ++ snap_state.target = NULL; ++ } ++ + savevm_cleanup_iothread(); ++ error_free(local_err); + save_snapshot_error("setup failed"); +} + @@ -764,6 +787,8 @@ index 0000000000..a0525d62dd + error_free(blocker); + blk_unref(be); + } ++ ++ error_free(local_err); + return ret; +} diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c diff --git a/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch b/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch index 06ec32f..55b3359 100644 --- a/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch +++ b/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch @@ -186,7 +186,7 @@ index a390554208..eda093b16a 100644 G_DEFINE_AUTOPTR_CLEANUP_FUNC(QEMUFile, qemu_fclose) diff --git a/migration/savevm-async.c b/migration/savevm-async.c -index a0525d62dd..7a66ede4d1 100644 +index ef014e93e8..f3179b8ad0 100644 --- a/migration/savevm-async.c +++ b/migration/savevm-async.c @@ -425,7 +425,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start, @@ -198,7 +198,7 @@ index a0525d62dd..7a66ede4d1 100644 object_unref(ioc); if (!snap_state.file) { -@@ -569,7 +569,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp) +@@ -591,7 +591,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp) /* restore the VM state */ QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(be, &bs_pos)); diff --git a/debian/patches/pve/0046-savevm-async-reuse-migration-blocker-check-for-snaps.patch b/debian/patches/pve/0046-savevm-async-reuse-migration-blocker-check-for-snaps.patch index b1ceef8..9801a2a 100644 --- a/debian/patches/pve/0046-savevm-async-reuse-migration-blocker-check-for-snaps.patch +++ b/debian/patches/pve/0046-savevm-async-reuse-migration-blocker-check-for-snaps.patch @@ -136,7 +136,7 @@ index b6888daced..80eb0dcd1f 100644 bool migration_in_postcopy(void); bool migration_postcopy_is_alive(MigrationStatus state); diff --git a/migration/savevm-async.c b/migration/savevm-async.c -index 7a66ede4d1..83548a8a13 100644 +index f3179b8ad0..41eabea480 100644 --- a/migration/savevm-async.c +++ b/migration/savevm-async.c @@ -391,7 +391,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start, -- 2.47.3