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

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

[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

Future work:

* 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.

* 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 I have to check this in more detail.

Note that patch 9 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/


Erik Fastermann (9):
  d/patches: normalize header of pbs-restore no-cache patch
  pbs-restore: keep callback data on the stack
  migration/qemu-file: allocate buffer with g_malloc
  migration/pbs-state: check allocation of incoming state buffer
  savevm-async: drop QIOChannel reference
  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 | 111 +++++++++++++-----
 ...add-optional-buffer-size-to-QEMUFile.patch |  29 ++---
 ...estore-new-command-to-restore-from-p.patch |  16 +--
 ...igrate-dirty-bitmap-state-via-savevm.patch |  25 ++--
 ...se-migration-blocker-check-for-snaps.patch |   4 +-
 ...no-cache-flag-to-skip-host-page-cach.patch |   7 +-
 6 files changed, 129 insertions(+), 63 deletions(-)

--
2.47.3

[PATCH qemu v2 1/9] d/patches: normalize header of pbs-restore no-cache patch

[Erik Fastermann]

Every other patch in the series is generated with --zero-commit and
--no-signature. Bring the outlier in line, so regenerating the series
does not produce unrelated churn.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 ...bs-restore-add-no-cache-flag-to-skip-host-page-cach.patch | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch b/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch
index fff7127..3ef6e69 100644
--- a/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch
+++ b/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch
@@ -1,4 +1,4 @@
-From d7ca855d826bb20cd896fe95e1cc2154ce32980d Mon Sep 17 00:00:00 2001
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Christian Ebner <c.ebner@proxmox.com>
 Date: Mon, 17 Aug 2026 10:21:58 +0200
 Subject: [PATCH] pbs-restore: add no-cache flag to skip host page cache on
@@ -67,6 +67,3 @@ index f165f418af..55a3cb235d 100644
      BlockBackend *blk = blk_new_open(target, NULL, options, flags, &local_err);
      if (!blk) {
          fprintf(stderr, "%s\n", error_get_pretty(local_err));
--- 
-2.47.3
-
-- 
2.47.3

[PATCH qemu v2 2/9] pbs-restore: keep callback data on the stack

[Erik Fastermann]

proxmox_restore_image() is synchronous and only hands the pointer back
to the callback, so the lifetime is the enclosing scope. Dropping the
heap allocation also drops its missing free and calloc() call, which is
discouraged in QEMU.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 ...s-restore-new-command-to-restore-from-p.patch | 16 +++++++++-------
 ...dd-no-cache-flag-to-skip-host-page-cach.patch |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/debian/patches/pve/0030-PVE-Backup-pbs-restore-new-command-to-restore-from-p.patch b/debian/patches/pve/0030-PVE-Backup-pbs-restore-new-command-to-restore-from-p.patch
index 6d5bb42..bd11662 100644
--- a/debian/patches/pve/0030-PVE-Backup-pbs-restore-new-command-to-restore-from-p.patch
+++ b/debian/patches/pve/0030-PVE-Backup-pbs-restore-new-command-to-restore-from-p.patch
@@ -7,6 +7,8 @@ Subject: [PATCH] PVE-Backup: pbs-restore - new command to restore from proxmox
 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 [WB: add namespace support]
 Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
+[EF: keep callback data on the stack]
+Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
 ---
  meson.build   |   4 +
  pbs-restore.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++
@@ -30,7 +32,7 @@ index 1eee80f7d8..b7a8702711 100644
    foreach exe: [ 'qemu-img', 'qemu-io', 'qemu-nbd', 'qemu-storage-daemon']
 diff --git a/pbs-restore.c b/pbs-restore.c
 new file mode 100644
-index 0000000000..f165f418af
+index 0000000000..f5e3552193
 --- /dev/null
 +++ b/pbs-restore.c
 @@ -0,0 +1,236 @@
@@ -240,11 +242,11 @@ index 0000000000..f165f418af
 +        return -1;
 +    }
 +
-+    CallbackData *callback_data = calloc(sizeof(CallbackData), 1);
-+
-+    callback_data->target = blk;
-+    callback_data->skip_zero = skip_zero;
-+    callback_data->last_offset = 0;
++    CallbackData callback_data = {
++        .target = blk,
++        .skip_zero = skip_zero,
++        .last_offset = 0,
++    };
 +
 +    // blk_set_enable_write_cache(blk, !writethrough);
 +
@@ -256,7 +258,7 @@ index 0000000000..f165f418af
 +        conn,
 +        archive_name,
 +        write_callback,
-+        callback_data,
++        &callback_data,
 +        &pbs_error,
 +        verbose);
 +
diff --git a/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch b/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch
index 3ef6e69..7b79722 100644
--- a/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch
+++ b/debian/patches/pve/0047-pbs-restore-add-no-cache-flag-to-skip-host-page-cach.patch
@@ -25,7 +25,7 @@ Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
  1 file changed, 11 insertions(+)
 
 diff --git a/pbs-restore.c b/pbs-restore.c
-index f165f418af..55a3cb235d 100644
+index f5e3552193..592b20f7ad 100644
 --- a/pbs-restore.c
 +++ b/pbs-restore.c
 @@ -81,6 +81,7 @@ int main(int argc, char **argv)
-- 
2.47.3

[PATCH qemu v2 3/9] migration/qemu-file: allocate buffer with g_malloc

[Erik Fastermann]

Plain malloc() can fail and the result was never checked, while
savevm-async asks for a 4 MiB buffer. g_malloc() exits instead of
leaving a NULL buffer for the first write to dereference.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 .../0018-PVE-add-optional-buffer-size-to-QEMUFile.patch   | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

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 ab8aac1..b56dd34 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
@@ -12,6 +12,8 @@ Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 [FE: rebase for 10.2.0]
 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+[EF: allocate buffer with g_malloc]
+Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
 ---
  migration/qemu-file.c    | 48 +++++++++++++++++++++++++++-------------
  migration/qemu-file.h    |  2 ++
@@ -19,7 +21,7 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
  3 files changed, 38 insertions(+), 17 deletions(-)
 
 diff --git a/migration/qemu-file.c b/migration/qemu-file.c
-index 9cf7dc3bd5..4a4de51fd9 100644
+index 9cf7dc3bd5..b1b7aa453e 100644
 --- a/migration/qemu-file.c
 +++ b/migration/qemu-file.c
 @@ -34,8 +34,8 @@
@@ -59,7 +61,7 @@ index 9cf7dc3bd5..4a4de51fd9 100644
      f->can_pass_fd = qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_FD_PASS);
      QTAILQ_INIT(&f->fds);
 +    f->buf_allocated_size = buffer_size;
-+    f->buf = malloc(buffer_size);
++    f->buf = g_malloc(buffer_size);
  
      return f;
  }
@@ -108,7 +110,7 @@ index 9cf7dc3bd5..4a4de51fd9 100644
      }
      g_clear_pointer(&f->ioc, object_unref);
 +
-+    free(f->buf);
++    g_free(f->buf);
 +
      error_free(f->last_error_obj);
      g_free(f);
-- 
2.47.3

[PATCH qemu v2 4/9] migration/pbs-state: check allocation of incoming state buffer

[Erik Fastermann]

The length is read from the migration stream, so it cannot be trusted.
An unchecked malloc() means a corrupt or truncated state file makes
qemu_get_buffer() write through a NULL pointer. Also the short read path
returned without freeing the buffer.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 ...igrate-dirty-bitmap-state-via-savevm.patch | 25 +++++++++++++------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/debian/patches/pve/0033-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0033-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
index adb41f4..4ce1d8c 100644
--- a/debian/patches/pve/0033-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
+++ b/debian/patches/pve/0033-PVE-Migrate-dirty-bitmap-state-via-savevm.patch
@@ -15,14 +15,16 @@ Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
 [FE: split up state_pending for 8.0]
 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+[EF: check allocation of incoming state buffer]
+Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
 ---
  include/migration/misc.h |   3 ++
  migration/meson.build    |   2 +
  migration/migration.c    |   1 +
- migration/pbs-state.c    | 104 +++++++++++++++++++++++++++++++++++++++
+ migration/pbs-state.c    | 111 +++++++++++++++++++++++++++++++++++++++
  pve-backup.c             |   1 +
  qapi/block-core.json     |   6 +++
- 6 files changed, 117 insertions(+)
+ 6 files changed, 124 insertions(+)
  create mode 100644 migration/pbs-state.c
 
 diff --git a/include/migration/misc.h b/include/migration/misc.h
@@ -71,10 +73,10 @@ index dfc60372cf..f415448689 100644
  typedef struct {
 diff --git a/migration/pbs-state.c b/migration/pbs-state.c
 new file mode 100644
-index 0000000000..a97187e4d7
+index 0000000000..c0c7f2ff6f
 --- /dev/null
 +++ b/migration/pbs-state.c
-@@ -0,0 +1,104 @@
+@@ -0,0 +1,111 @@
 +/*
 + * PBS (dirty-bitmap) state migration
 + */
@@ -104,18 +106,25 @@ index 0000000000..a97187e4d7
 +{
 +    /* safe cast, we cannot migrate to target with less bits than source */
 +    size_t buf_size = (size_t)qemu_get_be64(f);
++    if (buf_size == 0) {
++        return 0;
++    }
 +
-+    uint8_t *buf = (uint8_t *)malloc(buf_size);
-+    size_t read = qemu_get_buffer(f, buf, buf_size);
++    g_autofree uint8_t *buf = g_try_malloc(buf_size);
++    if (!buf) {
++        fprintf(stderr,
++                "error receiving PBS state: cannot allocate %zu bytes\n",
++                buf_size);
++        return -ENOMEM;
++    }
 +
++    size_t read = qemu_get_buffer(f, buf, buf_size);
 +    if (read < buf_size) {
 +        fprintf(stderr, "error receiving PBS state: not enough data\n");
 +        return -EIO;
 +    }
 +
 +    proxmox_import_state(buf, buf_size);
-+
-+    free(buf);
 +    return 0;
 +}
 +
-- 
2.47.3

[PATCH qemu v2 5/9] savevm-async: drop QIOChannel reference

[Erik Fastermann]

qemu_file_new_impl() takes its own reference and qemu_fclose() drops
exactly one, so the reference from qio_channel_savevm_async_new() was
never released. The channel also holds a BlockBackend reference that
only its finalizer drops, so both leaked on every snapshot.

Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
---
 ...async-for-background-state-snapshots.patch | 15 ++++++++++-----
 ...add-optional-buffer-size-to-QEMUFile.patch | 19 +++++++++----------
 ...se-migration-blocker-check-for-snaps.patch |  2 +-
 3 files changed, 20 insertions(+), 16 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 ce9df5c..f252236 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,19 +37,21 @@ 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]
+Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
 ---
  hmp-commands-info.hx         |  13 +
  hmp-commands.hx              |  17 +
  include/migration/snapshot.h |   2 +
  include/monitor/hmp.h        |   3 +
  migration/meson.build        |   1 +
- migration/savevm-async.c     | 597 +++++++++++++++++++++++++++++++++++
+ migration/savevm-async.c     | 600 +++++++++++++++++++++++++++++++++++
  monitor/hmp-cmds.c           |  38 +++
  qapi/migration.json          |  34 ++
  qapi/misc.json               |  25 ++
  qemu-options.hx              |  12 +
  system/vl.c                  |  10 +
- 11 files changed, 752 insertions(+)
+ 11 files changed, 755 insertions(+)
  create mode 100644 migration/savevm-async.c
 
 diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
@@ -147,10 +149,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..2a860f11a1
+index 0000000000..cab08643ad
 --- /dev/null
 +++ b/migration/savevm-async.c
-@@ -0,0 +1,597 @@
+@@ -0,0 +1,600 @@
 +#include "qemu/osdep.h"
 +#include "migration/channel-savevm-async.h"
 +#include "migration/migration.h"
@@ -572,6 +574,7 @@ index 0000000000..2a860f11a1
 +    QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(snap_state.target,
 +                                                               &snap_state.bs_pos));
 +    snap_state.file = qemu_file_new_output(ioc);
++    object_unref(ioc);
 +
 +    if (!snap_state.file) {
 +        error_setg(errp, "failed to open '%s'", statefile);
@@ -708,7 +711,9 @@ index 0000000000..2a860f11a1
 +    bdrv_op_block_all(bs, blocker);
 +
 +    /* restore the VM state */
-+    f = qemu_file_new_input(QIO_CHANNEL(qio_channel_savevm_async_new(be, &bs_pos)));
++    QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(be, &bs_pos));
++    f = qemu_file_new_input(ioc);
++    object_unref(ioc);
 +    if (!f) {
 +        error_setg(errp, "Could not open VM state file");
 +        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 b56dd34..39cacf7 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
@@ -17,8 +17,8 @@ Signed-off-by: Erik Fastermann <e.fastermann@proxmox.com>
 ---
  migration/qemu-file.c    | 48 +++++++++++++++++++++++++++-------------
  migration/qemu-file.h    |  2 ++
- migration/savevm-async.c |  5 +++--
- 3 files changed, 38 insertions(+), 17 deletions(-)
+ migration/savevm-async.c |  4 ++--
+ 3 files changed, 37 insertions(+), 17 deletions(-)
 
 diff --git a/migration/qemu-file.c b/migration/qemu-file.c
 index 9cf7dc3bd5..b1b7aa453e 100644
@@ -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 2a860f11a1..f5a4819e27 100644
+index cab08643ad..f9eaf3b8e6 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,
@@ -195,16 +195,15 @@ index 2a860f11a1..f5a4819e27 100644
                                                                 &snap_state.bs_pos));
 -    snap_state.file = qemu_file_new_output(ioc);
 +    snap_state.file = qemu_file_new_output_sized(ioc, 4 * 1024 * 1024);
+     object_unref(ioc);
  
      if (!snap_state.file) {
-         error_setg(errp, "failed to open '%s'", statefile);
-@@ -555,7 +555,8 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
-     bdrv_op_block_all(bs, blocker);
+@@ -557,7 +557,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
  
      /* restore the VM state */
--    f = qemu_file_new_input(QIO_CHANNEL(qio_channel_savevm_async_new(be, &bs_pos)));
-+    f = qemu_file_new_input_sized(QIO_CHANNEL(qio_channel_savevm_async_new(be, &bs_pos)),
-+                                  4 * 1024 * 1024);
+     QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(be, &bs_pos));
+-    f = qemu_file_new_input(ioc);
++    f = qemu_file_new_input_sized(ioc, 4 * 1024 * 1024);
+     object_unref(ioc);
      if (!f) {
          error_setg(errp, "Could not open VM state file");
-         goto the_end;
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 25a06a6..5cc3893 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 f5a4819e27..41376406eb 100644
+index f9eaf3b8e6..51340c5dcd 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,
-- 
2.47.3

Re: [PATCH qemu v2 5/9] savevm-async: drop QIOChannel reference

[Fiona Ebner]

Am 07.09.26 um 11:49 AM schrieb Erik Fastermann:
> qemu_file_new_impl() takes its own reference and qemu_fclose() drops
> exactly one, so the reference from qio_channel_savevm_async_new() was
> never released. The channel also holds a BlockBackend reference that
> only its finalizer drops, so both leaked on every snapshot.

>From a quick look, it seems like there are places in upstream code that
suffer from the same issue as well. For example, qemu_fopen_bdrv().
Could you check this and send patches upstream to fix the actually
affected ones?

https://www.qemu.org/docs/master/devel/submitting-a-patch.html

[PATCH qemu v2 6/9] savevm-async: free migration vmdesc

[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 | 27 ++++++++++++-------
 ...add-optional-buffer-size-to-QEMUFile.patch |  6 ++---
 ...se-migration-blocker-check-for-snaps.patch |  4 +--
 3 files changed, 23 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..8750897 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     | 608 +++++++++++++++++++++++++++++++++++
  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, 763 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..4f44c2f12d
 --- /dev/null
 +++ b/migration/savevm-async.c
-@@ -0,0 +1,600 @@
+@@ -0,0 +1,608 @@
 +#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
@@ -618,6 +625,8 @@ index 0000000000..cab08643ad
 +    return;
 +
 +fail:
++    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..d73d432 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 4f44c2f12d..0025d69284 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,
+@@ -424,7 +424,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)
+@@ -565,7 +565,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..66b6b7b 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 0025d69284..73ba96ba25 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,
+@@ -390,7 +390,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
          return;
      }
  
-- 
2.47.3

Re: [PATCH qemu v2 6/9] savevm-async: free migration vmdesc

[Fiona Ebner]

Am 07.09.26 um 11:48 AM schrieb Erik Fastermann:
>  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..4f44c2f12d
>  --- /dev/null
>  +++ b/migration/savevm-async.c
> -@@ -0,0 +1,600 @@
> +@@ -0,0 +1,608 @@
>  +#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
> @@ -618,6 +625,8 @@ index 0000000000..cab08643ad
>  +    return;
>  +
>  +fail:
> ++    migration_state_cleanup(ms, true);

While fine in practice, because the implementations handle it, it reads
wrong to have the migrate_set_state() and g_clear_pointer(&ms->vmdesc,
...) calls when we fail before having a successful migrate_init(). Can
we guard this call based on whether we already have a successful init?

> ++
>  +    savevm_cleanup_iothread();
>  +    save_snapshot_error("setup failed");
>  +}

[PATCH qemu v2 7/9] savevm-async: release resources on start failure path

[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 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 <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     | 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

[PATCH qemu v2 8/9] savevm-async: include reason when file open fails

[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 d21fb1f..94908d7 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     | 632 +++++++++++++++++++++++++++++++++++
+ migration/savevm-async.c     | 634 +++++++++++++++++++++++++++++++++++
  monitor/hmp-cmds.c           |  38 +++
  qapi/migration.json          |  34 ++
  qapi/misc.json               |  25 ++
  qemu-options.hx              |  12 +
  system/vl.c                  |  10 +
- 11 files changed, 787 insertions(+)
+ 11 files changed, 789 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..d688fc7112
+index 0000000000..56c60b345f
 --- /dev/null
 +++ b/migration/savevm-async.c
-@@ -0,0 +1,632 @@
+@@ -0,0 +1,634 @@
 +#include "qemu/osdep.h"
 +#include "migration/channel-savevm-async.h"
 +#include "migration/migration.h"
@@ -570,7 +571,8 @@ index 0000000000..d688fc7112
 +    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);
@@ -729,7 +731,8 @@ index 0000000000..d688fc7112
 +    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 cac1842..4345f42 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 d688fc7112..24f7ba60c9 100644
+index 56c60b345f..a460970dd5 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,
+@@ -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 d688fc7112..24f7ba60c9 100644
      object_unref(ioc);
  
      if (!snap_state.file) {
-@@ -587,7 +587,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
+@@ -589,7 +589,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 98a0ca4..d87875b 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 24f7ba60c9..78fc27b320 100644
+index a460970dd5..b703c5d5a3 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

[PATCH qemu v2 9/9] savevm-async: restore run state from before snapshot

[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 | 57 ++++++++++++-------
 ...add-optional-buffer-size-to-QEMUFile.patch |  6 +-
 ...se-migration-blocker-check-for-snaps.patch |  4 +-
 3 files changed, 41 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 94908d7..9a4a049 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     | 634 +++++++++++++++++++++++++++++++++++
- monitor/hmp-cmds.c           |  38 +++
+ migration/savevm-async.c     | 648 +++++++++++++++++++++++++++++++++++
+ monitor/hmp-cmds.c           |  38 ++
  qapi/migration.json          |  34 ++
  qapi/misc.json               |  25 ++
  qemu-options.hx              |  12 +
  system/vl.c                  |  10 +
- 11 files changed, 789 insertions(+)
+ 11 files changed, 803 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..56c60b345f
+index 0000000000..fe455016ab
 --- /dev/null
 +++ b/migration/savevm-async.c
-@@ -0,0 +1,634 @@
+@@ -0,0 +1,648 @@
 +#include "qemu/osdep.h"
 +#include "migration/channel-savevm-async.h"
 +#include "migration/migration.h"
@@ -210,7 +211,7 @@ index 0000000000..56c60b345f
 +    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..56c60b345f
 +    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..56c60b345f
 +     */
 +    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..56c60b345f
 +        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);
@@ -529,12 +543,18 @@ index 0000000000..56c60b345f
 +        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);
@@ -542,7 +562,7 @@ index 0000000000..56c60b345f
 +    }
 +
 +    if (!statefile) {
-+        snap_state.vm_needs_start = runstate_is_running();
++        snap_state.vm_old_state = runstate_get();
 +        vm_stop(RUN_STATE_SAVE_VM);
 +        snap_state.state = SAVE_STATE_COMPLETED;
 +        return;
@@ -703,12 +723,7 @@ index 0000000000..56c60b345f
 +        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 4345f42..e29c721 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 56c60b345f..a460970dd5 100644
+index fe455016ab..91a3f404fb 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,
+@@ -444,7 +444,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 56c60b345f..a460970dd5 100644
      object_unref(ioc);
  
      if (!snap_state.file) {
-@@ -589,7 +589,7 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
+@@ -603,7 +603,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 d87875b..410f13b 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 a460970dd5..b703c5d5a3 100644
+index 91a3f404fb..6c16000898 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,
+@@ -409,7 +409,7 @@ void qmp_savevm_start(const char *statefile, bool has_skip_vm_start,
          return;
      }
  
-- 
2.47.3

Re: [PATCH qemu v2 9/9] savevm-async: restore run state from before snapshot

[Fiona Ebner]

Am 07.09.26 um 11:49 AM schrieb Erik Fastermann:
> @@ -230,6 +231,24 @@ index 0000000000..56c60b345f
>  +    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)) {

I'd prefer this condition to be dropped or extended to also check for
RUN_STATE_SAVE_VM. Because when not using a statefile, we call
vm_stop(RUN_STATE_SAVE_VM) and do not set the state to
RUN_STATE_FINISH_MIGRATE. It might be fine in practice, because if the
runstate was not live, then RUN_STATE_SAVE_VM is not actually applied,
as vm_stop() skips setting the state if already not live. But still, it
would be good to be explicit about it.

> ++        runstate_set(old_state);
> ++    }
> ++}
> ++

partially-applied: [PATCH qemu v2 0/9] savevm-async: fix stuck paused vm after snapshot

[Fiona Ebner]

On Mon, 07 Sep 2026 11:47:34 +0200, Erik Fastermann wrote:
> 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.
> 
> [...]

Applied the first five patches, thanks!

[1/9] d/patches: normalize header of pbs-restore no-cache patch
      commit: 7498b1bd7b71da1465631267620400b541d1de15
[2/9] pbs-restore: keep callback data on the stack
      commit: 10fd013483270154354657766e6645ca3e63af24
[3/9] migration/qemu-file: allocate buffer with g_malloc
      commit: 45d5a170a7c8628b1c743417b7e3d2a65a67033f
[4/9] migration/pbs-state: check allocation of incoming state buffer
      commit: b6f2adbcea48c6b13613d638fc45643d4181471e
[5/9] savevm-async: drop QIOChannel reference
      commit: 513917f9a745c641c6a783d35dd340beb469b785