* [PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot
@ 2026-09-14 10:46 Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 1/4] savevm-async: free migration vmdesc Erik Fastermann
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Erik Fastermann @ 2026-09-14 10:46 UTC (permalink / raw)
To: pve-devel; +Cc: Erik Fastermann
Snapshotting a paused VM left it in finish-migrate, where it stayed
until the VM was restarted. Fix this by storing the run state instead
and put it back, similar to migration_iteration_finish().
While working on that, I noticed some memory leaks and other issues,
which I found by using sanitizer tools like ASAN and reading the code
and docs.
Changes since the RFC:
* store the run state via runstate_get() and restore it instead of
skipping the transition into finish-migrate (Thanks @Fiona)
* refuse a snapshot while an incoming migration is pending, which
migrate_prepare() already does, otherwise the restore aborts
* fixed multiple memory leaks
* fixed multiple discarded error messages
* removed raw uses of allocators
* folded the changes into the relevant debian/patches
Changes since v2 (Thanks @Fiona for the feedback):
* dropped patches 1-5, which were already applied [3]
* guard migration_state_cleanup() in qmp_savevm_start() failure case
* added comment when attempting to set RUN_STATE_SAVE_VM
Future work:
* Fiona mentioned that similar bugs to the ones I found also exist in
upstream QEMU. I will look into submitting patches.
* Once SAVE_STATE_ERROR is set, no new snapshot can be started and
savevm-start fails with "VM snapshot already started" until the VM is
restarted. Resetting that state looks difficult and could introduce
new bugs. A couple of people ran into this before, see [0] and [1].
It would definitely be nicer if a single failed snapshot did not
block further snapshots until a VM restart. I added a Bugzilla entry
[4].
* TSAN flags a data race on snap_state.bs_pos. Probably simply fixed
by using atomics. There might be some other issues in savevm-async,
but that has to be checked in more detail. I added a Bugzilla entry
[5].
Note that patch 4 touches the same lines as [2].
[0] https://forum.proxmox.com/threads/snapshot-creation-issue.176090/
[1] https://forum.proxmox.com/threads/snapshot-fails-to-cleanup-and-prevents-future-snapshots.144757/
[2] https://lore.proxmox.com/all/20260714142629.277917-2-f.ebner@proxmox.com/
[3] https://lore.proxmox.com/pve-devel/20260907094744.131193-1-e.fastermann@proxmox.com/
[4] https://bugzilla.proxmox.com/show_bug.cgi?id=8037
[5] https://bugzilla.proxmox.com/show_bug.cgi?id=8038
Erik Fastermann (4):
savevm-async: free migration vmdesc
savevm-async: release resources on start failure path
savevm-async: include reason when file open fails
savevm-async: restore run state from before snapshot
...async-for-background-state-snapshots.patch | 116 ++++++++++++++----
...add-optional-buffer-size-to-QEMUFile.patch | 6 +-
...se-migration-blocker-check-for-snaps.patch | 4 +-
3 files changed, 94 insertions(+), 32 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH qemu v3 1/4] savevm-async: free migration vmdesc
2026-09-14 10:46 [PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot Erik Fastermann
@ 2026-09-14 10:46 ` Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 2/4] savevm-async: release resources on start failure path Erik Fastermann
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Erik Fastermann @ 2026-09-14 10:46 UTC (permalink / raw)
To: pve-devel; +Cc: Erik Fastermann
migrate_init() allocates it and only migration_cleanup() frees it, which
savevm-async does not call. Free it with the rest of the migration
state, tens of KiB per snapshot even for a minimal guest.
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
...async-for-background-state-snapshots.patch | 31 +++++++++++++------
...add-optional-buffer-size-to-QEMUFile.patch | 6 ++--
...se-migration-blocker-check-for-snaps.patch | 4 +--
3 files changed, 27 insertions(+), 14 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 f252236..d4c3bb5 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
@@ -37,7 +37,8 @@ Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
add parameter to skip vm start to be used for hibernation
rebase for 11.0.0]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
-[EF: drop QIOChannel reference]
+[EF: drop QIOChannel reference
+ free migration vmdesc]
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
hmp-commands-info.hx | 13 +
@@ -45,13 +46,13 @@ Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
include/migration/snapshot.h | 2 +
include/monitor/hmp.h | 3 +
migration/meson.build | 1 +
- migration/savevm-async.c | 600 +++++++++++++++++++++++++++++++++++
+ migration/savevm-async.c | 612 +++++++++++++++++++++++++++++++++++
monitor/hmp-cmds.c | 38 +++
qapi/migration.json | 34 ++
qapi/misc.json | 25 ++
qemu-options.hx | 12 +
system/vl.c | 10 +
- 11 files changed, 755 insertions(+)
+ 11 files changed, 767 insertions(+)
create mode 100644 migration/savevm-async.c
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
@@ -149,10 +150,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..cab08643ad
+index 0000000000..a0525d62dd
--- /dev/null
+++ b/migration/savevm-async.c
-@@ -0,0 +1,600 @@
+@@ -0,0 +1,612 @@
+#include "qemu/osdep.h"
+#include "migration/channel-savevm-async.h"
+#include "migration/migration.h"
@@ -261,6 +262,15 @@ index 0000000000..cab08643ad
+ return info;
+}
+
++static void migration_state_cleanup(MigrationState *ms, bool failed)
++{
++ migrate_set_state(&ms->state, MIGRATION_STATUS_SETUP,
++ failed ? MIGRATION_STATUS_FAILED : MIGRATION_STATUS_COMPLETED);
++ ms->to_dst_file = NULL;
++ /* see migration_cleanup_json_writer */
++ g_clear_pointer(&ms->vmdesc, json_writer_free);
++}
++
+static int save_snapshot_cleanup(void)
+{
+ int ret = 0;
@@ -359,10 +369,7 @@ index 0000000000..cab08643ad
+ DPRINTF("timing: process_savevm_finalize (state saving) took %ld ms\n",
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - start_time);
+
-+ /* clear migration state */
-+ migrate_set_state(&ms->state, MIGRATION_STATUS_SETUP,
-+ ret || aborted ? MIGRATION_STATUS_FAILED : MIGRATION_STATUS_COMPLETED);
-+ ms->to_dst_file = NULL;
++ migration_state_cleanup(ms, ret || aborted);
+
+ /*
+ * Same as in migration_iteration_finish(): saving RAM might've turned on CPU throttling for
@@ -505,6 +512,7 @@ index 0000000000..cab08643ad
+{
+ Error *local_err = NULL;
+ MigrationState *ms = migrate_get_current();
++ bool migration_initialized = false;
+ BlockDriverState *target_bs = NULL;
+ int ret = 0;
+
@@ -591,6 +599,7 @@ index 0000000000..cab08643ad
+ }
+ memset(&mig_stats, 0, sizeof(mig_stats));
+ ms->to_dst_file = snap_state.file;
++ migration_initialized = true;
+
+ error_setg(&snap_state.blocker, "block device is in use by savevm");
+ bdrv_op_block_all(target_bs, snap_state.blocker);
@@ -618,6 +627,10 @@ index 0000000000..cab08643ad
+ return;
+
+fail:
++ if (migration_initialized) {
++ migration_state_cleanup(ms, true);
++ }
++
+ savevm_cleanup_iothread();
+ save_snapshot_error("setup failed");
+}
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 39cacf7..06ec32f 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,10 +186,10 @@ 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 cab08643ad..f9eaf3b8e6 100644
+index a0525d62dd..7a66ede4d1 100644
--- a/migration/savevm-async.c
+++ b/migration/savevm-async.c
-@@ -418,7 +418,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
+@@ -425,7 +425,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(snap_state.target,
&snap_state.bs_pos));
@@ -198,7 +198,7 @@ index cab08643ad..f9eaf3b8e6 100644
object_unref(ioc);
if (!snap_state.file) {
-@@ -557,7 +557,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
+@@ -569,7 +569,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 5cc3893..b1ceef8 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,10 +136,10 @@ 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 f9eaf3b8e6..51340c5dcd 100644
+index 7a66ede4d1..83548a8a13 100644
--- a/migration/savevm-async.c
+++ b/migration/savevm-async.c
-@@ -384,7 +384,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
+@@ -391,7 +391,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
return;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH qemu v3 2/4] savevm-async: release resources on start failure path
2026-09-14 10:46 [PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 1/4] savevm-async: free migration vmdesc Erik Fastermann
@ 2026-09-14 10:46 ` Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 3/4] savevm-async: include reason when file open fails Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 4/4] savevm-async: restore run state from before snapshot Erik Fastermann
3 siblings, 0 replies; 5+ messages in thread
From: Erik Fastermann @ 2026-09-14 10:46 UTC (permalink / raw)
To: pve-devel; +Cc: Erik Fastermann
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 <e.fastermann@proxmox.com>
---
...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 <s.reiter@proxmox.com>
rebase for 11.0.0]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
[EF: drop QIOChannel reference
- free migration vmdesc]
+ free migration vmdesc
+ release resources on start failure path]
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
hmp-commands-info.hx | 13 +
@@ -46,13 +47,13 @@ Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH qemu v3 3/4] savevm-async: include reason when file open fails
2026-09-14 10:46 [PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 1/4] savevm-async: free migration vmdesc Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 2/4] savevm-async: release resources on start failure path Erik Fastermann
@ 2026-09-14 10:46 ` Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 4/4] savevm-async: restore run state from before snapshot Erik Fastermann
3 siblings, 0 replies; 5+ messages in thread
From: Erik Fastermann @ 2026-09-14 10:46 UTC (permalink / raw)
To: pve-devel; +Cc: Erik Fastermann
blk_new_open() reports why it could not open the file, but the error was
discarded.
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
...m-async-for-background-state-snapshots.patch | 17 ++++++++++-------
...E-add-optional-buffer-size-to-QEMUFile.patch | 6 +++---
...euse-migration-blocker-check-for-snaps.patch | 2 +-
3 files changed, 14 insertions(+), 11 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 a4571d8..30400a1 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
@@ -39,7 +39,8 @@ Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
[EF: drop QIOChannel reference
free migration vmdesc
- release resources on start failure path]
+ release resources on start failure path
+ include reason when file open fails]
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
hmp-commands-info.hx | 13 +
@@ -47,13 +48,13 @@ Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
include/migration/snapshot.h | 2 +
include/monitor/hmp.h | 3 +
migration/meson.build | 1 +
- migration/savevm-async.c | 636 +++++++++++++++++++++++++++++++++++
+ migration/savevm-async.c | 638 +++++++++++++++++++++++++++++++++++
monitor/hmp-cmds.c | 38 +++
qapi/migration.json | 34 ++
qapi/misc.json | 25 ++
qemu-options.hx | 12 +
system/vl.c | 10 +
- 11 files changed, 791 insertions(+)
+ 11 files changed, 793 insertions(+)
create mode 100644 migration/savevm-async.c
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
@@ -151,10 +152,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..ef014e93e8
+index 0000000000..bf8509b261
--- /dev/null
+++ b/migration/savevm-async.c
-@@ -0,0 +1,636 @@
+@@ -0,0 +1,638 @@
+#include "qemu/osdep.h"
+#include "migration/channel-savevm-async.h"
+#include "migration/migration.h"
@@ -571,7 +572,8 @@ index 0000000000..ef014e93e8
+ qdict_put_str(options, "driver", "raw");
+ snap_state.target = blk_new_open(statefile, NULL, options, bdrv_oflags, &local_err);
+ if (!snap_state.target) {
-+ error_setg(errp, "failed to open '%s'", statefile);
++ error_setg(errp, "failed to open '%s' - %s", statefile,
++ local_err ? error_get_pretty(local_err) : "unknown error");
+ goto fail;
+ }
+ target_bs = blk_bs(snap_state.target);
@@ -733,7 +735,8 @@ index 0000000000..ef014e93e8
+ be = blk_new_open(filename, NULL, options, 0, &local_err);
+
+ if (!be) {
-+ error_setg(errp, "Could not open VM state file");
++ error_setg(errp, "Could not open VM state file - %s",
++ local_err ? error_get_pretty(local_err) : "unknown error");
+ goto the_end;
+ }
+
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 55b3359..caea753 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,10 +186,10 @@ 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 ef014e93e8..f3179b8ad0 100644
+index bf8509b261..94849f5219 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,
+@@ -426,7 +426,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(snap_state.target,
&snap_state.bs_pos));
@@ -198,7 +198,7 @@ index ef014e93e8..f3179b8ad0 100644
object_unref(ioc);
if (!snap_state.file) {
-@@ -591,7 +591,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
+@@ -593,7 +593,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 9801a2a..153a569 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 f3179b8ad0..41eabea480 100644
+index 94849f5219..4ed5d510bf 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH qemu v3 4/4] savevm-async: restore run state from before snapshot
2026-09-14 10:46 [PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot Erik Fastermann
` (2 preceding siblings ...)
2026-09-14 10:46 ` [PATCH qemu v3 3/4] savevm-async: include reason when file open fails Erik Fastermann
@ 2026-09-14 10:46 ` Erik Fastermann
3 siblings, 0 replies; 5+ messages in thread
From: Erik Fastermann @ 2026-09-14 10:46 UTC (permalink / raw)
To: pve-devel; +Cc: Erik Fastermann
A snapshot of a paused VM left it in finish-migrate, where it stayed
until the VM was restarted. vm_needs_start only recorded whether the VM
had been running, so there was nothing to restore a paused VM to.
Store the run state instead and put it back, similar to
migration_iteration_finish().
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
...async-for-background-state-snapshots.patch | 63 ++++++++++++-------
...add-optional-buffer-size-to-QEMUFile.patch | 6 +-
...se-migration-blocker-check-for-snaps.patch | 4 +-
3 files changed, 47 insertions(+), 26 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 30400a1..d7d5d0d 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
@@ -40,7 +40,8 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
[EF: drop QIOChannel reference
free migration vmdesc
release resources on start failure path
- include reason when file open fails]
+ include reason when file open fails
+ restore run state from before snapshot]
Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
hmp-commands-info.hx | 13 +
@@ -48,13 +49,13 @@ Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
include/migration/snapshot.h | 2 +
include/monitor/hmp.h | 3 +
migration/meson.build | 1 +
- migration/savevm-async.c | 638 +++++++++++++++++++++++++++++++++++
- monitor/hmp-cmds.c | 38 +++
+ migration/savevm-async.c | 658 +++++++++++++++++++++++++++++++++++
+ monitor/hmp-cmds.c | 38 ++
qapi/migration.json | 34 ++
qapi/misc.json | 25 ++
qemu-options.hx | 12 +
system/vl.c | 10 +
- 11 files changed, 793 insertions(+)
+ 11 files changed, 813 insertions(+)
create mode 100644 migration/savevm-async.c
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
@@ -152,10 +153,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..bf8509b261
+index 0000000000..0c8a10886f
--- /dev/null
+++ b/migration/savevm-async.c
-@@ -0,0 +1,638 @@
+@@ -0,0 +1,658 @@
+#include "qemu/osdep.h"
+#include "migration/channel-savevm-async.h"
+#include "migration/migration.h"
@@ -210,7 +211,7 @@ index 0000000000..bf8509b261
+ int state;
+ Error *error;
+ Error *blocker;
-+ bool vm_needs_start;
++ RunState vm_old_state;
+ bool skip_vm_start;
+ QEMUFile *file;
+ int64_t total_time;
@@ -230,6 +231,24 @@ index 0000000000..bf8509b261
+ return snap_state.state == SAVE_STATE_COMPLETED && snap_state.skip_vm_start;
+}
+
++static void restore_vm_old_state(void)
++{
++ RunState old_state = snap_state.vm_old_state;
++
++ if (old_state == RUN_STATE__MAX) {
++ return;
++ }
++ snap_state.vm_old_state = RUN_STATE__MAX;
++
++ if (runstate_is_live(old_state)) {
++ if (!should_skip_vm_start() && !runstate_check(RUN_STATE_SHUTDOWN)) {
++ vm_start();
++ }
++ } else if (runstate_check(RUN_STATE_FINISH_MIGRATE)) {
++ runstate_set(old_state);
++ }
++}
++
+SaveVMInfo *qmp_query_savevm(Error **errp)
+{
+ SaveVMInfo *info = g_malloc0(sizeof(*info));
@@ -352,7 +371,7 @@ index 0000000000..bf8509b261
+ */
+ blk_set_aio_context(snap_state.target, qemu_get_aio_context(), NULL);
+
-+ snap_state.vm_needs_start = runstate_is_running();
++ snap_state.vm_old_state = runstate_get();
+ ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+ if (ret < 0) {
+ save_snapshot_error("vm_stop_force_state error %d", ret);
@@ -396,12 +415,7 @@ index 0000000000..bf8509b261
+ save_snapshot_error("process_savevm_cleanup: invalid state: %d",
+ snap_state.state);
+ }
-+ if (snap_state.vm_needs_start) {
-+ if (!should_skip_vm_start()) {
-+ vm_start();
-+ }
-+ snap_state.vm_needs_start = false;
-+ }
++ restore_vm_old_state();
+
+ DPRINTF("timing: process_savevm_finalize (full) took %ld ms\n",
+ qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - start_time);
@@ -530,12 +544,18 @@ index 0000000000..bf8509b261
+ return;
+ }
+
++ if (runstate_check(RUN_STATE_INMIGRATE)) {
++ error_setg(errp, "There's an incoming migration in progress");
++ return;
++ }
++
+ /* initialize snapshot info */
+ snap_state.bs_pos = 0;
+ snap_state.total_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+ snap_state.blocker = NULL;
+ snap_state.target_close_wait = (QemuCoSleep){ .to_wake = NULL };
+ snap_state.skip_vm_start = has_skip_vm_start && skip_vm_start;
++ snap_state.vm_old_state = RUN_STATE__MAX;
+
+ if (snap_state.error) {
+ error_free(snap_state.error);
@@ -543,7 +563,13 @@ index 0000000000..bf8509b261
+ }
+
+ if (!statefile) {
-+ snap_state.vm_needs_start = runstate_is_running();
++ snap_state.vm_old_state = runstate_get();
++ /*
++ * vm_stop() only applies the run state if the VM is live, i.e. running
++ * or suspended. Otherwise the VM is already stopped and the run state
++ * is left alone, so restore_vm_old_state() has nothing to put back
++ * either.
++ */
+ vm_stop(RUN_STATE_SAVE_VM);
+ snap_state.state = SAVE_STATE_COMPLETED;
+ return;
@@ -707,12 +733,7 @@ index 0000000000..bf8509b261
+ return;
+ }
+
-+ if (snap_state.vm_needs_start) {
-+ if (!should_skip_vm_start()) {
-+ vm_start();
-+ }
-+ snap_state.vm_needs_start = false;
-+ }
++ restore_vm_old_state();
+
+ qemu_coroutine_enter(wait_for_close);
+}
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 caea753..60bed8c 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,10 +186,10 @@ 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 bf8509b261..94849f5219 100644
+index 0c8a10886f..fd64c209ea 100644
--- a/migration/savevm-async.c
+++ b/migration/savevm-async.c
-@@ -426,7 +426,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
+@@ -451,7 +451,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(snap_state.target,
&snap_state.bs_pos));
@@ -198,7 +198,7 @@ index bf8509b261..94849f5219 100644
object_unref(ioc);
if (!snap_state.file) {
-@@ -593,7 +593,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
+@@ -613,7 +613,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 153a569..5543423 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,10 +136,10 @@ 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 94849f5219..4ed5d510bf 100644
+index fd64c209ea..ae813c3d21 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,
+@@ -416,7 +416,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
return;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-14 10:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14 10:46 [PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 1/4] savevm-async: free migration vmdesc Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 2/4] savevm-async: release resources on start failure path Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 3/4] savevm-async: include reason when file open fails Erik Fastermann
2026-09-14 10:46 ` [PATCH qemu v3 4/4] savevm-async: restore run state from before snapshot Erik Fastermann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.