[Qemu-devel] [PATCH 0/5] block/raw: Strip "file:" prefix from filenames

[Qemu-devel] [PATCH 0/5] block/raw: Strip "file:" prefix from filenames

[Max Reitz]

The "file" protocol drivers (block/raw-posix and block/raw-win32) may be
explicitly selected by prepending a "file:" prefix to a filename (as
with all other block protocols). However, currently, they do not strip
this prefix as they should.

This series fixes this issue.


Max Reitz (5):
  block: Keep "filename" option after parsing
  block/raw-posix: Implement bdrv_parse_filename()
  block/raw-posix: Strip "file:" prefix on creation
  block/raw-win32: Implement bdrv_parse_filename()
  block/raw-win32: Strip "file:" prefix on creation

 block.c           |  7 ++++++-
 block/raw-posix.c | 14 ++++++++++++++
 block/raw-win32.c | 14 ++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

-- 
1.9.0

[Qemu-devel] [PATCH 1/5] block: Keep "filename" option after parsing

[Max Reitz]

Currently, bdrv_file_open() always removes the "filename" option from
the options QDict after bdrv_parse_filename() has been (successfully)
called. However, for drivers with bdrv_needs_filename, it makes more
sense for bdrv_parse_filename() to overwrite the "filename" option and
for bdrv_file_open() to fetch the filename from there.

Since there currently are no drivers that implement
bdrv_parse_filename() and have bdrv_needs_filename set, this does not
change current behavior.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index f01b91c..845cbfb 100644
--- a/block.c
+++ b/block.c
@@ -1017,7 +1017,12 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
             ret = -EINVAL;
             goto fail;
         }
-        qdict_del(*options, "filename");
+
+        if (!drv->bdrv_needs_filename) {
+            qdict_del(*options, "filename");
+        } else {
+            filename = qdict_get_str(*options, "filename");
+        }
     }
 
     if (!drv->bdrv_file_open) {
-- 
1.9.0

Re: [Qemu-devel] [PATCH 1/5] block: Keep "filename" option after parsing

[Benoît Canet]

The Wednesday 05 Mar 2014 à 22:41:36 (+0100), Max Reitz wrote :
> Currently, bdrv_file_open() always removes the "filename" option from
> the options QDict after bdrv_parse_filename() has been (successfully)
> called. However, for drivers with bdrv_needs_filename, it makes more
> sense for bdrv_parse_filename() to overwrite the "filename" option and
> for bdrv_file_open() to fetch the filename from there.
> 
> Since there currently are no drivers that implement
> bdrv_parse_filename() and have bdrv_needs_filename set, this does not
> change current behavior.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index f01b91c..845cbfb 100644
> --- a/block.c
> +++ b/block.c
> @@ -1017,7 +1017,12 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
>              ret = -EINVAL;
>              goto fail;
>          }
> -        qdict_del(*options, "filename");
> +
> +        if (!drv->bdrv_needs_filename) {
> +            qdict_del(*options, "filename");
> +        } else {
> +            filename = qdict_get_str(*options, "filename");
> +        }
>      }
>  
>      if (!drv->bdrv_file_open) {
> -- 
> 1.9.0
> 
> 
Looks innocuous.

Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH 2/5] block/raw-posix: Implement bdrv_parse_filename()

[Max Reitz]

The "file" protocol driver should strip the "file:" prefix from
filenames if present.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-posix.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 161ea14..892145c 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -336,6 +336,17 @@ error:
 }
 #endif
 
+static void raw_parse_filename(const char *filename, QDict *options,
+                               Error **errp)
+{
+    /* The filename does not have to be prefixed by the protocol name, since
+     * "file" is the default protocol; therefore, the return value of this
+     * function call can be ignored. */
+    strstart(filename, "file:", &filename);
+
+    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static QemuOptsList raw_runtime_opts = {
     .name = "raw",
     .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
@@ -1412,6 +1423,7 @@ static BlockDriver bdrv_file = {
     .instance_size = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_probe = NULL, /* no probe for protocols */
+    .bdrv_parse_filename = raw_parse_filename,
     .bdrv_file_open = raw_open,
     .bdrv_reopen_prepare = raw_reopen_prepare,
     .bdrv_reopen_commit = raw_reopen_commit,
-- 
1.9.0

Re: [Qemu-devel] [PATCH 2/5] block/raw-posix: Implement bdrv_parse_filename()

[Benoît Canet]

The Wednesday 05 Mar 2014 à 22:41:37 (+0100), Max Reitz wrote :
> The "file" protocol driver should strip the "file:" prefix from
> filenames if present.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-posix.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 161ea14..892145c 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -336,6 +336,17 @@ error:
>  }
>  #endif
>  
> +static void raw_parse_filename(const char *filename, QDict *options,
> +                               Error **errp)
> +{
> +    /* The filename does not have to be prefixed by the protocol name, since
> +     * "file" is the default protocol; therefore, the return value of this
> +     * function call can be ignored. */
> +    strstart(filename, "file:", &filename);
> +
> +    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
> +}
> +
>  static QemuOptsList raw_runtime_opts = {
>      .name = "raw",
>      .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
> @@ -1412,6 +1423,7 @@ static BlockDriver bdrv_file = {
>      .instance_size = sizeof(BDRVRawState),
>      .bdrv_needs_filename = true,
>      .bdrv_probe = NULL, /* no probe for protocols */
> +    .bdrv_parse_filename = raw_parse_filename,
>      .bdrv_file_open = raw_open,
>      .bdrv_reopen_prepare = raw_reopen_prepare,
>      .bdrv_reopen_commit = raw_reopen_commit,
> -- 
> 1.9.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH 3/5] block/raw-posix: Strip "file:" prefix on creation

[Max Reitz]

The bdrv_create() implementation of the block/raw-posix "file" protocol
driver should strip the "file:" prefix from filenames if present.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-posix.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 892145c..e6b4c1f 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1241,6 +1241,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     int result = 0;
     int64_t total_size = 0;
 
+    strstart(filename, "file:", &filename);
+
     /* Read out options */
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-- 
1.9.0

Re: [Qemu-devel] [PATCH 3/5] block/raw-posix: Strip "file:" prefix on creation

[Benoît Canet]

The Wednesday 05 Mar 2014 à 22:41:38 (+0100), Max Reitz wrote :
> The bdrv_create() implementation of the block/raw-posix "file" protocol
> driver should strip the "file:" prefix from filenames if present.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-posix.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 892145c..e6b4c1f 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1241,6 +1241,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      int result = 0;
>      int64_t total_size = 0;
>  
> +    strstart(filename, "file:", &filename);
> +
>      /* Read out options */
>      while (options && options->name) {
>          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> -- 
> 1.9.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH 4/5] block/raw-win32: Implement bdrv_parse_filename()

[Max Reitz]

The "file" protocol driver should strip the "file:" prefix from
filenames if present.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-win32.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index ae1c8e6..0755434 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -251,6 +251,17 @@ static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
     }
 }
 
+static void raw_parse_filename(const char *filename, QDict *options,
+                               Error **errp)
+{
+    /* The filename does not have to be prefixed by the protocol name, since
+     * "file" is the default protocol; therefore, the return value of this
+     * function call can be ignored. */
+    strstart(filename, "file:", &filename);
+
+    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static QemuOptsList raw_runtime_opts = {
     .name = "raw",
     .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
@@ -504,6 +515,7 @@ static BlockDriver bdrv_file = {
     .protocol_name	= "file",
     .instance_size	= sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
+    .bdrv_parse_filename = raw_parse_filename,
     .bdrv_file_open	= raw_open,
     .bdrv_close		= raw_close,
     .bdrv_create	= raw_create,
-- 
1.9.0

Re: [Qemu-devel] [PATCH 4/5] block/raw-win32: Implement bdrv_parse_filename()

[Benoît Canet]

The Wednesday 05 Mar 2014 à 22:41:39 (+0100), Max Reitz wrote :
> The "file" protocol driver should strip the "file:" prefix from
> filenames if present.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-win32.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index ae1c8e6..0755434 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -251,6 +251,17 @@ static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
>      }
>  }
>  
> +static void raw_parse_filename(const char *filename, QDict *options,
> +                               Error **errp)
> +{
> +    /* The filename does not have to be prefixed by the protocol name, since
> +     * "file" is the default protocol; therefore, the return value of this
> +     * function call can be ignored. */
> +    strstart(filename, "file:", &filename);
> +
> +    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
> +}
> +
>  static QemuOptsList raw_runtime_opts = {
>      .name = "raw",
>      .head = QTAILQ_HEAD_INITIALIZER(raw_runtime_opts.head),
> @@ -504,6 +515,7 @@ static BlockDriver bdrv_file = {
>      .protocol_name	= "file",
>      .instance_size	= sizeof(BDRVRawState),
>      .bdrv_needs_filename = true,
> +    .bdrv_parse_filename = raw_parse_filename,
>      .bdrv_file_open	= raw_open,
>      .bdrv_close		= raw_close,
>      .bdrv_create	= raw_create,
> -- 
> 1.9.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH 5/5] block/raw-win32: Strip "file:" prefix on creation

[Max Reitz]

The bdrv_create() implementation of the block/raw-win32 "file" protocol
driver should strip the "file:" prefix from filenames if present.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-win32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 0755434..9954748 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -481,6 +481,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
     int fd;
     int64_t total_size = 0;
 
+    strstart(filename, "file:", &filename);
+
     /* Read out options */
     while (options && options->name) {
         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
-- 
1.9.0

Re: [Qemu-devel] [PATCH 5/5] block/raw-win32: Strip "file:" prefix on creation

[Benoît Canet]

The Wednesday 05 Mar 2014 à 22:41:40 (+0100), Max Reitz wrote :
> The bdrv_create() implementation of the block/raw-win32 "file" protocol
> driver should strip the "file:" prefix from filenames if present.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-win32.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 0755434..9954748 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -481,6 +481,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>      int fd;
>      int64_t total_size = 0;
>  
> +    strstart(filename, "file:", &filename);
> +
>      /* Read out options */
>      while (options && options->name) {
>          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> -- 
> 1.9.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH 0/5] block/raw: Strip "file:" prefix from filenames

[Eric Blake]

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

On 03/05/2014 02:41 PM, Max Reitz wrote:
> The "file" protocol drivers (block/raw-posix and block/raw-win32) may be
> explicitly selected by prepending a "file:" prefix to a filename (as
> with all other block protocols). However, currently, they do not strip
> this prefix as they should.
> 
> This series fixes this issue.

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

-- 
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 0/5] block/raw: Strip "file:" prefix from filenames

[Kevin Wolf]

Am 05.03.2014 um 22:41 hat Max Reitz geschrieben:
> The "file" protocol drivers (block/raw-posix and block/raw-win32) may be
> explicitly selected by prepending a "file:" prefix to a filename (as
> with all other block protocols). However, currently, they do not strip
> this prefix as they should.
> 
> This series fixes this issue.

Thanks, applied to the block branch.

This is just for consistency and there is no real use case for this,
right? If so, it would be even more consistent to do the same for the
host_* drivers provided by block/raw-{posix,win32}.c

I talked about this multiple times in discussions about handling colons
in filenames, so that prefixing file: would make it unambigious that the
colon is part of the file name and not a protocol name, but these days I
think we already have enough other ways to express this (the easiest
being ./test:foo, because protocols can't contain slashes)

Kevin

Re: [Qemu-devel] [PATCH 0/5] block/raw: Strip "file:" prefix from filenames

[Benoît Canet]

The Thursday 06 Mar 2014 à 13:57:55 (+0100), Kevin Wolf wrote :
> Am 05.03.2014 um 22:41 hat Max Reitz geschrieben:
> > The "file" protocol drivers (block/raw-posix and block/raw-win32) may be
> > explicitly selected by prepending a "file:" prefix to a filename (as
> > with all other block protocols). However, currently, they do not strip
> > this prefix as they should.
> > 
> > This series fixes this issue.
> 
> Thanks, applied to the block branch.
Heh my review was too late :)

> 
> This is just for consistency and there is no real use case for this,
> right? If so, it would be even more consistent to do the same for the
> host_* drivers provided by block/raw-{posix,win32}.c
> 
> I talked about this multiple times in discussions about handling colons
> in filenames, so that prefixing file: would make it unambigious that the
> colon is part of the file name and not a protocol name, but these days I
> think we already have enough other ways to express this (the easiest
> being ./test:foo, because protocols can't contain slashes)
> 
> Kevin
> 

Re: [Qemu-devel] [PATCH 0/5] block/raw: Strip "file:" prefix from filenames

[Max Reitz]

On 06.03.2014 13:57, Kevin Wolf wrote:
> Am 05.03.2014 um 22:41 hat Max Reitz geschrieben:
>> The "file" protocol drivers (block/raw-posix and block/raw-win32) may be
>> explicitly selected by prepending a "file:" prefix to a filename (as
>> with all other block protocols). However, currently, they do not strip
>> this prefix as they should.
>>
>> This series fixes this issue.
> Thanks, applied to the block branch.
>
> This is just for consistency and there is no real use case for this,
> right? If so, it would be even more consistent to do the same for the
> host_* drivers provided by block/raw-{posix,win32}.c

The use case would be that this allows explicitly specifying the prefix 
in order to create image files with a colon ("file:foo:bar.qcow2" for an 
image named "foo:bar.qcow2", although "./foo:bar.qcow2" works just as well).

I only looked for the "file" protocol drivers, but I'll have a look for 
other protocol drivers which currently do not strip their prefix as well 
(such as host_*, yes).

> I talked about this multiple times in discussions about handling colons
> in filenames, so that prefixing file: would make it unambigious that the
> colon is part of the file name and not a protocol name, but these days I
> think we already have enough other ways to express this (the easiest
> being ./test:foo, because protocols can't contain slashes)

Yes, but considering my first idea was to explicitly state the "file" 
prefix instead of the simpler ./, maybe others think the same way. ;-)


Max