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

[Uri Lublin]

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 <address@hidden>
> ---
>  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.