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

[Qemu-devel] [PATCH v2 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.


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 v1:

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:[down] 'check-qdict: Add test for qdict_join()'
003/12:[down] 'block: Add "has_single_child" field for drivers'
004/12:[0008] [FC] 'block/json: Add JSON protocol driver'
005/12:[----] [--] 'block/json: Add functions for cache control'
006/12:[0004] [FC] 'block/json: Add functions for writing zeroes etc.'
007/12:[0001] [FC] 'block/json: Add bdrv_co_get_block_status()'
008/12:[----] [-C] 'block/json: Add ioctl etc.'
009/12:[----] [-C] 'block/json: Add bdrv_get_specific_info()'
010/12:[----] [--] 'block/raw_bsd: Add bdrv_get_specific_info()'
011/12:[0012] [FC] 'block/qapi: Ignore filters on top for format name'
012/12:[0017] [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               | 235 +++++++++++++++++++++++++++++++++++++++++++++
 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/084     | 123 ++++++++++++++++++++++++
 tests/qemu-iotests/084.out |  39 ++++++++
 tests/qemu-iotests/group   |   1 +
 12 files changed, 554 insertions(+), 3 deletions(-)
 create mode 100644 block/json.c
 create mode 100755 tests/qemu-iotests/084
 create mode 100644 tests/qemu-iotests/084.out

-- 
1.9.0

[Qemu-devel] [PATCH v2 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.0

[Qemu-devel] [PATCH v2 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>
---
 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.0

[Qemu-devel] [PATCH v2 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>
---
 block.c                   | 4 ++++
 include/block/block_int.h | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/block.c b/block.c
index 06fbc0a..aec325f 100644
--- a/block.c
+++ b/block.c
@@ -5418,6 +5418,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 4fc5ea8..7815587 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.0

[Qemu-devel] [PATCH v2 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>
---
 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.0

[Qemu-devel] [PATCH v2 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/json.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/block/json.c b/block/json.c
index 591bc47..f40343e 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)
+{
+    return bdrv_invalidate_cache(bs->file);
+}
+
 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.0

[Qemu-devel] [PATCH v2 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>
---
 block/json.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/block/json.c b/block/json.c
index f40343e..e4cdb68 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)
 {
     return bdrv_invalidate_cache(bs->file);
@@ -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.0

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

[Max Reitz]

Implement this function in the same way as raw_bsd does: Acknowledge
that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
and BDRV_BLOCK_DATA and derive the offset directly from the sector
index) and add BDRV_BLOCK_RAW to the returned value.

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

diff --git a/block/json.c b/block/json.c
index e4cdb68..966a5f5 100644
--- a/block/json.c
+++ b/block/json.c
@@ -110,6 +110,15 @@ 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)
+{
+    *pnum = nb_sectors;
+    return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
+           (sector_num << BDRV_SECTOR_BITS);
+}
+
 static void json_invalidate_cache(BlockDriverState *bs)
 {
     return bdrv_invalidate_cache(bs->file);
@@ -156,6 +165,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.0

[Qemu-devel] [PATCH v2 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 966a5f5..f21b7e3 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)
@@ -119,6 +127,31 @@ static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
            (sector_num << BDRV_SECTOR_BITS);
 }
 
+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)
 {
     return bdrv_invalidate_cache(bs->file);
@@ -163,10 +196,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.0

[Qemu-devel] [PATCH v2 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 f21b7e3..1bb3956 100644
--- a/block/json.c
+++ b/block/json.c
@@ -183,6 +183,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",
@@ -216,6 +221,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.0

[Qemu-devel] [PATCH v2 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.0

[Qemu-devel] [PATCH v2 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>
---
 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.0

[Qemu-devel] [PATCH v2 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>
---
 tests/qemu-iotests/084     | 123 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/084.out |  39 ++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 163 insertions(+)
 create mode 100755 tests/qemu-iotests/084
 create mode 100644 tests/qemu-iotests/084.out

diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084
new file mode 100755
index 0000000..75cb0c2
--- /dev/null
+++ b/tests/qemu-iotests/084
@@ -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/084.out b/tests/qemu-iotests/084.out
new file mode 100644
index 0000000..375dd4a
--- /dev/null
+++ b/tests/qemu-iotests/084.out
@@ -0,0 +1,39 @@
+QA output created by 084
+
+=== 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 9ec62d2..8f6e835 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -85,5 +85,6 @@
 079 rw auto
 081 rw auto
 082 rw auto quick
+084 rw auto quick
 086 rw auto quick
 087 rw auto quick
-- 
1.9.0

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

[Eric Blake]

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

On 03/07/2014 03:55 PM, Max Reitz wrote:
> Add some test cases for qdict_join().
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/check-qdict.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)

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

> +    /* Test everything once without overwrite and once with */
> +    do
> +    {

> +    }
> +    while (overwrite ^= true);

That has got to be the coolest do-while I've seen in a long time :)

-- 
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 v2 03/12] block: Add "has_single_child" field for drivers

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:47 (+0100), Max Reitz wrote :
> 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>
> ---
>  block.c                   | 4 ++++
>  include/block/block_int.h | 7 +++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 06fbc0a..aec325f 100644
> --- a/block.c
> +++ b/block.c
> @@ -5418,6 +5418,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 4fc5ea8..7815587 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.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:48 (+0100), Max Reitz wrote :
> 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>
> ---
>  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.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:50 (+0100), Max Reitz wrote :
> 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>
> ---
>  block/json.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/block/json.c b/block/json.c
> index f40343e..e4cdb68 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)
>  {
>      return bdrv_invalidate_cache(bs->file);
> @@ -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.0
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:55 (+0100), Max Reitz wrote :
> 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>
> ---
>  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.0
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:46 (+0100), Max Reitz wrote :
> Add some test cases for qdict_join().
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  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.0
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Max Reitz]

On 07.03.2014 23:55, 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'd appreciate it if someone could review patches 7 and 12. :-)

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

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:51 (+0100), Max Reitz wrote :
> Implement this function in the same way as raw_bsd does: Acknowledge
> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
> and BDRV_BLOCK_DATA and derive the offset directly from the sector
> index) and add BDRV_BLOCK_RAW to the returned value.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/json.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/json.c b/block/json.c
> index e4cdb68..966a5f5 100644
> --- a/block/json.c
> +++ b/block/json.c
> @@ -110,6 +110,15 @@ 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)
> +{
> +    *pnum = nb_sectors;
> +    return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
> +           (sector_num << BDRV_SECTOR_BITS);
> +}
> +
>  static void json_invalidate_cache(BlockDriverState *bs)
>  {
>      return bdrv_invalidate_cache(bs->file);
> @@ -156,6 +165,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.0
> 

If this filter is stacked on top of qcow2 even the simple BDRV_BLOCK_RAW feel
weird.
I think the best thing we can do is to ask to Peter Lieven if this code has the
intended meaning in a block filter.

Peter: 

You wrote the inspiration for this code in raw-posix.c.

What do you think of this piece of code designed to be stacked as a block filter
on top of regular block driver (qcow2 etc) ?

Does it makes sense ? Or would it be better to forward the call to the lower
level ?

Best regards

Benoît

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

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:56 (+0100), Max Reitz wrote :
> Add a test for the JSON protocol driver.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/084     | 123 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/084.out |  39 ++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 163 insertions(+)
>  create mode 100755 tests/qemu-iotests/084
>  create mode 100644 tests/qemu-iotests/084.out
> 
> diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084
> new file mode 100755
> index 0000000..75cb0c2
> --- /dev/null
> +++ b/tests/qemu-iotests/084
> @@ -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/084.out b/tests/qemu-iotests/084.out
> new file mode 100644
> index 0000000..375dd4a
> --- /dev/null
> +++ b/tests/qemu-iotests/084.out
> @@ -0,0 +1,39 @@
> +QA output created by 084
> +
> +=== 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 9ec62d2..8f6e835 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -85,5 +85,6 @@
>  079 rw auto
>  081 rw auto
>  082 rw auto quick
> +084 rw auto quick
>  086 rw auto quick
>  087 rw auto quick
> -- 
> 1.9.0
> 

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

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

[Max Reitz]

On 07.03.2014 23:55, Max Reitz wrote:
> Implement this function in the same way as raw_bsd does: Acknowledge
> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
> and BDRV_BLOCK_DATA and derive the offset directly from the sector
> index) and add BDRV_BLOCK_RAW to the returned value.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/json.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)

Ping – Benoît is unsure of BDRV_BLOCK_RAW, therefore he elected not to 
give a reviewed-by for this patch.

The commit introducing BDRV_BLOCK_RAW 
(92bc50a5ad7fbc9a0bd17240eaea5027a100ca79) is signed-off-by Peter, 
reviewed-by Eric and signed-off-by Kevin (as the committer, I suppose). 
Could anyone of you comment on this patch? The question is whether to 
use BDRV_BLOCK_RAW or a recursive call to bdrv_get_block_status() here. 
I mean, I could just replace the BDRV_BLOCK_RAW by a recursive call to 
bdrv_get_block_status() and Benoît would probably approve, but obviously 
that would be cheating.


Max

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

[Peter Lieven]

Am 08.04.2014 um 15:15 schrieb Max Reitz <mreitz@redhat.com>:

> On 07.03.2014 23:55, Max Reitz wrote:
>> Implement this function in the same way as raw_bsd does: Acknowledge
>> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
>> and BDRV_BLOCK_DATA and derive the offset directly from the sector
>> index) and add BDRV_BLOCK_RAW to the returned value.
>> 
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/json.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
> 
> Ping – Benoît is unsure of BDRV_BLOCK_RAW, therefore he elected not to give a reviewed-by for this patch.
> 
> The commit introducing BDRV_BLOCK_RAW (92bc50a5ad7fbc9a0bd17240eaea5027a100ca79) is signed-off-by Peter, reviewed-by Eric and signed-off-by Kevin (as the committer, I suppose). Could anyone of you comment on this patch? The question is whether to use BDRV_BLOCK_RAW or a recursive call to bdrv_get_block_status() here. I mean, I could just replace the BDRV_BLOCK_RAW by a recursive call to bdrv_get_block_status() and Benoît would probably approve, but obviously that would be cheating.

Sorry, I missed Benoits earlier email. I have not fully looked through the block/json patch set, but as far as I understand its a filter that can be on top of any format/protocol combination.
Therefore the only right solution can be to pass the bdrv_co_get_block_status call to the underlying format driver.

As for the BDRV_BLOCK_RAW flag we introduced this for the special case of the raw format which guarantees a linear mapping of the whole drive to the underlaying protocol e.g.
file, iscsi, host_device, nfs… Therefore we can derive the file offset from the sector. The allocation status has to be queried from the protocol driver. In fact in the raw format
case it would also work to pass the call to bs->file, but this would result in 2 calls to bs->file->drv->bdrv_get_block_status for unallocated blocks. Remember that bdrv_get_block_status
can be an expensive call e.g. in the iSCSI case. This is why I made commit 92bc50a5.

Peter

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

[Max Reitz]

On 08.04.2014 22:53, Peter Lieven wrote:
> Am 08.04.2014 um 15:15 schrieb Max Reitz <mreitz@redhat.com>:
>
>> On 07.03.2014 23:55, Max Reitz wrote:
>>> Implement this function in the same way as raw_bsd does: Acknowledge
>>> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
>>> and BDRV_BLOCK_DATA and derive the offset directly from the sector
>>> index) and add BDRV_BLOCK_RAW to the returned value.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>>>   block/json.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>> Ping – Benoît is unsure of BDRV_BLOCK_RAW, therefore he elected not to give a reviewed-by for this patch.
>>
>> The commit introducing BDRV_BLOCK_RAW (92bc50a5ad7fbc9a0bd17240eaea5027a100ca79) is signed-off-by Peter, reviewed-by Eric and signed-off-by Kevin (as the committer, I suppose). Could anyone of you comment on this patch? The question is whether to use BDRV_BLOCK_RAW or a recursive call to bdrv_get_block_status() here. I mean, I could just replace the BDRV_BLOCK_RAW by a recursive call to bdrv_get_block_status() and Benoît would probably approve, but obviously that would be cheating.
> Sorry, I missed Benoits earlier email. I have not fully looked through the block/json patch set, but as far as I understand its a filter that can be on top of any format/protocol combination.
> Therefore the only right solution can be to pass the bdrv_co_get_block_status call to the underlying format driver.
>
> As for the BDRV_BLOCK_RAW flag we introduced this for the special case of the raw format which guarantees a linear mapping of the whole drive to the underlaying protocol e.g.
> file, iscsi, host_device, nfs… Therefore we can derive the file offset from the sector. The allocation status has to be queried from the protocol driver. In fact in the raw format
> case it would also work to pass the call to bs->file, but this would result in 2 calls to bs->file->drv->bdrv_get_block_status for unallocated blocks. Remember that bdrv_get_block_status
> can be an expensive call e.g. in the iSCSI case. This is why I made commit 92bc50a5.
>
> Peter

Thank you very much. I'll send a v3 without BDRV_BLOCK_RAW then.

Max