[PATCH qemu v3 0/4] savevm-async: fix stuck paused vm after snapshot

public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed

* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Service provided by Proxmox Server Solutions GmbH | Privacy | Legal