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/[email protected]/


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



Reply via email to