From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58224) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMKua-000423-IA for qemu-devel@nongnu.org; Thu, 11 Oct 2012 11:43:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TMKuW-0007f1-6I for qemu-devel@nongnu.org; Thu, 11 Oct 2012 11:43:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8928) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TMKuV-0007et-SE for qemu-devel@nongnu.org; Thu, 11 Oct 2012 11:42:56 -0400 Message-ID: <5076E8F6.1020000@redhat.com> Date: Thu, 11 Oct 2012 09:42:46 -0600 From: Eric Blake MIME-Version: 1.0 References: <1348855033-17174-1-git-send-email-kwolf@redhat.com> <1348855033-17174-13-git-send-email-kwolf@redhat.com> In-Reply-To: <1348855033-17174-13-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enigE1472700C08EDB63725550E2" Subject: Re: [Qemu-devel] [PATCH 12/30] QAPI: add command for live block commit, 'block-commit' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Jeff Cody , qemu-devel@nongnu.org, anthony@codemonkey.ws This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigE1472700C08EDB63725550E2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 09/28/2012 11:56 AM, Kevin Wolf wrote: > From: Jeff Cody >=20 > The command for live block commit is added, which has the following > arguments: >=20 > + /* default top_bs is the active layer */ > + top_bs =3D bs; > + > + if (top) { > + if (strcmp(bs->filename, top) !=3D 0) { > + top_bs =3D bdrv_find_backing_image(bs, top); > + } > + } In light of Jeff's followup patches to fix bdrv_find_backing_image when dealing with relative file names, I see another bug here (but impact is limited to only a poor-quality error message, claiming that 'top was not found' instead of the intended 'commit of an active top is not supported'). That is, strcmp() on user-supplied 'top' is not guaranteed to succeed if the user provided a different spelling for the same file as was actually recorded in bs->filename, and since you have resorted to realpath() before strcmp() in bdrv_find_backing_image to cope with alternat user spellings, I think you also need to use realpath() before comparison here (probably a new helper function bdrv_is_equal() or some such name, that compares a user-supplied name with a bs). Saving this until a followup patch as part of your phase 2 series to add active image commit support is fine by me. --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enigE1472700C08EDB63725550E2 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 Mozilla - http://www.enigmail.net/ iQEcBAEBCAAGBQJQduj3AAoJEKeha0olJ0NqdygIAJMza3lnOaP7VvxkjlIU4GqU 7Hb6BEWfCUMuCvoQeezLXWtxnTGOjYD68MsUQS6KWrDJuqlnv9450GUCCZZVCU5d u94m87tbpbHi2GmE1pdiusYfsaJ4uYeUBMfFBYaOrc/IiUKgOUTluWdG9O6zhVW1 QrUkPyPcE/rPHRvIdwL4v+WjIFbHjiIpaly01lsX393A0cC6jAGA6A7DU4T29WWW 5eePXArs/47VikgyGRRyagevmzCWUihSonasay04SqfhQrvXBbyeLvLLiZ/F3dyU Xsxze0cdDi0UNX1jyo4C9z9xPfjW8jI4BXEu6gnDcu29+kyvNmtsWI/XBsEaLW8= =Zo5n -----END PGP SIGNATURE----- --------------enigE1472700C08EDB63725550E2--