From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34048) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV8aL-0006ip-JS for qemu-devel@nongnu.org; Wed, 24 Apr 2013 18:54:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UV8aI-0003yj-8i for qemu-devel@nongnu.org; Wed, 24 Apr 2013 18:54:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5469) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UV8aH-0003xT-Rd for qemu-devel@nongnu.org; Wed, 24 Apr 2013 18:54:42 -0400 Message-ID: <517862AC.3030500@redhat.com> Date: Wed, 24 Apr 2013 16:54:36 -0600 From: Eric Blake MIME-Version: 1.0 References: <2d4c5e75a5fa710d73fa4ffae03f4c143ba66519.1366817130.git.phrdina@redhat.com> In-Reply-To: <2d4c5e75a5fa710d73fa4ffae03f4c143ba66519.1366817130.git.phrdina@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2WJFXWEIGWJTNVLRJFWIX" Subject: Re: [Qemu-devel] [PATCH v2 04/12] qapi: Convert delvm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Hrdina Cc: kwolf@redhat.com, xiawenc@linux.vnet.ibm.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2WJFXWEIGWJTNVLRJFWIX Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/24/2013 09:32 AM, Pavel Hrdina wrote: > QMP command vm-snapshot-delete takes two parameters: name and id. They = are > optional, but one of the name or id must be provided. If both are provi= ded they > will match only the snapshot with the same name and id. The command ret= urns > SnapshotInfo only if the snapshot exists, otherwise it returns an error= message. >=20 > HMP command delvm now uses the new vm-snapshot-delete, but behave sligh= tly s/but behave/and behaves/ > different. The delvm takes one optional flag -i and one parameter name.= If you > provide only the name parameter, it will match only snapshots with that= name. > If you also provide the flag, it will match only snapshots with name as= id. > Information about successfully deleted snapshot will be printed, otherw= ise > an error message will be printed. >=20 > These improves behavior of the command to be more strict on selecting s= napshots > because actual behavior is wrong. Now if you want to delete snapshot wi= th name '2' > but there is no snapshot with that name it could delete snapshot with i= d '2' and > that isn't what you want. >=20 > There is also small hack to ensure that in each block device with diffe= rent > driver type the correct snapshot is deleted. The 'qcow2' and 'sheepdog'= internally > search at first for id but 'rbd' has only name and therefore search onl= y for name. >=20 > Signed-off-by: Pavel Hrdina > --- > hmp-commands.hx | 14 +++++----- > hmp.c | 33 +++++++++++++++++++++++ > hmp.h | 1 + > include/sysemu/sysemu.h | 1 - > qapi-schema.json | 17 ++++++++++++ > qmp-commands.hx | 38 +++++++++++++++++++++++++++ > savevm.c | 69 +++++++++++++++++++++++++++++++++++++++--= -------- > 7 files changed, 153 insertions(+), 20 deletions(-) >=20 > +void hmp_vm_snapshot_delete(Monitor *mon, const QDict *qdict) > +{ > + const char *name =3D qdict_get_try_str(qdict, "name"); > + const bool id =3D qdict_get_try_bool(qdict, "id", false); Don't know that the 'const' is really needed here, but doesn't hurt. > + Error *local_err =3D NULL; > + SnapshotInfo *info; > + > + if (id) { > + info =3D qmp_vm_snapshot_delete(false, NULL, true, name, &loca= l_err); > + } else { > + info =3D qmp_vm_snapshot_delete(true, name, false, NULL, &loca= l_err); > + } > + > + if (info) { > + char buf[256]; I know this fixed-size buffer is just a copy-and-paste from other code that displays snapshot information, but I really hate it. On the other hand, I can tolerate if we have it as an intermediate step between two series that both land in the same release. If your series goes in first, Wenchao's series that cleans up the fixed-size buffer will need to be rebased to tweak this additional spot. If Wenchao's patches go in first, then you will have a bit of rebase work to do. Since we are already deferring this series into 1.6, I think it would be nice to post a unified series of the best of both authors, rather than continuing to waffle on what should go in first. [And if I keep saying that often enough, I may end up getting my hands dirty and becoming the person that posts such a unified patch, although generally I don't like forcefully taking over someone else's initial work= =2E] > +++ b/qapi-schema.json > @@ -3505,3 +3505,20 @@ > '*asl_compiler_rev': 'uint32', > '*file': 'str', > '*data': 'str' }} > + > +## > +# @vm-snapshot-delete: > +# > +# Delete a snapshot identified by name or id or both. One of the name = or id > +# is required. It will returns SnapshotInfo of successfully deleted sn= apshot. s/returns/return/ > +# > +# @name: #optional tag of an existing snapshot > +# > +# @id: #optional id of an existing snapshot > +# > +# Returns: SnapshotInfo on success > +# > +# Since: 1.5 1.6, now that we are deferring. > +## > +{ 'command': 'vm-snapshot-delete', 'data': {'*name': 'str', '*id': 'st= r'}, > + 'returns': 'SnapshotInfo' } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 4d65422..9b15cb4 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1423,6 +1423,44 @@ Example: > =20 > EQMP > { > + .name =3D "vm-snapshot-delete", > + .args_type =3D "name:s?,id:s?", > + .params =3D "[tag] [id]", > + .help =3D "delete a VM snapshot from its tag or id", Sounds slightly better if you do s/from/based on/ > + .mhandler.cmd_new =3D qmp_marshal_input_vm_snapshot_delete > + }, > + > +SQMP > +vm-snapshot-delete > +------ > + > +Delete a snapshot identified by name or id or both. One of the name or= id > +is required. It will returns SnapshotInfo of successfully deleted snap= shot. s/returns/return/ > @@ -2556,31 +2557,73 @@ int load_vmstate(const char *name) > return 0; > } > =20 > -void do_delvm(Monitor *mon, const QDict *qdict) > +SnapshotInfo *qmp_vm_snapshot_delete(const bool has_name, const char *= name, > + const bool has_id, const char *id= , > + Error **errp) Actual function looks right to me. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2WJFXWEIGWJTNVLRJFWIX 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.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJReGKsAAoJEKeha0olJ0NqT58H/00aUqcWDwZ4R9rb+w20vGHE NCEuDyCs7x4LDiS+NP3BYBPIDbdAJWZP4ZKsIxJX9F1KqgECT1hLpEg1r6d1clTR tj4Emhqg2Ur7KVR3mYygVR934sFxyUnct7M1nWBMmTFYUfs18V46dlgDpsFYPH+B Nf/G+xvFJYN6X+u741K/XwlaHfEbB+W5C1bYI4ZTgGWMoCRzF/Wz+x66HUnlyy4n PuaEZX02J7cnbO6qSLCldD8qanfaqthhbKm9n/7bytGQYTgCBjrd1nB4dcZQrkKI P87XtyZRBIh4iN1sm6coLYMxLJ1fjvKAjaEYnRKOmk66Z7fh4tPrM35GYdmyasY= =30QE -----END PGP SIGNATURE----- ------enig2WJFXWEIGWJTNVLRJFWIX--