[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm |
Date: |
Tue, 24 Jul 2012 14:03:54 -0300 |
On Thu, 12 Jul 2012 18:55:16 +0200
Pavel Hrdina <address@hidden> wrote:
>
> Signed-off-by: Pavel Hrdina <address@hidden>
> ---
> 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.
> }
> }
>
- Re: [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm, (continued)
[Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots, Pavel Hrdina, 2012/07/12
[Qemu-devel] [RFC PATCH 2/4] qapi: Convert loadvm, Pavel Hrdina, 2012/07/12
Re: [Qemu-devel] [RFC PATCH 0/4] qapi: Convert snapshot's commands, Luiz Capitulino, 2012/07/24