From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55099) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv4Id-0004B0-Av for qemu-devel@nongnu.org; Thu, 12 Jun 2014 08:40:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wv4IU-0004bP-UY for qemu-devel@nongnu.org; Thu, 12 Jun 2014 08:40:11 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:46654 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv4IU-0004X6-Kj for qemu-devel@nongnu.org; Thu, 12 Jun 2014 08:40:02 -0400 Date: Thu, 12 Jun 2014 14:40:00 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140612123954.GD24528@irqsave.net> References: <1402495503-4722-1-git-send-email-kwolf@redhat.com> <1402495503-4722-5-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-5-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/9] block: Always pass driver name through options QDict 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:58 (+0200), Kevin Wolf wrote : > The "driver" entry in the options QDict is now only missing if we're > opening an image with format probing. >=20 > We also catch cases now where both the drv argument and a "driver" > option is specified, e.g. by specifying -drive format=3Dqcow2,driver=3D= raw >=20 > Signed-off-by: Kevin Wolf > --- > block.c | 76 ++++++++++++++++++++++++--------------= -------- > tests/qemu-iotests/051 | 1 + > tests/qemu-iotests/051.out | 5 ++- > 3 files changed, 45 insertions(+), 37 deletions(-) >=20 > diff --git a/block.c b/block.c > index b9c2b41..011cfc4 100644 > --- a/block.c > +++ b/block.c > @@ -1038,14 +1038,13 @@ static QDict *parse_json_filename(const char *f= ilename, Error **errp) > * filename/flags pair to option QDict entries. > */ > static int bdrv_fill_options(QDict **options, const char **pfilename, = int flags, > - Error **errp) > + BlockDriver *drv, Error **errp) > { > const char *filename =3D *pfilename; > const char *drvname; > bool protocol =3D flags & BDRV_O_PROTOCOL; > bool parse_filename =3D false; > Error *local_err =3D NULL; > - BlockDriver *drv; > =20 > /* Parse json: pseudo-protocol */ > if (filename && g_str_has_prefix(filename, "json:")) { > @@ -1062,12 +1061,8 @@ static int bdrv_fill_options(QDict **options, co= nst char **pfilename, int flags, > *pfilename =3D filename =3D NULL; > } > =20 > - if (!protocol) { > - return 0; > - } > - > /* Fetch the file name from the options QDict if necessary */ > - if (filename) { > + if (protocol && filename) { > if (filename && !qdict_haskey(*options, "filename")) { > qdict_put(*options, "filename", qstring_from_str(filename)= ); > parse_filename =3D true; > @@ -1082,30 +1077,41 @@ static int bdrv_fill_options(QDict **options, c= onst char **pfilename, int flags, > filename =3D qdict_get_try_str(*options, "filename"); > drvname =3D qdict_get_try_str(*options, "driver"); > =20 > - if (!drvname) { > - if (filename) { > - drv =3D bdrv_find_protocol(filename, parse_filename); > - if (!drv) { > - error_setg(errp, "Unknown protocol"); > + if (drv) { > + if (drvname) { > + error_setg(errp, "Driver specified twice"); > + return -EINVAL; > + } > + drvname =3D drv->format_name; > + qdict_put(*options, "driver", qstring_from_str(drvname)); > + } else { > + if (!drvname && protocol) { > + if (filename) { > + drv =3D bdrv_find_protocol(filename, parse_filename); > + if (!drv) { > + error_setg(errp, "Unknown protocol"); > + return -EINVAL; > + } > + > + drvname =3D drv->format_name; > + qdict_put(*options, "driver", qstring_from_str(drvname= )); > + } else { > + error_setg(errp, "Must specify either driver or file")= ; > return -EINVAL; > } > - > - drvname =3D drv->format_name; > - qdict_put(*options, "driver", qstring_from_str(drvname)); > - } else { > - error_setg(errp, "Must specify either driver or file"); > - return -EINVAL; > + } else if (drvname) { > + drv =3D bdrv_find_format(drvname); > + if (!drv) { > + error_setg(errp, "Unknown driver '%s'", drvname); > + return -ENOENT; > + } > } > } > =20 > - drv =3D bdrv_find_format(drvname); > - if (!drv) { > - error_setg(errp, "Unknown driver '%s'", drvname); > - return -ENOENT; > - } > + assert(drv || !protocol); > =20 > /* Driver-specific filename parsing */ > - if (drv->bdrv_parse_filename && parse_filename) { > + if (drv && drv->bdrv_parse_filename && parse_filename) { > drv->bdrv_parse_filename(filename, *options, &local_err); > if (local_err) { > error_propagate(errp, local_err); > @@ -1445,7 +1451,7 @@ int bdrv_open(BlockDriverState **pbs, const char = *filename, > options =3D qdict_new(); > } > =20 > - ret =3D bdrv_fill_options(&options, &filename, flags, &local_err); > + ret =3D bdrv_fill_options(&options, &filename, flags, drv, &local_= err); > if (local_err) { > error_propagate(errp, local_err); > return ret; > @@ -1486,7 +1492,10 @@ int bdrv_open(BlockDriverState **pbs, const char= *filename, > } > =20 > /* Find the right image format driver */ > + drv =3D NULL; > drvname =3D qdict_get_try_str(options, "driver"); > + assert(drvname || !(flags & BDRV_O_PROTOCOL)); > + > if (drvname) { > drv =3D bdrv_find_format(drvname); > qdict_del(options, "driver"); > @@ -1495,19 +1504,14 @@ int bdrv_open(BlockDriverState **pbs, const cha= r *filename, > ret =3D -EINVAL; > goto fail; > } > - } > - > - if (!drv) { > - if (file) { > - ret =3D find_image_format(file, filename, &drv, &local_err= ); > - } else { > - error_setg(errp, "Must specify either driver or file"); > - ret =3D -EINVAL; > + } else if (file) { > + ret =3D find_image_format(file, filename, &drv, &local_err); > + if (ret < 0) { > goto fail; > } > - } > - > - if (!drv) { > + } else { > + error_setg(errp, "Must specify either driver or file"); > + ret =3D -EINVAL; > goto fail; > } > =20 > diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 > index c4af131..30a712f 100755 > --- a/tests/qemu-iotests/051 > +++ b/tests/qemu-iotests/051 > @@ -92,6 +92,7 @@ echo > =20 > run_qemu -drive file=3D"$TEST_IMG",format=3Dfoo > run_qemu -drive file=3D"$TEST_IMG",driver=3Dfoo > +run_qemu -drive file=3D"$TEST_IMG",driver=3Draw,format=3Dqcow2 > =20 > echo > echo =3D=3D=3D Overriding backing file =3D=3D=3D > diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out > index ad60107..37cb049 100644 > --- a/tests/qemu-iotests/051.out > +++ b/tests/qemu-iotests/051.out > @@ -38,7 +38,10 @@ Testing: -drive file=3DTEST_DIR/t.qcow2,format=3Dfoo > QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,format=3Dfoo: 'foo' invalid = format > =20 > Testing: -drive file=3DTEST_DIR/t.qcow2,driver=3Dfoo > -QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,driver=3Dfoo: could not open= disk image TEST_DIR/t.qcow2: Invalid driver: 'foo' > +QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,driver=3Dfoo: could not open= disk image TEST_DIR/t.qcow2: Unknown driver 'foo' > + > +Testing: -drive file=3DTEST_DIR/t.qcow2,driver=3Draw,format=3Dqcow2 > +QEMU_PROG: -drive file=3DTEST_DIR/t.qcow2,driver=3Draw,format=3Dqcow2:= could not open disk image TEST_DIR/t.qcow2: Driver specified twice > =20 > =20 > =3D=3D=3D Overriding backing file =3D=3D=3D > --=20 > 1.8.3.1 >=20 >=20 That's faily huge patch. I didn't understood everything.