[pve-devel] [PATCH qemu 3/4] async snapshot: improve error handling for 'savevm-start' QMP command

[Fiona Ebner]

Return values for qemu_savevm_state_setup() and blk_set_aio_context()
now get checked.

Move the qemu_coroutine_create() call to after the new early return
to avoid a potential memory leak.

Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---
 ...async-for-background-state-snapshots.patch | 26 ++++++++++++++-----
 ...add-optional-buffer-size-to-QEMUFile.patch |  6 ++---
 2 files changed, 22 insertions(+), 10 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 1fe4648..4e9c6bf 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,13 +37,13 @@ Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
  include/migration/snapshot.h |   2 +
  include/monitor/hmp.h        |   3 +
  migration/meson.build        |   1 +
- migration/savevm-async.c     | 537 +++++++++++++++++++++++++++++++++++
+ migration/savevm-async.c     | 549 +++++++++++++++++++++++++++++++++++
  monitor/hmp-cmds.c           |  38 +++
  qapi/migration.json          |  34 +++
  qapi/misc.json               |  18 ++
  qemu-options.hx              |  12 +
  system/vl.c                  |  10 +
- 11 files changed, 685 insertions(+)
+ 11 files changed, 697 insertions(+)
  create mode 100644 migration/savevm-async.c
 
 diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
@@ -141,10 +141,10 @@ index 020127d901..4b0c4f0f51 100644
    'threadinfo.c',
 diff --git a/migration/savevm-async.c b/migration/savevm-async.c
 new file mode 100644
-index 0000000000..59bb0b57d9
+index 0000000000..4c90209188
 --- /dev/null
 +++ b/migration/savevm-async.c
-@@ -0,0 +1,537 @@
+@@ -0,0 +1,549 @@
 +#include "qemu/osdep.h"
 +#include "migration/channel-savevm-async.h"
 +#include "migration/migration.h"
@@ -167,6 +167,7 @@ index 0000000000..59bb0b57d9
 +#include "qapi/qapi-commands-misc.h"
 +#include "qapi/qapi-commands-block.h"
 +#include "qemu/cutils.h"
++#include "qemu/error-report.h"
 +#include "qemu/timer.h"
 +#include "qemu/main-loop.h"
 +#include "qemu/rcu.h"
@@ -479,6 +480,7 @@ index 0000000000..59bb0b57d9
 +    Error *local_err = NULL;
 +    MigrationState *ms = migrate_get_current();
 +    AioContext *iohandler_ctx = iohandler_get_aio_context();
++    int ret = 0;
 +
 +    int bdrv_oflags = BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_NO_FLUSH;
 +
@@ -549,15 +551,25 @@ index 0000000000..59bb0b57d9
 +
 +    snap_state.state = SAVE_STATE_ACTIVE;
 +    snap_state.finalize_bh = qemu_bh_new(process_savevm_finalize, &snap_state);
-+    snap_state.co = qemu_coroutine_create(&process_savevm_co, NULL);
 +    qemu_savevm_state_header(snap_state.file);
-+    qemu_savevm_state_setup(snap_state.file, &local_err);
++    ret = qemu_savevm_state_setup(snap_state.file, &local_err);
++    if (ret != 0) {
++        error_setg_errno(errp, -ret, "savevm state setup failed: %s",
++                         local_err ? error_get_pretty(local_err) : "unknown error");
++        return;
++    }
 +
 +    /* Async processing from here on out happens in iohandler context, so let
 +     * the target bdrv have its home there.
 +     */
-+    blk_set_aio_context(snap_state.target, iohandler_ctx, &local_err);
++    ret = blk_set_aio_context(snap_state.target, iohandler_ctx, &local_err);
++    if (ret != 0) {
++        warn_report("failed to set iohandler context for VM state target: %s %s",
++                    local_err ? error_get_pretty(local_err) : "unknown error",
++                    strerror(-ret));
++    }
 +
++    snap_state.co = qemu_coroutine_create(&process_savevm_co, NULL);
 +    aio_co_schedule(iohandler_ctx, snap_state.co);
 +
 +    return;
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 cd2e2d2..67d6ba8 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
@@ -184,10 +184,10 @@ index 11c2120edd..edf3c5d147 100644
  
  /*
 diff --git a/migration/savevm-async.c b/migration/savevm-async.c
-index 59bb0b57d9..9a4dd1e4f5 100644
+index 4c90209188..eb562d3dcf 100644
 --- a/migration/savevm-async.c
 +++ b/migration/savevm-async.c
-@@ -379,7 +379,7 @@ void qmp_savevm_start(const char *statefile, Error **errp)
+@@ -381,7 +381,7 @@ void qmp_savevm_start(const char *statefile, Error **errp)
  
      QIOChannel *ioc = QIO_CHANNEL(qio_channel_savevm_async_new(snap_state.target,
                                                                 &snap_state.bs_pos));
@@ -196,7 +196,7 @@ index 59bb0b57d9..9a4dd1e4f5 100644
  
      if (!snap_state.file) {
          error_setg(errp, "failed to open '%s'", statefile);
-@@ -502,7 +502,8 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
+@@ -514,7 +514,8 @@ int load_snapshot_from_blockdev(const char *filename, Error **errp)
      blk_op_block_all(be, blocker);
  
      /* restore the VM state */
-- 
2.39.5