[Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver

[Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver

[Max Reitz]

This series adds a passthrough JSON protocol block driver. Its filenames
are JSON objects prefixed by "json:". The objects are used as options
for opening another block device which will be the child of the JSON
device. Regarding this child device, the JSON driver behaves nearly the
same as raw_bsd in that it is just a passthrough driver. The only
difference is probably that the JSON driver identifies itself as a block
filter, in contrast to raw_bsd.

The purpose of this driver is that it may sometimes be desirable to
specify options for a block device where only a filename can be given,
e.g., for backing files. Using this should obviously be the exception,
but it is nice to have if actually needed.


v3:
 - rebased on top of Kevin's block branch
 - Patch 5: bdrv_invalidate_cache() now takes an Error parameter, add it
 - Patch 7: Don't use BDRV_BLOCK_RAW, but rather simply pass
   bdrv_get_block_status() to the underlying block device [Benoît/Peter]
 - Patch 12: Test number 084 is already taken, raise it to 089


v2:
 - rebased on top of Kevin's block branch and on Benoît's patch "block:
   Rewrite the snapshot authorization mechanism for block filters."
   (this series now depends on this patch)

 - Added patch 2: Adds test cases for the qdict_join() function
   introduced by patch 1
 - Added patch 3: Since Benoît changed the snapshot authorization
   mechanism, there is now no easy way of detecting whether a block
   filter only has a single child (which is however required for patch
   11). This patch introduces such a way.
 - Patch 4:
   - Simplified return in json_open() [Benoît]
   - Adjusted to fit the new authorization mechanism [Benoît], including
     patch 3 of this series
 - Patch 6: Recursive calls should go to bs->file, not bs itself
   [Benoît]
 - Patch 7: Added missing "*pnum = nb_sectors;" [Benoît]
 - Patch 11: Adjusted to fit the new authorization mechanism [Benoît]
   and especially patch 3
 - Patch 12:
   - Skip test if TEST_IMG contains a quotation mark [Eric]
   - Omit the test number from which two of the test cases are
     originally taken from the test output [Eric]



git-backport-diff against v2:

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/12:[----] [--] 'qdict: Add qdict_join()'
002/12:[----] [--] 'check-qdict: Add test for qdict_join()'
003/12:[----] [--] 'block: Add "has_single_child" field for drivers'
004/12:[----] [--] 'block/json: Add JSON protocol driver'
005/12:[0004] [FC] 'block/json: Add functions for cache control'
006/12:[----] [-C] 'block/json: Add functions for writing zeroes etc.'
007/12:[0004] [FC] 'block/json: Add bdrv_co_get_block_status()'
008/12:[----] [-C] 'block/json: Add ioctl etc.'
009/12:[----] [--] 'block/json: Add bdrv_get_specific_info()'
010/12:[----] [--] 'block/raw_bsd: Add bdrv_get_specific_info()'
011/12:[----] [--] 'block/qapi: Ignore filters on top for format name'
012/12:[0004] [FC] 'iotests: Add test for the JSON protocol'



Max Reitz (12):
  qdict: Add qdict_join()
  check-qdict: Add test for qdict_join()
  block: Add "has_single_child" field for drivers
  block/json: Add JSON protocol driver
  block/json: Add functions for cache control
  block/json: Add functions for writing zeroes etc.
  block/json: Add bdrv_co_get_block_status()
  block/json: Add ioctl etc.
  block/json: Add bdrv_get_specific_info()
  block/raw_bsd: Add bdrv_get_specific_info()
  block/qapi: Ignore filters on top for format name
  iotests: Add test for the JSON protocol

 block.c                    |   4 +
 block/Makefile.objs        |   2 +-
 block/json.c               | 233 +++++++++++++++++++++++++++++++++++++++++++++
 block/qapi.c               |  18 +++-
 block/raw_bsd.c            |   6 ++
 include/block/block_int.h  |   7 ++
 include/qapi/qmp/qdict.h   |   3 +
 qobject/qdict.c            |  32 +++++++
 tests/check-qdict.c        |  87 +++++++++++++++++
 tests/qemu-iotests/089     | 123 ++++++++++++++++++++++++
 tests/qemu-iotests/089.out |  39 ++++++++
 tests/qemu-iotests/group   |   1 +
 12 files changed, 552 insertions(+), 3 deletions(-)
 create mode 100644 block/json.c
 create mode 100755 tests/qemu-iotests/089
 create mode 100644 tests/qemu-iotests/089.out

-- 
1.9.1

[Qemu-devel] [PATCH v3 01/12] qdict: Add qdict_join()

[Max Reitz]

This function joins two QDicts by absorbing one into the other.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 include/qapi/qmp/qdict.h |  3 +++
 qobject/qdict.c          | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 1ddf97b..d68f4eb 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -16,6 +16,7 @@
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qlist.h"
 #include "qemu/queue.h"
+#include <stdbool.h>
 #include <stdint.h>
 
 #define QDICT_BUCKET_MAX 512
@@ -70,4 +71,6 @@ void qdict_flatten(QDict *qdict);
 void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
 void qdict_array_split(QDict *src, QList **dst);
 
+void qdict_join(QDict *dest, QDict *src, bool overwrite);
+
 #endif /* QDICT_H */
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 42ec4c0..ea239f0 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -665,3 +665,35 @@ void qdict_array_split(QDict *src, QList **dst)
         qlist_append_obj(*dst, subqobj ?: QOBJECT(subqdict));
     }
 }
+
+/**
+ * qdict_join(): Absorb the src QDict into the dest QDict, that is, move all
+ * elements from src to dest.
+ *
+ * If an element from src has a key already present in dest, it will not be
+ * moved unless overwrite is true.
+ *
+ * If overwrite is true, the conflicting values in dest will be discarded and
+ * replaced by the corresponding values from src.
+ *
+ * Therefore, with overwrite being true, the src QDict will always be empty when
+ * this function returns. If overwrite is false, the src QDict will be empty
+ * iff there were no conflicts.
+ */
+void qdict_join(QDict *dest, QDict *src, bool overwrite)
+{
+    const QDictEntry *entry, *next;
+
+    entry = qdict_first(src);
+    while (entry) {
+        next = qdict_next(src, entry);
+
+        if (overwrite || !qdict_haskey(dest, entry->key)) {
+            qobject_incref(entry->value);
+            qdict_put_obj(dest, entry->key, entry->value);
+            qdict_del(src, entry->key);
+        }
+
+        entry = next;
+    }
+}
-- 
1.9.1

[Qemu-devel] [PATCH v3 02/12] check-qdict: Add test for qdict_join()

[Max Reitz]

Add some test cases for qdict_join().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 tests/check-qdict.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 2ad0f78..a9296f0 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -444,6 +444,92 @@ static void qdict_array_split_test(void)
     QDECREF(test_dict);
 }
 
+static void qdict_join_test(void)
+{
+    QDict *dict1, *dict2;
+    bool overwrite = false;
+    int i;
+
+    dict1 = qdict_new();
+    dict2 = qdict_new();
+
+
+    /* Test everything once without overwrite and once with */
+    do
+    {
+        /* Test empty dicts */
+        qdict_join(dict1, dict2, overwrite);
+
+        g_assert(qdict_size(dict1) == 0);
+        g_assert(qdict_size(dict2) == 0);
+
+
+        /* First iteration: Test movement */
+        /* Second iteration: Test empty source and non-empty destination */
+        qdict_put(dict2, "foo", qint_from_int(42));
+
+        for (i = 0; i < 2; i++) {
+            qdict_join(dict1, dict2, overwrite);
+
+            g_assert(qdict_size(dict1) == 1);
+            g_assert(qdict_size(dict2) == 0);
+
+            g_assert(qdict_get_int(dict1, "foo") == 42);
+        }
+
+
+        /* Test non-empty source and destination without conflict */
+        qdict_put(dict2, "bar", qint_from_int(23));
+
+        qdict_join(dict1, dict2, overwrite);
+
+        g_assert(qdict_size(dict1) == 2);
+        g_assert(qdict_size(dict2) == 0);
+
+        g_assert(qdict_get_int(dict1, "foo") == 42);
+        g_assert(qdict_get_int(dict1, "bar") == 23);
+
+
+        /* Test conflict */
+        qdict_put(dict2, "foo", qint_from_int(84));
+
+        qdict_join(dict1, dict2, overwrite);
+
+        g_assert(qdict_size(dict1) == 2);
+        g_assert(qdict_size(dict2) == !overwrite);
+
+        g_assert(qdict_get_int(dict1, "foo") == overwrite ? 84 : 42);
+        g_assert(qdict_get_int(dict1, "bar") == 23);
+
+        if (!overwrite) {
+            g_assert(qdict_get_int(dict2, "foo") == 84);
+        }
+
+
+        /* Check the references */
+        g_assert(qdict_get(dict1, "foo")->refcnt == 1);
+        g_assert(qdict_get(dict1, "bar")->refcnt == 1);
+
+        if (!overwrite) {
+            g_assert(qdict_get(dict2, "foo")->refcnt == 1);
+        }
+
+
+        /* Clean up */
+        qdict_del(dict1, "foo");
+        qdict_del(dict1, "bar");
+
+        if (!overwrite) {
+            qdict_del(dict2, "foo");
+        }
+    }
+    while (overwrite ^= true);
+
+
+    QDECREF(dict1);
+    QDECREF(dict2);
+}
+
 /*
  * Errors test-cases
  */
@@ -584,6 +670,7 @@ int main(int argc, char **argv)
     g_test_add_func("/public/iterapi", qdict_iterapi_test);
     g_test_add_func("/public/flatten", qdict_flatten_test);
     g_test_add_func("/public/array_split", qdict_array_split_test);
+    g_test_add_func("/public/join", qdict_join_test);
 
     g_test_add_func("/errors/put_exists", qdict_put_exists_test);
     g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test);
-- 
1.9.1

[Qemu-devel] [PATCH v3 03/12] block: Add "has_single_child" field for drivers

[Max Reitz]

This field should be used by block drivers acting as filters which have
only a single child BDS which is referenced through the "file" field of
the BDS.

Setting this field allows other block functions to "access" the child
BDS, for instance in bdrv_recurse_is_first_non_filter(). Therefore, it
should not be set if the block layer should not have access to the child
through the filter.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block.c                   | 4 ++++
 include/block/block_int.h | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/block.c b/block.c
index 990a754..7662f99 100644
--- a/block.c
+++ b/block.c
@@ -5466,6 +5466,10 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
         return bs->drv->bdrv_recurse_is_first_non_filter(bs, candidate);
     }
 
+    if (bs->drv->has_single_child) {
+        return bdrv_recurse_is_first_non_filter(bs->file, candidate);
+    }
+
     /* the driver is a block filter but don't allow to recurse -> return false
      */
     return false;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cd5bc73..243e974 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -78,6 +78,13 @@ struct BlockDriver {
 
     /* set to true if the BlockDriver is a block filter */
     bool is_filter;
+    /* Set to true if the BlockDriver is a filter with a single child referenced
+     * through the "file" field in the BDS. This allows the block layer to
+     * access that child through the filter (e.g., for
+     * bdrv_recurse_is_first_non_filter()); if this is not desired, set it to
+     * false (the "file" field should not have been used in this case anyway,
+     * though). */
+    bool has_single_child;
     /* for snapshots block filter like Quorum can implement the
      * following recursive callback.
      * It's purpose is to recurse on the filter children while calling
-- 
1.9.1

[Qemu-devel] [PATCH v3 04/12] block/json: Add JSON protocol driver

[Max Reitz]

Add a JSON protocol driver which allows supplying block driver options
through the filename rather than separately. Other than that, it is a
pure passthrough driver which identifies itself as a filter.

This patch implements the functions bdrv_parse_filename(),
bdrv_file_open(), bdrv_close(), bdrv_aio_readv(), bdrv_aio_writev(),
bdrv_getlength(), bdrv_refresh_limits() and bdrv_get_info().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/Makefile.objs |   2 +-
 block/json.c        | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+), 1 deletion(-)
 create mode 100644 block/json.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index fd88c03..d4b70f4 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -5,7 +5,7 @@ block-obj-y += qed-check.o
 block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-$(CONFIG_QUORUM) += quorum.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
-block-obj-y += snapshot.o qapi.o
+block-obj-y += snapshot.o qapi.o json.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block/json.c b/block/json.c
new file mode 100644
index 0000000..591bc47
--- /dev/null
+++ b/block/json.c
@@ -0,0 +1,134 @@
+/*
+ * JSON filename wrapper protocol driver
+ *
+ * Copyright (c) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu-common.h"
+#include "block/block_int.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qjson.h"
+
+static void json_parse_filename(const char *filename, QDict *options,
+                                Error **errp)
+{
+    QObject *file_options_obj;
+    QDict *full_options;
+
+    if (!strstart(filename, "json:", &filename)) {
+        error_setg(errp, "Unknown protocol prefix for JSON block driver");
+        return;
+    }
+
+    file_options_obj = qobject_from_json(filename);
+    if (!file_options_obj) {
+        error_setg(errp, "Could not parse the JSON options");
+        return;
+    }
+
+    if (qobject_type(file_options_obj) != QTYPE_QDICT) {
+        qobject_decref(file_options_obj);
+        error_setg(errp, "Invalid JSON object");
+        return;
+    }
+
+    full_options = qdict_new();
+    qdict_put_obj(full_options, "x-options", file_options_obj);
+    qdict_flatten(full_options);
+
+    qdict_join(options, full_options, true);
+    assert(qdict_size(full_options) == 0);
+    QDECREF(full_options);
+}
+
+static int json_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
+{
+    int ret;
+
+    assert(bs->file == NULL);
+    ret = bdrv_open_image(&bs->file, NULL, options, "x-options", flags, false,
+                          errp);
+
+    return ret;
+}
+
+static void json_close(BlockDriverState *bs)
+{
+}
+
+static BlockDriverAIOCB *json_aio_readv(BlockDriverState *bs,
+                                        int64_t sector_num, QEMUIOVector *qiov,
+                                        int nb_sectors,
+                                        BlockDriverCompletionFunc *cb,
+                                        void *opaque)
+{
+    return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
+}
+
+static BlockDriverAIOCB *json_aio_writev(BlockDriverState *bs,
+                                         int64_t sector_num, QEMUIOVector *qiov,
+                                         int nb_sectors,
+                                         BlockDriverCompletionFunc *cb,
+                                         void *opaque)
+{
+    return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
+}
+
+static int64_t json_getlength(BlockDriverState *bs)
+{
+    return bdrv_getlength(bs->file);
+}
+
+static int json_refresh_limits(BlockDriverState *bs)
+{
+    bs->bl = bs->file->bl;
+    return 0;
+}
+
+static int json_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+    return bdrv_get_info(bs->file, bdi);
+}
+
+static BlockDriver bdrv_json = {
+    .format_name                = "json",
+    .protocol_name              = "json",
+    .instance_size              = 0,
+
+    .bdrv_parse_filename        = json_parse_filename,
+    .bdrv_file_open             = json_open,
+    .bdrv_close                 = json_close,
+
+    .bdrv_aio_readv             = json_aio_readv,
+    .bdrv_aio_writev            = json_aio_writev,
+
+    .has_variable_length        = true,
+    .bdrv_getlength             = json_getlength,
+
+    .bdrv_refresh_limits        = json_refresh_limits,
+    .bdrv_get_info              = json_get_info,
+
+    .is_filter                  = true,
+    .has_single_child           = true,
+};
+
+static void bdrv_json_init(void)
+{
+    bdrv_register(&bdrv_json);
+}
+
+block_init(bdrv_json_init);
-- 
1.9.1

[Qemu-devel] [PATCH v3 05/12] block/json: Add functions for cache control

[Max Reitz]

Add passthrough functions for bdrv_aio_flush() and
bdrv_invalidate_cache().

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

diff --git a/block/json.c b/block/json.c
index 591bc47..0e2d518 100644
--- a/block/json.c
+++ b/block/json.c
@@ -88,6 +88,18 @@ static BlockDriverAIOCB *json_aio_writev(BlockDriverState *bs,
     return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
 }
 
+static BlockDriverAIOCB *json_aio_flush(BlockDriverState *bs,
+                                        BlockDriverCompletionFunc *cb,
+                                        void *opaque)
+{
+    return bdrv_aio_flush(bs->file, cb, opaque);
+}
+
+static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
+{
+    return bdrv_invalidate_cache(bs->file, errp);
+}
+
 static int64_t json_getlength(BlockDriverState *bs)
 {
     return bdrv_getlength(bs->file);
@@ -115,6 +127,9 @@ static BlockDriver bdrv_json = {
 
     .bdrv_aio_readv             = json_aio_readv,
     .bdrv_aio_writev            = json_aio_writev,
+    .bdrv_aio_flush             = json_aio_flush,
+
+    .bdrv_invalidate_cache      = json_invalidate_cache,
 
     .has_variable_length        = true,
     .bdrv_getlength             = json_getlength,
-- 
1.9.1

[Qemu-devel] [PATCH v3 06/12] block/json: Add functions for writing zeroes etc.

[Max Reitz]

Add passthrough functions for bdrv_aio_discard(),
bdrv_co_write_zeroes(), bdrv_truncate() and bdrv_has_zero_init().

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

diff --git a/block/json.c b/block/json.c
index 0e2d518..cb83780 100644
--- a/block/json.c
+++ b/block/json.c
@@ -95,6 +95,21 @@ static BlockDriverAIOCB *json_aio_flush(BlockDriverState *bs,
     return bdrv_aio_flush(bs->file, cb, opaque);
 }
 
+static BlockDriverAIOCB *json_aio_discard(BlockDriverState *bs,
+                                          int64_t sector_num, int nb_sectors,
+                                          BlockDriverCompletionFunc *cb,
+                                          void *opaque)
+{
+    return bdrv_aio_discard(bs->file, sector_num, nb_sectors, cb, opaque);
+}
+
+static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
+                                             int64_t sector_num, int nb_sectors,
+                                             BdrvRequestFlags flags)
+{
+    return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
 {
     return bdrv_invalidate_cache(bs->file, errp);
@@ -105,6 +120,16 @@ static int64_t json_getlength(BlockDriverState *bs)
     return bdrv_getlength(bs->file);
 }
 
+static int json_truncate(BlockDriverState *bs, int64_t offset)
+{
+    return bdrv_truncate(bs->file, offset);
+}
+
+static int json_has_zero_init(BlockDriverState *bs)
+{
+    return bdrv_has_zero_init(bs->file);
+}
+
 static int json_refresh_limits(BlockDriverState *bs)
 {
     bs->bl = bs->file->bl;
@@ -128,12 +153,17 @@ static BlockDriver bdrv_json = {
     .bdrv_aio_readv             = json_aio_readv,
     .bdrv_aio_writev            = json_aio_writev,
     .bdrv_aio_flush             = json_aio_flush,
+    .bdrv_aio_discard           = json_aio_discard,
+
+    .bdrv_co_write_zeroes       = json_co_write_zeroes,
 
     .bdrv_invalidate_cache      = json_invalidate_cache,
 
     .has_variable_length        = true,
     .bdrv_getlength             = json_getlength,
+    .bdrv_truncate              = json_truncate,
 
+    .bdrv_has_zero_init         = json_has_zero_init,
     .bdrv_refresh_limits        = json_refresh_limits,
     .bdrv_get_info              = json_get_info,
 
-- 
1.9.1

[Qemu-devel] [PATCH v3 07/12] block/json: Add bdrv_co_get_block_status()

[Max Reitz]

Implement this function by passing it through to bs->file.

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

diff --git a/block/json.c b/block/json.c
index cb83780..dfeec81 100644
--- a/block/json.c
+++ b/block/json.c
@@ -110,6 +110,13 @@ static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
     return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
 }
 
+static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
+                                                     int64_t sector_num,
+                                                     int nb_sectors, int *pnum)
+{
+    return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
 {
     return bdrv_invalidate_cache(bs->file, errp);
@@ -156,6 +163,7 @@ static BlockDriver bdrv_json = {
     .bdrv_aio_discard           = json_aio_discard,
 
     .bdrv_co_write_zeroes       = json_co_write_zeroes,
+    .bdrv_co_get_block_status   = json_co_get_block_status,
 
     .bdrv_invalidate_cache      = json_invalidate_cache,
 
-- 
1.9.1

[Qemu-devel] [PATCH v3 08/12] block/json: Add ioctl etc.

[Max Reitz]

Add passthrough functions for bdrv_aio_ioctl(), bdrv_is_inserted(),
bdrv_media_changed(), bdrv_eject(), bdrv_lock_medium() and bdrv_ioctl().

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

diff --git a/block/json.c b/block/json.c
index dfeec81..0c7d90d 100644
--- a/block/json.c
+++ b/block/json.c
@@ -103,6 +103,14 @@ static BlockDriverAIOCB *json_aio_discard(BlockDriverState *bs,
     return bdrv_aio_discard(bs->file, sector_num, nb_sectors, cb, opaque);
 }
 
+static BlockDriverAIOCB *json_aio_ioctl(BlockDriverState *bs,
+                                        unsigned long int req, void *buf,
+                                        BlockDriverCompletionFunc *cb,
+                                        void *opaque)
+{
+    return bdrv_aio_ioctl(bs->file, req, buf, cb, opaque);
+}
+
 static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
                                              int64_t sector_num, int nb_sectors,
                                              BdrvRequestFlags flags)
@@ -117,6 +125,31 @@ static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
     return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
 }
 
+static int json_is_inserted(BlockDriverState *bs)
+{
+    return bdrv_is_inserted(bs->file);
+}
+
+static int json_media_changed(BlockDriverState *bs)
+{
+    return bdrv_media_changed(bs->file);
+}
+
+static void json_eject(BlockDriverState *bs, bool eject_flag)
+{
+    bdrv_eject(bs->file, eject_flag);
+}
+
+static void json_lock_medium(BlockDriverState *bs, bool locked)
+{
+    bdrv_lock_medium(bs->file, locked);
+}
+
+static int json_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
+{
+    return bdrv_ioctl(bs->file, req, buf);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
 {
     return bdrv_invalidate_cache(bs->file, errp);
@@ -161,10 +194,17 @@ static BlockDriver bdrv_json = {
     .bdrv_aio_writev            = json_aio_writev,
     .bdrv_aio_flush             = json_aio_flush,
     .bdrv_aio_discard           = json_aio_discard,
+    .bdrv_aio_ioctl             = json_aio_ioctl,
 
     .bdrv_co_write_zeroes       = json_co_write_zeroes,
     .bdrv_co_get_block_status   = json_co_get_block_status,
 
+    .bdrv_is_inserted           = json_is_inserted,
+    .bdrv_media_changed         = json_media_changed,
+    .bdrv_eject                 = json_eject,
+    .bdrv_lock_medium           = json_lock_medium,
+    .bdrv_ioctl                 = json_ioctl,
+
     .bdrv_invalidate_cache      = json_invalidate_cache,
 
     .has_variable_length        = true,
-- 
1.9.1

[Qemu-devel] [PATCH v3 09/12] block/json: Add bdrv_get_specific_info()

[Max Reitz]

Add a passthrough function for bdrv_get_specific_info().

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

diff --git a/block/json.c b/block/json.c
index 0c7d90d..d9348bc 100644
--- a/block/json.c
+++ b/block/json.c
@@ -181,6 +181,11 @@ static int json_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return bdrv_get_info(bs->file, bdi);
 }
 
+static ImageInfoSpecific *json_get_specific_info(BlockDriverState *bs)
+{
+    return bdrv_get_specific_info(bs->file);
+}
+
 static BlockDriver bdrv_json = {
     .format_name                = "json",
     .protocol_name              = "json",
@@ -214,6 +219,7 @@ static BlockDriver bdrv_json = {
     .bdrv_has_zero_init         = json_has_zero_init,
     .bdrv_refresh_limits        = json_refresh_limits,
     .bdrv_get_info              = json_get_info,
+    .bdrv_get_specific_info     = json_get_specific_info,
 
     .is_filter                  = true,
     .has_single_child           = true,
-- 
1.9.1

[Qemu-devel] [PATCH v3 10/12] block/raw_bsd: Add bdrv_get_specific_info()

[Max Reitz]

Add a passthrough function for bdrv_get_specific_info().

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

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 01ea692..e93ccd3 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -90,6 +90,11 @@ static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return bdrv_get_info(bs->file, bdi);
 }
 
+static ImageInfoSpecific *raw_get_specific_info(BlockDriverState *bs)
+{
+    return bdrv_get_specific_info(bs->file);
+}
+
 static int raw_refresh_limits(BlockDriverState *bs)
 {
     bs->bl = bs->file->bl;
@@ -187,6 +192,7 @@ static BlockDriver bdrv_raw = {
     .bdrv_getlength       = &raw_getlength,
     .has_variable_length  = true,
     .bdrv_get_info        = &raw_get_info,
+    .bdrv_get_specific_info = &raw_get_specific_info,
     .bdrv_refresh_limits  = &raw_refresh_limits,
     .bdrv_is_inserted     = &raw_is_inserted,
     .bdrv_media_changed   = &raw_media_changed,
-- 
1.9.1

[Qemu-devel] [PATCH v3 11/12] block/qapi: Ignore filters on top for format name

[Max Reitz]

bdrv_query_image_info() currently deduces the image filename and the
format name from the top BDS. However, it is probably more reasonable to
ignore as many filters as possible on top of the BDS chain since those
neither change the type nor the filename of the underlying image.

Filters like quorum which have multiple underlying BDS should not be
removed, however, since the underlying BDS chains may lead to different
image formats and most certainly to different filenames. Therefore, only
simple filters with a single underlying BDS may be skipped.

In addition, any "raw" BDS on top of such a simple filter should be
removed, since they have probably been automatically created by
bdrv_open() but basically function as a simple filter as well.

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

diff --git a/block/qapi.c b/block/qapi.c
index 8f2b4db..84b3097 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -171,11 +171,25 @@ void bdrv_query_image_info(BlockDriverState *bs,
     int ret;
     Error *err = NULL;
     ImageInfo *info = g_new0(ImageInfo, 1);
+    BlockDriverState *ubs;
 
     bdrv_get_geometry(bs, &total_sectors);
 
-    info->filename        = g_strdup(bs->filename);
-    info->format          = g_strdup(bdrv_get_format_name(bs));
+    /* Remove the top layer of filters; that is, remove every "raw" BDS on top
+     * of a single-child filter and every single-child filter itself until a BDS
+     * is encountered which cannot be removed through these rules */
+    ubs = bs;
+    while ((ubs->drv && ubs->drv->is_filter && ubs->drv->has_single_child) ||
+           (ubs->drv && !strcmp(ubs->drv->format_name, "raw") && ubs->file &&
+            ubs->file->drv && ubs->file->drv->is_filter &&
+            ubs->file->drv->has_single_child))
+    {
+        ubs = ubs->file;
+    }
+
+    info->filename        = g_strdup(ubs->filename);
+    info->format          = g_strdup(bdrv_get_format_name(ubs));
+
     info->virtual_size    = total_sectors * 512;
     info->actual_size     = bdrv_get_allocated_file_size(bs);
     info->has_actual_size = info->actual_size >= 0;
-- 
1.9.1

[Qemu-devel] [PATCH v3 12/12] iotests: Add test for the JSON protocol

[Max Reitz]

Add a test for the JSON protocol driver.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 tests/qemu-iotests/089     | 123 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/089.out |  39 ++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 163 insertions(+)
 create mode 100755 tests/qemu-iotests/089
 create mode 100644 tests/qemu-iotests/089.out

diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
new file mode 100755
index 0000000..75cb0c2
--- /dev/null
+++ b/tests/qemu-iotests/089
@@ -0,0 +1,123 @@
+#!/bin/bash
+#
+# Test case for the JSON block protocol
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Using an image filename containing quotation marks will render the JSON data
+# below invalid. In that case, we have little choice but simply not to run this
+# test.
+case $TEST_IMG in
+    *'"'*)
+        _notrun "image filename may not contain quotation marks"
+        ;;
+esac
+
+IMG_SIZE=64M
+
+# Taken from test 072
+echo
+echo "=== Testing nested image formats ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
+
+$QEMU_IO -c 'write -P 42 0 512' -c 'write -P 23 512 512' \
+         -c 'write -P 66 1024 512' "$TEST_IMG.base" | _filter_qemu_io
+
+$QEMU_IMG convert -f raw -O $IMGFMT "$TEST_IMG.base" "$TEST_IMG"
+
+$QEMU_IO -c 'read -P 42 0 512' -c 'read -P 23 512 512' \
+         -c 'read -P 66 1024 512' "json:{
+    \"driver\": \"$IMGFMT\",
+    \"file\": {
+        \"driver\": \"$IMGFMT\",
+        \"file\": {
+            \"filename\": \"$TEST_IMG\"
+        }
+    }
+}" | _filter_qemu_io
+
+# This should fail (see test 072)
+$QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
+
+
+# Taken from test 071
+echo
+echo "=== Testing blkdebug ==="
+echo
+
+_make_test_img $IMG_SIZE
+
+$QEMU_IO -c 'write -P 42 0x38000 512' "$TEST_IMG" | _filter_qemu_io
+
+# The "image.filename" part tests whether "a": { "b": "c" } and "a.b": "c" do
+# the same (which they should).
+# This has to use -g to force qemu-io to use BDRV_O_PROTOCOL, since it will try
+# to determine the format of the file otherwise; due to the complexity of the
+# filename, only raw (or json itself) will work and this will then result in an
+# error because of the blkdebug part. Thus, use -g.
+$QEMU_IO -c 'read -P 42 0x38000 512' -g "json:{
+    \"driver\": \"$IMGFMT\",
+    \"file\": {
+        \"driver\": \"blkdebug\",
+        \"inject-error\": [{
+            \"event\": \"l2_load\"
+        }],
+        \"image.filename\": \"$TEST_IMG\"
+    }
+}" | _filter_qemu_io
+
+
+echo
+echo "=== Testing qemu-img info output ==="
+echo
+
+# This should output information about the image itself, not about the JSON
+# block device.
+$QEMU_IMG info "json:{\"driver\":\"qcow2\",\"file.filename\":\"$TEST_IMG\"}" \
+    | _filter_testdir | _filter_imgfmt
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out
new file mode 100644
index 0000000..4804ff0
--- /dev/null
+++ b/tests/qemu-iotests/089.out
@@ -0,0 +1,39 @@
+QA output created by 089
+
+=== Testing nested image formats ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 1024
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 1024
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Pattern verification failed at offset 0, 512 bytes
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing blkdebug ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 512/512 bytes at offset 229376
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read failed: Input/output error
+
+=== Testing qemu-img info output ===
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+disk size: 324K
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 864643d..e975e93 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -95,3 +95,4 @@
 086 rw auto quick
 087 rw auto
 088 rw auto
+089 rw auto quick
-- 
1.9.1

Re: [Qemu-devel] [PATCH v3 05/12] block/json: Add functions for cache control

[Benoît Canet]

The Thursday 10 Apr 2014 à 20:43:36 (+0200), Max Reitz wrote :
> Add passthrough functions for bdrv_aio_flush() and
> bdrv_invalidate_cache().
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/json.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/block/json.c b/block/json.c
> index 591bc47..0e2d518 100644
> --- a/block/json.c
> +++ b/block/json.c
> @@ -88,6 +88,18 @@ static BlockDriverAIOCB *json_aio_writev(BlockDriverState *bs,
>      return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
>  }
>  
> +static BlockDriverAIOCB *json_aio_flush(BlockDriverState *bs,
> +                                        BlockDriverCompletionFunc *cb,
> +                                        void *opaque)
> +{
> +    return bdrv_aio_flush(bs->file, cb, opaque);
> +}
> +
> +static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
> +{
> +    return bdrv_invalidate_cache(bs->file, errp);
> +}
> +
>  static int64_t json_getlength(BlockDriverState *bs)
>  {
>      return bdrv_getlength(bs->file);
> @@ -115,6 +127,9 @@ static BlockDriver bdrv_json = {
>  
>      .bdrv_aio_readv             = json_aio_readv,
>      .bdrv_aio_writev            = json_aio_writev,
> +    .bdrv_aio_flush             = json_aio_flush,
> +
> +    .bdrv_invalidate_cache      = json_invalidate_cache,
>  
>      .has_variable_length        = true,
>      .bdrv_getlength             = json_getlength,
> -- 
> 1.9.1
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH v3 07/12] block/json: Add bdrv_co_get_block_status()

[Benoît Canet]

The Thursday 10 Apr 2014 à 20:43:38 (+0200), Max Reitz wrote :
> Implement this function by passing it through to bs->file.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/json.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/block/json.c b/block/json.c
> index cb83780..dfeec81 100644
> --- a/block/json.c
> +++ b/block/json.c
> @@ -110,6 +110,13 @@ static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
>      return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
>  }
>  
> +static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
> +                                                     int64_t sector_num,
> +                                                     int nb_sectors, int *pnum)
> +{
> +    return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
> +}
> +
>  static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
>  {
>      return bdrv_invalidate_cache(bs->file, errp);
> @@ -156,6 +163,7 @@ static BlockDriver bdrv_json = {
>      .bdrv_aio_discard           = json_aio_discard,
>  
>      .bdrv_co_write_zeroes       = json_co_write_zeroes,
> +    .bdrv_co_get_block_status   = json_co_get_block_status,
>  
>      .bdrv_invalidate_cache      = json_invalidate_cache,
>  
> -- 
> 1.9.1
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

Here you go :)

Re: [Qemu-devel] [PATCH v3 07/12] block/json: Add bdrv_co_get_block_status()

[Max Reitz]

On 10.04.2014 20:58, Benoît Canet wrote:
> The Thursday 10 Apr 2014 à 20:43:38 (+0200), Max Reitz wrote :
>> Implement this function by passing it through to bs->file.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/json.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/block/json.c b/block/json.c
>> index cb83780..dfeec81 100644
>> --- a/block/json.c
>> +++ b/block/json.c
>> @@ -110,6 +110,13 @@ static coroutine_fn int json_co_write_zeroes(BlockDriverState *bs,
>>       return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
>>   }
>>   
>> +static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
>> +                                                     int64_t sector_num,
>> +                                                     int nb_sectors, int *pnum)
>> +{
>> +    return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
>> +}
>> +
>>   static void json_invalidate_cache(BlockDriverState *bs, Error **errp)
>>   {
>>       return bdrv_invalidate_cache(bs->file, errp);
>> @@ -156,6 +163,7 @@ static BlockDriver bdrv_json = {
>>       .bdrv_aio_discard           = json_aio_discard,
>>   
>>       .bdrv_co_write_zeroes       = json_co_write_zeroes,
>> +    .bdrv_co_get_block_status   = json_co_get_block_status,
>>   
>>       .bdrv_invalidate_cache      = json_invalidate_cache,
>>   
>> -- 
>> 1.9.1
>>
> Reviewed-by: Benoit Canet <benoit@irqsave.net>
>
> Here you go :)

Thank you :D

Re: [Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver

[Max Reitz]

On 10.04.2014 20:43, Max Reitz wrote:
> This series adds a passthrough JSON protocol block driver. Its filenames
> are JSON objects prefixed by "json:". The objects are used as options
> for opening another block device which will be the child of the JSON
> device. Regarding this child device, the JSON driver behaves nearly the
> same as raw_bsd in that it is just a passthrough driver. The only
> difference is probably that the JSON driver identifies itself as a block
> filter, in contrast to raw_bsd.
>
> The purpose of this driver is that it may sometimes be desirable to
> specify options for a block device where only a filename can be given,
> e.g., for backing files. Using this should obviously be the exception,
> but it is nice to have if actually needed.

Ping – I do understand that Kevin has reservations against this series, 
but as long as he doesn't explicitly ask me to reimplement this in 
bdrv_open() without an own block driver (which I'd more or less gladly 
do), I do not see issues why this series should not be merged.

Max

Re: [Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver

[Stefan Hajnoczi]

On Mon, May 05, 2014 at 06:19:07PM +0200, Max Reitz wrote:
> On 10.04.2014 20:43, Max Reitz wrote:
> >This series adds a passthrough JSON protocol block driver. Its filenames
> >are JSON objects prefixed by "json:". The objects are used as options
> >for opening another block device which will be the child of the JSON
> >device. Regarding this child device, the JSON driver behaves nearly the
> >same as raw_bsd in that it is just a passthrough driver. The only
> >difference is probably that the JSON driver identifies itself as a block
> >filter, in contrast to raw_bsd.
> >
> >The purpose of this driver is that it may sometimes be desirable to
> >specify options for a block device where only a filename can be given,
> >e.g., for backing files. Using this should obviously be the exception,
> >but it is nice to have if actually needed.
> 
> Ping – I do understand that Kevin has reservations against this
> series, but as long as he doesn't explicitly ask me to reimplement
> this in bdrv_open() without an own block driver (which I'd more or
> less gladly do), I do not see issues why this series should not be
> merged.

I haven't reviewed it further because it seems like a kludge (that we
have to keep supporting once it's merged).  Was hoping you and Kevin
will come up with a long-term fix instead.

Kevin: Any new ideas on this patch series?

Stefan

Re: [Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver

[Max Reitz]

On 06.05.2014 10:10, Stefan Hajnoczi wrote:
> On Mon, May 05, 2014 at 06:19:07PM +0200, Max Reitz wrote:
>> On 10.04.2014 20:43, Max Reitz wrote:
>>> This series adds a passthrough JSON protocol block driver. Its filenames
>>> are JSON objects prefixed by "json:". The objects are used as options
>>> for opening another block device which will be the child of the JSON
>>> device. Regarding this child device, the JSON driver behaves nearly the
>>> same as raw_bsd in that it is just a passthrough driver. The only
>>> difference is probably that the JSON driver identifies itself as a block
>>> filter, in contrast to raw_bsd.
>>>
>>> The purpose of this driver is that it may sometimes be desirable to
>>> specify options for a block device where only a filename can be given,
>>> e.g., for backing files. Using this should obviously be the exception,
>>> but it is nice to have if actually needed.
>> Ping – I do understand that Kevin has reservations against this
>> series, but as long as he doesn't explicitly ask me to reimplement
>> this in bdrv_open() without an own block driver (which I'd more or
>> less gladly do), I do not see issues why this series should not be
>> merged.
> I haven't reviewed it further because it seems like a kludge (that we
> have to keep supporting once it's merged).  Was hoping you and Kevin
> will come up with a long-term fix instead.

Okay, if you think the same, I guess I'll have to rewrite this series. I 
agree that including this functionality in bdrv_open() is the nicer 
alternative from the user's point of view, whereas I think using a 
separate block driver results in nicer code.

I'll rewrite this series and then we'll see how bad it actually looks. ;-)

Max

Re: [Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver

[Stefan Hajnoczi]

On Tue, May 06, 2014 at 07:51:10PM +0200, Max Reitz wrote:
> On 06.05.2014 10:10, Stefan Hajnoczi wrote:
> >On Mon, May 05, 2014 at 06:19:07PM +0200, Max Reitz wrote:
> >>On 10.04.2014 20:43, Max Reitz wrote:
> >>>This series adds a passthrough JSON protocol block driver. Its filenames
> >>>are JSON objects prefixed by "json:". The objects are used as options
> >>>for opening another block device which will be the child of the JSON
> >>>device. Regarding this child device, the JSON driver behaves nearly the
> >>>same as raw_bsd in that it is just a passthrough driver. The only
> >>>difference is probably that the JSON driver identifies itself as a block
> >>>filter, in contrast to raw_bsd.
> >>>
> >>>The purpose of this driver is that it may sometimes be desirable to
> >>>specify options for a block device where only a filename can be given,
> >>>e.g., for backing files. Using this should obviously be the exception,
> >>>but it is nice to have if actually needed.
> >>Ping – I do understand that Kevin has reservations against this
> >>series, but as long as he doesn't explicitly ask me to reimplement
> >>this in bdrv_open() without an own block driver (which I'd more or
> >>less gladly do), I do not see issues why this series should not be
> >>merged.
> >I haven't reviewed it further because it seems like a kludge (that we
> >have to keep supporting once it's merged).  Was hoping you and Kevin
> >will come up with a long-term fix instead.
> 
> Okay, if you think the same, I guess I'll have to rewrite this
> series. I agree that including this functionality in bdrv_open() is
> the nicer alternative from the user's point of view, whereas I think
> using a separate block driver results in nicer code.
> 
> I'll rewrite this series and then we'll see how bad it actually looks. ;-)

Thanks!

Stefan