From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VO2Q6-0007Ro-AT for qemu-devel@nongnu.org; Mon, 23 Sep 2013 05:27:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VO2Q1-0001Mm-4t for qemu-devel@nongnu.org; Mon, 23 Sep 2013 05:27:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59098) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VO2Q0-0001MY-TW for qemu-devel@nongnu.org; Mon, 23 Sep 2013 05:27:01 -0400 Date: Mon, 23 Sep 2013 11:27:03 +0200 From: Kevin Wolf Message-ID: <20130923092703.GC2223@dhcp-200-207.str.redhat.com> References: <1379708668-13190-1-git-send-email-benoit@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1379708668-13190-1-git-send-email-benoit@irqsave.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] block: introduce BlockDriver.bdrv_needs_filename to enable some drivers. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: qemu-devel@nongnu.org, stefanha@redhat.com Am 20.09.2013 um 22:24 hat Beno=EEt Canet geschrieben: > Some drivers will have driver specifics options but no filename. > This new bool allow the block layer to treat them correctly. >=20 > The first driver of this type will be the quorum driver. >=20 > Signed-off-by: Benoit Canet > --- > block.c | 6 ++++-- > block/bochs.c | 1 + > block/cloop.c | 1 + > block/cow.c | 1 + > block/dmg.c | 1 + > block/gluster.c | 4 ++++ > block/iscsi.c | 1 + > block/parallels.c | 1 + > block/qcow.c | 1 + > block/qcow2.c | 1 + > block/qed.c | 1 + > block/raw-posix.c | 5 +++++ > block/raw-win32.c | 2 ++ > block/raw_bsd.c | 1 + > block/rbd.c | 1 + > block/sheepdog.c | 3 +++ > block/vdi.c | 1 + > block/vhdx.c | 1 + > block/vmdk.c | 1 + > block/vpc.c | 1 + > include/block/block_int.h | 1 + > 21 files changed, 34 insertions(+), 2 deletions(-) >=20 > diff --git a/block.c b/block.c > index 827549c..88099a1 100644 > --- a/block.c > +++ b/block.c > @@ -760,7 +760,8 @@ static int bdrv_open_common(BlockDriverState *bs, B= lockDriverState *file, > /* Open the image, either directly or using a protocol */ > if (drv->bdrv_file_open) { > assert(file =3D=3D NULL); > - assert(drv->bdrv_parse_filename || filename !=3D NULL); > + assert(drv->bdrv_parse_filename || !drv->bdrv_needs_filename |= | > + filename !=3D NULL); Doesn't drv->bdrv_parse_filename imply !drv->bdrv_needs_filename? (If I'm not mistaken, it is only checked here anyway because there was no bdrv_needs_filename yet.) I think it's enough to check the latter. > ret =3D drv->bdrv_file_open(bs, options, open_flags); > } else { > if (file =3D=3D NULL) { > @@ -870,7 +871,8 @@ int bdrv_file_open(BlockDriverState **pbs, const ch= ar *filename, > goto fail; > } > qdict_del(options, "filename"); > - } else if (!drv->bdrv_parse_filename && !filename) { > + } else if (!drv->bdrv_parse_filename && drv->bdrv_needs_filename &= & > + !filename) { > qerror_report(ERROR_CLASS_GENERIC_ERROR, > "The '%s' block driver requires a file name", > drv->format_name); > diff --git a/block/bochs.c b/block/bochs.c > index d7078c0..72a73aa 100644 > --- a/block/bochs.c > +++ b/block/bochs.c > @@ -237,6 +237,7 @@ static void bochs_close(BlockDriverState *bs) > static BlockDriver bdrv_bochs =3D { > .format_name =3D "bochs", > .instance_size =3D sizeof(BDRVBochsState), > + .bdrv_needs_filename =3D true, This looks wrong. Format drivers which only implement .bdrv_open() (and not .bdrv_file_open()) aren't even passed a filename. It would also be wrong because they can be used above non-file protocols. Do you get failures if you remove this line from format drivers? If so, we need to look into that and fix it. Kevin