From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50455) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvAnW-0003El-8Z for qemu-devel@nongnu.org; Tue, 15 Jan 2013 12:59:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TvAnU-0002Dr-Ph for qemu-devel@nongnu.org; Tue, 15 Jan 2013 12:59:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:7897) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvAnU-0002Dk-Hr for qemu-devel@nongnu.org; Tue, 15 Jan 2013 12:59:40 -0500 Message-ID: <50F598A2.9070903@redhat.com> Date: Tue, 15 Jan 2013 10:57:54 -0700 From: Eric Blake MIME-Version: 1.0 References: <1358147387-8221-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1358147387-8221-8-git-send-email-xiawenc@linux.vnet.ibm.com> <50F4973F.4090608@redhat.com> <50F52E41.90700@linux.vnet.ibm.com> In-Reply-To: <50F52E41.90700@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2LJLKGGUOUSPLRBSUHAPE" Subject: Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: aliguori@us.ibm.com, phrdina@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2LJLKGGUOUSPLRBSUHAPE Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/15/2013 03:24 AM, Wenchao Xia wrote: >>> >>> +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_in= fo, >>> + const char *name) >>> + if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) { >> >> >> This code comparison favors ids over names; so if I request to delvm 2= , >> I end up removing the second snapshot, not the first. This is okay, b= ut >> probably worth documenting, > how about: > /* if id is not NULL, try find it with id, if not exist, return NULL > * if id is NULL and name is not NULL, try find it with name. > */ if id and name is NULL, direct return fail. > int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,= > const char *id, const char *name) That would be pushing the burden onto the callers to decide whether they are doing an id lookup, a name lookup, or both. In QMP terms, that means that your QMP command to delete a snapshot would now need an additional optional argument to decide whether the associated name is only an id, only a name, or can match either. But I'm not sure you want that. What I was trying to get at is that given a single string "2", it does seem nicer to do both an id and a name lookup, and return the first hit; you just need to document that ids take preference over names (and thus, naming a snapshot "2" may make the snapshot become invisible by name, but not by id, if a later snapshot creation causes id 2 to be used). Then your QMP command for deleting a snapshot no longer needs to care whether "2" is an id or a name, just whether it matches. Hmm, while typing this, I thought of another snag. Suppose you have a VM with two disks, but where only the first disk previously had a snapshot with id 1. If I create a new snapshot across both disks, does that mean disk 1 gets id 2 while disk 2 gets id 1, or do both disks get id 2, even though that means disk 2 skips over id 1? As long as the snapshot is named, you can refer to the name to get the same snapshot across both disks, regardless of what id it has. But if the name is numeric, and id takes preference over name when doing a lookup, we could get ourselves into the situation where a snapshot created with name "2" can eventually never be restored in one piece, because the individual disks have different ids for the same snapshot name. So maybe we DO need a way after all for QMP to specify whether a name lookup is for id, name, or both. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2LJLKGGUOUSPLRBSUHAPE 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/ iQEcBAEBCAAGBQJQ9ZiiAAoJEKeha0olJ0NqKiAH/jUznnlYvmiBtO/IAbwjc9/w Lwrdc1Kn9w3PwsbiyRH7j4MFsOqUjy6ZGIZ6l900aDjrvAkzZt6pb8bhyFA0UpVO JvveCNvkkR9RFU8FX608wVh10L+6+aPAuDZIAHVXQ+ZLpF2j8rj1V1cjDID6vtZ6 w4KUAIFem++NGAY1EifRwjvhnibKX5e1baMRkmdLuXrN8Rb/IYyKVLfTFeAorLws 5OKOxYr+37/aLgz58StMADr59YHFhA76xgGYRDWizW8CmrrRKYYAF83blXaIGamb hfhfuVYAX/vP/EPP+PGsN7+aijpQ56uhYBcVWaI/TT5Jcjep+U4i4EZA9MEWeeQ= =rjtQ -----END PGP SIGNATURE----- ------enig2LJLKGGUOUSPLRBSUHAPE--