[Qemu-devel] [PATCH] Modify qemu-img can not create local filename contain ":"

[Qemu-devel] [PATCH] Modify qemu-img can not create local filename contain ":"

[Jun Li]

Such as how to visit glusterfs:
file=gluster://1.2.3.4/testvol/a.img
file=gluster+tcp://1.2.3.4/testvol/a.img
file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img
file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img
file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
file=gluster+rdma://1.2.3.4:24007/testvol/a.img
----
So if only the path contain "://", the path maybe contain a protocol. So use
strstr() to replace func strcspn().

Signed-off-by: Jun Li <junmuzi@gmail.com>
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 2fd5482..aead10e 100644
--- a/block.c
+++ b/block.c
@@ -237,12 +237,12 @@ static int path_has_protocol(const char *path)
         is_windows_drive_prefix(path)) {
         return 0;
     }
-    p = path + strcspn(path, ":/\\");
+    p = strstr(path, ":/\\");
 #else
-    p = path + strcspn(path, ":/");
+    p = strstr(path, "://");
 #endif
 
-    return *p == ':';
+    return p != NULL;
 }
 
 int path_is_absolute(const char *path)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] Modify qemu-img can not create local filename contain ":"

[Max Reitz]

On 03.03.2014 15:02, Jun Li wrote:
> Such as how to visit glusterfs:
> file=gluster://1.2.3.4/testvol/a.img
> file=gluster+tcp://1.2.3.4/testvol/a.img
> file=gluster+tcp://1.2.3.4:24007/testvol/dir/a.img
> file=gluster+tcp://[1:2:3:4:5:6:7:8]/testvol/dir/a.img
> file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir/a.img
> file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
> file=gluster+rdma://1.2.3.4:24007/testvol/a.img
> ----
> So if only the path contain "://", the path maybe contain a protocol. So use
> strstr() to replace func strcspn().
>
> Signed-off-by: Jun Li <junmuzi@gmail.com>
> ---
>   block.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>

Currently, a protocol prefix is only defined to end with a colon, not 
with ":/" or "://". There are already protocol block drivers which do 
not require a slash after the colon such as blkdebug or blkverify and I 
deem it rather impossible to redefine their filename format now (in 
order to make them use ":/").

Thus, I do not think it will be this easy. I am in fact not even sure 
whether it is possible at all (automagically doing the right thing) – 
currently, a colon simpy is the separator between protocol and filename. 
Just using the "file" protocol per default for the whole filename if 
there is no protocol with a name equal to the pre-colon part is probably 
not what we want, since the user may actually be referring to some 
protocol that the qemu version he/she is using does not support (yet), 
in which case creating a file with a name including the pre-colon part 
(the protocol name) is most probably not the right thing to do.

Henceforth, in my opinion, we either have to ask the user in that case 
or we introduce some new option which disables protocol prefixes. I 
think the easiest way to do the latter is to introduce a 
bdrv_parse_filename() function for the "file" protocol drivers which 
remove a "file:" prefix if given. Then, the user could just specify 
"file:foo:bar.img" to reference a file named "foo:bar.img".

Currently, the behavior is that such a prefix will be interpreted 
correctly (the "file" protocol is selected) but it is not removed. Thus, 
"file:foo:bar.img" will actually reference a file named 
"file:foo:bar.img". One could argue that removing the prefix therefore 
breaks current behavior, but I sincerely hope nobody has relied on that 
behavior so far.


Max

Re: [Qemu-devel] [PATCH] Modify qemu-img can not create local filename contain ":"

[Eric Blake]

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

On 03/03/2014 10:47 AM, Max Reitz wrote:

> 
> Currently, a protocol prefix is only defined to end with a colon, not
> with ":/" or "://". There are already protocol block drivers which do
> not require a slash after the colon such as blkdebug or blkverify and I
> deem it rather impossible to redefine their filename format now (in
> order to make them use ":/").

It should ALWAYS be possible to open a file via absolute path, or via
'./file:with:colon'.  That is, our check for a protocol should be any
prefix that contains a ':' prior to a '/'.

> 
> Thus, I do not think it will be this easy. I am in fact not even sure
> whether it is possible at all (automagically doing the right thing) –
> currently, a colon simpy is the separator between protocol and filename.
> Just using the "file" protocol per default for the whole filename if
> there is no protocol with a name equal to the pre-colon part is probably
> not what we want, since the user may actually be referring to some
> protocol that the qemu version he/she is using does not support (yet),
> in which case creating a file with a name including the pre-colon part
> (the protocol name) is most probably not the right thing to do.

If the user wants a file name that contains a colon, use either an
absolute path name, or prefix the relative name with './'.  Either way
should cause a '/' to occur before the first ':', and since a protocol
consists only of a prefix prior to ':' without '/', it then becomes
obvious that a file name is intended.

> 
> Henceforth, in my opinion, we either have to ask the user in that case
> or we introduce some new option which disables protocol prefixes. I
> think the easiest way to do the latter is to introduce a
> bdrv_parse_filename() function for the "file" protocol drivers which
> remove a "file:" prefix if given. Then, the user could just specify
> "file:foo:bar.img" to reference a file named "foo:bar.img".

Yes, this would also make sense, if you can't live with './foo:bar.img'.

> 
> Currently, the behavior is that such a prefix will be interpreted
> correctly (the "file" protocol is selected) but it is not removed. Thus,
> "file:foo:bar.img" will actually reference a file named
> "file:foo:bar.img". One could argue that removing the prefix therefore
> breaks current behavior, but I sincerely hope nobody has relied on that
> behavior so far.

I think the fact that the file: prefix is NOT removed before hitting the
filesystem is a bug worth fixing - it should be fairly obvious that the
relative name in the filesystem should not include the 'file:' prefix,
but that 'file:' was the protocol that forced interpretation of the rest
of the string as a local filename.

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH] Modify qemu-img can not create local filename contain ":"

[Max Reitz]

On 03.03.2014 19:03, Eric Blake wrote:
> On 03/03/2014 10:47 AM, Max Reitz wrote:
>
>> Currently, a protocol prefix is only defined to end with a colon, not
>> with ":/" or "://". There are already protocol block drivers which do
>> not require a slash after the colon such as blkdebug or blkverify and I
>> deem it rather impossible to redefine their filename format now (in
>> order to make them use ":/").
> It should ALWAYS be possible to open a file via absolute path, or via
> './file:with:colon'.  That is, our check for a protocol should be any
> prefix that contains a ':' prior to a '/'.

Oh, yes, you are right, I forgot about that.

Max

>
>> Thus, I do not think it will be this easy. I am in fact not even sure
>> whether it is possible at all (automagically doing the right thing) –
>> currently, a colon simpy is the separator between protocol and filename.
>> Just using the "file" protocol per default for the whole filename if
>> there is no protocol with a name equal to the pre-colon part is probably
>> not what we want, since the user may actually be referring to some
>> protocol that the qemu version he/she is using does not support (yet),
>> in which case creating a file with a name including the pre-colon part
>> (the protocol name) is most probably not the right thing to do.
> If the user wants a file name that contains a colon, use either an
> absolute path name, or prefix the relative name with './'.  Either way
> should cause a '/' to occur before the first ':', and since a protocol
> consists only of a prefix prior to ':' without '/', it then becomes
> obvious that a file name is intended.
>
>> Henceforth, in my opinion, we either have to ask the user in that case
>> or we introduce some new option which disables protocol prefixes. I
>> think the easiest way to do the latter is to introduce a
>> bdrv_parse_filename() function for the "file" protocol drivers which
>> remove a "file:" prefix if given. Then, the user could just specify
>> "file:foo:bar.img" to reference a file named "foo:bar.img".
> Yes, this would also make sense, if you can't live with './foo:bar.img'.
>
>> Currently, the behavior is that such a prefix will be interpreted
>> correctly (the "file" protocol is selected) but it is not removed. Thus,
>> "file:foo:bar.img" will actually reference a file named
>> "file:foo:bar.img". One could argue that removing the prefix therefore
>> breaks current behavior, but I sincerely hope nobody has relied on that
>> behavior so far.
> I think the fact that the file: prefix is NOT removed before hitting the
> filesystem is a bug worth fixing - it should be fairly obvious that the
> relative name in the filesystem should not include the 'file:' prefix,
> but that 'file:' was the protocol that forced interpretation of the rest
> of the string as a local filename.
>

Re: [Qemu-devel] [PATCH] Modify qemu-img can not create local filename contain ":"

[jun muzi]

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

>>* Currently, a protocol prefix is only defined to end with a colon, not*
>>* with ":/" or "://". There are already protocol block drivers which do*
>*> not require a slash after the colon such as blkdebug or blkverify and I*
>* >deem it rather impossible to redefine their filename format now (in*
>>* order to make them use ":/").*

>It should ALWAYS be possible to open a file via absolute path, or via
>'./file:with:colon'.  That is, our check for a protocol should be any
>prefix that contains a ':' prior to a '/'.


Yes, check in the code,
static int path_has_protocol(const char *path)
{
    ...
    p = path + strcspn(path, ":/");
    ...
}
strcspn() function will return the anterior location of ":" or "/" in the
path. When the filename contains ':' via absolute path(eg:
/home/file:with:colon) or relative path(eg:./file:with:colon), the
strcspn() function will return the location of "/", then the
path_has_protocol() function will return 0.
Based on above discussion, when create image via :
# qemu-img create -f qcow2 ip-192.168.254.185\:asdf 5G
let qemu give more error information except "qemu-img: Unknown protocol
'ip-192.168.254.185:asdf'". Such as:
qemu-img: Unknown protocol 'ip-192.168.254.185:asdf'
Or using absolute path or relative path to create locale file.
---------
This maybe the most simple method to fixed this issue. thx.

Jun Li

[-- Attachment #2: Type: text/html, Size: 1720 bytes --]