diff mbox series

Add save/load/del[vm] QMP api

Message ID 20180522065922.GA3462@rnd
State New
Headers show
Series Add save/load/del[vm] QMP api | expand

Commit Message

Pavel Balaev May 22, 2018, 6:59 a.m. UTC 
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(-)

Comments

Markus Armbruster May 22, 2018, 12:42 p.m. UTC | #1 
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
Eric Blake May 23, 2018, 3:19 p.m. UTC | #2 
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).
diff mbox series

Patch

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:
 #