[Qemu-devel] [PATCH v4 0/5] QMP wrappers for VM snapshot operations

[Qemu-devel] [PATCH v4 0/5] QMP wrappers for VM snapshot operations

[Denis V. Lunev]

EFI based VM with pflash storage for NVRAM could not be snapshoted as
libvirt configures storage as 'raw' and writable. OK, this is a libvirt
problem.

Another problem is that libvirt can not detect this failure at all
as it uses HMP for this operation. This create snapshot/delete snapshot
sequence passes silently.

The patchset adds QMP wrappers for the purpose.

Signed-off-by: "Denis V. Lunev" <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>

Changes from v3:
- wrong patch 1 is replaced

Changes from v2:
- patches 1/2 are resplit to move processing HMP specific handling
  of snapshot name generation to exclusive HMP code
- removed all '.' at the end of error_setg strings
- fixed too long lines with '-' in qmp-commands.hx
- error_setg_errno errno passing is fixed (-ret)
- fixed logical error in hmp_loadvm (vm_start on error)
- NOT switched to error_prepend code (it is not yet merged). Can we do this
  later? This will make my life easear merging code to our downstream.

Changes from v1:
- cosmetic fixes suggested by Markus. I pray I have added all of them
- patch 5 is rewritten completely. Original one was deadbeaf

Denis V. Lunev (5):
  migration: split hmp_savevm to migrate_savevm and hmp_savevm wrapper
  qmp: create qmp_savevm command
  qmp: create qmp_delvm command
  migration: improve error reporting for load_vmstate
  qmp: create QMP implementation of loadvm command

 hmp.c                         |  36 +++++++++++++++
 include/migration/migration.h |   1 +
 include/sysemu/sysemu.h       |   2 +-
 migration/savevm.c            | 100 +++++++++++++++++++++---------------------
 monitor.c                     |   9 ++--
 qapi-schema.json              |  39 ++++++++++++++++
 qmp-commands.hx               |  71 ++++++++++++++++++++++++++++++
 vl.c                          |   4 +-
 8 files changed, 206 insertions(+), 56 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to migrate_savevm and hmp_savevm wrapper

[Denis V. Lunev]

This would be useful in the next step when QMP version of this call will
be introduced. The patch also moves snapshot name generation to the
hmp specific code as QMP version of this code will require the name
on the protocol level.

Addition of migration_savevm to migration/migration.h is temporary. It
will be removed in the next patch with QMP level change.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
---
 hmp.c                         | 26 ++++++++++++++++++++++++
 include/migration/migration.h |  3 +++
 migration/savevm.c            | 46 +++++++++++++++++--------------------------
 3 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/hmp.c b/hmp.c
index c2b2c16..e7bab75 100644
--- a/hmp.c
+++ b/hmp.c
@@ -18,6 +18,7 @@
 #include "net/eth.h"
 #include "sysemu/char.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/sysemu.h"
 #include "qemu/option.h"
 #include "qemu/timer.h"
 #include "qmp-commands.h"
@@ -32,6 +33,7 @@
 #include "ui/console.h"
 #include "block/qapi.h"
 #include "qemu-io.h"
+#include "migration/migration.h"
 
 #ifdef CONFIG_SPICE
 #include <spice/enums.h>
@@ -2386,3 +2388,27 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
 
     qapi_free_RockerOfDpaGroupList(list);
 }
+
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+    const char *name = qdict_get_try_str(qdict, "name");
+    char name_buf[64];
+
+    if (name == NULL) {
+        qemu_timeval tv;
+        struct tm tm;
+
+        /* cast below needed for OpenBSD where tv_sec is still 'long' */
+        localtime_r((const time_t *)&tv.tv_sec, &tm);
+        strftime(name_buf, sizeof(name_buf), "vm-%Y%m%d%H%M%S", &tm);
+
+        name = name_buf;
+    }
+
+    migrate_savevm(name, &local_err);
+
+    if (local_err != NULL) {
+        error_report_err(local_err);
+    }
+}
diff --git a/include/migration/migration.h b/include/migration/migration.h
index d9494b8..73c8bb1 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -277,6 +277,8 @@ int migrate_compress_threads(void);
 int migrate_decompress_threads(void);
 bool migrate_use_events(void);
 
+void migrate_savevm(const char *name, Error **errp);
+
 /* Sending on the return path - generic and then for each message type */
 void migrate_send_rp_message(MigrationIncomingState *mis,
                              enum mig_rp_message_type message_type,
@@ -321,4 +323,5 @@ int ram_save_queue_pages(MigrationState *ms, const char *rbname,
 PostcopyState postcopy_state_get(void);
 /* Set the state and return the old state */
 PostcopyState postcopy_state_set(PostcopyState new_state);
+
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..308302a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
     return ret;
 }
 
-void hmp_savevm(Monitor *mon, const QDict *qdict)
+void migrate_savevm(const char *name, Error **errp)
 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
@@ -1914,29 +1914,27 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     int saved_vm_running;
     uint64_t vm_state_size;
     qemu_timeval tv;
-    struct tm tm;
-    const char *name = qdict_get_try_str(qdict, "name");
     Error *local_err = NULL;
     AioContext *aio_context;
 
     if (!bdrv_all_can_snapshot(&bs)) {
-        monitor_printf(mon, "Device '%s' is writable but does not "
-                       "support snapshots.\n", bdrv_get_device_name(bs));
+        error_setg(errp,
+                   "Device '%s' is writable but does not support snapshots",
+                   bdrv_get_device_name(bs));
         return;
     }
 
     /* Delete old snapshots of the same name */
-    if (name && bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
-        monitor_printf(mon,
-                       "Error while deleting snapshot on device '%s': %s\n",
-                       bdrv_get_device_name(bs1), error_get_pretty(local_err));
+    if (bdrv_all_delete_snapshot(name, &bs1, &local_err) < 0) {
+        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+                   bdrv_get_device_name(bs1), error_get_pretty(local_err));
         error_free(local_err);
         return;
     }
 
     bs = bdrv_all_find_vmstate_bs();
     if (bs == NULL) {
-        monitor_printf(mon, "No block device can accept snapshots\n");
+        error_setg(errp, "No block device can accept snapshots");
         return;
     }
     aio_context = bdrv_get_aio_context(bs);
@@ -1945,7 +1943,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 
     ret = global_state_store();
     if (ret) {
-        monitor_printf(mon, "Error saving global state\n");
+        error_setg(errp, "Error saving global state");
         return;
     }
     vm_stop(RUN_STATE_SAVE_VM);
@@ -1960,39 +1958,31 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     sn->date_nsec = tv.tv_usec * 1000;
     sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 
-    if (name) {
-        ret = bdrv_snapshot_find(bs, old_sn, name);
-        if (ret >= 0) {
-            pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
-            pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
-        } else {
-            pstrcpy(sn->name, sizeof(sn->name), name);
-        }
+    ret = bdrv_snapshot_find(bs, old_sn, name);
+    if (ret >= 0) {
+        pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
+        pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
     } else {
-        /* cast below needed for OpenBSD where tv_sec is still 'long' */
-        localtime_r((const time_t *)&tv.tv_sec, &tm);
-        strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm);
+        pstrcpy(sn->name, sizeof(sn->name), name);
     }
 
     /* save the VM state */
     f = qemu_fopen_bdrv(bs, 1);
     if (!f) {
-        monitor_printf(mon, "Could not open VM state file\n");
+        error_setg(errp, "Could not open VM state file");
         goto the_end;
     }
-    ret = qemu_savevm_state(f, &local_err);
+    ret = qemu_savevm_state(f, errp);
     vm_state_size = qemu_ftell(f);
     qemu_fclose(f);
     if (ret < 0) {
-        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
-        error_free(local_err);
         goto the_end;
     }
 
     ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, &bs);
     if (ret < 0) {
-        monitor_printf(mon, "Error while creating snapshot on '%s'\n",
-                       bdrv_get_device_name(bs));
+        error_setg(errp, "Error while creating snapshot on '%s'",
+                   bdrv_get_device_name(bs));
     }
 
  the_end:
-- 
2.5.0

[Qemu-devel] [PATCH 2/5] qmp: create qmp_savevm command

[Denis V. Lunev]

'name' attribute is made mandatory in distinction with HMP command.

The patch also moves hmp_savevm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 hmp.c                         |  3 +--
 include/migration/migration.h |  2 --
 migration/savevm.c            |  2 +-
 qapi-schema.json              | 13 +++++++++++++
 qmp-commands.hx               | 25 +++++++++++++++++++++++++
 5 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/hmp.c b/hmp.c
index e7bab75..8d6eda6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -33,7 +33,6 @@
 #include "ui/console.h"
 #include "block/qapi.h"
 #include "qemu-io.h"
-#include "migration/migration.h"
 
 #ifdef CONFIG_SPICE
 #include <spice/enums.h>
@@ -2406,7 +2405,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
         name = name_buf;
     }
 
-    migrate_savevm(name, &local_err);
+    qmp_savevm(name, &local_err);
 
     if (local_err != NULL) {
         error_report_err(local_err);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 73c8bb1..58c04a9 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -277,8 +277,6 @@ int migrate_compress_threads(void);
 int migrate_decompress_threads(void);
 bool migrate_use_events(void);
 
-void migrate_savevm(const char *name, Error **errp);
-
 /* Sending on the return path - generic and then for each message type */
 void migrate_send_rp_message(MigrationIncomingState *mis,
                              enum mig_rp_message_type message_type,
diff --git a/migration/savevm.c b/migration/savevm.c
index 308302a..0dbb15f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
     return ret;
 }
 
-void migrate_savevm(const char *name, Error **errp)
+void qmp_savevm(const char *name, Error **errp)
 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
diff --git a/qapi-schema.json b/qapi-schema.json
index 2e31733..09d1a1a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4054,3 +4054,16 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @savevm
+#
+# Save a VM snapshot. Old snapshot with the same name will be deleted if exists.
+#
+# @name: identifier of a snapshot to be created
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'savevm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db072a6..b7851e1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4795,3 +4795,28 @@ Example:
                  {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
                   "pop-vlan": 1, "id": 251658240}
    ]}
+
+EQMP
+
+SQMP
+savevm
+------
+
+Save a VM snapshot. Old snapshot with the same name will be deleted if exists.
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "savevm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "savevm",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_savevm,
+    },
-- 
2.5.0

[Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Denis V. Lunev]

The patch also moves hmp_delvm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 hmp.c              | 11 +++++++++++
 migration/savevm.c | 16 +++++++---------
 qapi-schema.json   | 13 +++++++++++++
 qmp-commands.hx    | 23 +++++++++++++++++++++++
 4 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/hmp.c b/hmp.c
index 8d6eda6..f65bbe7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2411,3 +2411,14 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
         error_report_err(local_err);
     }
 }
+
+void hmp_delvm(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+
+    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
+
+    if (local_err != NULL) {
+        error_report_err(local_err);
+    }
+}
diff --git a/migration/savevm.c b/migration/savevm.c
index 0dbb15f..1262b57 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2092,17 +2092,15 @@ int load_vmstate(const char *name)
     return 0;
 }
 
-void hmp_delvm(Monitor *mon, const QDict *qdict)
+void qmp_delvm(const char *name, Error **errp)
 {
     BlockDriverState *bs;
-    Error *err;
-    const char *name = qdict_get_str(qdict, "name");
-
-    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
-        monitor_printf(mon,
-                       "Error while deleting snapshot on device '%s': %s\n",
-                       bdrv_get_device_name(bs), error_get_pretty(err));
-        error_free(err);
+    Error *local_err = NULL;
+
+    if (bdrv_all_delete_snapshot(name, &bs, &local_err) < 0) {
+        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+                   bdrv_get_device_name(bs), error_get_pretty(local_err));
+        error_free(local_err);
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 09d1a1a..94a2764 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4067,3 +4067,16 @@
 # Since 2.6
 ##
 { 'command': 'savevm', 'data': {'name': 'str'} }
+
+##
+# @delvm
+#
+# Delete a VM snapshot
+#
+# @name: identifier of a snapshot to be deleted
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b7851e1..5e6f573 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4820,3 +4820,26 @@ EQMP
         .args_type  = "name:s",
         .mhandler.cmd_new = qmp_marshal_savevm,
     },
+
+SQMP
+delvm
+-----
+
+Delete a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "delvm",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_delvm,
+    },
-- 
2.5.0

[Qemu-devel] [PATCH 4/5] migration: improve error reporting for load_vmstate

[Denis V. Lunev]

The patch adds Error ** parameter to load_vmstate call and fills error
inside. The caller after that properly reports error either through
monitor or via local stderr facility during VM start.

This helper will be useful too for qmp_loadvm implementation.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 include/sysemu/sysemu.h |  2 +-
 migration/savevm.c      | 28 ++++++++++++++++------------
 monitor.c               |  5 ++++-
 vl.c                    |  4 +++-
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 3bb8897..d9ccf45 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -78,7 +78,7 @@ void qemu_remove_exit_notifier(Notifier *notify);
 void qemu_add_machine_init_done_notifier(Notifier *notify);
 
 void hmp_savevm(Monitor *mon, const QDict *qdict);
-int load_vmstate(const char *name);
+int load_vmstate(const char *name, Error **errp);
 void hmp_delvm(Monitor *mon, const QDict *qdict);
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 1262b57..3de917e 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2019,7 +2019,7 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
     }
 }
 
-int load_vmstate(const char *name)
+int load_vmstate(const char *name, Error **errp)
 {
     BlockDriverState *bs, *bs_vm_state;
     QEMUSnapshotInfo sn;
@@ -2028,20 +2028,22 @@ int load_vmstate(const char *name)
     AioContext *aio_context;
 
     if (!bdrv_all_can_snapshot(&bs)) {
-        error_report("Device '%s' is writable but does not support snapshots.",
-                     bdrv_get_device_name(bs));
+        error_setg(errp,
+                   "Device '%s' is writable but does not support snapshots",
+                   bdrv_get_device_name(bs));
         return -ENOTSUP;
     }
     ret = bdrv_all_find_snapshot(name, &bs);
     if (ret < 0) {
-        error_report("Device '%s' does not have the requested snapshot '%s'",
-                     bdrv_get_device_name(bs), name);
+        error_setg(errp,
+                   "Device '%s' does not have the requested snapshot '%s'",
+                   bdrv_get_device_name(bs), name);
         return ret;
     }
 
     bs_vm_state = bdrv_all_find_vmstate_bs();
     if (!bs_vm_state) {
-        error_report("No block device supports snapshots");
+        error_setg(errp, "No block device supports snapshots");
         return -ENOTSUP;
     }
     aio_context = bdrv_get_aio_context(bs_vm_state);
@@ -2051,10 +2053,11 @@ int load_vmstate(const char *name)
     ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
     aio_context_release(aio_context);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Snapshot '%s' not found", name);
         return ret;
     } else if (sn.vm_state_size == 0) {
-        error_report("This is a disk-only snapshot. Revert to it offline "
-            "using qemu-img.");
+        error_setg(errp, "Snapshot '%s' is a disk-only snapshot and must be "
+                   "reverted offline using qemu-img", name);
         return -EINVAL;
     }
 
@@ -2063,15 +2066,16 @@ int load_vmstate(const char *name)
 
     ret = bdrv_all_goto_snapshot(name, &bs);
     if (ret < 0) {
-        error_report("Error %d while activating snapshot '%s' on '%s'",
-                     ret, name, bdrv_get_device_name(bs));
+        error_setg_errno(errp, -ret,
+                         "Error while activating snapshot '%s' on '%s'",
+                         name, bdrv_get_device_name(bs));
         return ret;
     }
 
     /* restore the VM state */
     f = qemu_fopen_bdrv(bs_vm_state, 0);
     if (!f) {
-        error_report("Could not open VM state file");
+        error_setg(errp, "Could not open VM state file");
         return -EINVAL;
     }
 
@@ -2085,7 +2089,7 @@ int load_vmstate(const char *name)
 
     migration_incoming_state_destroy();
     if (ret < 0) {
-        error_report("Error %d while loading VM state", ret);
+        error_setg_errno(errp, -ret, "Error while loading VM state");
         return ret;
     }
 
diff --git a/monitor.c b/monitor.c
index e7e7ae2..2ddf6c1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1739,10 +1739,13 @@ static void hmp_loadvm(Monitor *mon, const QDict *qdict)
 {
     int saved_vm_running  = runstate_is_running();
     const char *name = qdict_get_str(qdict, "name");
+    Error *local_err = NULL;
 
     vm_stop(RUN_STATE_RESTORE_VM);
 
-    if (load_vmstate(name) == 0 && saved_vm_running) {
+    if (load_vmstate(name, &local_err) < 0) {
+        error_report_err(local_err);
+    } else if (saved_vm_running) {
         vm_start();
     }
 }
diff --git a/vl.c b/vl.c
index 5aaea77..0172e42 100644
--- a/vl.c
+++ b/vl.c
@@ -4648,8 +4648,10 @@ int main(int argc, char **argv, char **envp)
     qemu_system_reset(VMRESET_SILENT);
     register_global_state();
     if (loadvm) {
-        if (load_vmstate(loadvm) < 0) {
+        Error *local_err = NULL;
+        if (load_vmstate(loadvm, &local_err) < 0) {
             autostart = 0;
+            error_report_err(local_err);
         }
     }
 
-- 
2.5.0

[Qemu-devel] [PATCH 5/5] qmp: create QMP implementation of loadvm command

[Denis V. Lunev]

Unfortunately load_vmstate has a return code (int) and this code is checked
in the other places. Thus we could not just rename it to qmp_loadvm as
returns void.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 migration/savevm.c | 10 ++++++++++
 monitor.c          |  8 ++------
 qapi-schema.json   | 13 +++++++++++++
 qmp-commands.hx    | 23 +++++++++++++++++++++++
 4 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 3de917e..6b34017 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2096,6 +2096,16 @@ int load_vmstate(const char *name, Error **errp)
     return 0;
 }
 
+void qmp_loadvm(const char *name, Error **errp)
+{
+    int saved_vm_running  = runstate_is_running();
+    vm_stop(RUN_STATE_RESTORE_VM);
+
+    if (load_vmstate(name, errp) == 0 && saved_vm_running) {
+        vm_start();
+    }
+}
+
 void qmp_delvm(const char *name, Error **errp)
 {
     BlockDriverState *bs;
diff --git a/monitor.c b/monitor.c
index 2ddf6c1..c1e998c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1737,16 +1737,12 @@ void qmp_closefd(const char *fdname, Error **errp)
 
 static void hmp_loadvm(Monitor *mon, const QDict *qdict)
 {
-    int saved_vm_running  = runstate_is_running();
     const char *name = qdict_get_str(qdict, "name");
     Error *local_err = NULL;
 
-    vm_stop(RUN_STATE_RESTORE_VM);
-
-    if (load_vmstate(name, &local_err) < 0) {
+    qmp_loadvm(name, &local_err);
+    if (local_err != NULL) {
         error_report_err(local_err);
-    } else if (saved_vm_running) {
-        vm_start();
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 94a2764..7d48948 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4080,3 +4080,16 @@
 # Since 2.6
 ##
 { 'command': 'delvm', 'data': {'name': 'str'} }
+
+##
+# @loadvm
+#
+# Load a VM snapshot
+#
+# @name: identifier of a snapshot to be loaded
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'loadvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5e6f573..9cd1bfe 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4843,3 +4843,26 @@ EQMP
         .args_type  = "name:s",
         .mhandler.cmd_new = qmp_marshal_delvm,
     },
+
+SQMP
+loadvm
+------
+
+Load a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "loadvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "loadvm",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_loadvm,
+    },
-- 
2.5.0

[Qemu-devel] [PATCH v3 0/5] QMP wrappers for VM snapshot operations

[Denis V. Lunev]

EFI based VM with pflash storage for NVRAM could not be snapshoted as
libvirt configures storage as 'raw' and writable. OK, this is a libvirt
problem.

Another problem is that libvirt can not detect this failure at all
as it uses HMP for this operation. This create snapshot/delete snapshot
sequence passes silently.

The patchset adds QMP wrappers for the purpose.

Signed-off-by: "Denis V. Lunev" <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>

Changes from v2:
- patches 1/2 are resplit to move processing HMP specific handling
  of snapshot name generation to exclusive HMP code
- removed all '.' at the end of error_setg strings
- fixed too long lines with '-' in qmp-commands.hx
- error_setg_errno errno passing is fixed (-ret)
- fixed logical error in hmp_loadvm (vm_start on error)
- NOT switched to error_prepend code (it is not yet merged). Can we do this
  later? This will make my life easear merging code to our downstream.

Changes from v1:
- cosmetic fixes suggested by Markus. I pray I have added all of them
- patch 5 is rewritten completely. Original one was deadbeaf

Denis V. Lunev (5):
  qmp: process system-reset event in paused state
  qmp: create qmp_savevm command
  qmp: create qmp_delvm command
  migration: improve error reporting for load_vmstate
  qmp: create QMP implementation of loadvm command

 hmp.c                         | 14 +++++++--
 include/migration/migration.h |  2 --
 include/sysemu/sysemu.h       |  2 +-
 migration/savevm.c            | 56 ++++++++++++++++++++--------------
 monitor.c                     |  9 +++---
 qapi-schema.json              | 39 ++++++++++++++++++++++++
 qmp-commands.hx               | 71 +++++++++++++++++++++++++++++++++++++++++++
 qmp.c                         |  4 +++
 vl.c                          |  4 ++-
 9 files changed, 168 insertions(+), 33 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Denis V. Lunev]

The patch also moves hmp_delvm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 hmp.c              | 11 +++++++++++
 migration/savevm.c | 16 +++++++---------
 qapi-schema.json   | 13 +++++++++++++
 qmp-commands.hx    | 23 +++++++++++++++++++++++
 4 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/hmp.c b/hmp.c
index 8d6eda6..f65bbe7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2411,3 +2411,14 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
         error_report_err(local_err);
     }
 }
+
+void hmp_delvm(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+
+    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
+
+    if (local_err != NULL) {
+        error_report_err(local_err);
+    }
+}
diff --git a/migration/savevm.c b/migration/savevm.c
index 0dbb15f..1262b57 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2092,17 +2092,15 @@ int load_vmstate(const char *name)
     return 0;
 }
 
-void hmp_delvm(Monitor *mon, const QDict *qdict)
+void qmp_delvm(const char *name, Error **errp)
 {
     BlockDriverState *bs;
-    Error *err;
-    const char *name = qdict_get_str(qdict, "name");
-
-    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
-        monitor_printf(mon,
-                       "Error while deleting snapshot on device '%s': %s\n",
-                       bdrv_get_device_name(bs), error_get_pretty(err));
-        error_free(err);
+    Error *local_err = NULL;
+
+    if (bdrv_all_delete_snapshot(name, &bs, &local_err) < 0) {
+        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+                   bdrv_get_device_name(bs), error_get_pretty(local_err));
+        error_free(local_err);
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 09d1a1a..94a2764 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4067,3 +4067,16 @@
 # Since 2.6
 ##
 { 'command': 'savevm', 'data': {'name': 'str'} }
+
+##
+# @delvm
+#
+# Delete a VM snapshot
+#
+# @name: identifier of a snapshot to be deleted
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b7851e1..5e6f573 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4820,3 +4820,26 @@ EQMP
         .args_type  = "name:s",
         .mhandler.cmd_new = qmp_marshal_savevm,
     },
+
+SQMP
+delvm
+-----
+
+Delete a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "delvm",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_delvm,
+    },
-- 
2.5.0

[Qemu-devel] [PATCH v2 for 2.6 0/5] QMP wrappers for VM snapshot operations

[Denis V. Lunev]

EFI based VM with pflash storage for NVRAM could not be snapshoted as
libvirt configures storage as 'raw' and writable. OK, this is a libvirt
problem.

Another problem is that libvirt can not detect this failure at all
as it uses HMP for this operation. This create snapshot/delete snapshot
sequence passes silently.

The patchset adds QMP wrappers for the purpose.

Signed-off-by: "Denis V. Lunev" <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>

Changes from v1:
- cosmetic fixes suggested by Markus. I pray I have added all of them :)
- patch 5 is rewritten completely. Original one was deadbeaf

Denis V. Lunev (5):
  migration: split hmp_savevm to do_savevm and hmp_savevm wrapper
  qmp: create qmp_savevm command
  qmp: create qmp_delvm command
  migration: improve error reporting for hmp_loadvm
  qmp: create QMP implementation of loadvm command

 include/sysemu/sysemu.h |   2 +-
 migration/savevm.c      | 100 +++++++++++++++++++++++++++++++-----------------
 monitor.c               |   7 +++-
 qapi-schema.json        |  39 +++++++++++++++++++
 qmp-commands.hx         |  71 ++++++++++++++++++++++++++++++++++
 vl.c                    |   5 ++-
 6 files changed, 185 insertions(+), 39 deletions(-)

[Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Denis V. Lunev]

The patch also moves hmp_delvm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 hmp.c              | 11 +++++++++++
 migration/savevm.c | 16 +++++++---------
 qapi-schema.json   | 13 +++++++++++++
 qmp-commands.hx    | 23 +++++++++++++++++++++++
 4 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/hmp.c b/hmp.c
index c9c7100..a342c8c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2390,3 +2390,14 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
         error_report_err(local_err);
     }
 }
+
+void hmp_delvm(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+
+    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
+
+    if (local_err != NULL) {
+        error_report_err(local_err);
+    }
+}
diff --git a/migration/savevm.c b/migration/savevm.c
index 65e0081..f87a061 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2099,17 +2099,15 @@ int load_vmstate(const char *name)
     return 0;
 }
 
-void hmp_delvm(Monitor *mon, const QDict *qdict)
+void qmp_delvm(const char *name, Error **errp)
 {
     BlockDriverState *bs;
-    Error *err;
-    const char *name = qdict_get_str(qdict, "name");
-
-    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
-        monitor_printf(mon,
-                       "Error while deleting snapshot on device '%s': %s\n",
-                       bdrv_get_device_name(bs), error_get_pretty(err));
-        error_free(err);
+    Error *local_err = NULL;
+
+    if (bdrv_all_delete_snapshot(name, &bs, &local_err) < 0) {
+        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+                   bdrv_get_device_name(bs), error_get_pretty(local_err));
+        error_free(local_err);
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 0114132..18c9a6c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3984,3 +3984,16 @@
 # Since 2.6
 ##
 { 'command': 'savevm', 'data': {'name': 'str'} }
+
+##
+# @delvm
+#
+# Delete a VM snapshot
+#
+# @name: identifier of a snapshot to be deleted
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index f216c5e..a630e8a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4764,3 +4764,26 @@ EQMP
         .args_type  = "name:s?",
         .mhandler.cmd_new = qmp_marshal_savevm,
     },
+
+SQMP
+delvm
+------------------
+
+Delete a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "delvm",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_delvm,
+    },
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2380 bytes --]

On 12/04/2015 07:44 AM, Denis V. Lunev wrote:
> The patch also moves hmp_delvm implementation into hmp.c. This function
> is just a simple wrapper now and does not have knowledge about
> migration internals.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Amit Shah <amit.shah@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> ---
>  hmp.c              | 11 +++++++++++
>  migration/savevm.c | 16 +++++++---------
>  qapi-schema.json   | 13 +++++++++++++
>  qmp-commands.hx    | 23 +++++++++++++++++++++++
>  4 files changed, 54 insertions(+), 9 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index c9c7100..a342c8c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2390,3 +2390,14 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
>          error_report_err(local_err);
>      }
>  }
> +
> +void hmp_delvm(Monitor *mon, const QDict *qdict)
> +{
> +    Error *local_err = NULL;

Same comment as earlier, about 'err' vs. 'local_err' naming,

> +
> +    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
> +
> +    if (local_err != NULL) {

and redundant comparison to NULL.

> +++ b/migration/savevm.c
> @@ -2099,17 +2099,15 @@ int load_vmstate(const char *name)
>      return 0;
>  }
>  
> -void hmp_delvm(Monitor *mon, const QDict *qdict)
> +void qmp_delvm(const char *name, Error **errp)
>  {
>      BlockDriverState *bs;
> -    Error *err;
> -    const char *name = qdict_get_str(qdict, "name");
> -
> -    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
> -        monitor_printf(mon,
> -                       "Error while deleting snapshot on device '%s': %s\n",
> -                       bdrv_get_device_name(bs), error_get_pretty(err));
> -        error_free(err);
> +    Error *local_err = NULL;

Instead of renaming 'err' to 'local_err',

> +
> +    if (bdrv_all_delete_snapshot(name, &bs, &local_err) < 0) {
> +        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
> +                   bdrv_get_device_name(bs), error_get_pretty(local_err));

you'll want to use Markus' proposed error prefixing code.


> +SQMP
> +delvm
> +------------------

Divider too long.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH 0/5] QMP wrappers for VM snapshot operations

[Denis V. Lunev]

EFI based VM with pflash storage for NVRAM could not be snapshoted as
libvirt configures storage as 'raw' and writable. OK, this is a libvirt
problem.

Another problem is that libvirt can not detect this failure at all
as it uses HMP for this operation. This create snapshot/delete snapshot
sequence passes silently.

The patchset adds QMP wrappers for the purpose.

At the moment I have placed 2.6 version into QAPI. Though (if you feel
appropriate) I can change it to 2.5 :) This is up to you to decide.

Please note, this patchset is made on top of
  [PATCH for 2.5 v8 0/10] dataplane snapshot fixes

Signed-off-by: "Denis V. Lunev" <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>

Denis V. Lunev (5):
  migration: split hmp_savevm to do_savevm and hmp_savevm wrapper
  qmp: create qmp_savevm command
  qmp: create qmp_delvm command
  migration: improve error reporting for hmp_loadvm
  qmp: create QMP implementation of loadvm command

 include/sysemu/sysemu.h |   2 +-
 migration/savevm.c      | 100 +++++++++++++++++++++++++++++++-----------------
 monitor.c               |   7 +++-
 qapi-schema.json        |  39 +++++++++++++++++++
 qmp-commands.hx         |  71 ++++++++++++++++++++++++++++++++++
 vl.c                    |   5 ++-
 6 files changed, 185 insertions(+), 39 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Denis V. Lunev]

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Juan Quintela <quintela@redhat.com>
CC: Amit Shah <amit.shah@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
---
 migration/savevm.c | 27 ++++++++++++++++++---------
 qapi-schema.json   | 13 +++++++++++++
 qmp-commands.hx    | 23 +++++++++++++++++++++++
 3 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 565b10a..90b6850 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2115,17 +2115,26 @@ int load_vmstate(const char *name)
     return 0;
 }
 
-void hmp_delvm(Monitor *mon, const QDict *qdict)
+void qmp_delvm(const char *name, Error **errp)
 {
     BlockDriverState *bs;
-    Error *err;
-    const char *name = qdict_get_str(qdict, "name");
-
-    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
-        monitor_printf(mon,
-                       "Error while deleting snapshot on device '%s': %s\n",
-                       bdrv_get_device_name(bs), error_get_pretty(err));
-        error_free(err);
+    Error *local_err = NULL;
+
+    if (bdrv_all_delete_snapshot(name, &bs, errp) < 0) {
+        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+                   bdrv_get_device_name(bs), error_get_pretty(local_err));
+        error_free(local_err);
+    }
+}
+
+void hmp_delvm(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
+
+    if (local_err != NULL) {
+        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
+        error_free(local_err);
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 8cc8b44..193b34f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3975,3 +3975,16 @@
 # Since 2.6
 ##
 { 'command': 'savevm', 'data': {'*name': 'str'} }
+
+##
+# @delvm
+#
+# Delete a VM snapshot
+#
+# @name: identifier of a snapshot to be deleted
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cd895f6..b2b17ff 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4764,3 +4764,26 @@ EQMP
         .args_type  = "name:s?",
         .mhandler.cmd_new = qmp_marshal_savevm,
     },
+
+SQMP
+delvm
+------------------
+
+Delete a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "delvm",
+        .args_type  = "name:s",
+        .mhandler.cmd_new = qmp_marshal_delvm,
+    },
-- 
2.5.0

Re: [Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Markus Armbruster]

"Denis V. Lunev" <den@openvz.org> writes:

> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Amit Shah <amit.shah@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> ---
>  migration/savevm.c | 27 ++++++++++++++++++---------
>  qapi-schema.json   | 13 +++++++++++++
>  qmp-commands.hx    | 23 +++++++++++++++++++++++
>  3 files changed, 54 insertions(+), 9 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 565b10a..90b6850 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2115,17 +2115,26 @@ int load_vmstate(const char *name)
>      return 0;
>  }
>  
> -void hmp_delvm(Monitor *mon, const QDict *qdict)
> +void qmp_delvm(const char *name, Error **errp)
>  {
>      BlockDriverState *bs;
> -    Error *err;
> -    const char *name = qdict_get_str(qdict, "name");
> -
> -    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
> -        monitor_printf(mon,
> -                       "Error while deleting snapshot on device '%s': %s\n",
> -                       bdrv_get_device_name(bs), error_get_pretty(err));
> -        error_free(err);
> +    Error *local_err = NULL;
> +
> +    if (bdrv_all_delete_snapshot(name, &bs, errp) < 0) {
> +        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
> +                   bdrv_get_device_name(bs), error_get_pretty(local_err));
> +        error_free(local_err);
> +    }
> +}
> +
> +void hmp_delvm(Monitor *mon, const QDict *qdict)
> +{
> +    Error *local_err = NULL;
> +    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
> +
> +    if (local_err != NULL) {
> +        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
> +        error_free(local_err);

error_report_err(), please.

>      }
>  }

Juan, Amit, in case you'd prefer to move out the parts that implement
HMP on top of QMP: they can go into hmp.c as long as they're as simple
as this one.

> diff --git a/qapi-schema.json b/qapi-schema.json
> index 8cc8b44..193b34f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3975,3 +3975,16 @@
>  # Since 2.6
>  ##
>  { 'command': 'savevm', 'data': {'*name': 'str'} }
> +
> +##
> +# @delvm
> +#
> +# Delete a VM snapshot
> +#
> +# @name: identifier of a snapshot to be deleted
> +#
> +# Returns: Nothing on success
> +#
> +# Since 2.6
> +##
> +{ 'command': 'delvm', 'data': {'name': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index cd895f6..b2b17ff 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4764,3 +4764,26 @@ EQMP
>          .args_type  = "name:s?",
>          .mhandler.cmd_new = qmp_marshal_savevm,
>      },
> +
> +SQMP
> +delvm
> +------------------
> +
> +Delete a VM snapshot
> +
> +Arguments:
> +
> +- "name": snapshot name
> +
> +Example:
> +
> +-> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "delvm",
> +        .args_type  = "name:s",
> +        .mhandler.cmd_new = qmp_marshal_delvm,
> +    },

Re: [Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> "Denis V. Lunev" <den@openvz.org> writes:
>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Juan Quintela <quintela@redhat.com>
>> CC: Amit Shah <amit.shah@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> ---
>>  migration/savevm.c | 27 ++++++++++++++++++---------
>>  qapi-schema.json   | 13 +++++++++++++
>>  qmp-commands.hx    | 23 +++++++++++++++++++++++
>>  3 files changed, 54 insertions(+), 9 deletions(-)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 565b10a..90b6850 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -2115,17 +2115,26 @@ int load_vmstate(const char *name)
>>      return 0;
>>  }
>>  
>> -void hmp_delvm(Monitor *mon, const QDict *qdict)
>> +void qmp_delvm(const char *name, Error **errp)
>>  {
>>      BlockDriverState *bs;
>> -    Error *err;
>> -    const char *name = qdict_get_str(qdict, "name");
>> -
>> -    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
>> -        monitor_printf(mon,
>> -                       "Error while deleting snapshot on device '%s': %s\n",
>> -                       bdrv_get_device_name(bs), error_get_pretty(err));
>> -        error_free(err);
>> +    Error *local_err = NULL;
>> +
>> +    if (bdrv_all_delete_snapshot(name, &bs, errp) < 0) {
>> +        error_setg(errp, "Error while deleting snapshot on device '%s': %s",
>> +                   bdrv_get_device_name(bs), error_get_pretty(local_err));
>> +        error_free(local_err);
>> +    }
>> +}
>> +
>> +void hmp_delvm(Monitor *mon, const QDict *qdict)
>> +{
>> +    Error *local_err = NULL;
>> +    qmp_delvm(qdict_get_str(qdict, "name"), &local_err);
>> +
>> +    if (local_err != NULL) {
>> +        monitor_printf(mon, "%s\n", error_get_pretty(local_err));
>> +        error_free(local_err);
>
> error_report_err(), please.
>
>>      }
>>  }
>
> Juan, Amit, in case you'd prefer to move out the parts that implement
> HMP on top of QMP: they can go into hmp.c as long as they're as simple
> as this one.

Perfect for me.

Thanks, Juan.

Re: [Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Juan Quintela]

"Denis V. Lunev" <den@openvz.org> wrote:
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Amit Shah <amit.shah@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>

I agree with the changes, move them to hmp.c as Markus suggests.