* [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands
@ 2012-07-12 16:55 Pavel Hrdina
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm Pavel Hrdina
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-12 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Pavel Hrdina
This patch-series converts savevm, loadvm, delvm and info snapshots from HMP
into QMP as savevm, loadvm, delvm and query-snapshots.
All comments are welcome.
Pavel
Pavel Hrdina (4):
qapi: Convert savevm
qapi: Convert loadvm
qapi: Convert delvm
qapi: Convert info snapshots
hmp-commands.hx | 6 +--
hmp.c | 64 +++++++++++++++++++++++
hmp.h | 4 ++
monitor.c | 14 +----
qapi-schema.json | 96 ++++++++++++++++++++++++++++++++++
qerror.c | 44 ++++++++++++++++
qerror.h | 33 ++++++++++++
qmp-commands.hx | 106 +++++++++++++++++++++++++++++++++++++
savevm.c | 154 ++++++++++++++++++++++++++++--------------------------
sysemu.h | 5 --
vl.c | 7 ++-
11 files changed, 437 insertions(+), 96 deletions(-)
--
1.7.10.4
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm
2012-07-12 16:55 [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Pavel Hrdina
@ 2012-07-12 16:55 ` Pavel Hrdina
2012-07-12 17:53 ` Eric Blake
2012-07-23 20:41 ` Luiz Capitulino
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm Pavel Hrdina
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-12 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Pavel Hrdina
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
hmp-commands.hx | 2 +-
hmp.c | 10 ++++++++++
hmp.h | 1 +
qapi-schema.json | 22 ++++++++++++++++++++++
qerror.c | 24 ++++++++++++++++++++++++
qerror.h | 18 ++++++++++++++++++
qmp-commands.hx | 26 ++++++++++++++++++++++++++
savevm.c | 29 ++++++++++++++---------------
sysemu.h | 1 -
9 files changed, 116 insertions(+), 17 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f5d9d91..e8c5325 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -267,7 +267,7 @@ ETEXI
.args_type = "name:s?",
.params = "[tag|id]",
.help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
- .mhandler.cmd = do_savevm,
+ .mhandler.cmd = hmp_savevm,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 4c6d4ae..491599d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1002,3 +1002,13 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
qmp_netdev_del(id, &err);
hmp_handle_error(mon, &err);
}
+
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+ const char *name = qdict_get_try_str(qdict, "name");
+ Error *err = NULL;
+
+ qmp_savevm(!!name, name, &err);
+
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 79d138d..dc6410b 100644
--- a/hmp.h
+++ b/hmp.h
@@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_savevm(Monitor *mon, const QDict *qdict);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 1ab5dbd..4db1447 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1868,3 +1868,25 @@
# Since: 0.14.0
##
{ 'command': 'netdev_del', 'data': {'id': 'str'} }
+
+##
+# @savevm:
+#
+# Create a snapshot of the whole virtual machine. If 'tag' is provided,
+# it is used as human readable identifier. If there is already a snapshot
+# with the same tag or ID, it is replaced.
+#
+# @name: tag or id of new or existing snapshot
+#
+# Returns: Nothing on success
+# If there is a writable device not supporting snapshots,
+# SnapshotNotSupported
+# If no block device can accept snapshots, SnapshotNotAccepted
+# If an error occures while creating a snapshot, SnapshotCreateFailed
+# If open a block device for vm state fail, SnapshotOpenFailed
+# If an error uccures while writing vm state, SnapshotWriteFailed
+# If delete snapshot with same 'name' fail, SnapshotDeleteFailed
+#
+# Since: 1.2
+##
+{ 'command': 'savevm', 'data': {'*name': 'str'} }
\ No newline at end of file
diff --git a/qerror.c b/qerror.c
index 92c4eff..4e6efa1 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,6 +309,30 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
+ .error_fmt = QERR_SNAPSHOT_CREATE_FAILED,
+ .desc = "Error '%(errno)' while creating snapshot on '%(device)'",
+ },
+ {
+ .error_fmt = QERR_SNAPSHOT_DELETE_FAILED,
+ .desc = "Error '%(errno)' while deleting snapshot on '%(device)'",
+ },
+ {
+ .error_fmt = QERR_SNAPSHOT_NOT_ACCEPTED,
+ .desc = "No block device can accept snapshots",
+ },
+ {
+ .error_fmt = QERR_SNAPSHOT_NOT_SUPPORTED,
+ .desc = "Device '%(device)' is writable but does not support snapshots",
+ },
+ {
+ .error_fmt = QERR_SNAPSHOT_OPEN_FAILED,
+ .desc = "Error while opening snapshot on '%(device)'",
+ },
+ {
+ .error_fmt = QERR_SNAPSHOT_WRITE_FAILED,
+ .desc = "Error '%(errno)' while writing snapshot on '%(device)'",
+ },
+ {
.error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
.desc = "Connection can not be completed immediately",
},
diff --git a/qerror.h b/qerror.h
index b4c8758..bc47f4a 100644
--- a/qerror.h
+++ b/qerror.h
@@ -251,6 +251,24 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
+#define QERR_SNAPSHOT_CREATE_FAILED \
+ "{ 'class': 'SnapshotCreateFailed', 'data': { 'device': %s, 'errno': %d } }"
+
+#define QERR_SNAPSHOT_DELETE_FAILED \
+ "{ 'class': 'SnapshotDeleteFailed', 'data': { 'device': %s, 'errno': %d } }"
+
+#define QERR_SNAPSHOT_NOT_ACCEPTED \
+ "{ 'class': 'SnapshotNotAccepted', 'data': {} }"
+
+#define QERR_SNAPSHOT_NOT_SUPPORTED \
+ "{ 'class': 'SnapshotNotSupported', 'data': { 'device': %s } }"
+
+#define QERR_SNAPSHOT_OPEN_FAILED \
+ "{ 'class': 'SnapshotOpenFailed', 'data': { 'device': %s } }"
+
+#define QERR_SNAPSHOT_WRITE_FAILED \
+ "{ 'class': 'SnapshotWriteFailed', 'data': { 'device': %s, 'errno': %d } }"
+
#define QERR_SOCKET_CONNECT_IN_PROGRESS \
"{ 'class': 'SockConnectInprogress', 'data': {} }"
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2e1a38e..a1c82f7 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1061,6 +1061,32 @@ Example:
EQMP
{
+ .name = "savevm",
+ .args_type = "name:s?",
+ .params = "name",
+ .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
+ .mhandler.cmd_new = qmp_marshal_input_savevm
+ },
+
+SQMP
+savevm
+------
+
+Create a snapshot of the whole virtual machine. If 'tag' is provided,
+it is used as human readable identifier. If there is already a snapshot
+with the same tag or ID, it is replaced.
+
+Arguments:
+
+- "name": tag or id of new or existing snapshot
+
+Example:
+
+-> { "execute": "savevm", "arguments": { "name": "my_snapshot" } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "qmp_capabilities",
.args_type = "",
.params = "",
diff --git a/savevm.c b/savevm.c
index a15c163..9fc1b53 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2033,7 +2033,7 @@ static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
/*
* Deletes snapshots of a given name in all opened images.
*/
-static int del_existing_snapshots(Monitor *mon, const char *name)
+static int del_existing_snapshots(Error **errp, const char *name)
{
BlockDriverState *bs;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
@@ -2046,9 +2046,8 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
{
ret = bdrv_snapshot_delete(bs, name);
if (ret < 0) {
- monitor_printf(mon,
- "Error while deleting snapshot on '%s'\n",
- bdrv_get_device_name(bs));
+ error_set(errp, QERR_SNAPSHOT_DELETE_FAILED,
+ bdrv_get_device_name(bs), ret);
return -1;
}
}
@@ -2057,7 +2056,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
return 0;
}
-void do_savevm(Monitor *mon, const QDict *qdict)
+void qmp_savevm(bool has_name, const char *name, Error **errp)
{
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
@@ -2072,7 +2071,6 @@ void do_savevm(Monitor *mon, const QDict *qdict)
struct timeval tv;
struct tm tm;
#endif
- const char *name = qdict_get_try_str(qdict, "name");
/* Verify if there is a device that doesn't support snapshots and is writable */
bs = NULL;
@@ -2083,15 +2081,15 @@ void do_savevm(Monitor *mon, const QDict *qdict)
}
if (!bdrv_can_snapshot(bs)) {
- monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n",
- bdrv_get_device_name(bs));
+ error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED,
+ bdrv_get_device_name(bs));
return;
}
}
bs = bdrv_snapshots();
if (!bs) {
- monitor_printf(mon, "No block device can accept snapshots\n");
+ error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED);
return;
}
@@ -2112,7 +2110,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
#endif
sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
- if (name) {
+ if (has_name) {
ret = bdrv_snapshot_find(bs, old_sn, name);
if (ret >= 0) {
pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
@@ -2133,21 +2131,22 @@ void do_savevm(Monitor *mon, const QDict *qdict)
}
/* Delete old snapshots of the same name */
- if (name && del_existing_snapshots(mon, name) < 0) {
+ if (has_name && del_existing_snapshots(errp, name) < 0) {
goto the_end;
}
/* save the VM state */
f = qemu_fopen_bdrv(bs, 1);
if (!f) {
- monitor_printf(mon, "Could not open VM state file\n");
+ error_set(errp, QERR_SNAPSHOT_OPEN_FAILED, bdrv_get_device_name(bs));
goto the_end;
}
ret = qemu_savevm_state(f);
vm_state_size = qemu_ftell(f);
qemu_fclose(f);
if (ret < 0) {
- monitor_printf(mon, "Error %d while writing VM\n", ret);
+ error_set(errp, QERR_SNAPSHOT_WRITE_FAILED,
+ bdrv_get_device_name(bs), ret);
goto the_end;
}
@@ -2160,8 +2159,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
ret = bdrv_snapshot_create(bs1, sn);
if (ret < 0) {
- monitor_printf(mon, "Error while creating snapshot on '%s'\n",
- bdrv_get_device_name(bs1));
+ error_set(errp, QERR_SNAPSHOT_CREATE_FAILED,
+ bdrv_get_device_name(bs1), ret);
}
}
}
diff --git a/sysemu.h b/sysemu.h
index 6540c79..95d1207 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,7 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify);
void qemu_add_machine_init_done_notifier(Notifier *notify);
-void do_savevm(Monitor *mon, const QDict *qdict);
int load_vmstate(const char *name);
void do_delvm(Monitor *mon, const QDict *qdict);
void do_info_snapshots(Monitor *mon);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm
2012-07-12 16:55 [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Pavel Hrdina
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm Pavel Hrdina
@ 2012-07-12 16:55 ` Pavel Hrdina
2012-07-12 17:56 ` Eric Blake
2012-07-24 17:03 ` Luiz Capitulino
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm Pavel Hrdina
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-12 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Pavel Hrdina
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
hmp-commands.hx | 2 +-
hmp.c | 10 ++++++++++
hmp.h | 1 +
monitor.c | 12 ------------
qapi-schema.json | 24 +++++++++++++++++++++++-
qerror.c | 16 ++++++++++++++++
qerror.h | 12 ++++++++++++
qmp-commands.hx | 25 +++++++++++++++++++++++++
savevm.c | 52 +++++++++++++++++++++++++++++++---------------------
sysemu.h | 1 -
vl.c | 7 ++++++-
11 files changed, 125 insertions(+), 37 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e8c5325..b145612 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -284,7 +284,7 @@ ETEXI
.args_type = "name:s",
.params = "tag|id",
.help = "restore a VM snapshot from its tag or id",
- .mhandler.cmd = do_loadvm,
+ .mhandler.cmd = hmp_loadvm,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 491599d..6ed4c3f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1012,3 +1012,13 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+
+void hmp_loadvm(Monitor *mon, const QDict *qdict)
+{
+ const char *name = qdict_get_str(qdict, "name");
+ Error *err = NULL;
+
+ qmp_loadvm(name, &err);
+
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index dc6410b..5623ae7 100644
--- a/hmp.h
+++ b/hmp.h
@@ -65,5 +65,6 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
void hmp_savevm(Monitor *mon, const QDict *qdict);
+void hmp_loadvm(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor.c b/monitor.c
index f6107ba..3efbcc8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2385,18 +2385,6 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
return -1;
}
-static void do_loadvm(Monitor *mon, const QDict *qdict)
-{
- int saved_vm_running = runstate_is_running();
- const char *name = qdict_get_str(qdict, "name");
-
- vm_stop(RUN_STATE_RESTORE_VM);
-
- if (load_vmstate(name) == 0 && saved_vm_running) {
- vm_start();
- }
-}
-
int monitor_get_fd(Monitor *mon, const char *fdname)
{
mon_fd_t *monfd;
diff --git a/qapi-schema.json b/qapi-schema.json
index 4db1447..7df6a90 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1889,4 +1889,26 @@
#
# Since: 1.2
##
-{ 'command': 'savevm', 'data': {'*name': 'str'} }
\ No newline at end of file
+{ 'command': 'savevm', 'data': {'*name': 'str'} }
+
+##
+# @loadvm:
+#
+# Set the whole virtual machine to the snapshot identified by the tag
+# 'tag' or the unique snapshot ID 'id'.
+#
+# @name: tag or id of existing snapshot
+#
+# Returns: Nothing on success
+# If no block device can accept snapshots, SnapshotNotAccepted
+# If no snapshot is found, SnapshotNotFound
+# If snapshot is only disk snapshot, SnapshotInvalid
+# If there is a writable device not supporting snapshots,
+# SnapshotNotSupported
+# If snapshot activate failed, SnapshotActivateFailed
+# If open a block device for vm state fail, SnapshotOpenFailed
+# If an error occures while loading vm state, SnapshotLoadFailed
+#
+# Since: 1.2
+##
+{ 'command': 'loadvm', 'data': {'name': 'str'} }
diff --git a/qerror.c b/qerror.c
index 4e6efa1..b9c7352 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,6 +309,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
+ .error_fmt = QERR_SNAPSHOT_ACTIVATE_FAILED,
+ .desc = "Error '%(errno)' while activating snapshot '%(name)' on '%(device)'",
+ },
+ {
.error_fmt = QERR_SNAPSHOT_CREATE_FAILED,
.desc = "Error '%(errno)' while creating snapshot on '%(device)'",
},
@@ -317,10 +321,22 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Error '%(errno)' while deleting snapshot on '%(device)'",
},
{
+ .error_fmt = QERR_SNAPSHOT_INVALID,
+ .desc = "Device '%(device)' disk-only snapshot. Revert to it offline using qemu-img.",
+ },
+ {
+ .error_fmt = QERR_SNAPSHOT_LOAD_FAILED,
+ .desc = "Error '%(errno)' while loading snapshot for '%(device)'",
+ },
+ {
.error_fmt = QERR_SNAPSHOT_NOT_ACCEPTED,
.desc = "No block device can accept snapshots",
},
{
+ .error_fmt = QERR_SNAPSHOT_NOT_FOUND,
+ .desc = "Device '%(device)' does not have the requested snapshot '%(name)'",
+ },
+ {
.error_fmt = QERR_SNAPSHOT_NOT_SUPPORTED,
.desc = "Device '%(device)' is writable but does not support snapshots",
},
diff --git a/qerror.h b/qerror.h
index bc47f4a..da2abdd 100644
--- a/qerror.h
+++ b/qerror.h
@@ -251,15 +251,27 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
+#define QERR_SNAPSHOT_ACTIVATE_FAILED \
+ "{ 'class': 'SnapshotActivateFailed', 'data': { 'device': %s, 'name': %s, 'errno':%d } }"
+
#define QERR_SNAPSHOT_CREATE_FAILED \
"{ 'class': 'SnapshotCreateFailed', 'data': { 'device': %s, 'errno': %d } }"
#define QERR_SNAPSHOT_DELETE_FAILED \
"{ 'class': 'SnapshotDeleteFailed', 'data': { 'device': %s, 'errno': %d } }"
+#define QERR_SNAPSHOT_INVALID \
+ "{ 'class': 'SnapshotInvalid', 'data': { 'device': %s } }"
+
+#define QERR_SNAPSHOT_LOAD_FAILED \
+ "{ 'class': 'SnapshotLoadFailed', 'data': { 'device': %s, 'errno': %d } }"
+
#define QERR_SNAPSHOT_NOT_ACCEPTED \
"{ 'class': 'SnapshotNotAccepted', 'data': {} }"
+#define QERR_SNAPSHOT_NOT_FOUND \
+ "{ 'class': 'SnapshotNotFound', 'data': { 'device': %s, 'name': %s } }"
+
#define QERR_SNAPSHOT_NOT_SUPPORTED \
"{ 'class': 'SnapshotNotSupported', 'data': { 'device': %s } }"
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a1c82f7..02550f2 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1087,6 +1087,31 @@ Example:
EQMP
{
+ .name = "loadvm",
+ .args_type = "name:s",
+ .params = "name",
+ .help = "restore a VM snapshot from its tag or id",
+ .mhandler.cmd_new = qmp_marshal_input_loadvm
+ },
+
+SQMP
+loadvm
+------
+
+Set the whole virtual machine to the snapshot identified by the tag
+'tag' or the unique snapshot ID 'id'.
+
+Arguments:
+
+- "name": tag or id of existing snapshot
+
+Example:
+
+-> { "execute": "loadvm", "arguments": { "name": "my_snapshot" } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "qmp_capabilities",
.args_type = "",
.params = "",
diff --git a/savevm.c b/savevm.c
index 9fc1b53..c113dd4 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2196,27 +2196,30 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
return;
}
-int load_vmstate(const char *name)
+void qmp_loadvm(const char *name, Error **errp)
{
BlockDriverState *bs, *bs_vm_state;
QEMUSnapshotInfo sn;
QEMUFile *f;
int ret;
+ int saved_vm_running;
bs_vm_state = bdrv_snapshots();
if (!bs_vm_state) {
- error_report("No block device supports snapshots");
- return -ENOTSUP;
+ error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED);
+ return;
}
/* Don't even try to load empty VM states */
ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
if (ret < 0) {
- return ret;
+ error_set(errp, QERR_SNAPSHOT_NOT_FOUND,
+ bdrv_get_device_name(bs_vm_state), name);
+ return;
} else if (sn.vm_state_size == 0) {
- error_report("This is a disk-only snapshot. Revert to it offline "
- "using qemu-img.");
- return -EINVAL;
+ error_set(errp, QERR_SNAPSHOT_INVALID,
+ bdrv_get_device_name(bs_vm_state));
+ return;
}
/* Verify if there is any device that doesn't support snapshots and is
@@ -2229,19 +2232,22 @@ int load_vmstate(const char *name)
}
if (!bdrv_can_snapshot(bs)) {
- error_report("Device '%s' is writable but does not support snapshots.",
- bdrv_get_device_name(bs));
- return -ENOTSUP;
+ error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED,
+ bdrv_get_device_name(bs));
+ return;
}
ret = bdrv_snapshot_find(bs, &sn, name);
if (ret < 0) {
- error_report("Device '%s' does not have the requested snapshot '%s'",
- bdrv_get_device_name(bs), name);
- return ret;
+ error_set(errp, QERR_SNAPSHOT_NOT_FOUND,
+ bdrv_get_device_name(bs), name);
+ return;
}
}
+ saved_vm_running = runstate_is_running();
+ vm_stop(RUN_STATE_RESTORE_VM);
+
/* Flush all IO requests so they don't interfere with the new state. */
bdrv_drain_all();
@@ -2250,9 +2256,9 @@ int load_vmstate(const char *name)
if (bdrv_can_snapshot(bs)) {
ret = bdrv_snapshot_goto(bs, name);
if (ret < 0) {
- error_report("Error %d while activating snapshot '%s' on '%s'",
- ret, name, bdrv_get_device_name(bs));
- return ret;
+ error_set(errp, QERR_SNAPSHOT_ACTIVATE_FAILED,
+ bdrv_get_device_name(bs), name, ret);
+ return;
}
}
}
@@ -2260,8 +2266,9 @@ int load_vmstate(const char *name)
/* restore the VM state */
f = qemu_fopen_bdrv(bs_vm_state, 0);
if (!f) {
- error_report("Could not open VM state file");
- return -EINVAL;
+ error_set(errp, QERR_SNAPSHOT_OPEN_FAILED,
+ bdrv_get_device_name(bs_vm_state));
+ return;
}
qemu_system_reset(VMRESET_SILENT);
@@ -2269,11 +2276,14 @@ int load_vmstate(const char *name)
qemu_fclose(f);
if (ret < 0) {
- error_report("Error %d while loading VM state", ret);
- return ret;
+ error_set(errp, QERR_SNAPSHOT_LOAD_FAILED,
+ bdrv_get_device_name(bs_vm_state), ret);
+ return;
}
- return 0;
+ if (!error_is_set(errp) && saved_vm_running) {
+ vm_start();
+ }
}
void do_delvm(Monitor *mon, const QDict *qdict)
diff --git a/sysemu.h b/sysemu.h
index 95d1207..8e65651 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,7 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify);
void qemu_add_machine_init_done_notifier(Notifier *notify);
-int load_vmstate(const char *name);
void do_delvm(Monitor *mon, const QDict *qdict);
void do_info_snapshots(Monitor *mon);
diff --git a/vl.c b/vl.c
index 1329c30..28b982d 100644
--- a/vl.c
+++ b/vl.c
@@ -3630,8 +3630,13 @@ int main(int argc, char **argv, char **envp)
qemu_system_reset(VMRESET_SILENT);
if (loadvm) {
- if (load_vmstate(loadvm) < 0) {
+ Error *err = NULL;
+
+ qmp_loadvm(loadvm, &err);
+
+ if (error_is_set(&err)) {
autostart = 0;
+ error_report("%s\n", error_get_pretty(err));
}
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm
2012-07-12 16:55 [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Pavel Hrdina
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm Pavel Hrdina
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm Pavel Hrdina
@ 2012-07-12 16:55 ` Pavel Hrdina
2012-07-12 17:59 ` Eric Blake
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots Pavel Hrdina
2012-07-24 17:30 ` [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Luiz Capitulino
4 siblings, 1 reply; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-12 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Pavel Hrdina
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
hmp-commands.hx | 2 +-
hmp.c | 10 ++++++++++
hmp.h | 1 +
qapi-schema.json | 17 +++++++++++++++++
qmp-commands.hx | 24 ++++++++++++++++++++++++
savevm.c | 21 +++++++++++----------
sysemu.h | 1 -
7 files changed, 64 insertions(+), 12 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index b145612..3be9ad4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -299,7 +299,7 @@ ETEXI
.args_type = "name:s",
.params = "tag|id",
.help = "delete a VM snapshot from its tag or id",
- .mhandler.cmd = do_delvm,
+ .mhandler.cmd = hmp_delvm,
},
STEXI
diff --git a/hmp.c b/hmp.c
index 6ed4c3f..a4668ca 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1022,3 +1022,13 @@ void hmp_loadvm(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+
+void hmp_delvm(Monitor *mon, const QDict *qdict)
+{
+ const char *name = qdict_get_str(qdict, "name");
+ Error *err = NULL;
+
+ qmp_delvm(name, &err);
+
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 5623ae7..78710e2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -66,5 +66,6 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict);
void hmp_netdev_del(Monitor *mon, const QDict *qdict);
void hmp_savevm(Monitor *mon, const QDict *qdict);
void hmp_loadvm(Monitor *mon, const QDict *qdict);
+void hmp_delvm(Monitor *mon, const QDict *qdict);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 7df6a90..a22d26a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1912,3 +1912,20 @@
# Since: 1.2
##
{ 'command': 'loadvm', 'data': {'name': 'str'} }
+
+##
+# @delvm:
+#
+# Delete the snapshot identified by 'tag' or 'id'.
+#
+# @name: tag or id of existing snapshot
+#
+# Returns: Nothing on success
+# If no block device can accept snapshots, SnapshotNotAccepted
+# If there is a writable device not supporting snapshots,
+# SnapshotNotSupported
+# If delete snapshot with same 'name' fail, SnapshotDeleteFailed
+#
+# Since: 1.2
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 02550f2..1071bb9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1112,6 +1112,30 @@ Example:
EQMP
{
+ .name = "delvm",
+ .args_type = "name:s",
+ .params = "tag|id",
+ .help = "delete a VM snapshot from its tag or id",
+ .mhandler.cmd_new = qmp_marshal_input_delvm
+ },
+
+SQMP
+delvm
+------
+
+Delete the snapshot identified by 'tag' or 'id'.
+
+Arguments:
+
+- "name": tag or id of existing snapshot
+
+Example:
+
+-> { "execute": "delvm", "arguments": { "name": "my_snapshot" } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "qmp_capabilities",
.args_type = "",
.params = "",
diff --git a/savevm.c b/savevm.c
index c113dd4..d3e8b07 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2286,15 +2286,14 @@ void qmp_loadvm(const char *name, Error **errp)
}
}
-void do_delvm(Monitor *mon, const QDict *qdict)
+void qmp_delvm(const char *name, Error **errp)
{
BlockDriverState *bs, *bs1;
int ret;
- const char *name = qdict_get_str(qdict, "name");
bs = bdrv_snapshots();
if (!bs) {
- monitor_printf(mon, "No block device supports snapshots\n");
+ error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED);
return;
}
@@ -2303,13 +2302,15 @@ void do_delvm(Monitor *mon, const QDict *qdict)
if (bdrv_can_snapshot(bs1)) {
ret = bdrv_snapshot_delete(bs1, name);
if (ret < 0) {
- if (ret == -ENOTSUP)
- monitor_printf(mon,
- "Snapshots not supported on device '%s'\n",
- bdrv_get_device_name(bs1));
- else
- monitor_printf(mon, "Error %d while deleting snapshot on "
- "'%s'\n", ret, bdrv_get_device_name(bs1));
+ if (ret == -ENOTSUP) {
+ error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED,
+ bdrv_get_device_name(bs1));
+ return;
+ } else {
+ error_set(errp, QERR_SNAPSHOT_DELETE_FAILED,
+ bdrv_get_device_name(bs1), ret);
+ return;
+ }
}
}
}
diff --git a/sysemu.h b/sysemu.h
index 8e65651..45e071c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,7 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify);
void qemu_add_machine_init_done_notifier(Notifier *notify);
-void do_delvm(Monitor *mon, const QDict *qdict);
void do_info_snapshots(Monitor *mon);
void qemu_announce_self(void);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots
2012-07-12 16:55 [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Pavel Hrdina
` (2 preceding siblings ...)
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm Pavel Hrdina
@ 2012-07-12 16:55 ` Pavel Hrdina
2012-07-12 18:03 ` Eric Blake
2012-07-24 17:30 ` [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Luiz Capitulino
4 siblings, 1 reply; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-12 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Pavel Hrdina
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
hmp.c | 34 ++++++++++++++++++++++++++++++++++
hmp.h | 1 +
monitor.c | 2 +-
qapi-schema.json | 35 +++++++++++++++++++++++++++++++++++
qerror.c | 4 ++++
qerror.h | 3 +++
qmp-commands.hx | 31 +++++++++++++++++++++++++++++++
savevm.c | 52 ++++++++++++++++++++++++----------------------------
sysemu.h | 2 --
9 files changed, 133 insertions(+), 31 deletions(-)
diff --git a/hmp.c b/hmp.c
index a4668ca..e611329 100644
--- a/hmp.c
+++ b/hmp.c
@@ -553,6 +553,40 @@ void hmp_info_block_jobs(Monitor *mon)
}
}
+void hmp_info_snapshots(Monitor *mon)
+{
+ SnapshotInfoList *list;
+ Error *err = NULL;
+ char buf[256];
+ QEMUSnapshotInfo sn;
+
+ list = qmp_query_snapshots(&err);
+
+ if (error_is_set(&err)) {
+ hmp_handle_error(mon, &err);
+ return;
+ }
+
+ if (!list) {
+ monitor_printf(mon, "There is no snapshot available.\n");
+ return;
+ }
+
+ monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+ while (list) {
+ memcpy(&(sn.id_str), list->value->id, sizeof(sn.id_str));
+ memcpy(&(sn.name), list->value->tag, sizeof(sn.name));
+ sn.date_sec = list->value->date;
+ sn.date_nsec = sn.date_nsec * 1000;
+ sn.vm_clock_nsec = list->value->vm_clock;
+ sn.vm_state_size = list->value->vm_size;
+ monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+ list = list->next;
+ }
+
+ qapi_free_SnapshotInfoList(list);
+}
+
void hmp_quit(Monitor *mon, const QDict *qdict)
{
monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 78710e2..0c13209 100644
--- a/hmp.h
+++ b/hmp.h
@@ -33,6 +33,7 @@ void hmp_info_spice(Monitor *mon);
void hmp_info_balloon(Monitor *mon);
void hmp_info_pci(Monitor *mon);
void hmp_info_block_jobs(Monitor *mon);
+void hmp_info_snapshots(Monitor *mon);
void hmp_quit(Monitor *mon, const QDict *qdict);
void hmp_stop(Monitor *mon, const QDict *qdict);
void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index 3efbcc8..262f9ea 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2596,7 +2596,7 @@ static mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the currently saved VM snapshots",
- .mhandler.info = do_info_snapshots,
+ .mhandler.info = hmp_info_snapshots,
},
{
.name = "status",
diff --git a/qapi-schema.json b/qapi-schema.json
index a22d26a..e66156c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -934,6 +934,41 @@
{ 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
##
+# @SnapshotInfo:
+#
+# Snapshot list. This structure contains list of snapshots for virtual machine.
+#
+# @id: id of the snapshot.
+#
+# @tag: human readable tag of the snapshot.
+#
+# @vm_size: size of the snapshot in Bytes.
+#
+# @date: date and time of the snapshot as timestamp.
+#
+# @vm_clock: time in the guest in nsecs.
+#
+# Since: 1.2
+##
+{ 'type': 'SnapshotInfo',
+ 'data': {'id': 'str', 'tag': 'str', 'vm_size': 'int',
+ 'date': 'int', 'vm_clock': 'int'} }
+
+##
+# @query-snapshots:
+#
+# List available snapshots for VM.
+#
+# Returns: a list of @SnapshotInfo describing each snapshot or an empty list
+# on success
+# If no block device can accept snapshots, SnapshotNotAccepted
+# If loading snapshot list failed, SnapshotListFailed
+#
+# Since: 1.2
+##
+{ 'command': 'query-snapshots', 'returns': ['SnapshotInfo'] }
+
+##
# @quit:
#
# This command will cause the QEMU process to exit gracefully. While every
diff --git a/qerror.c b/qerror.c
index b9c7352..7c6c0b3 100644
--- a/qerror.c
+++ b/qerror.c
@@ -325,6 +325,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' disk-only snapshot. Revert to it offline using qemu-img.",
},
{
+ .error_fmt = QERR_SNAPSHOT_LIST_FAILED,
+ .desc = "Error '%(errno)' while listing snapshots on '%(device)'",
+ },
+ {
.error_fmt = QERR_SNAPSHOT_LOAD_FAILED,
.desc = "Error '%(errno)' while loading snapshot for '%(device)'",
},
diff --git a/qerror.h b/qerror.h
index da2abdd..d602f05 100644
--- a/qerror.h
+++ b/qerror.h
@@ -263,6 +263,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_SNAPSHOT_INVALID \
"{ 'class': 'SnapshotInvalid', 'data': { 'device': %s } }"
+#define QERR_SNAPSHOT_LIST_FAILED \
+ "{ 'class': 'SnapshotListFailed', 'data': { 'device': %s, 'errno': %d } }"
+
#define QERR_SNAPSHOT_LOAD_FAILED \
"{ 'class': 'SnapshotLoadFailed', 'data': { 'device': %s, 'errno': %d } }"
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1071bb9..949efd2 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2284,3 +2284,34 @@ EQMP
.args_type = "implements:s?,abstract:b?",
.mhandler.cmd_new = qmp_marshal_input_qom_list_types,
},
+
+SQMP
+query-snapshots
+----------
+
+List available snapshots for VM.
+
+Return a json-object with the following information:
+
+- "id": id of the snapshot.
+
+- "tag": human readable tag of the snapshot.
+
+- "vm_size": size of the snapshot in Bytes.
+
+- "date": date and time of the snapshot as timestamp.
+
+- "vm_clock": time in the guest in nsecs.
+
+Example:
+
+-> { "execute": "query-snapshots" }
+<- {"return": [{"vm_size": 212309067, "tag": "my_snapshot",
+ "vm_clock": 630176706190, "id": "1", "date": 1341999856}]}
+
+EQMP
+ {
+ .name = "query-snapshots",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_query_snapshots,
+ },
diff --git a/savevm.c b/savevm.c
index d3e8b07..7454bd4 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2316,34 +2316,26 @@ void qmp_delvm(const char *name, Error **errp)
}
}
-void do_info_snapshots(Monitor *mon)
+SnapshotInfoList *qmp_query_snapshots(Error **errp)
{
+ SnapshotInfoList *snapshot_list = NULL, *last = NULL;
BlockDriverState *bs, *bs1;
QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
int nb_sns, i, ret, available;
- int total;
- int *available_snapshots;
- char buf[256];
bs = bdrv_snapshots();
if (!bs) {
- monitor_printf(mon, "No available block device supports snapshots\n");
- return;
+ error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED);
+ return NULL;
}
nb_sns = bdrv_snapshot_list(bs, &sn_tab);
if (nb_sns < 0) {
- monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
- return;
- }
-
- if (nb_sns == 0) {
- monitor_printf(mon, "There is no snapshot available.\n");
- return;
+ error_set(errp, QERR_SNAPSHOT_LIST_FAILED,
+ bdrv_get_device_name(bs), nb_sns);
+ return NULL;
}
- available_snapshots = g_malloc0(sizeof(int) * nb_sns);
- total = 0;
for (i = 0; i < nb_sns; i++) {
sn = &sn_tab[i];
available = 1;
@@ -2360,23 +2352,27 @@ void do_info_snapshots(Monitor *mon)
}
if (available) {
- available_snapshots[total] = i;
- total++;
- }
- }
-
- if (total > 0) {
- monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
- for (i = 0; i < total; i++) {
- sn = &sn_tab[available_snapshots[i]];
- monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+ SnapshotInfoList *info = g_malloc0(sizeof(*info));
+ info->value = g_malloc0(sizeof(*info->value));
+ info->value->id = g_strdup(sn->id_str);
+ info->value->tag = g_strdup(sn->name);
+ info->value->vm_size = sn->vm_state_size;
+ info->value->date = sn->date_sec;
+ info->value->vm_clock = sn->vm_clock_nsec;
+
+ if (!snapshot_list) {
+ snapshot_list = info;
+ last = info;
+ } else {
+ last->next = info;
+ last = info;
+ }
}
- } else {
- monitor_printf(mon, "There is no suitable snapshot available\n");
}
g_free(sn_tab);
- g_free(available_snapshots);
+
+ return snapshot_list;
}
diff --git a/sysemu.h b/sysemu.h
index 45e071c..b79ce73 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,8 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify);
void qemu_add_machine_init_done_notifier(Notifier *notify);
-void do_info_snapshots(Monitor *mon);
-
void qemu_announce_self(void);
bool qemu_savevm_state_blocked(Error **errp);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm Pavel Hrdina
@ 2012-07-12 17:53 ` Eric Blake
2012-07-16 8:12 ` Pavel Hrdina
2012-07-23 20:41 ` Luiz Capitulino
1 sibling, 1 reply; 19+ messages in thread
From: Eric Blake @ 2012-07-12 17:53 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]
On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -1868,3 +1868,25 @@
> # Since: 0.14.0
> ##
> { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @savevm:
> +#
> +# Create a snapshot of the whole virtual machine. If 'tag' is provided,
> +# it is used as human readable identifier. If there is already a snapshot
> +# with the same tag or ID, it is replaced.
> +#
> +# @name: tag or id of new or existing snapshot
Needs an #optional designation, given the syntax below.
> +#
> +# Returns: Nothing on success
> +# If there is a writable device not supporting snapshots,
> +# SnapshotNotSupported
> +# If no block device can accept snapshots, SnapshotNotAccepted
> +# If an error occures while creating a snapshot, SnapshotCreateFailed
s/occures/occurs/
> +# If open a block device for vm state fail, SnapshotOpenFailed
> +# If an error uccures while writing vm state, SnapshotWriteFailed
s/uccures/occurs/
> +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed
The notion of blindly overwriting the existing snapshot of the same name
seems a bit dangerous; should we take this opportunity to enhance the
command, and add a force flag, where things fail if the flag is false
but the name already exists, and where the reuse only happens if the
flag is present? (In fact, it would make my life in libvirt easier, as
I have an action item to make libvirt reject attempts to create a
snapshot with tag named '1' if an existing snapshot already has an id of
'1'.)
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> \ No newline at end of file
Fix that.
> @@ -1061,6 +1061,32 @@ Example:
>
> EQMP
> {
> + .name = "savevm",
> + .args_type = "name:s?",
> + .params = "name",
> + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
> + .mhandler.cmd_new = qmp_marshal_input_savevm
> + },
> +
> +SQMP
> +savevm
I know the HMP command is short, for ease of typing; but since 'savevm'
is not an English word, should we name the QMP command 'save-vm'?
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm Pavel Hrdina
@ 2012-07-12 17:56 ` Eric Blake
2012-07-24 17:04 ` Luiz Capitulino
2012-07-24 17:03 ` Luiz Capitulino
1 sibling, 1 reply; 19+ messages in thread
From: Eric Blake @ 2012-07-12 17:56 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]
On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -1889,4 +1889,26 @@
> #
> # Since: 1.2
> ##
> -{ 'command': 'savevm', 'data': {'*name': 'str'} }
> \ No newline at end of file
> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> +
> +##
> +# @loadvm:
Similar to my question in 1/4, should this be named 'load-vm'?
> +#
> +# Set the whole virtual machine to the snapshot identified by the tag
> +# 'tag' or the unique snapshot ID 'id'.
> +#
> +# @name: tag or id of existing snapshot
> +#
> +# Returns: Nothing on success
> +# If no block device can accept snapshots, SnapshotNotAccepted
> +# If no snapshot is found, SnapshotNotFound
> +# If snapshot is only disk snapshot, SnapshotInvalid
> +# If there is a writable device not supporting snapshots,
> +# SnapshotNotSupported
> +# If snapshot activate failed, SnapshotActivateFailed
> +# If open a block device for vm state fail, SnapshotOpenFailed
> +# If an error occures while loading vm state, SnapshotLoadFailed
s/occures/occurs/
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm Pavel Hrdina
@ 2012-07-12 17:59 ` Eric Blake
2012-07-16 8:12 ` Pavel Hrdina
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2012-07-12 17:59 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 863 bytes --]
On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> hmp-commands.hx | 2 +-
> hmp.c | 10 ++++++++++
> hmp.h | 1 +
> qapi-schema.json | 17 +++++++++++++++++
> qmp-commands.hx | 24 ++++++++++++++++++++++++
> savevm.c | 21 +++++++++++----------
> sysemu.h | 1 -
> 7 files changed, 64 insertions(+), 12 deletions(-)
> +
> +##
> +# @delvm:
This name is worse than 'loadvm' or 'savevm', in that we are NOT
deleting the entire vm, but a named snapshot stored within the vm.
Would naming it 'delete-vm-snapshot' make more sense (in which case the
others might make more sense as 'save-vm-snapshot' and 'load-vm-snapshot')?
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots Pavel Hrdina
@ 2012-07-12 18:03 ` Eric Blake
2012-07-16 8:15 ` Pavel Hrdina
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2012-07-12 18:03 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -934,6 +934,41 @@
> { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
>
> ##
> +# @SnapshotInfo:
> +#
> +# Snapshot list. This structure contains list of snapshots for virtual machine.
> +#
> +# @id: id of the snapshot.
> +#
> +# @tag: human readable tag of the snapshot.
Tag was optional on creation, should that field be marked optional and
omitted on output when there was no tag on input?
> +#
> +# @vm_size: size of the snapshot in Bytes.
> +#
> +# @date: date and time of the snapshot as timestamp.
What type of timestamp? Seconds since Epoch?
> +#
> +# @vm_clock: time in the guest in nsecs.
If vm_clock is in nsecs, should we also be reporting sub-second
resolution in date?
> +SQMP
> +query-snapshots
> +----------
> +
> +List available snapshots for VM.
> +
> +Return a json-object with the following information:
Returns an array of json-objects, each with the following information:
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm
2012-07-12 17:53 ` Eric Blake
@ 2012-07-16 8:12 ` Pavel Hrdina
2012-07-23 20:41 ` Luiz Capitulino
0 siblings, 1 reply; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-16 8:12 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
On 07/12/2012 07:53 PM, Eric Blake wrote:
> On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -1868,3 +1868,25 @@
>> # Since: 0.14.0
>> ##
>> { 'command': 'netdev_del', 'data': {'id': 'str'} }
>> +
>> +##
>> +# @savevm:
>> +#
>> +# Create a snapshot of the whole virtual machine. If 'tag' is provided,
>> +# it is used as human readable identifier. If there is already a snapshot
>> +# with the same tag or ID, it is replaced.
>> +#
>> +# @name: tag or id of new or existing snapshot
> Needs an #optional designation, given the syntax below.
>
>> +#
>> +# Returns: Nothing on success
>> +# If there is a writable device not supporting snapshots,
>> +# SnapshotNotSupported
>> +# If no block device can accept snapshots, SnapshotNotAccepted
>> +# If an error occures while creating a snapshot, SnapshotCreateFailed
> s/occures/occurs/
>
>> +# If open a block device for vm state fail, SnapshotOpenFailed
>> +# If an error uccures while writing vm state, SnapshotWriteFailed
> s/uccures/occurs/
>
>> +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed
> The notion of blindly overwriting the existing snapshot of the same name
> seems a bit dangerous; should we take this opportunity to enhance the
> command, and add a force flag, where things fail if the flag is false
> but the name already exists, and where the reuse only happens if the
> flag is present? (In fact, it would make my life in libvirt easier, as
> I have an action item to make libvirt reject attempts to create a
> snapshot with tag named '1' if an existing snapshot already has an id of
> '1'.)
This sounds reasonable and I think that this could be also good for HMP,
not only for QMP.
If there isn't anyone who disagree, I'll implement it.
Pavel
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
>> \ No newline at end of file
> Fix that.
>
>> @@ -1061,6 +1061,32 @@ Example:
>>
>> EQMP
>> {
>> + .name = "savevm",
>> + .args_type = "name:s?",
>> + .params = "name",
>> + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
>> + .mhandler.cmd_new = qmp_marshal_input_savevm
>> + },
>> +
>> +SQMP
>> +savevm
> I know the HMP command is short, for ease of typing; but since 'savevm'
> is not an English word, should we name the QMP command 'save-vm'?
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm
2012-07-12 17:59 ` Eric Blake
@ 2012-07-16 8:12 ` Pavel Hrdina
2012-07-16 16:37 ` Eric Blake
0 siblings, 1 reply; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-16 8:12 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
On 07/12/2012 07:59 PM, Eric Blake wrote:
> On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> ---
>> hmp-commands.hx | 2 +-
>> hmp.c | 10 ++++++++++
>> hmp.h | 1 +
>> qapi-schema.json | 17 +++++++++++++++++
>> qmp-commands.hx | 24 ++++++++++++++++++++++++
>> savevm.c | 21 +++++++++++----------
>> sysemu.h | 1 -
>> 7 files changed, 64 insertions(+), 12 deletions(-)
>> +
>> +##
>> +# @delvm:
> This name is worse than 'loadvm' or 'savevm', in that we are NOT
> deleting the entire vm, but a named snapshot stored within the vm.
> Would naming it 'delete-vm-snapshot' make more sense (in which case the
> others might make more sense as 'save-vm-snapshot' and 'load-vm-snapshot')?
>
This naming looks nice. I definitely agree that it could be
save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and
query-vm-snapshots.
Pavel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots
2012-07-12 18:03 ` Eric Blake
@ 2012-07-16 8:15 ` Pavel Hrdina
0 siblings, 0 replies; 19+ messages in thread
From: Pavel Hrdina @ 2012-07-16 8:15 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
On 07/12/2012 08:03 PM, Eric Blake wrote:
> On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -934,6 +934,41 @@
>> { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
>>
>> ##
>> +# @SnapshotInfo:
>> +#
>> +# Snapshot list. This structure contains list of snapshots for virtual machine.
>> +#
>> +# @id: id of the snapshot.
>> +#
>> +# @tag: human readable tag of the snapshot.
> Tag was optional on creation, should that field be marked optional and
> omitted on output when there was no tag on input?
If you create snapshot using an ID, Tag is created automatically. Even
if it is ugly tag, I think that we shouldn't omit it on output.
>> +#
>> +# @vm_size: size of the snapshot in Bytes.
>> +#
>> +# @date: date and time of the snapshot as timestamp.
> What type of timestamp? Seconds since Epoch?
It is unix timestamp in seconds since Epoch.
>> +#
>> +# @vm_clock: time in the guest in nsecs.
> If vm_clock is in nsecs, should we also be reporting sub-second
> resolution in date?
Well, I thought that this could be enough, but we could return the same
structure as "QEMUSnapshotInfo".
>> +SQMP
>> +query-snapshots
>> +----------
>> +
>> +List available snapshots for VM.
>> +
>> +Return a json-object with the following information:
> Returns an array of json-objects, each with the following information:
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm
2012-07-16 8:12 ` Pavel Hrdina
@ 2012-07-16 16:37 ` Eric Blake
2012-07-17 10:55 ` Kevin Wolf
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2012-07-16 16:37 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]
On 07/16/2012 02:12 AM, Pavel Hrdina wrote:
> On 07/12/2012 07:59 PM, Eric Blake wrote:
>> On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
>>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>>> ---
>>> hmp-commands.hx | 2 +-
>>> hmp.c | 10 ++++++++++
>>> hmp.h | 1 +
>>> qapi-schema.json | 17 +++++++++++++++++
>>> qmp-commands.hx | 24 ++++++++++++++++++++++++
>>> savevm.c | 21 +++++++++++----------
>>> sysemu.h | 1 -
>>> 7 files changed, 64 insertions(+), 12 deletions(-)
>>> +
>>> +##
>>> +# @delvm:
>> This name is worse than 'loadvm' or 'savevm', in that we are NOT
>> deleting the entire vm, but a named snapshot stored within the vm.
>> Would naming it 'delete-vm-snapshot' make more sense (in which case the
>> others might make more sense as 'save-vm-snapshot' and
>> 'load-vm-snapshot')?
>>
> This naming looks nice. I definitely agree that it could be
> save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and
> query-vm-snapshots.
On seeing that spelled out, I wonder if the '-vm' is just noise; where
we could use 'query-snapshots' instead of 'query-vm-snapshots'. Then
again, we already have 'blockdev-snapshot-sync', which is an entirely
different snapshot (just a block device, rather than the entire VM), so
dropping -vm is probably a bad idea. Anyone else want to chime in on
the bikeshed painting discussion of the best QMP name?
--
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: 620 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm
2012-07-16 16:37 ` Eric Blake
@ 2012-07-17 10:55 ` Kevin Wolf
0 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2012-07-17 10:55 UTC (permalink / raw)
To: Eric Blake; +Cc: Pavel Hrdina, qemu-devel
Am 16.07.2012 18:37, schrieb Eric Blake:
> On 07/16/2012 02:12 AM, Pavel Hrdina wrote:
>> On 07/12/2012 07:59 PM, Eric Blake wrote:
>>> On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
>>>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>>>> ---
>>>> hmp-commands.hx | 2 +-
>>>> hmp.c | 10 ++++++++++
>>>> hmp.h | 1 +
>>>> qapi-schema.json | 17 +++++++++++++++++
>>>> qmp-commands.hx | 24 ++++++++++++++++++++++++
>>>> savevm.c | 21 +++++++++++----------
>>>> sysemu.h | 1 -
>>>> 7 files changed, 64 insertions(+), 12 deletions(-)
>>>> +
>>>> +##
>>>> +# @delvm:
>>> This name is worse than 'loadvm' or 'savevm', in that we are NOT
>>> deleting the entire vm, but a named snapshot stored within the vm.
>>> Would naming it 'delete-vm-snapshot' make more sense (in which case the
>>> others might make more sense as 'save-vm-snapshot' and
>>> 'load-vm-snapshot')?
>>>
>> This naming looks nice. I definitely agree that it could be
>> save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and
>> query-vm-snapshots.
>
> On seeing that spelled out, I wonder if the '-vm' is just noise; where
> we could use 'query-snapshots' instead of 'query-vm-snapshots'. Then
> again, we already have 'blockdev-snapshot-sync', which is an entirely
> different snapshot (just a block device, rather than the entire VM), so
> dropping -vm is probably a bad idea. Anyone else want to chime in on
> the bikeshed painting discussion of the best QMP name?
Sure. ;-)
If there is a blockdev-snapshot-sync, then the counterparts for the
whole VM shouldn't be called load/save/del-vm-snapshot, but
vm-snapshot-load/save/del. And possibly add a -sync as well.
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm Pavel Hrdina
2012-07-12 17:53 ` Eric Blake
@ 2012-07-23 20:41 ` Luiz Capitulino
1 sibling, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2012-07-23 20:41 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
On Thu, 12 Jul 2012 18:55:15 +0200
Pavel Hrdina <phrdina@redhat.com> wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> hmp-commands.hx | 2 +-
> hmp.c | 10 ++++++++++
> hmp.h | 1 +
> qapi-schema.json | 22 ++++++++++++++++++++++
> qerror.c | 24 ++++++++++++++++++++++++
> qerror.h | 18 ++++++++++++++++++
> qmp-commands.hx | 26 ++++++++++++++++++++++++++
> savevm.c | 29 ++++++++++++++---------------
> sysemu.h | 1 -
> 9 files changed, 116 insertions(+), 17 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f5d9d91..e8c5325 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -267,7 +267,7 @@ ETEXI
> .args_type = "name:s?",
> .params = "[tag|id]",
> .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
> - .mhandler.cmd = do_savevm,
> + .mhandler.cmd = hmp_savevm,
> },
>
> STEXI
> diff --git a/hmp.c b/hmp.c
> index 4c6d4ae..491599d 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1002,3 +1002,13 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
> qmp_netdev_del(id, &err);
> hmp_handle_error(mon, &err);
> }
> +
> +void hmp_savevm(Monitor *mon, const QDict *qdict)
> +{
> + const char *name = qdict_get_try_str(qdict, "name");
> + Error *err = NULL;
> +
> + qmp_savevm(!!name, name, &err);
> +
> + hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 79d138d..dc6410b 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const QDict *qdict);
> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_savevm(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1ab5dbd..4db1447 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1868,3 +1868,25 @@
> # Since: 0.14.0
> ##
> { 'command': 'netdev_del', 'data': {'id': 'str'} }
> +
> +##
> +# @savevm:
Let's call it save-vm.
> +#
> +# Create a snapshot of the whole virtual machine. If 'tag' is provided,
> +# it is used as human readable identifier. If there is already a snapshot
> +# with the same tag or ID, it is replaced.
Please, document that the vm is automatically stopped and resumed, and that
this can take a long time.
> +#
> +# @name: tag or id of new or existing snapshot
I don't like the idea of making 'name' optional in QMP. We could (and should)
have it as optional in HMP though. This means that we should move the code
that auto-generates it to HMP.
Also, I remember an old discussion about distinguishing between 'tag' and 'id'.
I remember it was difficult to do, so we could choose not to do it, but let's
re-evaluate this again.
> +#
> +# Returns: Nothing on success
> +# If there is a writable device not supporting snapshots,
> +# SnapshotNotSupported
> +# If no block device can accept snapshots, SnapshotNotAccepted
> +# If an error occures while creating a snapshot, SnapshotCreateFailed
> +# If open a block device for vm state fail, SnapshotOpenFailed
> +# If an error uccures while writing vm state, SnapshotWriteFailed
> +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed
I don't think we should re-create all these errors, more comments on that
below.
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> \ No newline at end of file
> diff --git a/qerror.c b/qerror.c
> index 92c4eff..4e6efa1 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -309,6 +309,30 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Could not start VNC server on %(target)",
> },
> {
> + .error_fmt = QERR_SNAPSHOT_CREATE_FAILED,
> + .desc = "Error '%(errno)' while creating snapshot on '%(device)'",
> + },
> + {
> + .error_fmt = QERR_SNAPSHOT_DELETE_FAILED,
> + .desc = "Error '%(errno)' while deleting snapshot on '%(device)'",
> + },
> + {
> + .error_fmt = QERR_SNAPSHOT_NOT_ACCEPTED,
> + .desc = "No block device can accept snapshots",
> + },
> + {
> + .error_fmt = QERR_SNAPSHOT_NOT_SUPPORTED,
> + .desc = "Device '%(device)' is writable but does not support snapshots",
> + },
> + {
> + .error_fmt = QERR_SNAPSHOT_OPEN_FAILED,
> + .desc = "Error while opening snapshot on '%(device)'",
> + },
> + {
> + .error_fmt = QERR_SNAPSHOT_WRITE_FAILED,
> + .desc = "Error '%(errno)' while writing snapshot on '%(device)'",
> + },
> + {
> .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
> .desc = "Connection can not be completed immediately",
> },
> diff --git a/qerror.h b/qerror.h
> index b4c8758..bc47f4a 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -251,6 +251,24 @@ QError *qobject_to_qerror(const QObject *obj);
> #define QERR_VNC_SERVER_FAILED \
> "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
>
> +#define QERR_SNAPSHOT_CREATE_FAILED \
> + "{ 'class': 'SnapshotCreateFailed', 'data': { 'device': %s, 'errno': %d } }"
> +
> +#define QERR_SNAPSHOT_DELETE_FAILED \
> + "{ 'class': 'SnapshotDeleteFailed', 'data': { 'device': %s, 'errno': %d } }"
> +
> +#define QERR_SNAPSHOT_NOT_ACCEPTED \
> + "{ 'class': 'SnapshotNotAccepted', 'data': {} }"
> +
> +#define QERR_SNAPSHOT_NOT_SUPPORTED \
> + "{ 'class': 'SnapshotNotSupported', 'data': { 'device': %s } }"
> +
> +#define QERR_SNAPSHOT_OPEN_FAILED \
> + "{ 'class': 'SnapshotOpenFailed', 'data': { 'device': %s } }"
> +
> +#define QERR_SNAPSHOT_WRITE_FAILED \
> + "{ 'class': 'SnapshotWriteFailed', 'data': { 'device': %s, 'errno': %d } }"
> +
> #define QERR_SOCKET_CONNECT_IN_PROGRESS \
> "{ 'class': 'SockConnectInprogress', 'data': {} }"
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e1a38e..a1c82f7 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1061,6 +1061,32 @@ Example:
>
> EQMP
> {
> + .name = "savevm",
> + .args_type = "name:s?",
> + .params = "name",
> + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
> + .mhandler.cmd_new = qmp_marshal_input_savevm
> + },
> +
> +SQMP
> +savevm
> +------
> +
> +Create a snapshot of the whole virtual machine. If 'tag' is provided,
> +it is used as human readable identifier. If there is already a snapshot
> +with the same tag or ID, it is replaced.
> +
> +Arguments:
> +
> +- "name": tag or id of new or existing snapshot
> +
> +Example:
> +
> +-> { "execute": "savevm", "arguments": { "name": "my_snapshot" } }
> +<- { "return": {} }
> +
> +EQMP
> + {
> .name = "qmp_capabilities",
> .args_type = "",
> .params = "",
> diff --git a/savevm.c b/savevm.c
> index a15c163..9fc1b53 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2033,7 +2033,7 @@ static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
> /*
> * Deletes snapshots of a given name in all opened images.
> */
> -static int del_existing_snapshots(Monitor *mon, const char *name)
> +static int del_existing_snapshots(Error **errp, const char *name)
> {
> BlockDriverState *bs;
> QEMUSnapshotInfo sn1, *snapshot = &sn1;
> @@ -2046,9 +2046,8 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
> {
> ret = bdrv_snapshot_delete(bs, name);
> if (ret < 0) {
> - monitor_printf(mon,
> - "Error while deleting snapshot on '%s'\n",
> - bdrv_get_device_name(bs));
> + error_set(errp, QERR_SNAPSHOT_DELETE_FAILED,
> + bdrv_get_device_name(bs), ret);
The Right Thing to do here is to change bdrv_snapshot_delete() to take an Error
argument and let it set the real error cause. Three considerations:
1. The QMP command shouldn't delete existing snapshots by default. Either,
we add a 'force' argument to it or don't delete snapshots in save-vm
at all (in which case a mngt app would have to delete the snapshots with the
same name manually, I prefer this approach btw)
2. This has to be done in a separate series
3. It's a good idea to wait for my series improving error_set() to be merged
before doing this
> return -1;
> }
> }
> @@ -2057,7 +2056,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
> return 0;
> }
>
> -void do_savevm(Monitor *mon, const QDict *qdict)
> +void qmp_savevm(bool has_name, const char *name, Error **errp)
> {
> BlockDriverState *bs, *bs1;
> QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
> @@ -2072,7 +2071,6 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> struct timeval tv;
> struct tm tm;
> #endif
> - const char *name = qdict_get_try_str(qdict, "name");
>
> /* Verify if there is a device that doesn't support snapshots and is writable */
> bs = NULL;
> @@ -2083,15 +2081,15 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> }
>
> if (!bdrv_can_snapshot(bs)) {
> - monitor_printf(mon, "Device '%s' is writable but does not support snapshots.\n",
> - bdrv_get_device_name(bs));
> + error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED,
> + bdrv_get_device_name(bs));
Please, use QERR_NOT_SUPPORTED instead. I know it doesn't allow any arguments
today, but I'm working on a series which will allow you to pass any arguments
you wish.
> return;
> }
> }
>
> bs = bdrv_snapshots();
> if (!bs) {
> - monitor_printf(mon, "No block device can accept snapshots\n");
> + error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED);
I think we should use QERR_NOT_SUPPORTED here too.
> return;
> }
>
> @@ -2112,7 +2110,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> #endif
> sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
>
> - if (name) {
> + if (has_name) {
> ret = bdrv_snapshot_find(bs, old_sn, name);
> if (ret >= 0) {
> pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
> @@ -2133,21 +2131,22 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> }
>
> /* Delete old snapshots of the same name */
> - if (name && del_existing_snapshots(mon, name) < 0) {
> + if (has_name && del_existing_snapshots(errp, name) < 0) {
> goto the_end;
> }
The QMP command should not delete existing snapshots, as said above.
>
> /* save the VM state */
> f = qemu_fopen_bdrv(bs, 1);
> if (!f) {
> - monitor_printf(mon, "Could not open VM state file\n");
> + error_set(errp, QERR_SNAPSHOT_OPEN_FAILED, bdrv_get_device_name(bs));
Please, use QERR_OPEN_FILE_FAILED instead. The filename should be the backing
file name.
> goto the_end;
> }
> ret = qemu_savevm_state(f);
> vm_state_size = qemu_ftell(f);
> qemu_fclose(f);
> if (ret < 0) {
> - monitor_printf(mon, "Error %d while writing VM\n", ret);
> + error_set(errp, QERR_SNAPSHOT_WRITE_FAILED,
> + bdrv_get_device_name(bs), ret);
The Right Thing to do here it to convert qemu_savevm_sstate() to take
an Error argument. The same considerations I wrote above about
del_existing_snapshots() apply here, except that you can report IOError
for most qemu_savevm_state() errors.
> goto the_end;
> }
>
> @@ -2160,8 +2159,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
> ret = bdrv_snapshot_create(bs1, sn);
> if (ret < 0) {
> - monitor_printf(mon, "Error while creating snapshot on '%s'\n",
> - bdrv_get_device_name(bs1));
> + error_set(errp, QERR_SNAPSHOT_CREATE_FAILED,
> + bdrv_get_device_name(bs1), ret);
Here too, bdrv_snapshot_create() should be changed to take an Error
argument.
> }
> }
> }
> diff --git a/sysemu.h b/sysemu.h
> index 6540c79..95d1207 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -69,7 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify);
>
> void qemu_add_machine_init_done_notifier(Notifier *notify);
>
> -void do_savevm(Monitor *mon, const QDict *qdict);
> int load_vmstate(const char *name);
> void do_delvm(Monitor *mon, const QDict *qdict);
> void do_info_snapshots(Monitor *mon);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm
2012-07-16 8:12 ` Pavel Hrdina
@ 2012-07-23 20:41 ` Luiz Capitulino
0 siblings, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2012-07-23 20:41 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: Eric Blake, qemu-devel
On Mon, 16 Jul 2012 10:12:08 +0200
Pavel Hrdina <phrdina@redhat.com> wrote:
> On 07/12/2012 07:53 PM, Eric Blake wrote:
> > On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
> >> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
> >> ---
> >> +++ b/qapi-schema.json
> >> @@ -1868,3 +1868,25 @@
> >> # Since: 0.14.0
> >> ##
> >> { 'command': 'netdev_del', 'data': {'id': 'str'} }
> >> +
> >> +##
> >> +# @savevm:
> >> +#
> >> +# Create a snapshot of the whole virtual machine. If 'tag' is provided,
> >> +# it is used as human readable identifier. If there is already a snapshot
> >> +# with the same tag or ID, it is replaced.
> >> +#
> >> +# @name: tag or id of new or existing snapshot
> > Needs an #optional designation, given the syntax below.
> >
> >> +#
> >> +# Returns: Nothing on success
> >> +# If there is a writable device not supporting snapshots,
> >> +# SnapshotNotSupported
> >> +# If no block device can accept snapshots, SnapshotNotAccepted
> >> +# If an error occures while creating a snapshot, SnapshotCreateFailed
> > s/occures/occurs/
> >
> >> +# If open a block device for vm state fail, SnapshotOpenFailed
> >> +# If an error uccures while writing vm state, SnapshotWriteFailed
> > s/uccures/occurs/
> >
> >> +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed
> > The notion of blindly overwriting the existing snapshot of the same name
> > seems a bit dangerous; should we take this opportunity to enhance the
> > command, and add a force flag, where things fail if the flag is false
> > but the name already exists, and where the reuse only happens if the
> > flag is present? (In fact, it would make my life in libvirt easier, as
> > I have an action item to make libvirt reject attempts to create a
> > snapshot with tag named '1' if an existing snapshot already has an id of
> > '1'.)
> This sounds reasonable and I think that this could be also good for HMP,
> not only for QMP.
> If there isn't anyone who disagree, I'll implement it.
I agree with it.
>
> Pavel
> >> +#
> >> +# Since: 1.2
> >> +##
> >> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> >> \ No newline at end of file
> > Fix that.
> >
> >> @@ -1061,6 +1061,32 @@ Example:
> >>
> >> EQMP
> >> {
> >> + .name = "savevm",
> >> + .args_type = "name:s?",
> >> + .params = "name",
> >> + .help = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
> >> + .mhandler.cmd_new = qmp_marshal_input_savevm
> >> + },
> >> +
> >> +SQMP
> >> +savevm
> > I know the HMP command is short, for ease of typing; but since 'savevm'
> > is not an English word, should we name the QMP command 'save-vm'?
> >
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm Pavel Hrdina
2012-07-12 17:56 ` Eric Blake
@ 2012-07-24 17:03 ` Luiz Capitulino
1 sibling, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2012-07-24 17:03 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
On Thu, 12 Jul 2012 18:55:16 +0200
Pavel Hrdina <phrdina@redhat.com> wrote:
>
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
> hmp-commands.hx | 2 +-
> hmp.c | 10 ++++++++++
> hmp.h | 1 +
> monitor.c | 12 ------------
> qapi-schema.json | 24 +++++++++++++++++++++++-
> qerror.c | 16 ++++++++++++++++
> qerror.h | 12 ++++++++++++
> qmp-commands.hx | 25 +++++++++++++++++++++++++
> savevm.c | 52 +++++++++++++++++++++++++++++++---------------------
> sysemu.h | 1 -
> vl.c | 7 ++++++-
> 11 files changed, 125 insertions(+), 37 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e8c5325..b145612 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -284,7 +284,7 @@ ETEXI
> .args_type = "name:s",
> .params = "tag|id",
> .help = "restore a VM snapshot from its tag or id",
> - .mhandler.cmd = do_loadvm,
> + .mhandler.cmd = hmp_loadvm,
> },
>
> STEXI
> diff --git a/hmp.c b/hmp.c
> index 491599d..6ed4c3f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1012,3 +1012,13 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
>
> hmp_handle_error(mon, &err);
> }
> +
> +void hmp_loadvm(Monitor *mon, const QDict *qdict)
> +{
> + const char *name = qdict_get_str(qdict, "name");
> + Error *err = NULL;
> +
> + qmp_loadvm(name, &err);
> +
> + hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index dc6410b..5623ae7 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -65,5 +65,6 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> void hmp_savevm(Monitor *mon, const QDict *qdict);
> +void hmp_loadvm(Monitor *mon, const QDict *qdict);
>
> #endif
> diff --git a/monitor.c b/monitor.c
> index f6107ba..3efbcc8 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2385,18 +2385,6 @@ static int do_closefd(Monitor *mon, const QDict *qdict, QObject **ret_data)
> return -1;
> }
>
> -static void do_loadvm(Monitor *mon, const QDict *qdict)
> -{
> - int saved_vm_running = runstate_is_running();
> - const char *name = qdict_get_str(qdict, "name");
> -
> - vm_stop(RUN_STATE_RESTORE_VM);
> -
> - if (load_vmstate(name) == 0 && saved_vm_running) {
> - vm_start();
> - }
> -}
> -
> int monitor_get_fd(Monitor *mon, const char *fdname)
> {
> mon_fd_t *monfd;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4db1447..7df6a90 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1889,4 +1889,26 @@
> #
> # Since: 1.2
> ##
> -{ 'command': 'savevm', 'data': {'*name': 'str'} }
> \ No newline at end of file
> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> +
> +##
> +# @loadvm:
> +#
> +# Set the whole virtual machine to the snapshot identified by the tag
> +# 'tag' or the unique snapshot ID 'id'.
> +#
> +# @name: tag or id of existing snapshot
> +#
> +# Returns: Nothing on success
> +# If no block device can accept snapshots, SnapshotNotAccepted
> +# If no snapshot is found, SnapshotNotFound
> +# If snapshot is only disk snapshot, SnapshotInvalid
> +# If there is a writable device not supporting snapshots,
> +# SnapshotNotSupported
> +# If snapshot activate failed, SnapshotActivateFailed
> +# If open a block device for vm state fail, SnapshotOpenFailed
> +# If an error occures while loading vm state, SnapshotLoadFailed
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'loadvm', 'data': {'name': 'str'} }
> diff --git a/qerror.c b/qerror.c
> index 4e6efa1..b9c7352 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -309,6 +309,10 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Could not start VNC server on %(target)",
> },
> {
> + .error_fmt = QERR_SNAPSHOT_ACTIVATE_FAILED,
> + .desc = "Error '%(errno)' while activating snapshot '%(name)' on '%(device)'",
> + },
> + {
> .error_fmt = QERR_SNAPSHOT_CREATE_FAILED,
> .desc = "Error '%(errno)' while creating snapshot on '%(device)'",
> },
> @@ -317,10 +321,22 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Error '%(errno)' while deleting snapshot on '%(device)'",
> },
> {
> + .error_fmt = QERR_SNAPSHOT_INVALID,
> + .desc = "Device '%(device)' disk-only snapshot. Revert to it offline using qemu-img.",
> + },
> + {
> + .error_fmt = QERR_SNAPSHOT_LOAD_FAILED,
> + .desc = "Error '%(errno)' while loading snapshot for '%(device)'",
> + },
> + {
> .error_fmt = QERR_SNAPSHOT_NOT_ACCEPTED,
> .desc = "No block device can accept snapshots",
> },
> {
> + .error_fmt = QERR_SNAPSHOT_NOT_FOUND,
> + .desc = "Device '%(device)' does not have the requested snapshot '%(name)'",
> + },
> + {
> .error_fmt = QERR_SNAPSHOT_NOT_SUPPORTED,
> .desc = "Device '%(device)' is writable but does not support snapshots",
> },
> diff --git a/qerror.h b/qerror.h
> index bc47f4a..da2abdd 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -251,15 +251,27 @@ QError *qobject_to_qerror(const QObject *obj);
> #define QERR_VNC_SERVER_FAILED \
> "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
>
> +#define QERR_SNAPSHOT_ACTIVATE_FAILED \
> + "{ 'class': 'SnapshotActivateFailed', 'data': { 'device': %s, 'name': %s, 'errno':%d } }"
> +
> #define QERR_SNAPSHOT_CREATE_FAILED \
> "{ 'class': 'SnapshotCreateFailed', 'data': { 'device': %s, 'errno': %d } }"
>
> #define QERR_SNAPSHOT_DELETE_FAILED \
> "{ 'class': 'SnapshotDeleteFailed', 'data': { 'device': %s, 'errno': %d } }"
>
> +#define QERR_SNAPSHOT_INVALID \
> + "{ 'class': 'SnapshotInvalid', 'data': { 'device': %s } }"
> +
> +#define QERR_SNAPSHOT_LOAD_FAILED \
> + "{ 'class': 'SnapshotLoadFailed', 'data': { 'device': %s, 'errno': %d } }"
> +
> #define QERR_SNAPSHOT_NOT_ACCEPTED \
> "{ 'class': 'SnapshotNotAccepted', 'data': {} }"
>
> +#define QERR_SNAPSHOT_NOT_FOUND \
> + "{ 'class': 'SnapshotNotFound', 'data': { 'device': %s, 'name': %s } }"
> +
> #define QERR_SNAPSHOT_NOT_SUPPORTED \
> "{ 'class': 'SnapshotNotSupported', 'data': { 'device': %s } }"
>
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index a1c82f7..02550f2 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1087,6 +1087,31 @@ Example:
>
> EQMP
> {
> + .name = "loadvm",
> + .args_type = "name:s",
> + .params = "name",
> + .help = "restore a VM snapshot from its tag or id",
> + .mhandler.cmd_new = qmp_marshal_input_loadvm
> + },
> +
> +SQMP
> +loadvm
> +------
> +
> +Set the whole virtual machine to the snapshot identified by the tag
> +'tag' or the unique snapshot ID 'id'.
> +
> +Arguments:
> +
> +- "name": tag or id of existing snapshot
> +
> +Example:
> +
> +-> { "execute": "loadvm", "arguments": { "name": "my_snapshot" } }
> +<- { "return": {} }
> +
> +EQMP
> + {
> .name = "qmp_capabilities",
> .args_type = "",
> .params = "",
> diff --git a/savevm.c b/savevm.c
> index 9fc1b53..c113dd4 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2196,27 +2196,30 @@ void qmp_xen_save_devices_state(const char *filename, Error **errp)
> return;
> }
>
> -int load_vmstate(const char *name)
> +void qmp_loadvm(const char *name, Error **errp)
> {
> BlockDriverState *bs, *bs_vm_state;
> QEMUSnapshotInfo sn;
> QEMUFile *f;
> int ret;
> + int saved_vm_running;
>
> bs_vm_state = bdrv_snapshots();
> if (!bs_vm_state) {
> - error_report("No block device supports snapshots");
> - return -ENOTSUP;
> + error_set(errp, QERR_SNAPSHOT_NOT_ACCEPTED);
> + return;
> }
As I said for save-vm, we can use QERR_NOT_SUPPORTED here.
>
> /* Don't even try to load empty VM states */
> ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
> if (ret < 0) {
> - return ret;
> + error_set(errp, QERR_SNAPSHOT_NOT_FOUND,
> + bdrv_get_device_name(bs_vm_state), name);
This can be QERR_INVALID_PARAMETER.
> + return;
> } else if (sn.vm_state_size == 0) {
> - error_report("This is a disk-only snapshot. Revert to it offline "
> - "using qemu-img.");
> - return -EINVAL;
> + error_set(errp, QERR_SNAPSHOT_INVALID,
> + bdrv_get_device_name(bs_vm_state));
> + return;
> }
>
> /* Verify if there is any device that doesn't support snapshots and is
> @@ -2229,19 +2232,22 @@ int load_vmstate(const char *name)
> }
>
> if (!bdrv_can_snapshot(bs)) {
> - error_report("Device '%s' is writable but does not support snapshots.",
> - bdrv_get_device_name(bs));
> - return -ENOTSUP;
> + error_set(errp, QERR_SNAPSHOT_NOT_SUPPORTED,
> + bdrv_get_device_name(bs));
QERR_NOT_SUPPORTED is fine I think.
> + return;
> }
>
> ret = bdrv_snapshot_find(bs, &sn, name);
> if (ret < 0) {
> - error_report("Device '%s' does not have the requested snapshot '%s'",
> - bdrv_get_device_name(bs), name);
> - return ret;
> + error_set(errp, QERR_SNAPSHOT_NOT_FOUND,
> + bdrv_get_device_name(bs), name);
QERR_PARAMETER_INVALID.
> + return;
> }
> }
>
> + saved_vm_running = runstate_is_running();
> + vm_stop(RUN_STATE_RESTORE_VM);
> +
> /* Flush all IO requests so they don't interfere with the new state. */
> bdrv_drain_all();
>
> @@ -2250,9 +2256,9 @@ int load_vmstate(const char *name)
> if (bdrv_can_snapshot(bs)) {
> ret = bdrv_snapshot_goto(bs, name);
> if (ret < 0) {
> - error_report("Error %d while activating snapshot '%s' on '%s'",
> - ret, name, bdrv_get_device_name(bs));
> - return ret;
> + error_set(errp, QERR_SNAPSHOT_ACTIVATE_FAILED,
> + bdrv_get_device_name(bs), name, ret);
The Right Thing to do here is to convert bdrv_snapshot_goto() to take an
Error argument.
> + return;
> }
> }
> }
> @@ -2260,8 +2266,9 @@ int load_vmstate(const char *name)
> /* restore the VM state */
> f = qemu_fopen_bdrv(bs_vm_state, 0);
> if (!f) {
> - error_report("Could not open VM state file");
> - return -EINVAL;
> + error_set(errp, QERR_SNAPSHOT_OPEN_FAILED,
> + bdrv_get_device_name(bs_vm_state));
> + return;
> }
>
> qemu_system_reset(VMRESET_SILENT);
> @@ -2269,11 +2276,14 @@ int load_vmstate(const char *name)
>
> qemu_fclose(f);
> if (ret < 0) {
> - error_report("Error %d while loading VM state", ret);
> - return ret;
> + error_set(errp, QERR_SNAPSHOT_LOAD_FAILED,
> + bdrv_get_device_name(bs_vm_state), ret);
> + return;
> }
>
> - return 0;
> + if (!error_is_set(errp) && saved_vm_running) {
> + vm_start();
> + }
> }
>
> void do_delvm(Monitor *mon, const QDict *qdict)
> diff --git a/sysemu.h b/sysemu.h
> index 95d1207..8e65651 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -69,7 +69,6 @@ void qemu_remove_exit_notifier(Notifier *notify);
>
> void qemu_add_machine_init_done_notifier(Notifier *notify);
>
> -int load_vmstate(const char *name);
> void do_delvm(Monitor *mon, const QDict *qdict);
> void do_info_snapshots(Monitor *mon);
>
> diff --git a/vl.c b/vl.c
> index 1329c30..28b982d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3630,8 +3630,13 @@ int main(int argc, char **argv, char **envp)
>
> qemu_system_reset(VMRESET_SILENT);
> if (loadvm) {
> - if (load_vmstate(loadvm) < 0) {
> + Error *err = NULL;
> +
> + qmp_loadvm(loadvm, &err);
> +
> + if (error_is_set(&err)) {
> autostart = 0;
> + error_report("%s\n", error_get_pretty(err));
You can use fprintf() here, and err has to be freed.
> }
> }
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm
2012-07-12 17:56 ` Eric Blake
@ 2012-07-24 17:04 ` Luiz Capitulino
0 siblings, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2012-07-24 17:04 UTC (permalink / raw)
To: Eric Blake; +Cc: Pavel Hrdina, qemu-devel
On Thu, 12 Jul 2012 11:56:28 -0600
Eric Blake <eblake@redhat.com> wrote:
> On 07/12/2012 10:55 AM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
>
> > +++ b/qapi-schema.json
> > @@ -1889,4 +1889,26 @@
> > #
> > # Since: 1.2
> > ##
> > -{ 'command': 'savevm', 'data': {'*name': 'str'} }
> > \ No newline at end of file
> > +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> > +
> > +##
> > +# @loadvm:
>
> Similar to my question in 1/4, should this be named 'load-vm'?
Yes.
>
> > +#
> > +# Set the whole virtual machine to the snapshot identified by the tag
> > +# 'tag' or the unique snapshot ID 'id'.
> > +#
> > +# @name: tag or id of existing snapshot
> > +#
> > +# Returns: Nothing on success
> > +# If no block device can accept snapshots, SnapshotNotAccepted
> > +# If no snapshot is found, SnapshotNotFound
> > +# If snapshot is only disk snapshot, SnapshotInvalid
> > +# If there is a writable device not supporting snapshots,
> > +# SnapshotNotSupported
> > +# If snapshot activate failed, SnapshotActivateFailed
> > +# If open a block device for vm state fail, SnapshotOpenFailed
> > +# If an error occures while loading vm state, SnapshotLoadFailed
>
> s/occures/occurs/
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands
2012-07-12 16:55 [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Pavel Hrdina
` (3 preceding siblings ...)
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots Pavel Hrdina
@ 2012-07-24 17:30 ` Luiz Capitulino
4 siblings, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2012-07-24 17:30 UTC (permalink / raw)
To: Pavel Hrdina; +Cc: qemu-devel
On Thu, 12 Jul 2012 18:55:14 +0200
Pavel Hrdina <phrdina@redhat.com> wrote:
> This patch-series converts savevm, loadvm, delvm and info snapshots from HMP
> into QMP as savevm, loadvm, delvm and query-snapshots.
I think we'll have to start propagating errors correctly as I pointed out
in my review, and then put this series on top. Let's wait for my series that
will improve error_set().
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-07-24 17:30 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-12 16:55 [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Pavel Hrdina
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm Pavel Hrdina
2012-07-12 17:53 ` Eric Blake
2012-07-16 8:12 ` Pavel Hrdina
2012-07-23 20:41 ` Luiz Capitulino
2012-07-23 20:41 ` Luiz Capitulino
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm Pavel Hrdina
2012-07-12 17:56 ` Eric Blake
2012-07-24 17:04 ` Luiz Capitulino
2012-07-24 17:03 ` Luiz Capitulino
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm Pavel Hrdina
2012-07-12 17:59 ` Eric Blake
2012-07-16 8:12 ` Pavel Hrdina
2012-07-16 16:37 ` Eric Blake
2012-07-17 10:55 ` Kevin Wolf
2012-07-12 16:55 ` [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots Pavel Hrdina
2012-07-12 18:03 ` Eric Blake
2012-07-16 8:15 ` Pavel Hrdina
2012-07-24 17:30 ` [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands Luiz Capitulino
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.