* [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
@ 2009-03-06 21:28 Eduardo Habkost
2009-03-08 11:28 ` Uri Lublin
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2009-03-06 21:28 UTC (permalink / raw)
To: qemu-devel
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
$
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?
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;
+
p = strchr(filename, ':');
if (!p)
return &bdrv_raw;
--
1.6.1
--
Eduardo
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-06 21:28 [Qemu-devel] [PATCH][RFC] Handling ':' on filenames Eduardo Habkost
@ 2009-03-08 11:28 ` Uri Lublin
2009-03-08 11:49 ` Stuart Brady
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Uri Lublin @ 2009-03-08 11:28 UTC (permalink / raw)
To: qemu-devel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-08 11:28 ` Uri Lublin
@ 2009-03-08 11:49 ` Stuart Brady
2009-03-09 14:32 ` Eduardo Habkost
2009-03-09 14:21 ` Eduardo Habkost
2009-03-09 14:27 ` Eduardo Habkost
2 siblings, 1 reply; 8+ messages in thread
From: Stuart Brady @ 2009-03-08 11:49 UTC (permalink / raw)
To: qemu-devel
On Sun, Mar 08, 2009 at 01:28:31PM +0200, Uri Lublin wrote:
> 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 ").
I can't help but feel that any approach to extracting filenames from the
middle of arguments will always have problems.
I suppose a syntax like this:
-drive file=/tmp/file,with,commas:and:colons ,format=qcow2
(i.e. with the next parameter to -drive starting in a new argument)...
wouldn't win many supporters though?
Cheers,
--
Stuart Brady
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-08 11:28 ` Uri Lublin
2009-03-08 11:49 ` Stuart Brady
@ 2009-03-09 14:21 ` Eduardo Habkost
2009-03-09 14:27 ` Eduardo Habkost
2 siblings, 0 replies; 8+ messages in thread
From: Eduardo Habkost @ 2009-03-09 14:21 UTC (permalink / raw)
To: kvm_autotest; +Cc: qemu-devel
Excerpts from kvm_autotest's message of Dom Mar 08 08:28:31 -0300 2009:
> 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 ").
Oops. Ignore that, I've pasted the wrong command-line. That was when I
was testing the behaviour of qemu when I specified other things before
the colon. The error appears when I do this:
$ qemu-system-x86_64 -hda /tmp/a:b
> 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.
>
Yes.
> >
> > 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 ?
Where possible, yes.
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-08 11:28 ` Uri Lublin
2009-03-08 11:49 ` Stuart Brady
2009-03-09 14:21 ` Eduardo Habkost
@ 2009-03-09 14:27 ` Eduardo Habkost
2009-03-10 3:59 ` Amit Shah
2 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2009-03-09 14:27 UTC (permalink / raw)
To: kvm_autotest; +Cc: qemu-devel
Excerpts from kvm_autotest's message of Dom Mar 08 08:28:31 -0300 2009:
<snip>
> > ---
> > 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;
Is ':' a valid character for Windows filenames? In this case, we
may want to check for backslashes also.
The "./filename" approach for relative paths seems nice to complete the fix,
but I still see this is a workaround to a more fundamental problem.
>
> 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.
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-08 11:49 ` Stuart Brady
@ 2009-03-09 14:32 ` Eduardo Habkost
2009-03-09 21:11 ` Jamie Lokier
0 siblings, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2009-03-09 14:32 UTC (permalink / raw)
To: Stuart Brady; +Cc: qemu-devel
Excerpts from Stuart Brady's message of Dom Mar 08 08:49:41 -0300 2009:
> On Sun, Mar 08, 2009 at 01:28:31PM +0200, Uri Lublin wrote:
> > 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 ").
>
> I can't help but feel that any approach to extracting filenames from the
> middle of arguments will always have problems.
>
> I suppose a syntax like this:
>
> -drive file=/tmp/file,with,commas:and:colons ,format=qcow2
>
> (i.e. with the next parameter to -drive starting in a new argument)...
> wouldn't win many supporters though?
The -drive option has a way to escape colons. For the filename
"/tmp/a,b,c", you can use:
-drive file=/tmp/a,,b,,c,format=qcow2
Not pretty, but it seems to work.
--
Eduardo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-09 14:32 ` Eduardo Habkost
@ 2009-03-09 21:11 ` Jamie Lokier
0 siblings, 0 replies; 8+ messages in thread
From: Jamie Lokier @ 2009-03-09 21:11 UTC (permalink / raw)
To: qemu-devel
Eduardo Habkost wrote:
> The -drive option has a way to escape colons. For the filename
> "/tmp/a,b,c", you can use:
>
> -drive file=/tmp/a,,b,,c,format=qcow2
>
> Not pretty, but it seems to work.
I wonder how you quote filenames from the monitor.
In my own management scripts I've taken to making a symlink with a
"safe" name to the real image file, and passing the safe name to KVM,
so that it's unambiguous. Causes a bit of fun with qcow2 files, which
refer to their backing files relative to the symlink not the target
file.
-- Jamie
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH][RFC] Handling ':' on filenames
2009-03-09 14:27 ` Eduardo Habkost
@ 2009-03-10 3:59 ` Amit Shah
0 siblings, 0 replies; 8+ messages in thread
From: Amit Shah @ 2009-03-10 3:59 UTC (permalink / raw)
To: qemu-devel; +Cc: kvm_autotest
On (Mon) Mar 09 2009 [11:27:53], Eduardo Habkost wrote:
> > > +
> > > + /* 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;
>
> Is ':' a valid character for Windows filenames? In this case, we
> may want to check for backslashes also.
I guess so; full paths are (at least used to be) something like
"C:\some-dir\some-file"
Amit
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-03-10 3:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-06 21:28 [Qemu-devel] [PATCH][RFC] Handling ':' on filenames Eduardo Habkost
2009-03-08 11:28 ` Uri Lublin
2009-03-08 11:49 ` Stuart Brady
2009-03-09 14:32 ` Eduardo Habkost
2009-03-09 21:11 ` Jamie Lokier
2009-03-09 14:21 ` Eduardo Habkost
2009-03-09 14:27 ` Eduardo Habkost
2009-03-10 3:59 ` Amit Shah
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.