[Qemu-devel] [PATCH v3 00/21] block: Cache mode for children etc.

[Qemu-devel] [PATCH v3 00/21] block: Cache mode for children etc.

[Kevin Wolf]

This is part three (or four, depending on whether you count the bdrv_swap
removal) of what I had sent earlier as "[PATCH 00/34] block: Cache mode for
children, reopen overhaul and more". Most of the patches were actually already
reviewed in v1.

This part contains the remaining functional changes that the cover letter for
v1 advertised, and a bit more:

- You can now use node name references for backing files
- bdrv_reopen() works now properly for inherited options (don't exist before
  this series; after the series the cache options)
- bdrv_reopen() works now properly with semantically overlapping options
- bdrv_reopen() can change child node options
- And finally you can set cache mode options for backing files and other
  children now (and the reopen behaviour even makes sense

v3:
- Patch 3 ('mirror: Error out when a BDS would get two BBs')
  Fixed typo in a comment [Max]

- Patch 4 ('block: Allow references for backing files')
  Removed meanwhile redundant assertion [Max]
  s/backing_hd/backing/ in the commit message [Max]

- Patch 6 ('block: Exclude nested options only for children in append_open_options()')
  g_strdup() the child name to avoid use after free [Wen Congyang]

- Patch 14 ('blockdev: Set 'format' indicates non-empty drive')
  Fix qemu commandline in a qtest case [Wen Congyang]

- Patch 20 ('qemu-iotests: Test cache mode option inheritance')
  s/backing_hd/backing/ in some comments [Max]
  Enabled commented out 'grep Cache' filter for saner output [Max]


Kevin Wolf (21):
  qcow2: Add .bdrv_join_options callback
  block: Fix reopen with semantically overlapping options
  mirror: Error out when a BDS would get two BBs
  block: Allow references for backing files
  block: Consider all block layer options in append_open_options
  block: Exclude nested options only for children in
    append_open_options()
  block: Pass driver-specific options to .bdrv_refresh_filename()
  block: Keep "driver" in bs->options
  block: Allow specifying child options in reopen
  block: reopen: Document option precedence and refactor accordingly
  block: Add infrastructure for option inheritance
  block: Split out parse_json_protocol()
  block: Introduce bs->explicit_options
  blockdev: Set 'format' indicates non-empty drive
  qemu-iotests: Remove cache mode test without medium
  block: reopen: Extract QemuOpts for generic block layer options
  block: Move cache options into options QDict
  blkdebug: Enable reopen
  qemu-iotests: Try setting cache mode for children
  qemu-iotests: Test cache mode option inheritance
  qemu-iotests: Test reopen with node-name/driver options

 block.c                       | 459 +++++++++++++++++++------
 block/blkdebug.c              |  24 +-
 block/blkverify.c             |   2 +-
 block/mirror.c                |  30 +-
 block/nbd.c                   |  10 +-
 block/qcow2.c                 |  47 +++
 block/quorum.c                |   2 +-
 blockdev.c                    |  57 +---
 include/block/block.h         |   4 +-
 include/block/block_int.h     |   8 +-
 tests/hd-geo-test.c           |   4 +-
 tests/qemu-iotests/051        |  22 +-
 tests/qemu-iotests/051.out    |  74 +++-
 tests/qemu-iotests/133        |  90 +++++
 tests/qemu-iotests/133.out    |  22 ++
 tests/qemu-iotests/142        | 354 +++++++++++++++++++
 tests/qemu-iotests/142.out    | 773 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group      |   2 +
 tests/qemu-iotests/iotests.py |   4 +-
 19 files changed, 1811 insertions(+), 177 deletions(-)
 create mode 100755 tests/qemu-iotests/133
 create mode 100644 tests/qemu-iotests/133.out
 create mode 100755 tests/qemu-iotests/142
 create mode 100644 tests/qemu-iotests/142.out

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 01/21] qcow2: Add .bdrv_join_options callback

[Kevin Wolf]

qcow2 accepts a few driver-specific options that overlap semantically
(e.g. "overlap-check" is an alias of "overlap-check.template", and any
missing cache size option is derived from the given ones).

When bdrv_reopen() merges the set of updated options with left out
options that should be kept at their old value, we need to consider this
and filter out any duplicates (which would generally cause errors
because new and old value would contradict each other).

This patch adds a .bdrv_join_options callback to BlockDriver and
implements it for qcow2.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2.c             | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block_int.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 88f56c8..9baaf4d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1282,6 +1282,52 @@ static void qcow2_reopen_abort(BDRVReopenState *state)
     g_free(state->opaque);
 }
 
+static void qcow2_join_options(QDict *options, QDict *old_options)
+{
+    bool has_new_overlap_template =
+        qdict_haskey(options, QCOW2_OPT_OVERLAP) ||
+        qdict_haskey(options, QCOW2_OPT_OVERLAP_TEMPLATE);
+    bool has_new_total_cache_size =
+        qdict_haskey(options, QCOW2_OPT_CACHE_SIZE);
+    bool has_all_cache_options;
+
+    /* New overlap template overrides all old overlap options */
+    if (has_new_overlap_template) {
+        qdict_del(old_options, QCOW2_OPT_OVERLAP);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_TEMPLATE);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_MAIN_HEADER);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_ACTIVE_L1);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_ACTIVE_L2);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_REFCOUNT_TABLE);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_INACTIVE_L1);
+        qdict_del(old_options, QCOW2_OPT_OVERLAP_INACTIVE_L2);
+    }
+
+    /* New total cache size overrides all old options */
+    if (qdict_haskey(options, QCOW2_OPT_CACHE_SIZE)) {
+        qdict_del(old_options, QCOW2_OPT_L2_CACHE_SIZE);
+        qdict_del(old_options, QCOW2_OPT_REFCOUNT_CACHE_SIZE);
+    }
+
+    qdict_join(options, old_options, false);
+
+    /*
+     * If after merging all cache size options are set, an old total size is
+     * overwritten. Do keep all options, however, if all three are new. The
+     * resulting error message is what we want to happen.
+     */
+    has_all_cache_options =
+        qdict_haskey(options, QCOW2_OPT_CACHE_SIZE) ||
+        qdict_haskey(options, QCOW2_OPT_L2_CACHE_SIZE) ||
+        qdict_haskey(options, QCOW2_OPT_REFCOUNT_CACHE_SIZE);
+
+    if (has_all_cache_options && !has_new_total_cache_size) {
+        qdict_del(options, QCOW2_OPT_CACHE_SIZE);
+    }
+}
+
 static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, int *pnum)
 {
@@ -3145,6 +3191,7 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_reopen_prepare  = qcow2_reopen_prepare,
     .bdrv_reopen_commit   = qcow2_reopen_commit,
     .bdrv_reopen_abort    = qcow2_reopen_abort,
+    .bdrv_join_options    = qcow2_join_options,
     .bdrv_create        = qcow2_create,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = qcow2_co_get_block_status,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4012e36..77dc165 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -121,6 +121,7 @@ struct BlockDriver {
                                BlockReopenQueue *queue, Error **errp);
     void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
     void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
+    void (*bdrv_join_options)(QDict *options, QDict *old_options);
 
     int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 02/21] block: Fix reopen with semantically overlapping options

[Kevin Wolf]

This fixes bdrv_reopen() calls like the following one:

    qemu-io -c 'open -o overlap-check.template=all /tmp/test.qcow2' \
    -c 'reopen -o overlap-check=none'

The approach taken so far would result in an options QDict that has both
"overlap-check.template=all" and "overlap-check=none", which obviously
conflicts. In this case, the old option should be overridden by the
newly specified option.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 3a7324b..675e5a8 100644
--- a/block.c
+++ b/block.c
@@ -624,6 +624,20 @@ static int refresh_total_sectors(BlockDriverState *bs, int64_t hint)
 }
 
 /**
+ * Combines a QDict of new block driver @options with any missing options taken
+ * from @old_options, so that leaving out an option defaults to its old value.
+ */
+static void bdrv_join_options(BlockDriverState *bs, QDict *options,
+                              QDict *old_options)
+{
+    if (bs->drv && bs->drv->bdrv_join_options) {
+        bs->drv->bdrv_join_options(options, old_options);
+    } else {
+        qdict_join(options, old_options, false);
+    }
+}
+
+/**
  * Set open flags for a given discard mode
  *
  * Return 0 on success, -1 if the discard mode was invalid.
@@ -1663,7 +1677,7 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
     }
 
     old_options = qdict_clone_shallow(bs->options);
-    qdict_join(options, old_options, false);
+    bdrv_join_options(bs, options, old_options);
     QDECREF(old_options);
 
     /* bdrv_open() masks this flag out */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 03/21] mirror: Error out when a BDS would get two BBs

[Kevin Wolf]

bdrv_replace_in_backing_chain() asserts that not both old and new
BlockDdriverState have a BlockBackend attached to them because both
would have to end up pointing to the new BDS and we don't support more
than one BB per BDS yet.

Before we can safely allow references to existing nodes as backing
files, we need to make sure that even if a backing file has a BB on it,
this doesn't crash qemu.

There are probably also some cases with the 'replaces' option set where
drive-mirror could fail this assertion today. They are fixed with this
error check as well.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 0e8f556..8e3f524 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -18,6 +18,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/ratelimit.h"
 #include "qemu/bitmap.h"
+#include "qemu/error-report.h"
 
 #define SLICE_TIME    100000000ULL /* ns */
 #define MAX_IN_FLIGHT 16
@@ -370,11 +371,22 @@ static void mirror_exit(BlockJob *job, void *opaque)
         if (s->to_replace) {
             to_replace = s->to_replace;
         }
+
+        /* This was checked in mirror_start_job(), but meanwhile one of the
+         * nodes could have been newly attached to a BlockBackend. */
+        if (to_replace->blk && s->target->blk) {
+            error_report("block job: Can't create node with two BlockBackends");
+            data->ret = -EINVAL;
+            goto out;
+        }
+
         if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
             bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
         }
         bdrv_replace_in_backing_chain(to_replace, s->target);
     }
+
+out:
     if (s->to_replace) {
         bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
         error_free(s->replace_blocker);
@@ -705,6 +717,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
                              bool is_none_mode, BlockDriverState *base)
 {
     MirrorBlockJob *s;
+    BlockDriverState *replaced_bs;
 
     if (granularity == 0) {
         granularity = bdrv_get_default_bitmap_granularity(target);
@@ -728,6 +741,21 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
         buf_size = DEFAULT_MIRROR_BUF_SIZE;
     }
 
+    /* We can't support this case as long as the block layer can't handle
+     * multiple BlockBackends per BlockDriverState. */
+    if (replaces) {
+        replaced_bs = bdrv_lookup_bs(replaces, replaces, errp);
+        if (replaced_bs == NULL) {
+            return;
+        }
+    } else {
+        replaced_bs = bs;
+    }
+    if (replaced_bs->blk && target->blk) {
+        error_setg(errp, "Can't create node with two BlockBackends");
+        return;
+    }
+
     s = block_job_create(driver, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/21] block: Allow references for backing files

[Kevin Wolf]

For bs->file, using references to existing BDSes has been possible for a
while already. This patch enables the same for bs->backing.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 48 +++++++++++++++++++++++++++++-------------------
 block/mirror.c        |  2 +-
 include/block/block.h |  3 ++-
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/block.c b/block.c
index 675e5a8..0241acd 100644
--- a/block.c
+++ b/block.c
@@ -1182,30 +1182,43 @@ out:
 /*
  * Opens the backing file for a BlockDriverState if not yet open
  *
- * options is a QDict of options to pass to the block drivers, or NULL for an
- * empty set of options. The reference to the QDict is transferred to this
- * function (even on failure), so if the caller intends to reuse the dictionary,
- * it needs to use QINCREF() before calling bdrv_file_open.
+ * bdref_key specifies the key for the image's BlockdevRef in the options QDict.
+ * That QDict has to be flattened; therefore, if the BlockdevRef is a QDict
+ * itself, all options starting with "${bdref_key}." are considered part of the
+ * BlockdevRef.
+ *
+ * TODO Can this be unified with bdrv_open_image()?
  */
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
+                           const char *bdref_key, Error **errp)
 {
     char *backing_filename = g_malloc0(PATH_MAX);
+    char *bdref_key_dot;
+    const char *reference = NULL;
     int ret = 0;
     BlockDriverState *backing_hd;
+    QDict *options;
+    QDict *tmp_parent_options = NULL;
     Error *local_err = NULL;
 
     if (bs->backing != NULL) {
-        QDECREF(options);
         goto free_exit;
     }
 
     /* NULL means an empty set of options */
-    if (options == NULL) {
-        options = qdict_new();
+    if (parent_options == NULL) {
+        tmp_parent_options = qdict_new();
+        parent_options = tmp_parent_options;
     }
 
     bs->open_flags &= ~BDRV_O_NO_BACKING;
-    if (qdict_haskey(options, "file.filename")) {
+
+    bdref_key_dot = g_strdup_printf("%s.", bdref_key);
+    qdict_extract_subqdict(parent_options, &options, bdref_key_dot);
+    g_free(bdref_key_dot);
+
+    reference = qdict_get_try_str(parent_options, bdref_key);
+    if (reference || qdict_haskey(options, "file.filename")) {
         backing_filename[0] = '\0';
     } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
         QDECREF(options);
@@ -1228,19 +1241,16 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
         goto free_exit;
     }
 
-    backing_hd = bdrv_new();
-
     if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
         qdict_put(options, "driver", qstring_from_str(bs->backing_format));
     }
 
-    assert(bs->backing == NULL);
+    backing_hd = NULL;
     ret = bdrv_open_inherit(&backing_hd,
                             *backing_filename ? backing_filename : NULL,
-                            NULL, options, 0, bs, &child_backing, &local_err);
+                            reference, options, 0, bs, &child_backing,
+                            &local_err);
     if (ret < 0) {
-        bdrv_unref(backing_hd);
-        backing_hd = NULL;
         bs->open_flags |= BDRV_O_NO_BACKING;
         error_setg(errp, "Could not open backing file: %s",
                    error_get_pretty(local_err));
@@ -1253,8 +1263,11 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
     bdrv_set_backing_hd(bs, backing_hd);
     bdrv_unref(backing_hd);
 
+    qdict_del(parent_options, bdref_key);
+
 free_exit:
     g_free(backing_filename);
+    QDECREF(tmp_parent_options);
     return ret;
 }
 
@@ -1537,10 +1550,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
 
     /* If there is a backing file, use it */
     if ((flags & BDRV_O_NO_BACKING) == 0) {
-        QDict *backing_options;
-
-        qdict_extract_subqdict(options, &backing_options, "backing.");
-        ret = bdrv_open_backing_file(bs, backing_options, &local_err);
+        ret = bdrv_open_backing_file(bs, options, "backing", &local_err);
         if (ret < 0) {
             goto close_and_fail;
         }
diff --git a/block/mirror.c b/block/mirror.c
index 8e3f524..fc34a9c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -652,7 +652,7 @@ static void mirror_complete(BlockJob *job, Error **errp)
     Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open_backing_file(s->target, NULL, &local_err);
+    ret = bdrv_open_backing_file(s->target, NULL, "backing", &local_err);
     if (ret < 0) {
         error_propagate(errp, local_err);
         return;
diff --git a/include/block/block.h b/include/block/block.h
index 3477328..589be29 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -210,7 +210,8 @@ BdrvChild *bdrv_open_child(const char *filename,
                            const BdrvChildRole *child_role,
                            bool allow_none, Error **errp);
 void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
+                           const char *bdref_key, Error **errp);
 int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
 int bdrv_open(BlockDriverState **pbs, const char *filename,
               const char *reference, QDict *options, int flags, Error **errp);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/21] block: Consider all block layer options in append_open_options

[Kevin Wolf]

The code already special-cased "node-name", which is currently the only
option passed in the QDict that isn't driver-specific. Generalise the
code to take all general block layer options into consideration.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 0241acd..73f0816 100644
--- a/block.c
+++ b/block.c
@@ -3950,20 +3950,30 @@ out:
 static bool append_open_options(QDict *d, BlockDriverState *bs)
 {
     const QDictEntry *entry;
+    QemuOptDesc *desc;
     bool found_any = false;
 
     for (entry = qdict_first(bs->options); entry;
          entry = qdict_next(bs->options, entry))
     {
-        /* Only take options for this level and exclude all non-driver-specific
-         * options */
-        if (!strchr(qdict_entry_key(entry), '.') &&
-            strcmp(qdict_entry_key(entry), "node-name"))
-        {
-            qobject_incref(qdict_entry_value(entry));
-            qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry));
-            found_any = true;
+        /* Only take options for this level */
+        if (strchr(qdict_entry_key(entry), '.')) {
+            continue;
         }
+
+        /* And exclude all non-driver-specific options */
+        for (desc = bdrv_runtime_opts.desc; desc->name; desc++) {
+            if (!strcmp(qdict_entry_key(entry), desc->name)) {
+                break;
+            }
+        }
+        if (desc->name) {
+            continue;
+        }
+
+        qobject_incref(qdict_entry_value(entry));
+        qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry));
+        found_any = true;
     }
 
     return found_any;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 06/21] block: Exclude nested options only for children in append_open_options()

[Kevin Wolf]

Some drivers have nested options (e.g. blkdebug rule arrays), which
don't belong to a child node and shouldn't be removed. Don't remove all
options with "." in their name, but check for the complete prefixes of
actually existing child nodes.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   | 20 ++++++++++++++++----
 include/block/block_int.h |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 73f0816..0dfff7a 100644
--- a/block.c
+++ b/block.c
@@ -1101,11 +1101,13 @@ static int bdrv_fill_options(QDict **options, const char **pfilename,
 
 static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
                                     BlockDriverState *child_bs,
+                                    const char *child_name,
                                     const BdrvChildRole *child_role)
 {
     BdrvChild *child = g_new(BdrvChild, 1);
     *child = (BdrvChild) {
         .bs     = child_bs,
+        .name   = g_strdup(child_name),
         .role   = child_role,
     };
 
@@ -1119,6 +1121,7 @@ static void bdrv_detach_child(BdrvChild *child)
 {
     QLIST_REMOVE(child, next);
     QLIST_REMOVE(child, next_parent);
+    g_free(child->name);
     g_free(child);
 }
 
@@ -1165,7 +1168,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
         bs->backing = NULL;
         goto out;
     }
-    bs->backing = bdrv_attach_child(bs, backing_hd, &child_backing);
+    bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing);
     bs->open_flags &= ~BDRV_O_NO_BACKING;
     pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
     pstrcpy(bs->backing_format, sizeof(bs->backing_format),
@@ -1321,7 +1324,7 @@ BdrvChild *bdrv_open_child(const char *filename,
         goto done;
     }
 
-    c = bdrv_attach_child(parent, bs, child_role);
+    c = bdrv_attach_child(parent, bs, bdref_key, child_role);
 
 done:
     qdict_del(options, bdref_key);
@@ -3951,13 +3954,22 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
 {
     const QDictEntry *entry;
     QemuOptDesc *desc;
+    BdrvChild *child;
     bool found_any = false;
+    const char *p;
 
     for (entry = qdict_first(bs->options); entry;
          entry = qdict_next(bs->options, entry))
     {
-        /* Only take options for this level */
-        if (strchr(qdict_entry_key(entry), '.')) {
+        /* Exclude options for children */
+        QLIST_FOREACH(child, &bs->children, next) {
+            if (strstart(qdict_entry_key(entry), child->name, &p)
+                && (!*p || *p == '.'))
+            {
+                break;
+            }
+        }
+        if (child) {
             continue;
         }
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 77dc165..7265247 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -351,6 +351,7 @@ extern const BdrvChildRole child_format;
 
 struct BdrvChild {
     BlockDriverState *bs;
+    char *name;
     const BdrvChildRole *role;
     QLIST_ENTRY(BdrvChild) next;
     QLIST_ENTRY(BdrvChild) next_parent;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 07/21] block: Pass driver-specific options to .bdrv_refresh_filename()

[Kevin Wolf]

In order to decide whether a blkdebug: filename can be produced or a
json: one is necessary, blkdebug checked whether bs->options had more
options than just "config", "x-image" or "image" (the latter including
nested options). That doesn't work well when generic block layer options
are present.

This patch passes an option QDict to the driver that contains only
driver-specific options, i.e. the options for the general block layer as
well as child nodes are already filtered out. Works much better this
way.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block.c                   |  5 ++++-
 block/blkdebug.c          | 17 ++++++-----------
 block/blkverify.c         |  2 +-
 block/nbd.c               | 10 +++++-----
 block/quorum.c            |  2 +-
 include/block/block_int.h |  2 +-
 6 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/block.c b/block.c
index 0dfff7a..5ca7c71 100644
--- a/block.c
+++ b/block.c
@@ -4027,7 +4027,10 @@ void bdrv_refresh_filename(BlockDriverState *bs)
             bs->full_open_options = NULL;
         }
 
-        drv->bdrv_refresh_filename(bs);
+        opts = qdict_new();
+        append_open_options(opts, bs);
+        drv->bdrv_refresh_filename(bs, opts);
+        QDECREF(opts);
     } else if (bs->file) {
         /* Try to reconstruct valid information from the underlying file */
         bool has_open_options;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index dee3a0e..48b0812 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -731,17 +731,15 @@ static int blkdebug_truncate(BlockDriverState *bs, int64_t offset)
     return bdrv_truncate(bs->file->bs, offset);
 }
 
-static void blkdebug_refresh_filename(BlockDriverState *bs)
+static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
 {
     QDict *opts;
     const QDictEntry *e;
     bool force_json = false;
 
-    for (e = qdict_first(bs->options); e; e = qdict_next(bs->options, e)) {
+    for (e = qdict_first(options); e; e = qdict_next(options, e)) {
         if (strcmp(qdict_entry_key(e), "config") &&
-            strcmp(qdict_entry_key(e), "x-image") &&
-            strcmp(qdict_entry_key(e), "image") &&
-            strncmp(qdict_entry_key(e), "image.", strlen("image.")))
+            strcmp(qdict_entry_key(e), "x-image"))
         {
             force_json = true;
             break;
@@ -757,7 +755,7 @@ static void blkdebug_refresh_filename(BlockDriverState *bs)
     if (!force_json && bs->file->bs->exact_filename[0]) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
                  "blkdebug:%s:%s",
-                 qdict_get_try_str(bs->options, "config") ?: "",
+                 qdict_get_try_str(options, "config") ?: "",
                  bs->file->bs->exact_filename);
     }
 
@@ -767,11 +765,8 @@ static void blkdebug_refresh_filename(BlockDriverState *bs)
     QINCREF(bs->file->bs->full_open_options);
     qdict_put_obj(opts, "image", QOBJECT(bs->file->bs->full_open_options));
 
-    for (e = qdict_first(bs->options); e; e = qdict_next(bs->options, e)) {
-        if (strcmp(qdict_entry_key(e), "x-image") &&
-            strcmp(qdict_entry_key(e), "image") &&
-            strncmp(qdict_entry_key(e), "image.", strlen("image.")))
-        {
+    for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+        if (strcmp(qdict_entry_key(e), "x-image")) {
             qobject_incref(qdict_entry_value(e));
             qdict_put_obj(opts, qdict_entry_key(e), qdict_entry_value(e));
         }
diff --git a/block/blkverify.c b/block/blkverify.c
index c5f8e8d..1d75449 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -307,7 +307,7 @@ static void blkverify_attach_aio_context(BlockDriverState *bs,
     bdrv_attach_aio_context(s->test_file->bs, new_context);
 }
 
-static void blkverify_refresh_filename(BlockDriverState *bs)
+static void blkverify_refresh_filename(BlockDriverState *bs, QDict *options)
 {
     BDRVBlkverifyState *s = bs->opaque;
 
diff --git a/block/nbd.c b/block/nbd.c
index cd6a587..416f42b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -342,13 +342,13 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
     nbd_client_attach_aio_context(bs, new_context);
 }
 
-static void nbd_refresh_filename(BlockDriverState *bs)
+static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
 {
     QDict *opts = qdict_new();
-    const char *path   = qdict_get_try_str(bs->options, "path");
-    const char *host   = qdict_get_try_str(bs->options, "host");
-    const char *port   = qdict_get_try_str(bs->options, "port");
-    const char *export = qdict_get_try_str(bs->options, "export");
+    const char *path   = qdict_get_try_str(options, "path");
+    const char *host   = qdict_get_try_str(options, "host");
+    const char *port   = qdict_get_try_str(options, "port");
+    const char *export = qdict_get_try_str(options, "export");
 
     qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
 
diff --git a/block/quorum.c b/block/quorum.c
index b9ba028..2810e37 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -997,7 +997,7 @@ static void quorum_attach_aio_context(BlockDriverState *bs,
     }
 }
 
-static void quorum_refresh_filename(BlockDriverState *bs)
+static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
 {
     BDRVQuorumState *s = bs->opaque;
     QDict *opts;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7265247..136635a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -136,7 +136,7 @@ struct BlockDriver {
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
     int (*bdrv_make_empty)(BlockDriverState *bs);
 
-    void (*bdrv_refresh_filename)(BlockDriverState *bs);
+    void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
 
     /* aio */
     BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/21] block: Keep "driver" in bs->options

[Kevin Wolf]

Instead of passing a separate drv argument to bdrv_open_common(), just
make sure that a "driver" option is set in the QDict. This also means
that a "driver" entry is consistently present in bs->options now.

This is another step towards keeping all options in the QDict (which is
the represenation of the blockdev-add QMP command).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 57 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 5ca7c71..2728bc5 100644
--- a/block.c
+++ b/block.c
@@ -817,6 +817,11 @@ static QemuOptsList bdrv_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "Node name of the block device node",
         },
+        {
+            .name = "driver",
+            .type = QEMU_OPT_STRING,
+            .help = "Block driver to use for the node",
+        },
         { /* end of list */ }
     },
 };
@@ -827,18 +832,31 @@ static QemuOptsList bdrv_runtime_opts = {
  * Removes all processed options from *options.
  */
 static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
-    QDict *options, int flags, BlockDriver *drv, Error **errp)
+                            QDict *options, int flags, Error **errp)
 {
     int ret, open_flags;
     const char *filename;
+    const char *driver_name = NULL;
     const char *node_name = NULL;
     QemuOpts *opts;
+    BlockDriver *drv;
     Error *local_err = NULL;
 
-    assert(drv != NULL);
     assert(bs->file == NULL);
     assert(options != NULL && bs->options != options);
 
+    opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail_opts;
+    }
+
+    driver_name = qemu_opt_get(opts, "driver");
+    drv = bdrv_find_format(driver_name);
+    assert(drv != NULL);
+
     if (file != NULL) {
         filename = file->bs->filename;
     } else {
@@ -848,19 +866,12 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     if (drv->bdrv_needs_filename && !filename) {
         error_setg(errp, "The '%s' block driver requires a file name",
                    drv->format_name);
-        return -EINVAL;
-    }
-
-    trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
-
-    opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
-    qemu_opts_absorb_qdict(opts, options, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
         ret = -EINVAL;
         goto fail_opts;
     }
 
+    trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
+
     node_name = qemu_opt_get(opts, "node-name");
     bdrv_assign_node_name(bs, node_name, &local_err);
     if (local_err) {
@@ -1477,11 +1488,14 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         goto fail;
     }
 
+    bs->open_flags = flags;
+    bs->options = options;
+    options = qdict_clone_shallow(options);
+
     /* Find the right image format driver */
     drvname = qdict_get_try_str(options, "driver");
     if (drvname) {
         drv = bdrv_find_format(drvname);
-        qdict_del(options, "driver");
         if (!drv) {
             error_setg(errp, "Unknown driver: '%s'", drvname);
             ret = -EINVAL;
@@ -1497,10 +1511,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         qdict_del(options, "backing");
     }
 
-    bs->open_flags = flags;
-    bs->options = options;
-    options = qdict_clone_shallow(options);
-
     /* Open image file without format layer */
     if ((flags & BDRV_O_PROTOCOL) == 0) {
         if (flags & BDRV_O_RDWR) {
@@ -1528,6 +1538,19 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         if (ret < 0) {
             goto fail;
         }
+        /*
+         * This option update would logically belong in bdrv_fill_options(),
+         * but we first need to open bs->file for the probing to work, while
+         * opening bs->file already requires the (mostly) final set of options
+         * so that cache mode etc. can be inherited.
+         *
+         * Adding the driver later is somewhat ugly, but it's not an option
+         * that would ever be inherited, so it's correct. We just need to make
+         * sure to update both bs->options (which has the full effective
+         * options for bs) and options (which has file.* already removed).
+         */
+        qdict_put(bs->options, "driver", qstring_from_str(drv->format_name));
+        qdict_put(options, "driver", qstring_from_str(drv->format_name));
     } else if (!drv) {
         error_setg(errp, "Must specify either driver or file");
         ret = -EINVAL;
@@ -1541,7 +1564,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
     assert(!(flags & BDRV_O_PROTOCOL) || !file);
 
     /* Open the image */
-    ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
+    ret = bdrv_open_common(bs, file, options, flags, &local_err);
     if (ret < 0) {
         goto fail;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 09/21] block: Allow specifying child options in reopen

[Kevin Wolf]

If the child was defined in the same context (-drive argument or
blockdev-add QMP command) as its parent, a reopen of the parent should
work the same and allow changing options of the child.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 2728bc5..a800d9e 100644
--- a/block.c
+++ b/block.c
@@ -1720,15 +1720,23 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
     flags &= ~BDRV_O_PROTOCOL;
 
     QLIST_FOREACH(child, &bs->children, next) {
+        QDict *new_child_options;
+        char *child_key_dot;
         int child_flags;
 
+        /* reopen can only change the options of block devices that were
+         * implicitly created and inherited options. For other (referenced)
+         * block devices, a syntax like "backing.foo" results in an error. */
         if (child->bs->inherits_from != bs) {
             continue;
         }
 
+        child_key_dot = g_strdup_printf("%s.", child->name);
+        qdict_extract_subqdict(options, &new_child_options, child_key_dot);
+        g_free(child_key_dot);
+
         child_flags = child->role->inherit_flags(flags);
-        /* TODO Pass down child flags (backing.*, extents.*, ...) */
-        bdrv_reopen_queue(bs_queue, child->bs, NULL, child_flags);
+        bdrv_reopen_queue(bs_queue, child->bs, new_child_options, child_flags);
     }
 
     bs_entry = g_new0(BlockReopenQueueEntry, 1);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 10/21] block: reopen: Document option precedence and refactor accordingly

[Kevin Wolf]

The interesting part of reopening an image is from which sources the
effective options should be taken, i.e. which options take precedence
over which other options. This patch documents the precedence that will
be implemented in the following patches.

It also refactors bdrv_reopen_queue(), so that the top-level reopened
node is handled the same way as children are. Option/flag inheritance
from the parent becomes just one item in the list and is done at the
beginning of the function, similar to how the other items are/will be
handled.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index a800d9e..235aa72 100644
--- a/block.c
+++ b/block.c
@@ -1693,9 +1693,13 @@ typedef struct BlockReopenQueueEntry {
  * bs_queue, or the existing bs_queue being used.
  *
  */
-BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
-                                    BlockDriverState *bs,
-                                    QDict *options, int flags)
+static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
+                                                 BlockDriverState *bs,
+                                                 QDict *options,
+                                                 int flags,
+                                                 const BdrvChildRole *role,
+                                                 QDict *parent_options,
+                                                 int parent_flags)
 {
     assert(bs != NULL);
 
@@ -1712,6 +1716,22 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
         options = qdict_new();
     }
 
+    /*
+     * Precedence of options:
+     * 1. Explicitly passed in options (highest)
+     * 2. TODO Set in flags (only for top level)
+     * 3. TODO Retained from explicitly set options of bs
+     * 4. TODO Inherited from parent node
+     * 5. Retained from effective options of bs
+     */
+
+    /* Inherit from parent node */
+    if (parent_options) {
+        assert(!flags);
+        flags = role->inherit_flags(parent_flags);
+    }
+
+    /* Old values are used for options that aren't set yet */
     old_options = qdict_clone_shallow(bs->options);
     bdrv_join_options(bs, options, old_options);
     QDECREF(old_options);
@@ -1722,7 +1742,6 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
     QLIST_FOREACH(child, &bs->children, next) {
         QDict *new_child_options;
         char *child_key_dot;
-        int child_flags;
 
         /* reopen can only change the options of block devices that were
          * implicitly created and inherited options. For other (referenced)
@@ -1735,8 +1754,8 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
         qdict_extract_subqdict(options, &new_child_options, child_key_dot);
         g_free(child_key_dot);
 
-        child_flags = child->role->inherit_flags(flags);
-        bdrv_reopen_queue(bs_queue, child->bs, new_child_options, child_flags);
+        bdrv_reopen_queue_child(bs_queue, child->bs, new_child_options, 0,
+                                child->role, options, flags);
     }
 
     bs_entry = g_new0(BlockReopenQueueEntry, 1);
@@ -1749,6 +1768,14 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
     return bs_queue;
 }
 
+BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
+                                    BlockDriverState *bs,
+                                    QDict *options, int flags)
+{
+    return bdrv_reopen_queue_child(bs_queue, bs, options, flags,
+                                   NULL, NULL, 0);
+}
+
 /*
  * Reopen multiple BlockDriverStates atomically & transactionally.
  *
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 11/21] block: Add infrastructure for option inheritance

[Kevin Wolf]

Options are not actually inherited from the parent node yet, but this
commit lays the grounds for doing so.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c                   | 52 ++++++++++++++++++++++++++++-------------------
 include/block/block_int.h |  3 ++-
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 235aa72..5b0183e 100644
--- a/block.c
+++ b/block.c
@@ -695,11 +695,14 @@ static int bdrv_temp_snapshot_flags(int flags)
 }
 
 /*
- * Returns the flags that bs->file should get if a protocol driver is expected,
- * based on the given flags for the parent BDS
+ * Returns the options and flags that bs->file should get if a protocol driver
+ * is expected, based on the given options and flags for the parent BDS
  */
-static int bdrv_inherited_flags(int flags)
+static void bdrv_inherited_options(int *child_flags, QDict *child_options,
+                                   int parent_flags, QDict *parent_options)
 {
+    int flags = parent_flags;
+
     /* Enable protocol handling, disable format probing for bs->file */
     flags |= BDRV_O_PROTOCOL;
 
@@ -710,45 +713,51 @@ static int bdrv_inherited_flags(int flags)
     /* Clear flags that only apply to the top layer */
     flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
 
-    return flags;
+    *child_flags = flags;
 }
 
 const BdrvChildRole child_file = {
-    .inherit_flags = bdrv_inherited_flags,
+    .inherit_options = bdrv_inherited_options,
 };
 
 /*
- * Returns the flags that bs->file should get if the use of formats (and not
- * only protocols) is permitted for it, based on the given flags for the parent
- * BDS
+ * Returns the options and flags that bs->file should get if the use of formats
+ * (and not only protocols) is permitted for it, based on the given options and
+ * flags for the parent BDS
  */
-static int bdrv_inherited_fmt_flags(int parent_flags)
+static void bdrv_inherited_fmt_options(int *child_flags, QDict *child_options,
+                                       int parent_flags, QDict *parent_options)
 {
-    int flags = child_file.inherit_flags(parent_flags);
-    return flags & ~BDRV_O_PROTOCOL;
+    child_file.inherit_options(child_flags, child_options,
+                               parent_flags, parent_options);
+
+    *child_flags &= ~BDRV_O_PROTOCOL;
 }
 
 const BdrvChildRole child_format = {
-    .inherit_flags = bdrv_inherited_fmt_flags,
+    .inherit_options = bdrv_inherited_fmt_options,
 };
 
 /*
- * Returns the flags that bs->backing should get, based on the given flags
- * for the parent BDS
+ * Returns the options and flags that bs->backing should get, based on the
+ * given options and flags for the parent BDS
  */
-static int bdrv_backing_flags(int flags)
+static void bdrv_backing_options(int *child_flags, QDict *child_options,
+                                 int parent_flags, QDict *parent_options)
 {
+    int flags = parent_flags;
+
     /* backing files always opened read-only */
     flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
 
     /* snapshot=on is handled on the top layer */
     flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_TEMPORARY);
 
-    return flags;
+    *child_flags = flags;
 }
 
 static const BdrvChildRole child_backing = {
-    .inherit_flags = bdrv_backing_flags,
+    .inherit_options = bdrv_backing_options,
 };
 
 static int bdrv_open_flags(BlockDriverState *bs, int flags)
@@ -1480,7 +1489,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
 
     if (child_role) {
         bs->inherits_from = parent;
-        flags = child_role->inherit_flags(parent->open_flags);
+        child_role->inherit_options(&flags, options,
+                                    parent->open_flags, parent->options);
     }
 
     ret = bdrv_fill_options(&options, &filename, &flags, &local_err);
@@ -1518,7 +1528,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         }
         if (flags & BDRV_O_SNAPSHOT) {
             snapshot_flags = bdrv_temp_snapshot_flags(flags);
-            flags = bdrv_backing_flags(flags);
+            bdrv_backing_options(&flags, options, flags, options);
         }
 
         bs->open_flags = flags;
@@ -1721,14 +1731,14 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
      * 1. Explicitly passed in options (highest)
      * 2. TODO Set in flags (only for top level)
      * 3. TODO Retained from explicitly set options of bs
-     * 4. TODO Inherited from parent node
+     * 4. Inherited from parent node
      * 5. Retained from effective options of bs
      */
 
     /* Inherit from parent node */
     if (parent_options) {
         assert(!flags);
-        flags = role->inherit_flags(parent_flags);
+        role->inherit_options(&flags, options, parent_flags, parent_options);
     }
 
     /* Old values are used for options that aren't set yet */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 136635a..04daa55 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -343,7 +343,8 @@ typedef struct BdrvAioNotifier {
 } BdrvAioNotifier;
 
 struct BdrvChildRole {
-    int (*inherit_flags)(int parent_flags);
+    void (*inherit_options)(int *child_flags, QDict *child_options,
+                            int parent_flags, QDict *parent_options);
 };
 
 extern const BdrvChildRole child_file;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 12/21] block: Split out parse_json_protocol()

[Kevin Wolf]

The next patch distinguishes options that were explicitly set and
options that were derived. bdrv_fill_option() added options of both
types: Options given by json: syntax should be counted as explicit, but
the rest is derived.

In preparation for the distinction, move json: parse to a separate
function.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 50 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/block.c b/block.c
index 5b0183e..8ac1669 100644
--- a/block.c
+++ b/block.c
@@ -1018,37 +1018,45 @@ static QDict *parse_json_filename(const char *filename, Error **errp)
     return options;
 }
 
+static void parse_json_protocol(QDict *options, const char **pfilename,
+                                Error **errp)
+{
+    QDict *json_options;
+    Error *local_err = NULL;
+
+    /* Parse json: pseudo-protocol */
+    if (!*pfilename || !g_str_has_prefix(*pfilename, "json:")) {
+        return;
+    }
+
+    json_options = parse_json_filename(*pfilename, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /* Options given in the filename have lower priority than options
+     * specified directly */
+    qdict_join(options, json_options, false);
+    QDECREF(json_options);
+    *pfilename = NULL;
+}
+
 /*
  * Fills in default options for opening images and converts the legacy
  * filename/flags pair to option QDict entries.
  * The BDRV_O_PROTOCOL flag in *flags will be set or cleared accordingly if a
  * block driver has been specified explicitly.
  */
-static int bdrv_fill_options(QDict **options, const char **pfilename,
+static int bdrv_fill_options(QDict **options, const char *filename,
                              int *flags, Error **errp)
 {
-    const char *filename = *pfilename;
     const char *drvname;
     bool protocol = *flags & BDRV_O_PROTOCOL;
     bool parse_filename = false;
     BlockDriver *drv = NULL;
     Error *local_err = NULL;
 
-    /* Parse json: pseudo-protocol */
-    if (filename && g_str_has_prefix(filename, "json:")) {
-        QDict *json_options = parse_json_filename(filename, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return -EINVAL;
-        }
-
-        /* Options given in the filename have lower priority than options
-         * specified directly */
-        qdict_join(*options, json_options, false);
-        QDECREF(json_options);
-        *pfilename = filename = NULL;
-    }
-
     drvname = qdict_get_try_str(*options, "driver");
     if (drvname) {
         drv = bdrv_find_format(drvname);
@@ -1487,13 +1495,19 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         options = qdict_new();
     }
 
+    parse_json_protocol(options, &filename, &local_err);
+    if (local_err) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
     if (child_role) {
         bs->inherits_from = parent;
         child_role->inherit_options(&flags, options,
                                     parent->open_flags, parent->options);
     }
 
-    ret = bdrv_fill_options(&options, &filename, &flags, &local_err);
+    ret = bdrv_fill_options(&options, filename, &flags, &local_err);
     if (local_err) {
         goto fail;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 13/21] block: Introduce bs->explicit_options

[Kevin Wolf]

bs->options doesn't only contain options that the user explicitly
requested, but also option that were derived from flags, the filename or
inherited from the parent node.

For reopen, it is important to know the difference because reopening the
parent can change inherited values in child nodes, but it shouldn't
change any options that were explicitly specified for the child.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c                   | 24 ++++++++++++++++++++++--
 include/block/block.h     |  1 +
 include/block/block_int.h |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 8ac1669..2dd2830 100644
--- a/block.c
+++ b/block.c
@@ -1495,12 +1495,15 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
         options = qdict_new();
     }
 
+    /* json: syntax counts as explicit options, as if in the QDict */
     parse_json_protocol(options, &filename, &local_err);
     if (local_err) {
         ret = -EINVAL;
         goto fail;
     }
 
+    bs->explicit_options = qdict_clone_shallow(options);
+
     if (child_role) {
         bs->inherits_from = parent;
         child_role->inherit_options(&flags, options,
@@ -1655,6 +1658,7 @@ fail:
     if (file != NULL) {
         bdrv_unref_child(bs, file);
     }
+    QDECREF(bs->explicit_options);
     QDECREF(bs->options);
     QDECREF(options);
     bs->options = NULL;
@@ -1729,7 +1733,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
 
     BlockReopenQueueEntry *bs_entry;
     BdrvChild *child;
-    QDict *old_options;
+    QDict *old_options, *explicit_options;
 
     if (bs_queue == NULL) {
         bs_queue = g_new0(BlockReopenQueue, 1);
@@ -1744,11 +1748,18 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
      * Precedence of options:
      * 1. Explicitly passed in options (highest)
      * 2. TODO Set in flags (only for top level)
-     * 3. TODO Retained from explicitly set options of bs
+     * 3. Retained from explicitly set options of bs
      * 4. Inherited from parent node
      * 5. Retained from effective options of bs
      */
 
+    /* Old explicitly set values (don't overwrite by inherited value) */
+    old_options = qdict_clone_shallow(bs->explicit_options);
+    bdrv_join_options(bs, options, old_options);
+    QDECREF(old_options);
+
+    explicit_options = qdict_clone_shallow(options);
+
     /* Inherit from parent node */
     if (parent_options) {
         assert(!flags);
@@ -1787,6 +1798,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
 
     bs_entry->state.bs = bs;
     bs_entry->state.options = options;
+    bs_entry->state.explicit_options = explicit_options;
     bs_entry->state.flags = flags;
 
     return bs_queue;
@@ -1846,6 +1858,8 @@ cleanup:
     QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
         if (ret && bs_entry->prepared) {
             bdrv_reopen_abort(&bs_entry->state);
+        } else if (ret) {
+            QDECREF(bs_entry->state.explicit_options);
         }
         QDECREF(bs_entry->state.options);
         g_free(bs_entry);
@@ -1980,6 +1994,9 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
     }
 
     /* set BDS specific flags now */
+    QDECREF(reopen_state->bs->explicit_options);
+
+    reopen_state->bs->explicit_options   = reopen_state->explicit_options;
     reopen_state->bs->open_flags         = reopen_state->flags;
     reopen_state->bs->enable_write_cache = !!(reopen_state->flags &
                                               BDRV_O_CACHE_WB);
@@ -2003,6 +2020,8 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
     if (drv->bdrv_reopen_abort) {
         drv->bdrv_reopen_abort(reopen_state);
     }
+
+    QDECREF(reopen_state->explicit_options);
 }
 
 
@@ -2061,6 +2080,7 @@ void bdrv_close(BlockDriverState *bs)
         bs->sg = 0;
         bs->zero_beyond_eof = false;
         QDECREF(bs->options);
+        QDECREF(bs->explicit_options);
         bs->options = NULL;
         QDECREF(bs->full_open_options);
         bs->full_open_options = NULL;
diff --git a/include/block/block.h b/include/block/block.h
index 589be29..17557c3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -150,6 +150,7 @@ typedef struct BDRVReopenState {
     BlockDriverState *bs;
     int flags;
     QDict *options;
+    QDict *explicit_options;
     void *opaque;
 } BDRVReopenState;
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 04daa55..6d7bd3b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -459,6 +459,7 @@ struct BlockDriverState {
     QLIST_HEAD(, BdrvChild) parents;
 
     QDict *options;
+    QDict *explicit_options;
     BlockdevDetectZeroesOptions detect_zeroes;
 
     /* The error object in use for blocking operations on backing_hd */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 14/21] blockdev: Set 'format' indicates non-empty drive

[Kevin Wolf]

Creating an empty drive while specifying 'format' doesn't make sense.
The specified format driver would simply be ignored.

Make a set 'format' option an indication that a non-empty drive should
be created. This makes 'format' consistent with 'driver' and allows
using it with a block driver that doesn't need any other options (like
null-co/null-aio).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c                    | 5 +----
 tests/hd-geo-test.c           | 4 ++--
 tests/qemu-iotests/iotests.py | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 313841b..afaeef9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -490,7 +490,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
     QDict *interval_dict = NULL;
     QList *interval_list = NULL;
     const char *id;
-    bool has_driver_specific_opts;
     BlockdevDetectZeroesOptions detect_zeroes =
         BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
     const char *throttling_group = NULL;
@@ -514,8 +513,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         qdict_del(bs_opts, "id");
     }
 
-    has_driver_specific_opts = !!qdict_size(bs_opts);
-
     /* extract parameters */
     snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
 
@@ -578,7 +575,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
     }
 
     /* init */
-    if ((!file || !*file) && !has_driver_specific_opts) {
+    if ((!file || !*file) && !qdict_size(bs_opts)) {
         BlockBackendRootState *blk_rs;
 
         blk = blk_new(qemu_opts_id(opts), errp);
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 00afc20..13b763d 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -206,13 +206,13 @@ static int setup_ide(int argc, char *argv[], int argv_sz,
 {
     char *s1, *s2, *s3;
 
-    s1 = g_strdup_printf("-drive id=drive%d,if=%s,format=raw",
+    s1 = g_strdup_printf("-drive id=drive%d,if=%s",
                          ide_idx, dev ? "none" : "ide");
     s2 = dev ? g_strdup("") : g_strdup_printf(",index=%d", ide_idx);
 
     if (img_secs[img_idx] >= 0) {
         setup_mbr(img_idx, mbr);
-        s3 = g_strdup_printf(",file=%s", img_file_name[img_idx]);
+        s3 = g_strdup_printf(",format=raw,file=%s", img_file_name[img_idx]);
     } else {
         s3 = g_strdup(",media=cdrom");
     }
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e02245e..27eddec 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -148,12 +148,12 @@ class VM(object):
     def add_drive(self, path, opts='', interface='virtio'):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=%s' % interface,
-                   'format=%s' % imgfmt,
                    'cache=%s' % cachemode,
                    'id=drive%d' % self._num_drives]
 
         if path is not None:
             options.append('file=%s' % path)
+            options.append('format=%s' % imgfmt)
 
         if opts:
             options.append(opts)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 15/21] qemu-iotests: Remove cache mode test without medium

[Kevin Wolf]

Specifying the cache mode for a driver without a medium is not a useful
thing to do: As long as there is no medium, the cache mode doesn't make
a difference, and once the 'change' command is used to insert a medium,
it ignores the old cache mode and makes the new medium use
cache=writethrough.

Later patches will make it an error to specify the cache mode for an
empty drive. Remove the corresponding test case.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/051        | 12 ++++++------
 tests/qemu-iotests/051.out    | 14 +++++++-------
 tests/qemu-iotests/iotests.py |  2 +-
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 17dbf04..f6f0f4d 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -183,12 +183,12 @@ echo
 # Cannot use the test image because cache=none might not work on the host FS
 # Use cdrom so that we won't get errors about missing media
 
-run_qemu -drive media=cdrom,cache=none
-run_qemu -drive media=cdrom,cache=directsync
-run_qemu -drive media=cdrom,cache=writeback
-run_qemu -drive media=cdrom,cache=writethrough
-run_qemu -drive media=cdrom,cache=unsafe
-run_qemu -drive media=cdrom,cache=invalid_value
+run_qemu -drive driver=null-co,cache=none
+run_qemu -drive driver=null-co,cache=directsync
+run_qemu -drive driver=null-co,cache=writeback
+run_qemu -drive driver=null-co,cache=writethrough
+run_qemu -drive driver=null-co,cache=unsafe
+run_qemu -drive driver=null-co,cache=invalid_value
 
 echo
 echo === Specifying the protocol layer ===
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 7765aa0..7a459a3 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -215,28 +215,28 @@ QEMU X.Y.Z monitor - type 'help' for more information
 
 === Cache modes ===
 
-Testing: -drive media=cdrom,cache=none
+Testing: -drive driver=null-co,cache=none
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
-Testing: -drive media=cdrom,cache=directsync
+Testing: -drive driver=null-co,cache=directsync
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
-Testing: -drive media=cdrom,cache=writeback
+Testing: -drive driver=null-co,cache=writeback
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
-Testing: -drive media=cdrom,cache=writethrough
+Testing: -drive driver=null-co,cache=writethrough
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
-Testing: -drive media=cdrom,cache=unsafe
+Testing: -drive driver=null-co,cache=unsafe
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
-Testing: -drive media=cdrom,cache=invalid_value
-QEMU_PROG: -drive media=cdrom,cache=invalid_value: invalid cache option
+Testing: -drive driver=null-co,cache=invalid_value
+QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
 
 
 === Specifying the protocol layer ===
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 27eddec..0a238ec 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -148,12 +148,12 @@ class VM(object):
     def add_drive(self, path, opts='', interface='virtio'):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=%s' % interface,
-                   'cache=%s' % cachemode,
                    'id=drive%d' % self._num_drives]
 
         if path is not None:
             options.append('file=%s' % path)
             options.append('format=%s' % imgfmt)
+            options.append('cache=%s' % cachemode)
 
         if opts:
             options.append(opts)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 16/21] block: reopen: Extract QemuOpts for generic block layer options

[Kevin Wolf]

This patch adds a QemuOpts for generic block layer options to
bdrv_reopen_prepare(). The only two options that currently exist
(node-name and driver) cannot be changed, so the only thing we do is
putting them right back into the QDict so that we check at the end that
they are indeed unchanged.

We will add new options soon that can actually be changed.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/block.c b/block.c
index 2dd2830..07763ff 100644
--- a/block.c
+++ b/block.c
@@ -1907,11 +1907,34 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
     int ret = -1;
     Error *local_err = NULL;
     BlockDriver *drv;
+    QemuOpts *opts;
+    const char *value;
 
     assert(reopen_state != NULL);
     assert(reopen_state->bs->drv != NULL);
     drv = reopen_state->bs->drv;
 
+    /* Process generic block layer options */
+    opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, reopen_state->options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto error;
+    }
+
+    /* node-name and driver must be unchanged. Put them back into the QDict, so
+     * that they are checked at the end of this function. */
+    value = qemu_opt_get(opts, "node-name");
+    if (value) {
+        qdict_put(reopen_state->options, "node-name", qstring_from_str(value));
+    }
+
+    value = qemu_opt_get(opts, "driver");
+    if (value) {
+        qdict_put(reopen_state->options, "driver", qstring_from_str(value));
+    }
+
     /* if we are to stay read-only, do not allow permission change
      * to r/w */
     if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
@@ -1972,6 +1995,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
     ret = 0;
 
 error:
+    qemu_opts_del(opts);
     return ret;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 17/21] block: Move cache options into options QDict

[Kevin Wolf]

This adds the cache mode options to the QDict, so that they can be
specified for child nodes (e.g. backing.cache.direct=off).

The cache modes are not removed from the flags at this point; instead,
options and flags are kept in sync. If the user specifies both flags and
options, the options take precedence.

Child node inherit cache modes as options now, they don't use flags any
more.

Note that this forbids specifying the cache mode for empty drives. It
didn't make sense anyway to specify it there, because it didn't have any
effect. blockdev_init() considers the cache options now bdrv_open()
options and therefore doesn't create an empty drive any more but calls
into bdrv_open(). This in turn will fail with no driver and filename
specified.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c    | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 blockdev.c |  52 ++++++++++----------------------
 2 files changed, 111 insertions(+), 41 deletions(-)

diff --git a/block.c b/block.c
index 07763ff..99db955 100644
--- a/block.c
+++ b/block.c
@@ -29,6 +29,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qjson.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/sysemu.h"
@@ -706,9 +707,16 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
     /* Enable protocol handling, disable format probing for bs->file */
     flags |= BDRV_O_PROTOCOL;
 
+    /* If the cache mode isn't explicitly set, inherit direct and no-flush from
+     * the parent. */
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+
     /* Our block drivers take care to send flushes and respect unmap policy,
-     * so we can enable both unconditionally on lower layers. */
-    flags |= BDRV_O_CACHE_WB | BDRV_O_UNMAP;
+     * so we can default to enable both on lower layers regardless of the
+     * corresponding parent options. */
+    qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
+    flags |= BDRV_O_UNMAP;
 
     /* Clear flags that only apply to the top layer */
     flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
@@ -747,6 +755,11 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
 {
     int flags = parent_flags;
 
+    /* The cache mode is inherited unmodified for backing files */
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_WB);
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+
     /* backing files always opened read-only */
     flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
 
@@ -780,6 +793,42 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
     return open_flags;
 }
 
+static void update_flags_from_options(int *flags, QemuOpts *opts)
+{
+    *flags &= ~BDRV_O_CACHE_MASK;
+
+    assert(qemu_opt_find(opts, BDRV_OPT_CACHE_WB));
+    if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, false)) {
+        *flags |= BDRV_O_CACHE_WB;
+    }
+
+    assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH));
+    if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
+        *flags |= BDRV_O_NO_FLUSH;
+    }
+
+    assert(qemu_opt_find(opts, BDRV_OPT_CACHE_DIRECT));
+    if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) {
+        *flags |= BDRV_O_NOCACHE;
+    }
+}
+
+static void update_options_from_flags(QDict *options, int flags)
+{
+    if (!qdict_haskey(options, BDRV_OPT_CACHE_WB)) {
+        qdict_put(options, BDRV_OPT_CACHE_WB,
+                  qbool_from_bool(flags & BDRV_O_CACHE_WB));
+    }
+    if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
+        qdict_put(options, BDRV_OPT_CACHE_DIRECT,
+                  qbool_from_bool(flags & BDRV_O_NOCACHE));
+    }
+    if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
+        qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
+                  qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+    }
+}
+
 static void bdrv_assign_node_name(BlockDriverState *bs,
                                   const char *node_name,
                                   Error **errp)
@@ -831,6 +880,21 @@ static QemuOptsList bdrv_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "Block driver to use for the node",
         },
+        {
+            .name = BDRV_OPT_CACHE_WB,
+            .type = QEMU_OPT_BOOL,
+            .help = "Enable writeback mode",
+        },
+        {
+            .name = BDRV_OPT_CACHE_DIRECT,
+            .type = QEMU_OPT_BOOL,
+            .help = "Bypass software writeback cache on the host",
+        },
+        {
+            .name = BDRV_OPT_CACHE_NO_FLUSH,
+            .type = QEMU_OPT_BOOL,
+            .help = "Ignore flush requests",
+        },
         { /* end of list */ }
     },
 };
@@ -925,7 +989,9 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     bs->drv = drv;
     bs->opaque = g_malloc0(drv->instance_size);
 
-    bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
+    /* Apply cache mode options */
+    update_flags_from_options(&bs->open_flags, opts);
+    bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
 
     /* Open the image, either directly or using a protocol */
     if (drv->bdrv_file_open) {
@@ -1075,6 +1141,9 @@ static int bdrv_fill_options(QDict **options, const char *filename,
         *flags &= ~BDRV_O_PROTOCOL;
     }
 
+    /* Translate cache options from flags into options */
+    update_options_from_flags(*options, *flags);
+
     /* Fetch the file name from the options QDict if necessary */
     if (protocol && filename) {
         if (!qdict_haskey(*options, "filename")) {
@@ -1747,12 +1816,22 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
     /*
      * Precedence of options:
      * 1. Explicitly passed in options (highest)
-     * 2. TODO Set in flags (only for top level)
+     * 2. Set in flags (only for top level)
      * 3. Retained from explicitly set options of bs
      * 4. Inherited from parent node
      * 5. Retained from effective options of bs
      */
 
+    if (!parent_options) {
+        /*
+         * Any setting represented by flags is always updated. If the
+         * corresponding QDict option is set, it takes precedence. Otherwise
+         * the flag is translated into a QDict option. The old setting of bs is
+         * not considered.
+         */
+        update_options_from_flags(options, flags);
+    }
+
     /* Old explicitly set values (don't overwrite by inherited value) */
     old_options = qdict_clone_shallow(bs->explicit_options);
     bdrv_join_options(bs, options, old_options);
@@ -1923,6 +2002,19 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
         goto error;
     }
 
+    update_flags_from_options(&reopen_state->flags, opts);
+
+    /* If a guest device is attached, it owns WCE */
+    if (reopen_state->bs->blk && blk_get_attached_dev(reopen_state->bs->blk)) {
+        bool old_wce = bdrv_enable_write_cache(reopen_state->bs);
+        bool new_wce = (reopen_state->flags & BDRV_O_CACHE_WB);
+        if (old_wce != new_wce) {
+            error_setg(errp, "Cannot change cache.writeback: Device attached");
+            ret = -EINVAL;
+            goto error;
+        }
+    }
+
     /* node-name and driver must be unchanged. Put them back into the QDict, so
      * that they are checked at the end of this function. */
     value = qemu_opt_get(opts, "node-name");
diff --git a/blockdev.c b/blockdev.c
index afaeef9..2b076fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -387,16 +387,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
             }
         }
 
-        if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true)) {
-            *bdrv_flags |= BDRV_O_CACHE_WB;
-        }
-        if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) {
-            *bdrv_flags |= BDRV_O_NOCACHE;
-        }
-        if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
-            *bdrv_flags |= BDRV_O_NO_FLUSH;
-        }
-
         if ((aio = qemu_opt_get(opts, "aio")) != NULL) {
             if (!strcmp(aio, "native")) {
                 *bdrv_flags |= BDRV_O_NATIVE_AIO;
@@ -569,9 +559,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
     }
 
     if (snapshot) {
-        /* always use cache=unsafe with snapshot */
-        bdrv_flags &= ~BDRV_O_CACHE_MASK;
-        bdrv_flags |= (BDRV_O_SNAPSHOT|BDRV_O_CACHE_WB|BDRV_O_NO_FLUSH);
+        bdrv_flags |= BDRV_O_SNAPSHOT;
     }
 
     /* init */
@@ -603,6 +591,20 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
             file = NULL;
         }
 
+        /* bdrv_open() defaults to the values in bdrv_flags (for compatibility
+         * with other callers) rather than what we want as the real defaults.
+         * Apply the defaults here instead. */
+        qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, "on");
+        qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
+        qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
+
+        if (snapshot) {
+            /* always use cache=unsafe with snapshot */
+            qdict_put(bs_opts, BDRV_OPT_CACHE_WB, qstring_from_str("on"));
+            qdict_put(bs_opts, BDRV_OPT_CACHE_DIRECT, qstring_from_str("off"));
+            qdict_put(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, qstring_from_str("on"));
+        }
+
         blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags,
                            errp);
         if (!blk) {
@@ -3870,18 +3872,6 @@ QemuOptsList qemu_common_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "discard operation (ignore/off, unmap/on)",
         },{
-            .name = BDRV_OPT_CACHE_WB,
-            .type = QEMU_OPT_BOOL,
-            .help = "enables writeback mode for any caches",
-        },{
-            .name = BDRV_OPT_CACHE_DIRECT,
-            .type = QEMU_OPT_BOOL,
-            .help = "enables use of O_DIRECT (bypass the host page cache)",
-        },{
-            .name = BDRV_OPT_CACHE_NO_FLUSH,
-            .type = QEMU_OPT_BOOL,
-            .help = "ignore any flush requests for the device",
-        },{
             .name = "aio",
             .type = QEMU_OPT_STRING,
             .help = "host AIO implementation (threads, native)",
@@ -3989,18 +3979,6 @@ static QemuOptsList qemu_root_bds_opts = {
             .type = QEMU_OPT_STRING,
             .help = "discard operation (ignore/off, unmap/on)",
         },{
-            .name = "cache.writeback",
-            .type = QEMU_OPT_BOOL,
-            .help = "enables writeback mode for any caches",
-        },{
-            .name = "cache.direct",
-            .type = QEMU_OPT_BOOL,
-            .help = "enables use of O_DIRECT (bypass the host page cache)",
-        },{
-            .name = "cache.no-flush",
-            .type = QEMU_OPT_BOOL,
-            .help = "ignore any flush requests for the device",
-        },{
             .name = "aio",
             .type = QEMU_OPT_STRING,
             .help = "host AIO implementation (threads, native)",
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 18/21] blkdebug: Enable reopen

[Kevin Wolf]

Just reopening the children (as block.c does now) is enough.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block/blkdebug.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 48b0812..7132e2c 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -775,6 +775,12 @@ static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
     bs->full_open_options = opts;
 }
 
+static int blkdebug_reopen_prepare(BDRVReopenState *reopen_state,
+                                   BlockReopenQueue *queue, Error **errp)
+{
+    return 0;
+}
+
 static BlockDriver bdrv_blkdebug = {
     .format_name            = "blkdebug",
     .protocol_name          = "blkdebug",
@@ -783,6 +789,7 @@ static BlockDriver bdrv_blkdebug = {
     .bdrv_parse_filename    = blkdebug_parse_filename,
     .bdrv_file_open         = blkdebug_open,
     .bdrv_close             = blkdebug_close,
+    .bdrv_reopen_prepare    = blkdebug_reopen_prepare,
     .bdrv_getlength         = blkdebug_getlength,
     .bdrv_truncate          = blkdebug_truncate,
     .bdrv_refresh_filename  = blkdebug_refresh_filename,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 19/21] qemu-iotests: Try setting cache mode for children

[Kevin Wolf]

This is a basic test for specifying cache modes for child nodes on the
command line. It doesn't take much time and works without O_DIRECT
support.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/051     | 10 +++++++-
 tests/qemu-iotests/051.out | 60 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index f6f0f4d..da90f59 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -61,7 +61,7 @@ function do_run_qemu()
 
 function run_qemu()
 {
-    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_generated_node_ids
 }
 
 size=128M
@@ -190,6 +190,14 @@ run_qemu -drive driver=null-co,cache=writethrough
 run_qemu -drive driver=null-co,cache=unsafe
 run_qemu -drive driver=null-co,cache=invalid_value
 
+# Can't test direct=on here because O_DIRECT might not be supported on this FS
+# Test 142 checks the direct=on cases
+
+for cache in writeback writethrough unsafe invalid_value; do
+    echo -e "info block\ninfo block file\ninfo block backing\ninfo block backing-file" | \
+    run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file -nodefaults
+done
+
 echo
 echo === Specifying the protocol layer ===
 echo
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 7a459a3..070d318 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -238,6 +238,66 @@ QEMU X.Y.Z monitor - type 'help' for more information
 Testing: -drive driver=null-co,cache=invalid_value
 QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
 
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file -nodefaults
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
+ide0-hd0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+    Cache mode:       writeback
+    Backing file:     TEST_DIR/t.qcow2.base (chain depth: 1)
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block f^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block fi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block fil^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block file^[[K
+
+file: TEST_DIR/t.qcow2 (file)
+    Cache mode:       writeback
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block b^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ba^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block bac^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block back^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backin^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing^[[K
+backing: TEST_DIR/t.qcow2.base (qcow2, read-only)
+    Cache mode:       writeback, ignore flushes
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block b^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ba^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block bac^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block back^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backin^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-f^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-fi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D!
 ^[[D^[[Dinfo block backing-fil^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-file^[[K
+
+backing-file: TEST_DIR/t.qcow2.base (file, read-only)
+    Cache mode:       writeback, ignore flushes
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file -nodefaults
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
+ide0-hd0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+    Cache mode:       writethrough
+    Backing file:     TEST_DIR/t.qcow2.base (chain depth: 1)
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block f^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block fi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block fil^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block file^[[K
+
+file: TEST_DIR/t.qcow2 (file)
+    Cache mode:       writeback
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block b^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ba^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block bac^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block back^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backin^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing^[[K
+backing: TEST_DIR/t.qcow2.base (qcow2, read-only)
+    Cache mode:       writeback, ignore flushes
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block b^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ba^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block bac^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block back^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backin^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-f^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-fi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D!
 ^[[D^[[Dinfo block backing-fil^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-file^[[K
+
+backing-file: TEST_DIR/t.qcow2.base (file, read-only)
+    Cache mode:       writeback, ignore flushes
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file -nodefaults
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
+ide0-hd0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+    Cache mode:       writeback, ignore flushes
+    Backing file:     TEST_DIR/t.qcow2.base (chain depth: 1)
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block f^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block fi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block fil^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block file^[[K
+
+file: TEST_DIR/t.qcow2 (file)
+    Cache mode:       writeback, ignore flushes
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block b^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ba^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block bac^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block back^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backin^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing^[[K
+backing: TEST_DIR/t.qcow2.base (qcow2, read-only)
+    Cache mode:       writeback, ignore flushes
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block b^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block ba^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block bac^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block back^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backin^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-f^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-fi^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D!
 ^[[D^[[Dinfo block backing-fil^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block backing-file^[[K
+
+backing-file: TEST_DIR/t.qcow2.base (file, read-only)
+    Cache mode:       writeback, ignore flushes
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file -nodefaults
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file: invalid cache option
+
 
 === Specifying the protocol layer ===
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 20/21] qemu-iotests: Test cache mode option inheritance

[Kevin Wolf]

This is doing a more complete test on setting cache modes both while
opening an image (i.e. in a -drive command line) and in reopen
situations. It checks that reopen can specify options for child nodes
and that cache modes are correctly inherited from parent nodes where
they are not specified.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/142     | 354 +++++++++++++++++++++
 tests/qemu-iotests/142.out | 773 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 1128 insertions(+)
 create mode 100755 tests/qemu-iotests/142
 create mode 100644 tests/qemu-iotests/142.out

diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
new file mode 100755
index 0000000..8aa50f8
--- /dev/null
+++ b/tests/qemu-iotests/142
@@ -0,0 +1,354 @@
+#!/bin/bash
+#
+# Test for configuring cache modes of arbitrary nodes (requires O_DIRECT)
+#
+# Copyright (C) 2015 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=kwolf@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
+    rm -f $TEST_IMG.snap
+}
+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
+
+# We test all cache modes anyway, but O_DIRECT needs to be supported
+_default_cache_mode none
+_supported_cache_modes none directsync
+
+function do_run_qemu()
+{
+    echo Testing: "$@"
+    (
+        if ! test -t 0; then
+            while read cmd; do
+                echo $cmd
+            done
+        fi
+        echo quit
+    ) | $QEMU -nographic -monitor stdio -nodefaults "$@"
+    echo
+}
+
+function run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu
+}
+
+size=128M
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+TEST_IMG="$TEST_IMG.snap" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base" $size
+
+echo
+echo === Simple test for all cache modes ===
+echo
+
+run_qemu -drive file="$TEST_IMG",cache=none
+run_qemu -drive file="$TEST_IMG",cache=directsync
+run_qemu -drive file="$TEST_IMG",cache=writeback
+run_qemu -drive file="$TEST_IMG",cache=writethrough
+run_qemu -drive file="$TEST_IMG",cache=unsafe
+run_qemu -drive file="$TEST_IMG",cache=invalid_value
+
+echo
+echo === Check inheritance of cache modes ===
+echo
+
+files="if=none,file=$TEST_IMG,backing.file.filename=$TEST_IMG.base"
+ids="node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file"
+
+function check_cache_all()
+{
+    # cache.direct is supposed to be inherited by both bs->file and
+    # bs->backing
+
+    echo -e "cache.direct=on on none0"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.direct=on | grep "Cache"
+    echo -e "\ncache.direct=on on file"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.direct=on | grep "Cache"
+    echo -e "\ncache.direct=on on backing"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.direct=on | grep "Cache"
+    echo -e "\ncache.direct=on on backing-file"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.direct=on | grep "Cache"
+
+    # cache.writeback is supposed to be inherited by bs->backing; bs->file
+    # always gets cache.writeback=on
+
+    echo -e "\n\ncache.writeback=off on none0"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | grep "Cache"
+    echo -e "\ncache.writeback=off on file"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep "Cache"
+    echo -e "\ncache.writeback=off on backing"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep "Cache"
+    echo -e "\ncache.writeback=off on backing-file"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep "Cache"
+
+    # cache.no-flush is supposed to be inherited by both bs->file and bs->backing
+
+    echo -e "\n\ncache.no-flush=on on none0"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.no-flush=on | grep "Cache"
+    echo -e "\ncache.no-flush=on on file"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.no-flush=on | grep "Cache"
+    echo -e "\ncache.no-flush=on on backing"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.no-flush=on | grep "Cache"
+    echo -e "\ncache.no-flush=on on backing-file"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.no-flush=on | grep "Cache"
+}
+
+echo
+echo "--- Configure cache modes on the command line ---"
+echo
+
+# First check the inherited cache mode after opening the image.
+
+hmp_cmds="info block image
+info block file
+info block backing
+info block backing-file"
+
+check_cache_all
+
+echo
+echo "--- Cache modes after reopen (live snapshot) ---"
+echo
+
+# Then trigger a reopen and check that the cache modes are still the same.
+
+hmp_cmds="snapshot_blkdev -n none0 $TEST_IMG.snap $IMGFMT
+info block
+info block image
+info block file
+info block backing
+info block backing-file"
+
+check_cache_all
+
+echo
+echo "--- Change cache modes with reopen (qemu-io command, flags) ---"
+echo
+
+# This one actually changes the cache mode with the reopen. For this test, the
+# new cache mode is specified in the flags, not as an option.
+
+hmp_cmds='qemu-io none0 "reopen -c none"
+info block image
+info block file
+info block backing
+info block backing-file'
+
+check_cache_all
+
+echo
+echo "--- Change cache modes with reopen (qemu-io command, options) ---"
+echo
+
+# This one actually changes the cache mode with the reopen. For this test, the
+# new cache mode is specified as an option, not in the flags.
+
+hmp_cmds='qemu-io none0 "reopen -o cache.direct=on"
+info block image
+info block file
+info block backing
+info block backing-file'
+
+check_cache_all
+
+echo
+echo "--- Change cache modes after snapshot ---"
+echo
+
+# This checks that the original image doesn't inherit from the snapshot
+
+hmp_cmds="snapshot_blkdev -n none0 $TEST_IMG.snap $IMGFMT
+qemu-io none0 \"reopen -c none\"
+info block none0
+info block image
+info block file
+info block backing
+info block backing-file"
+
+check_cache_all
+
+echo
+echo "--- Change cache mode in parent, child has explicit option in JSON ---"
+echo
+
+# This checks that children with options explicitly set by the json:
+# pseudo-protocol don't inherit these options from their parents.
+#
+# Yes, blkdebug::json:... is criminal, but I can't see another way to have a
+# BDS initialised with the json: pseudo-protocol, but still have it inherit
+# options from its parent node.
+
+hmp_cmds="qemu-io none0 \"reopen -o cache.writeback=off,cache.direct=on,cache.no-flush=on\"
+info block image
+info block blkdebug
+info block file"
+
+echo "$hmp_cmds" | run_qemu -drive if=none,file="blkdebug::json:{\"filename\":\"$TEST_IMG\",,\"cache\":{\"writeback\":false,,\"direct\":false}}",node-name=image,file.node-name=blkdebug,file.image.node-name=file | grep "Cache"
+
+echo
+echo "=== Check that referenced BDSes don't inherit ==="
+echo
+
+drv_bkfile="if=none,driver=file,filename=$TEST_IMG.base,node-name=backing-file"
+drv_bk="if=none,file=json:{'driver':'$IMGFMT',,'file':'backing-file',,'node-name':'backing'}"
+drv_file="if=none,driver=file,filename=$TEST_IMG,node-name=file"
+drv_img="if=none,id=blk,file=json:{'driver':'$IMGFMT',,'file':'file',,'backing':'backing',,'node-name':'image'}"
+
+function check_cache_all_separate()
+{
+    # Check cache.direct
+
+    echo -e "cache.direct=on on blk"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.direct=on | grep "Cache"
+    echo -e "\ncache.direct=on on file"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.direct=on -drive "$drv_img" | grep "Cache"
+    echo -e "\ncache.direct=on on backing"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.direct=on -drive "$drv_file" -drive "$drv_img" | grep "Cache"
+    echo -e "\ncache.direct=on on backing-file"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.direct=on -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep "Cache"
+
+    # Check cache.writeback
+
+    echo -e "\n\ncache.writeback=off on blk"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.writeback=off | grep "Cache"
+    echo -e "\ncache.writeback=off on file"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.writeback=off -drive "$drv_img" | grep "Cache"
+    echo -e "\ncache.writeback=off on backing"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.writeback=off -drive "$drv_file" -drive "$drv_img" | grep "Cache"
+    echo -e "\ncache.writeback=off on backing-file"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.writeback=off -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep "Cache"
+
+    # Check cache.no-flush
+
+    echo -e "\n\ncache.no-flush=on on blk"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.no-flush=on | grep "Cache"
+    echo -e "\ncache.no-flush=on on file"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.no-flush=on -drive "$drv_img" | grep "Cache"
+    echo -e "\ncache.no-flush=on on backing"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.no-flush=on -drive "$drv_file" -drive "$drv_img" | grep "Cache"
+    echo -e "\ncache.no-flush=on on backing-file"
+    echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.no-flush=on -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep "Cache"
+}
+
+echo
+echo "--- Configure cache modes on the command line ---"
+echo
+
+# First check the inherited cache mode after opening the image.
+
+hmp_cmds="info block image
+info block file
+info block backing
+info block backing-file"
+
+check_cache_all_separate
+
+echo
+echo "--- Cache modes after reopen (live snapshot) ---"
+echo
+
+# Then trigger a reopen and check that the cache modes are still the same.
+
+hmp_cmds="snapshot_blkdev -n blk $TEST_IMG.snap $IMGFMT
+info block blk
+info block image
+info block file
+info block backing
+info block backing-file"
+
+check_cache_all_separate
+
+echo
+echo "--- Change cache modes with reopen (qemu-io command, flags) ---"
+echo
+
+# This one actually changes the cache mode with the reopen. For this test, the
+# new cache mode is specified as flags, not as option.
+
+hmp_cmds='qemu-io blk "reopen -c none"
+info block image
+info block file
+info block backing
+info block backing-file'
+
+check_cache_all_separate
+
+
+echo
+echo "=== Reopening children instead of the root ==="
+echo
+
+files="if=none,file=$TEST_IMG,backing.file.filename=$TEST_IMG.base"
+ids="node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file"
+
+echo
+echo "--- Basic reopen ---"
+echo
+
+hmp_cmds='qemu-io none0 "reopen -o backing.cache.direct=on"
+info block image
+info block file
+info block backing
+info block backing-file'
+
+check_cache_all
+
+echo
+echo "--- Change cache mode after reopening child ---"
+echo
+
+# This checks that children with options explicitly set with reopen don't
+# inherit these options from their parents any more
+
+# TODO Implement node-name support for 'qemu-io' HMP command for -c
+# Can use only -o to access child node options for now
+
+hmp_cmds="qemu-io none0 \"reopen -o file.cache.writeback=off,file.cache.direct=off,file.cache.no-flush=off\"
+qemu-io none0 \"reopen -o backing.file.cache.writeback=on,backing.file.cache.direct=off,backing.file.cache.no-flush=on\"
+qemu-io none0 \"reopen -c none\"
+info block image
+info block file
+info block backing
+info block backing-file"
+
+echo "$hmp_cmds" | run_qemu -drive "$files","$ids" | grep "Cache"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out
new file mode 100644
index 0000000..b555d5a
--- /dev/null
+++ b/tests/qemu-iotests/142.out
@@ -0,0 +1,773 @@
+QA output created by 142
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.snap', fmt=IMGFMT size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+
+=== Simple test for all cache modes ===
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=directsync
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value: invalid cache option
+
+
+=== Check inheritance of cache modes ===
+
+
+--- Configure cache modes on the command line ---
+
+cache.direct=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on none0
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on file
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+
+
+cache.no-flush=on on none0
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+
+cache.no-flush=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+
+--- Cache modes after reopen (live snapshot) ---
+
+cache.direct=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on none0
+    Cache mode:       writethrough
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+
+
+cache.no-flush=on on none0
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+
+cache.no-flush=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+
+--- Change cache modes with reopen (qemu-io command, flags) ---
+
+cache.direct=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on file
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+
+
+cache.no-flush=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.no-flush=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.no-flush=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+
+--- Change cache modes with reopen (qemu-io command, options) ---
+
+cache.direct=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on none0
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on file
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+
+
+cache.no-flush=on on none0
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+
+cache.no-flush=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.no-flush=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+
+--- Change cache modes after snapshot ---
+
+cache.direct=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+
+
+cache.no-flush=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+
+cache.no-flush=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+
+--- Change cache mode in parent, child has explicit option in JSON ---
+
+    Cache mode:       writethrough, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writethrough, ignore flushes
+
+=== Check that referenced BDSes don't inherit ===
+
+
+--- Configure cache modes on the command line ---
+
+cache.direct=on on blk
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+
+cache.direct=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on blk
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on file
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+
+
+cache.no-flush=on on blk
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+
+--- Cache modes after reopen (live snapshot) ---
+
+cache.direct=on on blk
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+
+cache.direct=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on blk
+    Cache mode:       writethrough
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+
+
+cache.no-flush=on on blk
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+
+--- Change cache modes with reopen (qemu-io command, flags) ---
+
+cache.direct=on on blk
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.direct=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+
+cache.direct=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on blk
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on file
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.writeback=off on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough
+
+
+cache.no-flush=on on blk
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+    Cache mode:       writeback
+
+cache.no-flush=on on backing
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback, direct
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+
+=== Reopening children instead of the root ===
+
+
+--- Basic reopen ---
+
+cache.direct=on on none0
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.direct=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+
+cache.writeback=off on none0
+    Cache mode:       writethrough
+    Cache mode:       writeback
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on file
+    Cache mode:       writeback
+    Cache mode:       writethrough
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writethrough, direct
+    Cache mode:       writeback, direct
+
+cache.writeback=off on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough, direct
+
+
+cache.no-flush=on on none0
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+
+cache.no-flush=on on file
+    Cache mode:       writeback
+    Cache mode:       writeback, ignore flushes
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct
+
+cache.no-flush=on on backing
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct, ignore flushes
+    Cache mode:       writeback, direct, ignore flushes
+
+cache.no-flush=on on backing-file
+    Cache mode:       writeback
+    Cache mode:       writeback
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, direct, ignore flushes
+
+--- Change cache mode after reopening child ---
+
+    Cache mode:       writeback, direct
+    Cache mode:       writethrough
+    Cache mode:       writeback, direct
+    Cache mode:       writeback, ignore flushes
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5a08808..ed6ce09 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -140,3 +140,4 @@
 137 rw auto
 138 rw auto quick
 139 rw auto quick
+142 auto
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 21/21] qemu-iotests: Test reopen with node-name/driver options

[Kevin Wolf]

'node-name' and 'driver' should not be changed during a reopen
operation. It is, however, valid to specify them with the same value as
they already had.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/133     | 90 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/133.out | 22 ++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 113 insertions(+)
 create mode 100755 tests/qemu-iotests/133
 create mode 100644 tests/qemu-iotests/133.out

diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133
new file mode 100755
index 0000000..8587102
--- /dev/null
+++ b/tests/qemu-iotests/133
@@ -0,0 +1,90 @@
+#!/bin/bash
+#
+# Test for reopen
+#
+# Copyright (C) 2015 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=kwolf@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
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+_make_test_img -b "$TEST_IMG.base"
+
+echo
+echo "=== Check that node-name can't be changed ==="
+echo
+
+$QEMU_IO -c 'reopen -o node-name=foo' $TEST_IMG
+$QEMU_IO -c 'reopen -o file.node-name=foo' $TEST_IMG
+$QEMU_IO -c 'reopen -o backing.node-name=foo' $TEST_IMG
+
+echo
+echo "=== Check that unchanged node-name is okay ==="
+echo
+
+# Explicitly repeated
+$QEMU_IO -c "open -o node-name=foo $TEST_IMG" -c 'reopen -o node-name=foo'
+$QEMU_IO -c "open -o file.node-name=foo $TEST_IMG" -c 'reopen -o file.node-name=foo'
+$QEMU_IO -c "open -o backing.node-name=foo $TEST_IMG" -c 'reopen -o backing.node-name=foo'
+
+# Implicitly retained
+$QEMU_IO -c "open -o node-name=foo $TEST_IMG" -c 'reopen'
+$QEMU_IO -c "open -o file.node-name=foo $TEST_IMG" -c 'reopen'
+$QEMU_IO -c "open -o backing.node-name=foo $TEST_IMG" -c 'reopen'
+
+echo
+echo "=== Check that driver can't be changed ==="
+echo
+
+$QEMU_IO -c 'reopen -o driver=raw' $TEST_IMG
+$QEMU_IO -c 'reopen -o file.driver=qcow2' $TEST_IMG
+$QEMU_IO -c 'reopen -o backing.driver=file' $TEST_IMG
+
+echo
+echo "=== Check that unchanged driver is okay ==="
+echo
+
+# Explicitly repeated (implicit case is covered in node-name test)
+$QEMU_IO -c 'reopen -o driver=qcow2' $TEST_IMG
+$QEMU_IO -c 'reopen -o file.driver=file' $TEST_IMG
+$QEMU_IO -c 'reopen -o backing.driver=qcow2' $TEST_IMG
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
new file mode 100644
index 0000000..cc86b94
--- /dev/null
+++ b/tests/qemu-iotests/133.out
@@ -0,0 +1,22 @@
+QA output created by 133
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
+
+=== Check that node-name can't be changed ===
+
+Cannot change the option 'node-name'
+Cannot change the option 'node-name'
+Cannot change the option 'node-name'
+
+=== Check that unchanged node-name is okay ===
+
+
+=== Check that driver can't be changed ===
+
+Cannot change the option 'driver'
+Cannot change the option 'driver'
+Cannot change the option 'driver'
+
+=== Check that unchanged driver is okay ===
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ed6ce09..d6e9219 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -134,6 +134,7 @@
 130 rw auto quick
 131 rw auto quick
 132 rw auto quick
+133 auto quick
 134 rw auto quick
 135 rw auto
 136 rw auto
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 06/21] block: Exclude nested options only for children in append_open_options()

[Max Reitz]

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

On 04.12.2015 14:35, Kevin Wolf wrote:
> Some drivers have nested options (e.g. blkdebug rule arrays), which
> don't belong to a child node and shouldn't be removed. Don't remove all
> options with "." in their name, but check for the complete prefixes of
> actually existing child nodes.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                   | 20 ++++++++++++++++----
>  include/block/block_int.h |  1 +
>  2 files changed, 17 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 14/21] blockdev: Set 'format' indicates non-empty drive

[Max Reitz]

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

On 04.12.2015 14:35, Kevin Wolf wrote:
> Creating an empty drive while specifying 'format' doesn't make sense.
> The specified format driver would simply be ignored.
> 
> Make a set 'format' option an indication that a non-empty drive should
> be created. This makes 'format' consistent with 'driver' and allows
> using it with a block driver that doesn't need any other options (like
> null-co/null-aio).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c                    | 5 +----
>  tests/hd-geo-test.c           | 4 ++--
>  tests/qemu-iotests/iotests.py | 2 +-
>  3 files changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 17/21] block: Move cache options into options QDict

[Max Reitz]

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

On 04.12.2015 14:35, Kevin Wolf wrote:
> This adds the cache mode options to the QDict, so that they can be
> specified for child nodes (e.g. backing.cache.direct=off).
> 
> The cache modes are not removed from the flags at this point; instead,
> options and flags are kept in sync. If the user specifies both flags and
> options, the options take precedence.
> 
> Child node inherit cache modes as options now, they don't use flags any
> more.
> 
> Note that this forbids specifying the cache mode for empty drives. It
> didn't make sense anyway to specify it there, because it didn't have any
> effect. blockdev_init() considers the cache options now bdrv_open()
> options and therefore doesn't create an empty drive any more but calls
> into bdrv_open(). This in turn will fail with no driver and filename
> specified.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c    | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  blockdev.c |  52 ++++++++++----------------------
>  2 files changed, 111 insertions(+), 41 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 00/21] block: Cache mode for children etc.

[Kevin Wolf]

Am 04.12.2015 um 14:35 hat Kevin Wolf geschrieben:
> This is part three (or four, depending on whether you count the bdrv_swap
> removal) of what I had sent earlier as "[PATCH 00/34] block: Cache mode for
> children, reopen overhaul and more". Most of the patches were actually already
> reviewed in v1.
> 
> This part contains the remaining functional changes that the cover letter for
> v1 advertised, and a bit more:
> 
> - You can now use node name references for backing files
> - bdrv_reopen() works now properly for inherited options (don't exist before
>   this series; after the series the cache options)
> - bdrv_reopen() works now properly with semantically overlapping options
> - bdrv_reopen() can change child node options
> - And finally you can set cache mode options for backing files and other
>   children now (and the reopen behaviour even makes sense

Applied to block-next.

Kevin

Re: [Qemu-devel] [PATCH v3 06/21] block: Exclude nested options only for children in append_open_options()

[Wen Congyang]

On 12/04/2015 09:35 PM, Kevin Wolf wrote:
> Some drivers have nested options (e.g. blkdebug rule arrays), which
> don't belong to a child node and shouldn't be removed. Don't remove all
> options with "." in their name, but check for the complete prefixes of
> actually existing child nodes.

I think we should have some way to get the child->name. For example, the
monitor command 'info block' or 'query-block' display it.

Thanks
Wen Congyang

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                   | 20 ++++++++++++++++----
>  include/block/block_int.h |  1 +
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 73f0816..0dfff7a 100644
> --- a/block.c
> +++ b/block.c
> @@ -1101,11 +1101,13 @@ static int bdrv_fill_options(QDict **options, const char **pfilename,
>  
>  static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
>                                      BlockDriverState *child_bs,
> +                                    const char *child_name,
>                                      const BdrvChildRole *child_role)
>  {
>      BdrvChild *child = g_new(BdrvChild, 1);
>      *child = (BdrvChild) {
>          .bs     = child_bs,
> +        .name   = g_strdup(child_name),
>          .role   = child_role,
>      };
>  
> @@ -1119,6 +1121,7 @@ static void bdrv_detach_child(BdrvChild *child)
>  {
>      QLIST_REMOVE(child, next);
>      QLIST_REMOVE(child, next_parent);
> +    g_free(child->name);
>      g_free(child);
>  }
>  
> @@ -1165,7 +1168,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
>          bs->backing = NULL;
>          goto out;
>      }
> -    bs->backing = bdrv_attach_child(bs, backing_hd, &child_backing);
> +    bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing);
>      bs->open_flags &= ~BDRV_O_NO_BACKING;
>      pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
>      pstrcpy(bs->backing_format, sizeof(bs->backing_format),
> @@ -1321,7 +1324,7 @@ BdrvChild *bdrv_open_child(const char *filename,
>          goto done;
>      }
>  
> -    c = bdrv_attach_child(parent, bs, child_role);
> +    c = bdrv_attach_child(parent, bs, bdref_key, child_role);
>  
>  done:
>      qdict_del(options, bdref_key);
> @@ -3951,13 +3954,22 @@ static bool append_open_options(QDict *d, BlockDriverState *bs)
>  {
>      const QDictEntry *entry;
>      QemuOptDesc *desc;
> +    BdrvChild *child;
>      bool found_any = false;
> +    const char *p;
>  
>      for (entry = qdict_first(bs->options); entry;
>           entry = qdict_next(bs->options, entry))
>      {
> -        /* Only take options for this level */
> -        if (strchr(qdict_entry_key(entry), '.')) {
> +        /* Exclude options for children */
> +        QLIST_FOREACH(child, &bs->children, next) {
> +            if (strstart(qdict_entry_key(entry), child->name, &p)
> +                && (!*p || *p == '.'))
> +            {
> +                break;
> +            }
> +        }
> +        if (child) {
>              continue;
>          }
>  
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 77dc165..7265247 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -351,6 +351,7 @@ extern const BdrvChildRole child_format;
>  
>  struct BdrvChild {
>      BlockDriverState *bs;
> +    char *name;
>      const BdrvChildRole *role;
>      QLIST_ENTRY(BdrvChild) next;
>      QLIST_ENTRY(BdrvChild) next_parent;
>