[Qemu-devel] [PATCH] Add save/load/del[vm] QMP api

Pavel Balaev posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180522065922.GA3462@rnd
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
hmp.c              | 22 +++++++++-------------
migration/savevm.c | 27 +++++++++++++++++++++++++++
qapi/misc.json     | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 90 insertions(+), 13 deletions(-)
[Qemu-devel] [PATCH] Add save/load/del[vm] QMP api
Posted by Pavel Balaev 5 years, 10 months ago
Hello,

Now savevm, loadvm and delvm commands only allowed from hmp monitor.
This patch adds ability to send them via QMP api. 

Signed-off-by: Pavel Balaev <mail@void.so>
---
 hmp.c              | 22 +++++++++-------------
 migration/savevm.c | 27 +++++++++++++++++++++++++++
 qapi/misc.json     | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/hmp.c b/hmp.c
index ef93f4878b..7ae5852c77 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1313,37 +1313,33 @@ void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict)
 
 void hmp_loadvm(Monitor *mon, const QDict *qdict)
 {
-    int saved_vm_running  = runstate_is_running();
     const char *name = qdict_get_str(qdict, "name");
     Error *err = NULL;
 
-    vm_stop(RUN_STATE_RESTORE_VM);
-
-    if (load_snapshot(name, &err) == 0 && saved_vm_running) {
-        vm_start();
-    }
+    qmp_loadvm(name, &err);
     hmp_handle_error(mon, &err);
 }
 
 void hmp_savevm(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
+    bool has_name = TRUE;
+    const char *name = qdict_get_try_str(qdict, "name");
 
-    save_snapshot(qdict_get_try_str(qdict, "name"), &err);
+    if (name == NULL) {
+        has_name = FALSE;
+    }
+
+    qmp_savevm(has_name, name, &err);
     hmp_handle_error(mon, &err);
 }
 
 void hmp_delvm(Monitor *mon, const QDict *qdict)
 {
-    BlockDriverState *bs;
     Error *err = NULL;
     const char *name = qdict_get_str(qdict, "name");
 
-    if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
-        error_reportf_err(err,
-                          "Error while deleting snapshot on device '%s': ",
-                          bdrv_get_device_name(bs));
-    }
+    qmp_delvm(name, &err);
 }
 
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
diff --git a/migration/savevm.c b/migration/savevm.c
index 4251125831..3c830ca5a4 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2515,6 +2515,33 @@ int save_snapshot(const char *name, Error **errp)
     return ret;
 }
 
+void qmp_savevm(bool has_name, const char *name, Error **errp)
+{
+    save_snapshot(has_name ? name : NULL, errp);
+}
+
+void qmp_loadvm(const char *name, Error **errp)
+{
+    int saved_vm_running  = runstate_is_running();
+
+    vm_stop(RUN_STATE_RESTORE_VM);
+
+    if (load_snapshot(name, errp) == 0 && saved_vm_running) {
+        vm_start();
+    }
+}
+
+void qmp_delvm(const char *name, Error **errp)
+{
+    BlockDriverState *bs;
+
+    if (bdrv_all_delete_snapshot(name, &bs, errp) < 0) {
+        error_reportf_err(*errp,
+                          "Error while deleting snapshot on device '%s': ",
+                          bdrv_get_device_name(bs));
+    }
+}
+
 void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
                                 Error **errp)
 {
diff --git a/qapi/misc.json b/qapi/misc.json
index f5988cc0b5..cca7df0202 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3097,6 +3097,60 @@
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
 
+##
+# @savevm:
+#
+# Save a VM snapshot. Without a name new snapshot is created.
+#
+# @name: identifier of a snapshot to be saved
+#
+# Returns: Nothing on success
+#
+# Since: 2.12
+#
+# Example:
+#
+# -> { "execute": "savevm", "arguments": { "name": "snapshot1" } }
+# <- { "return": {} }
+##
+{ 'command': 'savevm', 'data': {'*name': 'str'} }
+
+##
+# @loadvm:
+#
+# Load a VM snapshot.
+#
+# @name: identifier of a snapshot to be loaded
+#
+# Returns: Nothing on success
+#
+# Since: 2.12
+#
+# Example:
+#
+# -> { "execute": "loadvm", "arguments": { "name": "snapshot1" } }
+# <- { "return": {} }
+##
+{ 'command': 'loadvm', 'data': {'name': 'str'} }
+
+##
+# @delvm:
+#
+# Delete a VM snapshot.
+#
+# @name: identifier of a snapshot to be deleted
+#
+# Returns: Nothing on success
+#
+# Since: 2.12
+#
+# Example:
+#
+# -> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
+# <- { "return": {} }
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
+
 ##
 # @xen-load-devices-state:
 #
-- 
2.16.1


Re: [Qemu-devel] [PATCH] Add save/load/del[vm] QMP api
Posted by Markus Armbruster 5 years, 10 months ago
Pavel Balaev <mail@void.so> writes:

> Hello,
>
> Now savevm, loadvm and delvm commands only allowed from hmp monitor.
> This patch adds ability to send them via QMP api. 
>
> Signed-off-by: Pavel Balaev <mail@void.so>

Quoting my reply to a prior similar patch:

    savevm and loadvm are HMP only precisely because an exact QMP
    equivalent wouldn't be a sane interface, and designing a sane QMP
    interface would be work.  Things that won't do for QMP include
    automatic selection of the block device receiving the VM state, and
    synchronous operation.

    Building blocks might be a better fit for QMP than a
    one-size-fits-all savevm command.

Message-ID: <87po5m2odo.fsf@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00615.html

Re: [Qemu-devel] [PATCH] Add save/load/del[vm] QMP api
Posted by Eric Blake 5 years, 10 months ago
On 05/22/2018 01:59 AM, Pavel Balaev wrote:
> Hello,
> 
> Now savevm, loadvm and delvm commands only allowed from hmp monitor.
> This patch adds ability to send them via QMP api.

Quoting my reply from an earlier similar proposal:
https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg01864.html

> Straightforward mapping of the existing HMP commands into QMP without
> any thought about the design won't make the errors any clearer. My
> argument is that any QMP design for managing internal snapshots must be
> well-designed, but that since we discourage internal snapshots, no one
> has been actively working on that design.

Or an even earlier series that also attempted the same thing, and was 
rejected:
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02427.html

You need to actually propose a sane design, and not just a mapping of 
the (awkward) HMP commands into blindly identical QMP commands.


> +# @savevm:
> +#
> +# Save a VM snapshot. Without a name new snapshot is created.
> +#
> +# @name: identifier of a snapshot to be saved
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 2.12

Furthermore, you've missed the 2.12 release.  The next release will be 
3.0 (although you'll find mentions of 2.13 throughout list archives, as 
the decision to use 3.0 instead of 2.13 as the next release is fairly 
recent).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org