From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36468)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mreitz@redhat.com>) id 1a4BEq-0004c4-RY
	for qemu-devel@nongnu.org; Wed, 02 Dec 2015 12:30:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <mreitz@redhat.com>) id 1a4BEm-000602-SQ
	for qemu-devel@nongnu.org; Wed, 02 Dec 2015 12:30:44 -0500
References: <1447127063-4662-1-git-send-email-mreitz@redhat.com>
	<1447127063-4662-8-git-send-email-mreitz@redhat.com>
	<w51zixugrex.fsf@maestria.local.igalia.com>
From: Max Reitz <mreitz@redhat.com>
Message-ID: <565F2AB6.8030902@redhat.com>
Date: Wed, 2 Dec 2015 18:30:30 +0100
MIME-Version: 1.0
In-Reply-To: <w51zixugrex.fsf@maestria.local.igalia.com>
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature";
	boundary="GVwMs0Ba1ejmKR21p0m2piRju53kjGDPS"
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 7/8] block: Make bdrv_open()
	return a BDS
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alberto Garcia <berto@igalia.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--GVwMs0Ba1ejmKR21p0m2piRju53kjGDPS
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable

On 01.12.2015 15:44, Alberto Garcia wrote:
> On Tue 10 Nov 2015 04:44:22 AM CET, Max Reitz wrote:
>> @@ -1398,32 +1397,21 @@ static int bdrv_open_inherit(BlockDriverState =
**pbs, const char *filename,
>>          bool options_non_empty =3D options ? qdict_size(options) : fa=
lse;
>>          QDECREF(options);
>> =20
>> -        if (*pbs) {
>> -            error_setg(errp, "Cannot reuse an existing BDS when refer=
encing "
>> -                       "another block device");
>> -            return -EINVAL;
>> -        }
>> -
>>          if (filename || options_non_empty) {
>>              error_setg(errp, "Cannot reference an existing block devi=
ce with "
>>                         "additional options or a new filename");
>> -            return -EINVAL;
>> +            return NULL;
>>          }
>> =20
>>          bs =3D bdrv_lookup_bs(reference, reference, errp);
>>          if (!bs) {
>> -            return -ENODEV;
>> +            return NULL;
>>          }
>>          bdrv_ref(bs);
>> -        *pbs =3D bs;
>> -        return 0;
>> +        return bs;
>>      }
>=20
> This last part is probably a bit simpler with just one return statement=
:
>=20
>            bs =3D bdrv_lookup_bs(reference, reference, errp);
>            if (bs) {
>                bdrv_ref(bs);
>            }
>            return bs;
>=20
> but I'm fine either way.

Yes, thanks for the suggestion; but I think I like it to be explicit
("ret =3D do_something(); if (ret indicates failure) { fail; } go_on;").
Maybe I'm just not flexible enough to discard my precious patterns. :-)

> Reviewed-by: Alberto Garcia <berto@igalia.com>

Thanks!

Max


--GVwMs0Ba1ejmKR21p0m2piRju53kjGDPS
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWXyq2AAoJEDuxQgLoOKytEroH/3JWrL2xER++Q0ybIQkhIHF/
flRHrJWOgxTlcONM/HcIk07riJ6itYDJHgxNhqazkEUlE9mx1usSDm3mkLP6+97o
SuD9L+q8Chn/+f3ebDzezxYVwtscsks4LTIJYJNM3QFJLGVyLA0I5voMwuCp9mMT
kIqNFUq2fK4zpcfET/v4FRzLMjVgiEdTCfWAcSo1cpjeM2b0lVxryyKAmQCdMa/W
qrsaHjKZFjD3ENjyxtaLiV4bz1mdIuwYqWKhc2n6hBovT/X+8gvj0dc9T8CaFx9t
Lf5JSZYGwdjwEcHzQnFOboGqs/g+Y1WFKIC/n1/9ahKcFtQpHFZaGWh7TqHGkc0=
=Be8O
-----END PGP SIGNATURE-----

--GVwMs0Ba1ejmKR21p0m2piRju53kjGDPS--