[Qemu-devel] [PATCH v2 0/7] Add "read-only" to the options QDict

[Qemu-devel] [PATCH v2 0/7] Add "read-only" to the options QDict

[Alberto Garcia]

This series adds "read-only" to the options QDict, fixing a
long-standing problem with the reopening code.

   [E] <- [D] <- [C] <- [B] <- [A]

In a normal scenario, the active layer [A] is in read-write mode and
everything else is read-only. If we reopen [D] in read-write mode and
later reopen [B], then [D] will become read-only when inheriting the
flags from its parent (see bdrv_backing_options()). With this series,
inheriting options doesn't need to override values that have been
explicitly set before.

The BDRV_O_RDWR flag is not removed yet, but its keep in sync with the
value of the "read-only" option. Removing the BDRV_O_RDWR flag is not
straightforward in all cases and can result in code that is
significantly slower and less readable. Therefore it will be dealt
with in the future.

Regards,

Berto

v2:
- Patch 2:
  - Update description [Kevin]
- Patch 4:
  - Inherit the value of "read-only" in bdrv_temp_snapshot_options().
    [Kevin]
  - Remove "read-only" from qemu_root_bds_opts and simply set a
    default value in bds_tree_init(). [Kevin]
- Patch 7:
  - Rebase after the changes made to qemu_root_bds_opts in patch 4.

v1: https://lists.gnu.org/archive/html/qemu-block/2016-09/msg00191.html
- Initial release

git backport-diff against v1:

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

001/7:[----] [--] 'block: Remove bdrv_is_snapshot'
002/7:[----] [--] 'block: Set BDRV_O_ALLOW_RDWR and snapshot_options before storing the flags'
003/7:[----] [--] 'block: Update bs->open_flags earlier in bdrv_open_common()'
004/7:[0013] [FC] 'block: Add "read-only" to the options QDict'
005/7:[----] [--] 'block: Don't queue the same BDS twice in bdrv_reopen_queue_child()'
006/7:[----] [--] 'commit: Add 'base' to the reopen queue before 'overlay_bs''
007/7:[0002] [FC] 'block: rename "read-only" to BDRV_OPT_READ_ONLY'

Alberto Garcia (7):
  block: Remove bdrv_is_snapshot
  block: Set BDRV_O_ALLOW_RDWR and snapshot_options before storing the
    flags
  block: Update bs->open_flags earlier in bdrv_open_common()
  block: Add "read-only" to the options QDict
  block: Don't queue the same BDS twice in bdrv_reopen_queue_child()
  commit: Add 'base' to the reopen queue before 'overlay_bs'
  block: rename "read-only" to BDRV_OPT_READ_ONLY

 block.c               | 89 +++++++++++++++++++++++++++++++++++++--------------
 block/commit.c        |  8 ++---
 block/vvfat.c         |  3 +-
 blockdev.c            | 26 +++++++--------
 include/block/block.h |  2 +-
 5 files changed, 84 insertions(+), 44 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/7] block: Remove bdrv_is_snapshot

[Alberto Garcia]

This is unnecessary and has been unused since 5433c24f0f9306c82ad9bcc.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c               | 5 -----
 include/block/block.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/block.c b/block.c
index 66ed1c0..1c75a6f 100644
--- a/block.c
+++ b/block.c
@@ -2965,11 +2965,6 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag)
     return false;
 }
 
-int bdrv_is_snapshot(BlockDriverState *bs)
-{
-    return !!(bs->open_flags & BDRV_O_SNAPSHOT);
-}
-
 /* backing_file can either be relative, or absolute, or a protocol.  If it is
  * relative, it must be relative to the chain.  So, passing in bs->filename
  * from a BDS as backing_file should not be done, as that may be relative to
diff --git a/include/block/block.h b/include/block/block.h
index 7edce5c..9f0399f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -420,7 +420,6 @@ void bdrv_get_full_backing_filename_from_filename(const char *backed,
                                                   const char *backing,
                                                   char *dest, size_t sz,
                                                   Error **errp);
-int bdrv_is_snapshot(BlockDriverState *bs);
 
 int path_has_protocol(const char *path);
 int path_is_absolute(const char *path);
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/7] block: Set BDRV_O_ALLOW_RDWR and snapshot_options before storing the flags

[Alberto Garcia]

If an image is opened with snapshot=on, its flags are modified by
bdrv_backing_options() and then bs->open_flags is updated accordingly.
This last step is unnecessary if we calculate the new flags before
setting bs->open_flags.

Soon we'll introduce the "read-only" option, and then we'll need to
be able to modify its value in the QDict when snapshot=on. This is
more cumbersome if bs->options is already set. This patch simplifies
that. Other than that, there are no semantic changes. Although it
might seem that bs->options can have a different value now because
it is stored after calling bdrv_backing_options(), this call doesn't
actually modify them in this scenario.

The code that sets BDRV_O_ALLOW_RDWR is also moved for the same
reason.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/block.c b/block.c
index 1c75a6f..7cae841 100644
--- a/block.c
+++ b/block.c
@@ -1627,6 +1627,17 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
         goto fail;
     }
 
+    if (flags & BDRV_O_RDWR) {
+        flags |= BDRV_O_ALLOW_RDWR;
+    }
+
+    if (flags & BDRV_O_SNAPSHOT) {
+        snapshot_options = qdict_new();
+        bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
+                                   flags, options);
+        bdrv_backing_options(&flags, options, flags, options);
+    }
+
     bs->open_flags = flags;
     bs->options = options;
     options = qdict_clone_shallow(options);
@@ -1651,18 +1662,6 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
 
     /* Open image file without format layer */
     if ((flags & BDRV_O_PROTOCOL) == 0) {
-        if (flags & BDRV_O_RDWR) {
-            flags |= BDRV_O_ALLOW_RDWR;
-        }
-        if (flags & BDRV_O_SNAPSHOT) {
-            snapshot_options = qdict_new();
-            bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
-                                       flags, options);
-            bdrv_backing_options(&flags, options, flags, options);
-        }
-
-        bs->open_flags = flags;
-
         file = bdrv_open_child(filename, options, "file", bs,
                                &child_file, true, &local_err);
         if (local_err) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 3/7] block: Update bs->open_flags earlier in bdrv_open_common()

[Alberto Garcia]

We're only doing this immediately before opening the image, but
bs->open_flags is used earlier in the function. At the moment this is
not causing problems because none of the checked flags are modified by
update_flags_from_options(), but this will change when we introduce
the "read-only" option.

This patch calls update_flags_from_options() at the beginning of the
function, immediately after creating the QemuOpts.

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

diff --git a/block.c b/block.c
index 7cae841..f56d703 100644
--- a/block.c
+++ b/block.c
@@ -913,6 +913,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
         goto fail_opts;
     }
 
+    update_flags_from_options(&bs->open_flags, opts);
+
     driver_name = qemu_opt_get(opts, "driver");
     drv = bdrv_find_format(driver_name);
     assert(drv != NULL);
@@ -974,9 +976,6 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     bs->drv = drv;
     bs->opaque = g_malloc0(drv->instance_size);
 
-    /* Apply cache mode options */
-    update_flags_from_options(&bs->open_flags, opts);
-
     /* Open the image, either directly or using a protocol */
     open_flags = bdrv_open_flags(bs, bs->open_flags);
     if (drv->bdrv_file_open) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 4/7] block: Add "read-only" to the options QDict

[Alberto Garcia]

This adds the "read-only" option to the QDict. One important effect of
this change is that when a child inherits options from its parent, the
existing "read-only" mode can be preserved if it was explicitly set
previously.

This addresses scenarios like this:

   [E] <- [D] <- [C] <- [B] <- [A]

In this case, if we reopen [D] with read-only=off, and later reopen
[B], then [D] will not inherit read-only=on from its parent during the
bdrv_reopen_queue_child() stage.

The BDRV_O_RDWR flag is not removed yet, but its keep in sync with the
value of the "read-only" option.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block.c               | 38 +++++++++++++++++++++++++++++++++++---
 block/vvfat.c         |  3 ++-
 blockdev.c            | 16 +++++++---------
 include/block/block.h |  1 +
 4 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index f56d703..663104e 100644
--- a/block.c
+++ b/block.c
@@ -685,6 +685,9 @@ static void bdrv_temp_snapshot_options(int *child_flags, QDict *child_options,
     qdict_set_default_str(child_options, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(child_options, BDRV_OPT_CACHE_NO_FLUSH, "on");
 
+    /* Copy the read-only option from the parent */
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);
+
     /* aio=native doesn't work for cache.direct=off, so disable it for the
      * temporary snapshot */
     *child_flags &= ~BDRV_O_NATIVE_AIO;
@@ -707,6 +710,9 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
 
+    /* Inherit the read-only option from the parent if it's not set */
+    qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);
+
     /* Our block drivers take care to send flushes and respect unmap policy,
      * so we can default to enable both on lower layers regardless of the
      * corresponding parent options. */
@@ -760,7 +766,8 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
     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);
+    qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "on");
+    flags &= ~BDRV_O_COPY_ON_READ;
 
     /* snapshot=on is handled on the top layer */
     flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_TEMPORARY);
@@ -807,6 +814,14 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
     if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) {
         *flags |= BDRV_O_NOCACHE;
     }
+
+    *flags &= ~BDRV_O_RDWR;
+
+    assert(qemu_opt_find(opts, BDRV_OPT_READ_ONLY));
+    if (!qemu_opt_get_bool(opts, BDRV_OPT_READ_ONLY, false)) {
+        *flags |= BDRV_O_RDWR;
+    }
+
 }
 
 static void update_options_from_flags(QDict *options, int flags)
@@ -819,6 +834,10 @@ static void update_options_from_flags(QDict *options, int flags)
         qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
                   qbool_from_bool(flags & BDRV_O_NO_FLUSH));
     }
+    if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
+        qdict_put(options, BDRV_OPT_READ_ONLY,
+                  qbool_from_bool(!(flags & BDRV_O_RDWR)));
+    }
 }
 
 static void bdrv_assign_node_name(BlockDriverState *bs,
@@ -882,6 +901,11 @@ static QemuOptsList bdrv_runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Ignore flush requests",
         },
+        {
+            .name = BDRV_OPT_READ_ONLY,
+            .type = QEMU_OPT_BOOL,
+            .help = "Node is opened in read-only mode",
+        },
         { /* end of list */ }
     },
 };
@@ -1626,14 +1650,22 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
         goto fail;
     }
 
-    if (flags & BDRV_O_RDWR) {
-        flags |= BDRV_O_ALLOW_RDWR;
+    /* Set the BDRV_O_RDWR and BDRV_O_ALLOW_RDWR flags.
+     * FIXME: we're parsing the QDict to avoid having to create a
+     * QemuOpts just for this, but neither option is optimal. */
+    if (g_strcmp0(qdict_get_try_str(options, BDRV_OPT_READ_ONLY), "on") &&
+        !qdict_get_try_bool(options, BDRV_OPT_READ_ONLY, false)) {
+        flags |= (BDRV_O_RDWR | BDRV_O_ALLOW_RDWR);
+    } else {
+        flags &= ~BDRV_O_RDWR;
     }
 
     if (flags & BDRV_O_SNAPSHOT) {
         snapshot_options = qdict_new();
         bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
                                    flags, options);
+        /* Let bdrv_backing_options() override "read-only" */
+        qdict_del(options, BDRV_OPT_READ_ONLY);
         bdrv_backing_options(&flags, options, flags, options);
     }
 
diff --git a/block/vvfat.c b/block/vvfat.c
index ba2620f..ded2109 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2971,7 +2971,8 @@ static BlockDriver vvfat_write_target = {
 static void vvfat_qcow_options(int *child_flags, QDict *child_options,
                                int parent_flags, QDict *parent_options)
 {
-    *child_flags = BDRV_O_RDWR | BDRV_O_NO_FLUSH;
+    qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "off");
+    *child_flags = BDRV_O_NO_FLUSH;
 }
 
 static const BdrvChildRole child_vvfat_qcow = {
diff --git a/blockdev.c b/blockdev.c
index 3010393..c3f8da4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -360,9 +360,6 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
     const char *aio;
 
     if (bdrv_flags) {
-        if (!qemu_opt_get_bool(opts, "read-only", false)) {
-            *bdrv_flags |= BDRV_O_RDWR;
-        }
         if (qemu_opt_get_bool(opts, "copy-on-read", false)) {
             *bdrv_flags |= BDRV_O_COPY_ON_READ;
         }
@@ -471,7 +468,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
     int bdrv_flags = 0;
     int on_read_error, on_write_error;
     bool account_invalid, account_failed;
-    bool writethrough;
+    bool writethrough, read_only;
     BlockBackend *blk;
     BlockDriverState *bs;
     ThrottleConfig cfg;
@@ -567,6 +564,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         bdrv_flags |= BDRV_O_SNAPSHOT;
     }
 
+    read_only = qemu_opt_get_bool(opts, BDRV_OPT_READ_ONLY, false);
+
     /* init */
     if ((!file || !*file) && !qdict_size(bs_opts)) {
         BlockBackendRootState *blk_rs;
@@ -574,7 +573,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         blk = blk_new();
         blk_rs = blk_get_root_state(blk);
         blk_rs->open_flags    = bdrv_flags;
-        blk_rs->read_only     = !(bdrv_flags & BDRV_O_RDWR);
+        blk_rs->read_only     = read_only;
         blk_rs->detect_zeroes = detect_zeroes;
 
         QDECREF(bs_opts);
@@ -588,6 +587,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
          * Apply the defaults here instead. */
         qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
         qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
+        qdict_set_default_str(bs_opts, BDRV_OPT_READ_ONLY,
+                              read_only ? "on" : "off");
         assert((bdrv_flags & BDRV_O_CACHE_MASK) == 0);
 
         if (runstate_check(RUN_STATE_INMIGRATE)) {
@@ -682,6 +683,7 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
      * Apply the defaults here instead. */
     qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
+    qdict_set_default_str(bs_opts, BDRV_OPT_READ_ONLY, "off");
 
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         bdrv_flags |= BDRV_O_INACTIVE;
@@ -4159,10 +4161,6 @@ static QemuOptsList qemu_root_bds_opts = {
             .type = QEMU_OPT_STRING,
             .help = "host AIO implementation (threads, native)",
         },{
-            .name = "read-only",
-            .type = QEMU_OPT_BOOL,
-            .help = "open drive file as read-only",
-        },{
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image file",
diff --git a/include/block/block.h b/include/block/block.h
index 9f0399f..57ae122 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -107,6 +107,7 @@ typedef struct HDGeometry {
 #define BDRV_OPT_CACHE_WB       "cache.writeback"
 #define BDRV_OPT_CACHE_DIRECT   "cache.direct"
 #define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"
+#define BDRV_OPT_READ_ONLY      "read-only"
 
 
 #define BDRV_SECTOR_BITS   9
-- 
2.9.3

[Qemu-devel] [PATCH v2 5/7] block: Don't queue the same BDS twice in bdrv_reopen_queue_child()

[Alberto Garcia]

bdrv_reopen_queue_child() assumes that a BlockDriverState is never
added twice to BlockReopenQueue.

That's however not the case: commit_start() adds 'base' (and its
children) to a new reopen queue, and then 'overlay_bs' (and its
children, which include 'base') to the same queue. The effect of this
is that the first set of options is ignored and overriden by the
second.

We fixed this by swapping the order in which both BDSs were added to
the queue in 3db2bd5508c86a1605258bc77c9672d93b5c350e. This patch
checks if a BDS is already in the reopen queue and keeps its options.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 663104e..1e9d66b 100644
--- a/block.c
+++ b/block.c
@@ -1877,6 +1877,13 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
         options = qdict_new();
     }
 
+    /* Check if this BlockDriverState is already in the queue */
+    QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+        if (bs == bs_entry->state.bs) {
+            break;
+        }
+    }
+
     /*
      * Precedence of options:
      * 1. Explicitly passed in options (highest)
@@ -1897,7 +1904,11 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
     }
 
     /* Old explicitly set values (don't overwrite by inherited value) */
-    old_options = qdict_clone_shallow(bs->explicit_options);
+    if (bs_entry) {
+        old_options = qdict_clone_shallow(bs_entry->state.explicit_options);
+    } else {
+        old_options = qdict_clone_shallow(bs->explicit_options);
+    }
     bdrv_join_options(bs, options, old_options);
     QDECREF(old_options);
 
@@ -1936,8 +1947,13 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
                                 child->role, options, flags);
     }
 
-    bs_entry = g_new0(BlockReopenQueueEntry, 1);
-    QSIMPLEQ_INSERT_TAIL(bs_queue, bs_entry, entry);
+    if (!bs_entry) {
+        bs_entry = g_new0(BlockReopenQueueEntry, 1);
+        QSIMPLEQ_INSERT_TAIL(bs_queue, bs_entry, entry);
+    } else {
+        QDECREF(bs_entry->state.options);
+        QDECREF(bs_entry->state.explicit_options);
+    }
 
     bs_entry->state.bs = bs;
     bs_entry->state.options = options;
-- 
2.9.3

[Qemu-devel] [PATCH v2 6/7] commit: Add 'base' to the reopen queue before 'overlay_bs'

[Alberto Garcia]

Now that we're checking for duplicates in the reopen queue, there's no
need to force a specific order in which the queue is constructed so we
can revert 3db2bd5508c86a1605258bc77c9672d93b5c350e.

Since both ways of constructing the queue are now valid, this patch
doesn't have any effect on the behavior of QEMU and is not strictly
necessary. However it can help us check that the fix for the reopen
queue is robust: if it stops working properly at some point, iotest
040 will break.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/commit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 553e18d..3ab5e0c 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -243,14 +243,14 @@ void commit_start(const char *job_id, BlockDriverState *bs,
     orig_overlay_flags = bdrv_get_flags(overlay_bs);
 
     /* convert base & overlay_bs to r/w, if necessary */
-    if (!(orig_overlay_flags & BDRV_O_RDWR)) {
-        reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL,
-                                         orig_overlay_flags | BDRV_O_RDWR);
-    }
     if (!(orig_base_flags & BDRV_O_RDWR)) {
         reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
                                          orig_base_flags | BDRV_O_RDWR);
     }
+    if (!(orig_overlay_flags & BDRV_O_RDWR)) {
+        reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs, NULL,
+                                         orig_overlay_flags | BDRV_O_RDWR);
+    }
     if (reopen_queue) {
         bdrv_reopen_multiple(reopen_queue, &local_err);
         if (local_err != NULL) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 7/7] block: rename "read-only" to BDRV_OPT_READ_ONLY

[Alberto Garcia]

There were a few instances left. After this patch we're using the
macro in all places.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index c3f8da4..fb207cd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -807,7 +807,7 @@ QemuOptsList qemu_legacy_drive_opts = {
 
         /* Options that are passed on, but have special semantics with -drive */
         {
-            .name = "read-only",
+            .name = BDRV_OPT_READ_ONLY,
             .type = QEMU_OPT_BOOL,
             .help = "open drive file as read-only",
         },{
@@ -873,7 +873,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
 
         { "group",          "throttling.group" },
 
-        { "readonly",       "read-only" },
+        { "readonly",       BDRV_OPT_READ_ONLY },
     };
 
     for (i = 0; i < ARRAY_SIZE(opt_renames); i++) {
@@ -945,7 +945,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     }
 
     /* copy-on-read is disabled with a warning for read-only devices */
-    read_only |= qemu_opt_get_bool(legacy_opts, "read-only", false);
+    read_only |= qemu_opt_get_bool(legacy_opts, BDRV_OPT_READ_ONLY, false);
     copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
 
     if (read_only && copy_on_read) {
@@ -953,7 +953,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
         copy_on_read = false;
     }
 
-    qdict_put(bs_opts, "read-only",
+    qdict_put(bs_opts, BDRV_OPT_READ_ONLY,
               qstring_from_str(read_only ? "on" : "off"));
     qdict_put(bs_opts, "copy-on-read",
               qstring_from_str(copy_on_read ? "on" :"off"));
@@ -4042,7 +4042,7 @@ QemuOptsList qemu_common_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "write error action",
         },{
-            .name = "read-only",
+            .name = BDRV_OPT_READ_ONLY,
             .type = QEMU_OPT_BOOL,
             .help = "open drive file as read-only",
         },{
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 0/7] Add "read-only" to the options QDict

[Kevin Wolf]

Am 15.09.2016 um 16:52 hat Alberto Garcia geschrieben:
> This series adds "read-only" to the options QDict, fixing a
> long-standing problem with the reopening code.
> 
>    [E] <- [D] <- [C] <- [B] <- [A]
> 
> In a normal scenario, the active layer [A] is in read-write mode and
> everything else is read-only. If we reopen [D] in read-write mode and
> later reopen [B], then [D] will become read-only when inheriting the
> flags from its parent (see bdrv_backing_options()). With this series,
> inheriting options doesn't need to override values that have been
> explicitly set before.
> 
> The BDRV_O_RDWR flag is not removed yet, but its keep in sync with the
> value of the "read-only" option. Removing the BDRV_O_RDWR flag is not
> straightforward in all cases and can result in code that is
> significantly slower and less readable. Therefore it will be dealt
> with in the future.

Thanks, applied to the block branch.

Kevin