[9/9] block: Allow changing 'force-share' on reopen

Commit Message

Alberto Garcia Aug. 26, 2018, 2:09 p.m. UTC

'force-share' is one of the basic BlockdevOptions available for all
drivers, but it's silently ignored by bdrv_reopen_prepare/commit(), so
the user cannot change it and doesn't get an error explaining that it
can't be changed.

One example of how this option is being ignored is that on a
read-write image this should produce an error:

   (qemu) qemu-io virtio0 "reopen -o force-share=on"
   force-share=on can only be used with read-only images

Since there's no reason why we shouldn't allow changing it and the
implementation is simple let's just do it.

Signed-off-by: Alberto Garcia <[e-mail]>
---
 block.c               | 30 ++++++++++++++++++++++++------
 include/block/block.h |  1 +
 2 files changed, 25 insertions(+), 6 deletions(-)

Comments

Max Reitz Aug. 29, 2018, 12:01 p.m. UTC | #1

On 2018-08-26 16:09, Alberto Garcia wrote:
> 'force-share' is one of the basic BlockdevOptions available for all
> drivers, but it's silently ignored by bdrv_reopen_prepare/commit(), so
> the user cannot change it and doesn't get an error explaining that it
> can't be changed.
> 
> One example of how this option is being ignored is that on a
> read-write image this should produce an error:
> 
>    (qemu) qemu-io virtio0 "reopen -o force-share=on"
>    force-share=on can only be used with read-only images
> 
> Since there's no reason why we shouldn't allow changing it and the
> implementation is simple let's just do it.
> 
> Signed-off-by: Alberto Garcia <[e-mail]>
> ---
>  block.c               | 30 ++++++++++++++++++++++++------
>  include/block/block.h |  1 +
>  2 files changed, 25 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz <[e-mail]>

Patch

diff --git a/block.c b/block.c
index 9aed5c19f4..b6e495b716 100644
--- a/block.c
+++ b/block.c
@@ -788,6 +788,18 @@  static BlockdevDetectZeroesOptions bdrv_parse_detect_zeroes(QemuOpts *opts,
     return detect_zeroes;
 }
 
+static bool bdrv_parse_force_share(QemuOpts *opts, int flags, Error **errp)
+{
+    bool value = qemu_opt_get_bool(opts, BDRV_OPT_FORCE_SHARE, false);
+
+    if (value && (flags & BDRV_O_RDWR)) {
+        error_setg(errp, BDRV_OPT_FORCE_SHARE
+                   "=on can only be used with read-only images");
+    }
+
+    return value;
+}
+
 /**
  * Set open flags for a given discard mode
  *
@@ -1373,12 +1385,9 @@  static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file,
     drv = bdrv_find_format(driver_name);
     assert(drv != NULL);
 
-    bs->force_share = qemu_opt_get_bool(opts, BDRV_OPT_FORCE_SHARE, false);
-
-    if (bs->force_share && (bs->open_flags & BDRV_O_RDWR)) {
-        error_setg(errp,
-                   BDRV_OPT_FORCE_SHARE
-                   "=on can only be used with read-only images");
+    bs->force_share = bdrv_parse_force_share(opts, bs->open_flags, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         ret = -EINVAL;
         goto fail_opts;
     }
@@ -3201,6 +3210,14 @@  int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
         goto error;
     }
 
+    reopen_state->force_share =
+        bdrv_parse_force_share(opts, reopen_state->flags, &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");
@@ -3359,6 +3376,7 @@  void bdrv_reopen_commit(BDRVReopenState *reopen_state)
     bs->open_flags         = reopen_state->flags;
     bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
     bs->detect_zeroes      = reopen_state->detect_zeroes;
+    bs->force_share        = reopen_state->force_share;
 
     /* Remove child references from bs->options and bs->explicit_options */
     QLIST_FOREACH(child, &bs->children, next) {
diff --git a/include/block/block.h b/include/block/block.h
index f71fa5a1c4..a49a027c54 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -185,6 +185,7 @@  typedef struct BDRVReopenState {
     BlockDriverState *bs;
     int flags;
     BlockdevDetectZeroesOptions detect_zeroes;
+    bool force_share;
     uint64_t perm, shared_perm;
     QDict *options;
     QDict *explicit_options;