From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVjwS-0000Rl-Ru for qemu-devel@nongnu.org; Fri, 26 Apr 2013 10:48:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVjwO-0007qj-Qq for qemu-devel@nongnu.org; Fri, 26 Apr 2013 10:48:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27871) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVjwO-0007qN-K9 for qemu-devel@nongnu.org; Fri, 26 Apr 2013 10:48:00 -0400 Message-ID: <517A9389.5070700@redhat.com> Date: Fri, 26 Apr 2013 08:47:37 -0600 From: Eric Blake MIME-Version: 1.0 References: <1366968675-1451-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1366968675-1451-5-git-send-email-xiawenc@linux.vnet.ibm.com> <20130426143418.GA7648@stefanha-thinkpad.redhat.com> In-Reply-To: <20130426143418.GA7648@stefanha-thinkpad.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2XTGXQLAHWGCIPEDGFKLF" Subject: Re: [Qemu-devel] [PATCH 4/7] block: distinguish id and name in bdrv_find_snapshot() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, phrdina@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com, pbonzini@redhat.com, Wenchao Xia This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2XTGXQLAHWGCIPEDGFKLF Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/26/2013 08:34 AM, Stefan Hajnoczi wrote: > On Fri, Apr 26, 2013 at 05:31:12PM +0800, Wenchao Xia wrote: >> To make it clear about id and name in searching, the API is changed >> a bit to distinguish them, and caller can choose to search by id or na= me. >> If not found, *errp will be set to tip why. >> >> Note that the caller logic is changed a bit: >> 1) In del_existing_snapshots() called by do_savevm(), it travers twice= >=20 > s/travers/traverse/ >=20 > Also in comments in the code. >=20 >> to find the snapshot, instead once, so matching sequence may change >> if there are unwisely chosen, mixed id and names. >> 2) In do_savevm(), same with del_existing_snapshot(), when it tries to= >> find the snapshot to overwrite, matching sequence may change for same >> reason. >> 3) In load_vmstate(), first when it tries to find the snapshot to be l= oaded, >> sequence may change for the same reason of above. Later in validation,= the >> logic is changed to be more strict to require both id and name matchin= g. >> 4) In do_info_snapshot(), in validation, the logic is changed to be mo= re >> strict to require both id and name matching. >=20 > It's easy to avoid changing semantics: keep the old name or id behavior= > around. Use the new name-and-id behavior for #3 and #4. >=20 > Please include a justification for breaking the search order. A good start for such a justification would be to edit parts of my email [1] into your commit message, showing how the old semantics make it IMPOSSIBLE to delete or load 'id 2 tag 1' without first getting 'id 1 tag 2' out of the way, assuming that a qcow2 file with poor naming conventions exists (even if such a bad file can only be created externally). The goal of a commit message is to convince the readers that a change in semantics is appropriate because the alternative of leaving things broken is worse. [1]https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03501.html --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2XTGXQLAHWGCIPEDGFKLF 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/ iQEcBAEBCAAGBQJRepOJAAoJEKeha0olJ0NqpPgH/RXQVLM+qIgAtifmlA9D9hAM cdkHQH7CZUdhb+s7LFcWgh3fDnwg8mm1hOFlvDrgJ7R1jWfxJ8giJWGG6Y8qskKT DGexLWNUks4NcDuwSOwsSdPQphPauI8JwWWz7R1A+aEEDl3XRvpAAmhW2Lce76Dt 7em6W7etshgmOW436ZYjZLZ36Y1bbRbtUVfx2eLOg9T7lAcVSak0UuXCXL9NFIZM CE9T6/PaigBLtNr1FfSNH5599E0VE1fGCb0GHQPCh2ne3wxFmKBbZHYbeoU5wL1L lVKJUvcrpNZNJ4bDSrG1qBUEgeIR9lQFJh55eMEBirquQevC/kW5Qrsr4FVKCtA= =6/8b -----END PGP SIGNATURE----- ------enig2XTGXQLAHWGCIPEDGFKLF--