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 2EE051FF0AB for ; Mon, 07 Sep 2026 11:48:54 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id EE7F4216A4; Mon, 07 Sep 2026 11:48:00 +0200 (CEST) From: Erik Fastermann To: pve-devel@lists.proxmox.com Subject: [PATCH qemu v2 7/9] savevm-async: release resources on start failure path Date: Mon, 7 Sep 2026 11:47:41 +0200 Message-ID: <20260907094744.131193-8-e.fastermann@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260907094744.131193-1-e.fastermann@proxmox.com> References: <20260907094744.131193-1-e.fastermann@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.475 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: R3G6AV6IDZVPGQJUIY5I2MQQUJF3C5OE X-Message-ID-Hash: R3G6AV6IDZVPGQJUIY5I2MQQUJF3C5OE 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 8750897..d21fb1f 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 | 608 +++++++++++++++++++++++++++++++++++ + migration/savevm-async.c | 632 +++++++++++++++++++++++++++++++++++ monitor/hmp-cmds.c | 38 +++ qapi/migration.json | 34 ++ qapi/misc.json | 25 ++ qemu-options.hx | 12 + system/vl.c | 10 + - 11 files changed, 763 insertions(+) + 11 files changed, 787 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..4f44c2f12d +index 0000000000..d688fc7112 --- /dev/null +++ b/migration/savevm-async.c -@@ -0,0 +1,608 @@ +@@ -0,0 +1,632 @@ +#include "qemu/osdep.h" +#include "migration/channel-savevm-async.h" +#include "migration/migration.h" @@ -625,9 +626,31 @@ index 0000000000..4f44c2f12d + 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; ++ } ++ + 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"); +} + @@ -760,6 +783,8 @@ index 0000000000..4f44c2f12d + 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 d73d432..cac1842 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 4f44c2f12d..0025d69284 100644 +index d688fc7112..24f7ba60c9 100644 --- a/migration/savevm-async.c +++ b/migration/savevm-async.c @@ -424,7 +424,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start, @@ -198,7 +198,7 @@ index 4f44c2f12d..0025d69284 100644 object_unref(ioc); if (!snap_state.file) { -@@ -565,7 +565,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp) +@@ -587,7 +587,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 66b6b7b..98a0ca4 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 0025d69284..73ba96ba25 100644 +index 24f7ba60c9..78fc27b320 100644 --- a/migration/savevm-async.c +++ b/migration/savevm-async.c @@ -390,7 +390,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start, -- 2.47.3