[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/13] qapi: Convert savevm
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 07/13] qapi: Convert savevm |
Date: |
Wed, 09 Jan 2013 15:32:28 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 |
On 01/09/2013 08:18 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <address@hidden>
> ---
> hmp-commands.hx | 4 ++--
> hmp.c | 9 +++++++++
> hmp.h | 1 +
> include/sysemu/sysemu.h | 1 -
> qapi-schema.json | 18 ++++++++++++++++++
> qmp-commands.hx | 29 +++++++++++++++++++++++++++++
> savevm.c | 27 ++++++++++++---------------
> 7 files changed, 71 insertions(+), 18 deletions(-)
>
For patches 1-6, you may add Reviewed-by: Eric Blake <address@hidden>
However, for this patch, I think we may need to think more about the design.
> +++ b/qapi-schema.json
> @@ -3017,3 +3017,21 @@
> # Since: 1.3.0
> ##
> { 'command': 'nbd-server-stop' }
> +
> +##
> +# @vm-snapshot-save:
> +#
> +# Create a snapshot of the whole virtual machine. If tag is provided as
> @name,
> +# it is used as human readable identifier. If there is already a snapshot
> +# with the same tag or ID, it is replaced.
> +#
> +# The VM is automatically stopped and resumed and saving a snapshot can take
> +# a long time.
Since my initial review on your earlier RFC, I have changed my mind a
bit and started thinking that merely exposing the existing HMP commands
as raw QMP commands is gross. The HMP design is blocking - you can't
abort the snapshot, and since it is long-running, you can't do anything
else with the guest during that time, either. Of course, the guest is
paused, but why does it have to be? Couldn't we enhance the snapshot
code to use the same principles as live migration to take an internal
snapshot without pausing the guest up front?
This our chance to make life better by breaking the QMP interface into a
sequence of commands, and merely rewriting the existing HMP command call
the entire QMP sequence under the hood, rather than just exposing the
same clunky one-command design in QMP. In other words, I think it would
be nicer to have a job-based snapshot, where one command starts the
snapshot, and then another command can query status to see if the
snapshot is still underway, an event gets fired when the snapshot
completes, and where yet another command can be used to cleanly abort a
long-running snapshot if it turns out the snapshot is no longer needed
because something else came up that must be dealt with now.
In other words, I think this series and Wenchao Xia's series[1] need to
coordinate on a common design.
[1] https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg00645.html
> +#
> +# @name: #optional tag of new snapshot or tag|id of existing snapshot
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.4.0
> +##
> +{ 'command': 'vm-snapshot-save', 'data': {'*name': 'str'} }
But if we insist making the same clunky QMP interface, then this (plus
the followup improvements in 13/13 for adding a force option) seems like
a valid translation from HMP.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 00/13] convert savevm to use qapi and introduce qmp command, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 01/13] block: add error parameter to bdrv_snapshot_create() and related functions, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 03/13] savevm: add error parameter to qemu_savevm_state_begin(), Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 02/13] block: add error parameter to del_existing_snapshots(), Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 04/13] savevm: add error parameter to qemu_savevm_state_iterate(), Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 05/13] savevm: add error parameter to qemu_savevm_state_complete(), Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 07/13] qapi: Convert savevm, Pavel Hrdina, 2013/01/09
- Re: [Qemu-devel] [PATCH 07/13] qapi: Convert savevm,
Eric Blake <=
- [Qemu-devel] [PATCH 10/13] savevm: update return value from qemu_savevm_state_begin, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 08/13] qemu-img: introduce qemu_img_handle_error, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 09/13] block: update return value from bdrv_snapshot_create, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 06/13] savevm: add error parameter to qemu_savevm_state(), Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 11/13] savevm: update return value from qemu_savevm_state_complete, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 12/13] savevm: update return value from qemu_savevm_state, Pavel Hrdina, 2013/01/09
- [Qemu-devel] [PATCH 13/13] vm-snapshot-save: add force parameter, Pavel Hrdina, 2013/01/09