[Qemu-devel] [PATCH v2 0/5] block: Strip protocol prefixes from filenames

[Qemu-devel] [PATCH v2 0/5] block: Strip protocol prefixes from filenames

[Max Reitz]

As some kind of follow-up to the "block: Strip 'file:' prefix from
filenames" series, this series does the same thing for other protocol
drivers.

All protocol drivers which implement bdrv_probe() may rely on them being
selected based on that function returning success alone. However, they
may have been chosen through a protocol prefix as well. Thus, if they
currently do not implement bdrv_parse_filename(), they may be unaware of
that possible prefix and therefore fail to interpret such filenames
(and, in fact, all of those are unaware).

This series makes these drivers strip their respective prefix through
bdrv_parse_filename() and in bdrv_create(), if implemented.

The following protocol drivers are not touched by this series since they
already implement bdrv_parse_filename() and are thus very likely aware
of the prefix:
 - vvfat, nbd, blkdebug, blkverify, ssh, curl

The following protocol drivers are not touched by this series since they
do not implement bdrv_probe() and therefore always receive a prefixed
filename (unless they are selected through QMP options) which makes them
pretty much guaranteed to handle these prefixes correctly:
 - nfs, sheepdog, rbd, quorum, gluster, iscsi

Thus, only drivers implementing bdrv_probe() and not implementing
bdrv_parse_filename() have been touched.


Please note that this series does not strip the prefix in bdrv_probe().
This is due to the driver being selected anyway later on through the
protocol prefix, even though bdrv_probe() returned 0. More importantly,
according to a comment in bdrv_find_protocol() in block.c about why
bdrv_probe() occurs before the protocol prefix is interpreted, it seems
actually more desirable not to strip the prefix in bdrv_probe() (since
it may in fact not be a prefix but rather some obscure device naming
schema).


v2:
 - Patch 3: Use a common cdrom_parse_filename() for both Linux and
            FreeBSD [Benoît]
 - Patch 4: Fixed commit message [Eric]



Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/5:[----] [--] 'block/raw-posix: bdrv_parse_filename() for hdev'
002/5:[----] [--] 'block/raw-posix: bdrv_parse_filename() for floppy'
003/5:[0013] [FC] 'block/raw-posix: bdrv_parse_filename() for cdrom'
004/5:[----] [--] 'block/raw-posix: Strip protocol prefix on creation'
005/5:[----] [--] 'block/raw-win32: bdrv_parse_filename() for hdev'



Max Reitz (5):
  block/raw-posix: bdrv_parse_filename() for hdev
  block/raw-posix: bdrv_parse_filename() for floppy
  block/raw-posix: bdrv_parse_filename() for cdrom
  block/raw-posix: Strip protocol prefix on creation
  block/raw-win32: bdrv_parse_filename() for hdev

 block/raw-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 block/raw-win32.c | 10 ++++++++++
 2 files changed, 57 insertions(+)

-- 
1.9.0

[Qemu-devel] [PATCH v2 1/5] block/raw-posix: bdrv_parse_filename() for hdev

[Max Reitz]

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

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/raw-posix.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index e6b4c1f..ab32ff9 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1561,6 +1561,15 @@ static int check_hdev_writable(BDRVRawState *s)
     return 0;
 }
 
+static void hdev_parse_filename(const char *filename, QDict *options,
+                                Error **errp)
+{
+    /* The prefix is optional, just as for "file". */
+    strstart(filename, "host_device:", &filename);
+
+    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp)
 {
@@ -1805,6 +1814,7 @@ static BlockDriver bdrv_host_device = {
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_probe_device  = hdev_probe_device,
+    .bdrv_parse_filename = hdev_parse_filename,
     .bdrv_file_open     = hdev_open,
     .bdrv_close         = raw_close,
     .bdrv_reopen_prepare = raw_reopen_prepare,
-- 
1.9.0

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

[Max Reitz]

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

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/raw-posix.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index ab32ff9..4b8c183 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1844,6 +1844,15 @@ static BlockDriver bdrv_host_device = {
 };
 
 #ifdef __linux__
+static void floppy_parse_filename(const char *filename, QDict *options,
+                                  Error **errp)
+{
+    /* The prefix is optional, just as for "file". */
+    strstart(filename, "host_floppy:", &filename);
+
+    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static int floppy_open(BlockDriverState *bs, QDict *options, int flags,
                        Error **errp)
 {
@@ -1949,6 +1958,7 @@ static BlockDriver bdrv_host_floppy = {
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_probe_device	= floppy_probe_device,
+    .bdrv_parse_filename = floppy_parse_filename,
     .bdrv_file_open     = floppy_open,
     .bdrv_close         = raw_close,
     .bdrv_reopen_prepare = raw_reopen_prepare,
-- 
1.9.0

[Qemu-devel] [PATCH v2 3/5] block/raw-posix: bdrv_parse_filename() for cdrom

[Max Reitz]

The "host_cdrom" protocol drivers should strip the "host_cdrom:" prefix
from filenames if present.

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

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 4b8c183..697cd2e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1983,7 +1983,20 @@ static BlockDriver bdrv_host_floppy = {
     .bdrv_media_changed = floppy_media_changed,
     .bdrv_eject         = floppy_eject,
 };
+#endif
 
+#if defined(__linux__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+static void cdrom_parse_filename(const char *filename, QDict *options,
+                                 Error **errp)
+{
+    /* The prefix is optional, just as for "file". */
+    strstart(filename, "host_cdrom:", &filename);
+
+    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+#endif
+
+#ifdef __linux__
 static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
@@ -2070,6 +2083,7 @@ static BlockDriver bdrv_host_cdrom = {
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_probe_device	= cdrom_probe_device,
+    .bdrv_parse_filename = cdrom_parse_filename,
     .bdrv_file_open     = cdrom_open,
     .bdrv_close         = raw_close,
     .bdrv_reopen_prepare = raw_reopen_prepare,
@@ -2200,6 +2214,7 @@ static BlockDriver bdrv_host_cdrom = {
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_probe_device	= cdrom_probe_device,
+    .bdrv_parse_filename = cdrom_parse_filename,
     .bdrv_file_open     = cdrom_open,
     .bdrv_close         = raw_close,
     .bdrv_reopen_prepare = raw_reopen_prepare,
-- 
1.9.0

[Qemu-devel] [PATCH v2 4/5] block/raw-posix: Strip protocol prefix on creation

[Max Reitz]

The hdev_create() implementation in block/raw-posix.c is used by the
"host_device", "host_cdrom" and "host_floppy" protocol block drivers
together. Thus, any of the associated prefixes may occur and exactly one
should should be stripped, if it does (thus,
"host_device:host_cdrom:/dev/cdrom" is not shortened to "/dev/cdrom").

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

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 697cd2e..1688e16 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1776,6 +1776,18 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options,
     int ret = 0;
     struct stat stat_buf;
     int64_t total_size = 0;
+    bool has_prefix;
+
+    /* This function is used by all three protocol block drivers and therefore
+     * any of these three prefixes may be given.
+     * The return value has to be stored somewhere, otherwise this is an error
+     * due to -Werror=unused-value. */
+    has_prefix =
+        strstart(filename, "host_device:", &filename) ||
+        strstart(filename, "host_cdrom:" , &filename) ||
+        strstart(filename, "host_floppy:", &filename);
+
+    (void)has_prefix;
 
     /* Read out options */
     while (options && options->name) {
-- 
1.9.0

[Qemu-devel] [PATCH v2 5/5] block/raw-win32: bdrv_parse_filename() for hdev

[Max Reitz]

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

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/raw-win32.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 9954748..48cb2c2 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -593,6 +593,15 @@ static int hdev_probe_device(const char *filename)
     return 0;
 }
 
+static void hdev_parse_filename(const char *filename, QDict *options,
+                                Error **errp)
+{
+    /* The prefix is optional, just as for "file". */
+    strstart(filename, "host_device:", &filename);
+
+    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
+}
+
 static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp)
 {
@@ -663,6 +672,7 @@ static BlockDriver bdrv_host_device = {
     .protocol_name	= "host_device",
     .instance_size	= sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
+    .bdrv_parse_filename = hdev_parse_filename,
     .bdrv_probe_device	= hdev_probe_device,
     .bdrv_file_open	= hdev_open,
     .bdrv_close		= raw_close,
-- 
1.9.0

Re: [Qemu-devel] [PATCH v2 3/5] block/raw-posix: bdrv_parse_filename() for cdrom

[Benoît Canet]

The Saturday 08 Mar 2014 à 00:39:43 (+0100), Max Reitz wrote :
> The "host_cdrom" protocol drivers should strip the "host_cdrom:" prefix
> from filenames if present.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/raw-posix.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 4b8c183..697cd2e 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1983,7 +1983,20 @@ static BlockDriver bdrv_host_floppy = {
>      .bdrv_media_changed = floppy_media_changed,
>      .bdrv_eject         = floppy_eject,
>  };
> +#endif
>  
> +#if defined(__linux__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> +static void cdrom_parse_filename(const char *filename, QDict *options,
> +                                 Error **errp)
> +{
> +    /* The prefix is optional, just as for "file". */
> +    strstart(filename, "host_cdrom:", &filename);
> +
> +    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
> +}
> +#endif
> +
> +#ifdef __linux__
>  static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
>                        Error **errp)
>  {
> @@ -2070,6 +2083,7 @@ static BlockDriver bdrv_host_cdrom = {
>      .instance_size      = sizeof(BDRVRawState),
>      .bdrv_needs_filename = true,
>      .bdrv_probe_device	= cdrom_probe_device,
> +    .bdrv_parse_filename = cdrom_parse_filename,
>      .bdrv_file_open     = cdrom_open,
>      .bdrv_close         = raw_close,
>      .bdrv_reopen_prepare = raw_reopen_prepare,
> @@ -2200,6 +2214,7 @@ static BlockDriver bdrv_host_cdrom = {
>      .instance_size      = sizeof(BDRVRawState),
>      .bdrv_needs_filename = true,
>      .bdrv_probe_device	= cdrom_probe_device,
> +    .bdrv_parse_filename = cdrom_parse_filename,
>      .bdrv_file_open     = cdrom_open,
>      .bdrv_close         = raw_close,
>      .bdrv_reopen_prepare = raw_reopen_prepare,
> -- 
> 1.9.0
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH v2 0/5] block: Strip protocol prefixes from filenames

[Stefan Hajnoczi]

On Sat, Mar 08, 2014 at 12:39:40AM +0100, Max Reitz wrote:
> As some kind of follow-up to the "block: Strip 'file:' prefix from
> filenames" series, this series does the same thing for other protocol
> drivers.
> 
> All protocol drivers which implement bdrv_probe() may rely on them being
> selected based on that function returning success alone. However, they
> may have been chosen through a protocol prefix as well. Thus, if they
> currently do not implement bdrv_parse_filename(), they may be unaware of
> that possible prefix and therefore fail to interpret such filenames
> (and, in fact, all of those are unaware).
> 
> This series makes these drivers strip their respective prefix through
> bdrv_parse_filename() and in bdrv_create(), if implemented.
> 
> The following protocol drivers are not touched by this series since they
> already implement bdrv_parse_filename() and are thus very likely aware
> of the prefix:
>  - vvfat, nbd, blkdebug, blkverify, ssh, curl
> 
> The following protocol drivers are not touched by this series since they
> do not implement bdrv_probe() and therefore always receive a prefixed
> filename (unless they are selected through QMP options) which makes them
> pretty much guaranteed to handle these prefixes correctly:
>  - nfs, sheepdog, rbd, quorum, gluster, iscsi
> 
> Thus, only drivers implementing bdrv_probe() and not implementing
> bdrv_parse_filename() have been touched.
> 
> 
> Please note that this series does not strip the prefix in bdrv_probe().
> This is due to the driver being selected anyway later on through the
> protocol prefix, even though bdrv_probe() returned 0. More importantly,
> according to a comment in bdrv_find_protocol() in block.c about why
> bdrv_probe() occurs before the protocol prefix is interpreted, it seems
> actually more desirable not to strip the prefix in bdrv_probe() (since
> it may in fact not be a prefix but rather some obscure device naming
> schema).
> 
> 
> v2:
>  - Patch 3: Use a common cdrom_parse_filename() for both Linux and
>             FreeBSD [Benoît]
>  - Patch 4: Fixed commit message [Eric]
> 
> 
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/5:[----] [--] 'block/raw-posix: bdrv_parse_filename() for hdev'
> 002/5:[----] [--] 'block/raw-posix: bdrv_parse_filename() for floppy'
> 003/5:[0013] [FC] 'block/raw-posix: bdrv_parse_filename() for cdrom'
> 004/5:[----] [--] 'block/raw-posix: Strip protocol prefix on creation'
> 005/5:[----] [--] 'block/raw-win32: bdrv_parse_filename() for hdev'
> 
> 
> 
> Max Reitz (5):
>   block/raw-posix: bdrv_parse_filename() for hdev
>   block/raw-posix: bdrv_parse_filename() for floppy
>   block/raw-posix: bdrv_parse_filename() for cdrom
>   block/raw-posix: Strip protocol prefix on creation
>   block/raw-win32: bdrv_parse_filename() for hdev
> 
>  block/raw-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  block/raw-win32.c | 10 ++++++++++
>  2 files changed, 57 insertions(+)
> 
> -- 
> 1.9.0
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan