From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33133) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLb3d-0004dr-GV for qemu-devel@nongnu.org; Thu, 06 Mar 2014 11:22:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLb3Y-0008V0-La for qemu-devel@nongnu.org; Thu, 06 Mar 2014 11:22:05 -0500 Received: from mx1.redhat.com ([209.132.183.28]:63652) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLb3Y-0008Up-CK for qemu-devel@nongnu.org; Thu, 06 Mar 2014 11:22:00 -0500 Message-ID: <5318A0A3.4010400@redhat.com> Date: Thu, 06 Mar 2014 09:21:55 -0700 From: Eric Blake MIME-Version: 1.0 References: <1394120669-4675-1-git-send-email-kwolf@redhat.com> <1394120669-4675-2-git-send-email-kwolf@redhat.com> <20140306160236.GD22291@irqsave.net> In-Reply-To: <20140306160236.GD22291@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nskAthUMl473XmNXF60HxLjPaXvgkNkGv" Subject: Re: [Qemu-devel] [PATCH 1/3] blockdev: Fail blockdev-add with encrypted images List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nskAthUMl473XmNXF60HxLjPaXvgkNkGv Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 03/06/2014 09:02 AM, Beno=C3=AEt Canet wrote: > The Thursday 06 Mar 2014 =C3=A0 16:44:27 (+0100), Kevin Wolf wrote : >> Encrypted images need a password before they can be used, and we don't= >> want blockdev-add to create BDSes that aren't fully initialised. So fo= r >> now simply forbid encrypted images; we can come back to it later if we= >> need the functionality. >> >> - blockdev_init(NULL, qdict, &local_err); >> + dinfo =3D blockdev_init(NULL, qdict, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> goto fail; >> } >> =20 >> + if (bdrv_key_required(dinfo->bdrv)) { >> + drive_uninit(dinfo); >> + error_setg(errp, "blockdev-add doesn't support encrypted devi= ces"); >=20 >> + goto fail; > This goto fail; is an extra the code will flow to fail anyway. Ah, but notice how the above block also had the same pattern, and now that we injected a new if clause, the earlier block is still correct. This is defensive programming, so that future additions don't have to mess with control flow of earlier code when adding new code later on. I'm happy with the patch as-is: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --nskAthUMl473XmNXF60HxLjPaXvgkNkGv 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTGKCjAAoJEKeha0olJ0NqeUsH/iKH0oqV2iyZniSlIdYJ9hv3 Fg4DtoTJTo/2IzXf46jCisOtxsQ/wQPuJBMHC719e3oPiqZyxDbYNo9SJINof/Wa 0refqeOhwOWZi/VFi92YDcZiaz02qdwbMd1vSF9dBRebix8BbS6btENamqWAo2Zf mXsaCZHI98XMBo1BHUmoNWz9JUHKxb5bJ68toHp0f5hjDi0KSPDlY86Cdia08yGn d6XTZiMNM74AbvyaZrqKlKN4loQiF3Yq/afJqhbr/NVq/WrrAs6w1Bss6SND2AIv tVQ/VudLx7uqvkJqw4cREgFiZlIqm9h7aJGG+nmVgJCGbSMqPuDpL6pwRIC2qqM= =5AzM -----END PGP SIGNATURE----- --nskAthUMl473XmNXF60HxLjPaXvgkNkGv--