Re: [Qemu-devel] [PATCH] block: Don't parse protocol from file.filename

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]

On 07/10/2013 07:51 AM, Kevin Wolf wrote:
> One of the major reasons for doing something new for -blockdev and
> blockdev-add was that the old block layer code parses filenames instead
> of just taking them literally. So we should really leave it untouched
> when it's passing using the new interfaces (like -drive
> file.filename=...).
> 
> This allows opening relative file names that contain a colon.

Will a protocol prefix ever contain a '/'?  Would it be desirable to
state that relative file names containing a colon should be specified as
'./file:name', with the '/' serving as the escape that means relative
file rather than attempting to use protocol './file:', even when using
legacy options?

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                    | 17 ++++++++++++-----
>  block/sheepdog.c           |  2 +-
>  include/block/block.h      |  3 ++-
>  qemu-img.c                 |  4 ++--
>  tests/qemu-iotests/051     | 12 ++++++++++++
>  tests/qemu-iotests/051.out | 14 ++++++++++++++
>  6 files changed, 43 insertions(+), 9 deletions(-)

> 
> @@ -813,7 +817,10 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
>          drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR));
>          qdict_del(options, "driver");
>      } else if (filename) {
> -        drv = bdrv_find_protocol(filename);
> +        drv = bdrv_find_protocol(filename, allow_protocol_prefix);
> +        if (!drv) {
> +            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol");
> +        }

If you want to allow './' as a forceful non-protocol escape even in
legacy parsing, then this code may need tweaking.  Otherwise, I think
the code looks fine.

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

> +++ b/tests/qemu-iotests/051
> @@ -149,6 +149,18 @@ echo
>  run_qemu -drive file=$TEST_IMG,file.driver=file
>  run_qemu -drive file=$TEST_IMG,file.driver=qcow2
>  
> +echo
> +echo === Parsing protocol from file name ===
> +echo
> +
> +# Protocol strings are supposed to be parsed from traditional option strings,
> +# but not when using driver-specific options. We can distinguish them by the
> +# error message for non-existing files.

Is it also worth testing that we successfully open a file name with a
colon from driver-specific options, or is that harder to do portably
(since windows doesn't allow : in file names except for the drive prefix)?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]