From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43384) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tt4CM-0006p2-P8 for qemu-devel@nongnu.org; Wed, 09 Jan 2013 17:32:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tt4CL-0003HW-AC for qemu-devel@nongnu.org; Wed, 09 Jan 2013 17:32:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:9996) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tt4CL-0003HM-3B for qemu-devel@nongnu.org; Wed, 09 Jan 2013 17:32:37 -0500 Message-ID: <50EDEFFC.9090508@redhat.com> Date: Wed, 09 Jan 2013 15:32:28 -0700 From: Eric Blake MIME-Version: 1.0 References: <92c5c71b9467824fe0423debd5d62ad6c7321106.1357741229.git.phrdina@redhat.com> In-Reply-To: <92c5c71b9467824fe0423debd5d62ad6c7321106.1357741229.git.phrdina@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enigB29FA2584D42E67AF46B128E" Subject: Re: [Qemu-devel] [PATCH 07/13] qapi: Convert savevm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina Cc: Wenchao Xia , qemu-devel@nongnu.org, phrdina@redhat.c0m, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigB29FA2584D42E67AF46B128E Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/09/2013 08:18 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > 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(-) >=20 For patches 1-6, you may add Reviewed-by: Eric Blake However, for this patch, I think we may need to think more about the desi= gn. > +++ 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 a= s @name, > +# it is used as human readable identifier. If there is already a snaps= hot > +# with the same tag or ID, it is replaced. > +# > +# The VM is automatically stopped and resumed and saving a snapshot ca= n 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enigB29FA2584D42E67AF46B128E Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQEcBAEBCAAGBQJQ7e/9AAoJEKeha0olJ0NqGw0IAIW++j7ZkG68INGrWqa4QFRt ErHORKYIlAdFZnVhZmKLWyu6Ip58ojrld6SruJLrazfeDodgpXnhS6viRcxXSgUJ l14m08/xth0JXvLaqRkpD2SbZbmBoZKNTKk9+EHvHMMERjoZKfVVkEKwr9kJWt/c JYZYpUHtKH4z7wpfWeoGSDUXYIlP7+Bop5xitSRSpcGIZ/B/KwUU2BGOwWTelUT9 PgkBB5Kmt/M8IvHp8pdbRQqrR7GvoYklo4eVHiIEVfj7vN5k7px+aA+SMUywPAhI +cC/Pgl2ghbiD2nnFJ8ahUMcQ1COlfu2y2UV23Y/mForcMF2aXn8MExww9+dJOM= =SSkt -----END PGP SIGNATURE----- --------------enigB29FA2584D42E67AF46B128E--