Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames

[Uri Lublin <uril@redhat.com>]

Eduardo Habkost wrote:
> This patch fixes this issue:
> 
>   $ qemu-img create -f qcow2 /tmp/a:b 1G
>   $ qemu-system-x86_64 -hda qcow2:/tmp/a:b
>   qemu: could not open disk image /tmp/a:b
>   $

This patch looks good to me with the following comments:

1. I think the example above is wrong.
    a. There is no protocol "qcow2"
    b. We do not want to use "qcow2:/tmp/a:b" as filenames (been there).
       We want to use -drive file=/tmp/a:b,format=qcow2 (and
       similar "qemu-img info -f ").
    c. But, without this patch, even when using appropriate -drive, we'd
       still get the same result (could not open disk image).
       Qemu considers "/tmp/a" to be a protocol, which does not exist,
       so Qemu fails to open the image.
       With this patch, Qemu opens the file successfully.

> 
> Based on a suggestion by Daniel Berrange.
> 
> However, this is still just a workaround. The semantics of filenames
> containing colon characters (and how this can be escaped, avoided,
> or worked around) are not very clear.
> 
> Going further, what if we stop using "protocol:filename" strings
> internally, except where the user interface or external data really
> requires this format?

2. Do you mean something like adding a '-drive protocol=xxx' option ?

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  block.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 7c744c7..04488d6 100644
> --- a/block.c
> +++ b/block.c
> @@ -236,6 +236,14 @@ static BlockDriver *find_protocol(const char *filename)
>          is_windows_drive_prefix(filename))
>          return &bdrv_raw;
>  #endif
> +
> +    /* Protocol name will never start with a slash.
> +     * This allows the user to specify absolute filenames
> +     * containing a ":" character.
> +     */
> +    if (*filename == '/')
> +        return &bdrv_raw;

3. The patch limits protocols names to not start with '/' (full paths).
    I think we should apply the same logic to relative paths, so
    protocol names would not start with '.' as well (no protocol
    starts with '.' today):

  +   if ((*filename == '/') || (*filename == '.'))
  +       return &bdrv_raw;

4. Note that this patch does not limit image formats to raw, it
    just says "use no protocols for full/relative paths".

> +
>      p = strchr(filename, ':');
>      if (!p)
>          return &bdrv_raw;


Regards,
     Uri.