The Wednesday 11 Jun 2014 à 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.
>
> We also catch cases now where both the drv argument and a "driver"
> option is specified, e.g. by specifying -drive format=qcow2,driver=raw
>
> Signed-off-by: Kevin Wolf <kw...@redhat.com>
> ---
> block.c | 76
> ++++++++++++++++++++++++----------------------
> tests/qemu-iotests/051 | 1 +
> tests/qemu-iotests/051.out | 5 ++-
> 3 files changed, 45 insertions(+), 37 deletions(-)
>
> 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
> *filename, 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 = *pfilename;
> const char *drvname;
> bool protocol = flags & BDRV_O_PROTOCOL;
> bool parse_filename = false;
> Error *local_err = NULL;
> - BlockDriver *drv;
>
> /* Parse json: pseudo-protocol */
> if (filename && g_str_has_prefix(filename, "json:")) {
> @@ -1062,12 +1061,8 @@ static int bdrv_fill_options(QDict **options, const
> char **pfilename, int flags,
> *pfilename = filename = NULL;
> }
>
> - 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 = true;
> @@ -1082,30 +1077,41 @@ static int bdrv_fill_options(QDict **options, const
> char **pfilename, int flags,
> filename = qdict_get_try_str(*options, "filename");
> drvname = qdict_get_try_str(*options, "driver");
>
> - if (!drvname) {
> - if (filename) {
> - drv = 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 = drv->format_name;
> + qdict_put(*options, "driver", qstring_from_str(drvname));
> + } else {
> + if (!drvname && protocol) {
> + if (filename) {
> + drv = bdrv_find_protocol(filename, parse_filename);
> + if (!drv) {
> + error_setg(errp, "Unknown protocol");
> + return -EINVAL;
> + }
> +
> + drvname = drv->format_name;
> + qdict_put(*options, "driver", qstring_from_str(drvname));
> + } else {
> + error_setg(errp, "Must specify either driver or file");
> return -EINVAL;
> }
> -
> - drvname = 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 = bdrv_find_format(drvname);
> + if (!drv) {
> + error_setg(errp, "Unknown driver '%s'", drvname);
> + return -ENOENT;
> + }
> }
> }
>
> - drv = bdrv_find_format(drvname);
> - if (!drv) {
> - error_setg(errp, "Unknown driver '%s'", drvname);
> - return -ENOENT;
> - }
> + assert(drv || !protocol);
>
> /* 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 = qdict_new();
> }
>
> - ret = bdrv_fill_options(&options, &filename, flags, &local_err);
> + ret = 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,
> }
>
> /* Find the right image format driver */
> + drv = NULL;
> drvname = qdict_get_try_str(options, "driver");
> + assert(drvname || !(flags & BDRV_O_PROTOCOL));
> +
> if (drvname) {
> drv = bdrv_find_format(drvname);
> qdict_del(options, "driver");
> @@ -1495,19 +1504,14 @@ int bdrv_open(BlockDriverState **pbs, const char
> *filename,
> ret = -EINVAL;
> goto fail;
> }
> - }
> -
> - if (!drv) {
> - if (file) {
> - ret = find_image_format(file, filename, &drv, &local_err);
> - } else {
> - error_setg(errp, "Must specify either driver or file");
> - ret = -EINVAL;
> + } else if (file) {
> + ret = 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 = -EINVAL;
> goto fail;
> }
>
> 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
>
> run_qemu -drive file="$TEST_IMG",format=foo
> run_qemu -drive file="$TEST_IMG",driver=foo
> +run_qemu -drive file="$TEST_IMG",driver=raw,format=qcow2
>
> echo
> echo === Overriding backing file ===
> 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=TEST_DIR/t.qcow2,format=foo
> QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=foo: 'foo' invalid format
>
> Testing: -drive file=TEST_DIR/t.qcow2,driver=foo
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: could not open disk
> image TEST_DIR/t.qcow2: Invalid driver: 'foo'
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: could not open disk
> image TEST_DIR/t.qcow2: Unknown driver 'foo'
> +
> +Testing: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=raw,format=qcow2: could not
> open disk image TEST_DIR/t.qcow2: Driver specified twice
>
>
> === Overriding backing file ===
> --
> 1.8.3.1
>
>
That's faily huge patch. I didn't understood everything.
