[4/9] block: Always pass driver name through options QDict

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 <kwolf@redhat.com>
---
 block.c                    | 76 ++++++++++++++++++++++++----------------------
 tests/qemu-iotests/051     |  1 +
 tests/qemu-iotests/051.out |  5 ++-
 3 files changed, 45 insertions(+), 37 deletions(-)

On 06/11/2014 08:04 AM, 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 <kwolf@redhat.com>
> ---
>  block.c                    | 76 ++++++++++++++++++++++++----------------------
>  tests/qemu-iotests/051     |  1 +
>  tests/qemu-iotests/051.out |  5 ++-
>  3 files changed, 45 insertions(+), 37 deletions(-)
> 

> @@ -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")) {

Slight conflict here when you clean up the dead 'filename &&' in patch
1; obvious resolution.

Reviewed-by: Eric Blake <eblake@redhat.com>

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 <kwolf@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.