From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv4Qy-0007rJ-Qi for qemu-devel@nongnu.org; Thu, 12 Jun 2014 08:48:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wv4Qs-00085r-Po for qemu-devel@nongnu.org; Thu, 12 Jun 2014 08:48:48 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:46659 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv4Qs-00085Q-Fq for qemu-devel@nongnu.org; Thu, 12 Jun 2014 08:48:42 -0400 Date: Thu, 12 Jun 2014 14:48:40 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140612124839.GE24528@irqsave.net> References: <1402495503-4722-1-git-send-email-kwolf@redhat.com> <1402495503-4722-6-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1402495503-4722-6-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 5/9] block: Use common driver selection code for bdrv_open_file() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com The Wednesday 11 Jun 2014 =E0 16:04:59 (+0200), Kevin Wolf wrote : > This moves the bdrv_open_file() call a bit down so that it can use the > bdrv_open() code that selects the right block driver. >=20 > The code between the old and the new call site is either common code > (the error message for an unknown driver has been unified now) or > doesn't run with cleared BDRV_O_PROTOCOL (added an if block in one > place, whereas the right path was already asserted in another place) >=20 > Signed-off-by: Kevin Wolf > --- > block.c | 71 ++++++++++++++++++++++++++++-----------------------------= -------- > 1 file changed, 30 insertions(+), 41 deletions(-) >=20 > diff --git a/block.c b/block.c > index 011cfc4..2aa0a56 100644 > --- a/block.c > +++ b/block.c > @@ -1136,25 +1136,14 @@ static int bdrv_fill_options(QDict **options, c= onst char **pfilename, int flags, > * returns. Then, the caller is responsible for freeing it. If it inte= nds to > * reuse the QDict, QINCREF() should be called beforehand. > */ > -static int bdrv_file_open(BlockDriverState *bs, QDict **options, int f= lags, > - Error **errp) > +static int bdrv_file_open(BlockDriverState *bs, BlockDriver *drv, > + QDict **options, int flags, Error **errp) > { > - BlockDriver *drv; > const char *filename; > - const char *drvname; > Error *local_err =3D NULL; > int ret; > =20 > filename =3D qdict_get_try_str(*options, "filename"); > - drvname =3D qdict_get_str(*options, "driver"); > - > - drv =3D bdrv_find_format(drvname); > - if (!drv) { > - error_setg(errp, "Unknown driver '%s'", drvname); > - ret =3D -ENOENT; > - goto fail; > - } > - qdict_del(*options, "driver"); > =20 > /* Open the file */ > if (!drv->bdrv_file_open) { > @@ -1460,35 +1449,23 @@ int bdrv_open(BlockDriverState **pbs, const cha= r *filename, > bs->options =3D options; > options =3D qdict_clone_shallow(options); > =20 > - if (flags & BDRV_O_PROTOCOL) { maybe you could use a bool protocol =3D flags & BDRV_O_PROTOCOL at the be= gining of the function. > - assert(!drv); > - ret =3D bdrv_file_open(bs, &options, flags & ~BDRV_O_PROTOCOL, > - &local_err); > - if (!ret) { > - drv =3D bs->drv; > - goto done; > - } else if (bs->drv) { > - goto close_and_fail; > - } else { > - goto fail; > - } > - } > - > /* Open image file without format layer */ > - if (flags & BDRV_O_RDWR) { > - flags |=3D BDRV_O_ALLOW_RDWR; > - } > - if (flags & BDRV_O_SNAPSHOT) { > - snapshot_flags =3D bdrv_temp_snapshot_flags(flags); > - flags =3D bdrv_backing_flags(flags); > - } > + if ((flags & BDRV_O_PROTOCOL) =3D=3D 0) { > + if (flags & BDRV_O_RDWR) { > + flags |=3D BDRV_O_ALLOW_RDWR; > + } > + if (flags & BDRV_O_SNAPSHOT) { > + snapshot_flags =3D bdrv_temp_snapshot_flags(flags); > + flags =3D bdrv_backing_flags(flags); > + } > =20 > - assert(file =3D=3D NULL); > - ret =3D bdrv_open_image(&file, filename, options, "file", > - bdrv_inherited_flags(flags), > - true, &local_err); > - if (ret < 0) { > - goto fail; > + assert(file =3D=3D NULL); > + ret =3D bdrv_open_image(&file, filename, options, "file", > + bdrv_inherited_flags(flags), > + true, &local_err); > + if (ret < 0) { > + goto fail; > + } > } > =20 > /* Find the right image format driver */ > @@ -1500,7 +1477,7 @@ int bdrv_open(BlockDriverState **pbs, const char = *filename, > drv =3D bdrv_find_format(drvname); > qdict_del(options, "driver"); > if (!drv) { > - error_setg(errp, "Invalid driver: '%s'", drvname); > + error_setg(errp, "Unknown driver: '%s'", drvname); > ret =3D -EINVAL; > goto fail; > } > @@ -1516,6 +1493,18 @@ int bdrv_open(BlockDriverState **pbs, const char= *filename, > } > =20 > /* Open the image */ > + if (flags & BDRV_O_PROTOCOL) { > + ret =3D bdrv_file_open(bs, drv, &options, flags & ~BDRV_O_PROT= OCOL, > + &local_err); > + if (!ret) { > + goto done; > + } else if (bs->drv) { > + goto close_and_fail; > + } else { > + goto fail; > + } > + } > + > ret =3D bdrv_open_common(bs, file, options, flags, drv, &local_err= ); > if (ret < 0) { > goto fail; > --=20 > 1.8.3.1 >=20 >=20 Reviewed-by: Benoit Canet