[1/3] block: add options parameter to bdrv_new_open_driver()

Commit Message

Manos Pitsidianakis Aug. 1, 2017, 1:49 p.m. UTC

Allow passing a QDict *options parameter to bdrv_new_open_driver() so
that it can be used if a driver needs it upon creation. The previous
behaviour (empty bs->options and bs->explicit_options) remains when
options is NULL.

Signed-off-by: Manos Pitsidianakis <[e-mail]>
---
 block.c               | 16 +++++++++++++---
 block/commit.c        |  4 ++--
 block/mirror.c        |  2 +-
 block/vvfat.c         |  2 +-
 include/block/block.h |  2 +-
 5 files changed, 18 insertions(+), 8 deletions(-)

Comments

Stefan Hajnoczi Aug. 2, 2017, 9:33 a.m. UTC | #1

On Tue, Aug 01, 2017 at 04:49:05PM +0300, Manos Pitsidianakis wrote:
> diff --git a/block.c b/block.c
> index 37e72b7a96..886a457ab0 100644
> --- a/block.c
> +++ b/block.c
> @@ -1149,16 +1149,26 @@ free_and_fail:
>      return ret;
>  }
>  
> +/*
> + * If options is not NULL it is cloned (which adds another reference to the
> + * option entries). If the call to bdrv_new_open_driver() is successful, the
> + * caller should unref options to pass ownership.
> + * */

Please follow the ownership convention for the options argument.
Existing block.c function take ownership of options:

  * 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 belongs to the block layer
  * after the call (even on failure), so if the caller intends to reuse the
  * dictionary, it needs to use QINCREF() before calling bdrv_open.

>  BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
> -                                       int flags, Error **errp)
> +                                       int flags, QDict *options, Error **errp)

Patch

diff --git a/block.c b/block.c
index 37e72b7a96..886a457ab0 100644
--- a/block.c
+++ b/block.c
@@ -1149,16 +1149,26 @@  free_and_fail:
     return ret;
 }
 
+/*
+ * If options is not NULL it is cloned (which adds another reference to the
+ * option entries). If the call to bdrv_new_open_driver() is successful, the
+ * caller should unref options to pass ownership.
+ * */
 BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
-                                       int flags, Error **errp)
+                                       int flags, QDict *options, Error **errp)
 {
     BlockDriverState *bs;
     int ret;
 
     bs = bdrv_new();
     bs->open_flags = flags;
-    bs->explicit_options = qdict_new();
-    bs->options = qdict_new();
+    if (options) {
+        bs->explicit_options = qdict_clone_shallow(options);
+        bs->options = qdict_clone_shallow(options);
+    } else {
+        bs->explicit_options = qdict_new();
+        bs->options = qdict_new();
+    }
     bs->opaque = NULL;
 
     update_options_from_flags(bs->options, flags);
diff --git a/block/commit.c b/block/commit.c
index c7857c3321..539e23c3f8 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -342,7 +342,7 @@  void commit_start(const char *job_id, BlockDriverState *bs,
     /* Insert commit_top block node above top, so we can block consistent read
      * on the backing chain below it */
     commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, filter_node_name, 0,
-                                         errp);
+                                         NULL, errp);
     if (commit_top_bs == NULL) {
         goto fail;
     }
@@ -494,7 +494,7 @@  int bdrv_commit(BlockDriverState *bs)
     backing_file_bs = backing_bs(bs);
 
     commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, NULL, BDRV_O_RDWR,
-                                         &local_err);
+                                         NULL, &local_err);
     if (commit_top_bs == NULL) {
         error_report_err(local_err);
         goto ro_cleanup;
diff --git a/block/mirror.c b/block/mirror.c
index c9a6a3ca86..e1a160e6ea 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1164,7 +1164,7 @@  static void mirror_start_job(const char *job_id, BlockDriverState *bs,
      * reads on the top, while disabling it in the intermediate nodes, and make
      * the backing chain writable. */
     mirror_top_bs = bdrv_new_open_driver(&bdrv_mirror_top, filter_node_name,
-                                         BDRV_O_RDWR, errp);
+                                         BDRV_O_RDWR, NULL, errp);
     if (mirror_top_bs == NULL) {
         return;
     }
diff --git a/block/vvfat.c b/block/vvfat.c
index a9e207f7f0..6c59473baf 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3194,7 +3194,7 @@  static int enable_write_target(BlockDriverState *bs, Error **errp)
 #endif
 
     backing = bdrv_new_open_driver(&vvfat_write_target, NULL, BDRV_O_ALLOW_RDWR,
-                                   &error_abort);
+                                   NULL, &error_abort);
     *(void**) backing->opaque = s;
 
     bdrv_set_backing_hd(s->bs, backing, &error_abort);
diff --git a/include/block/block.h b/include/block/block.h
index 34770bb33a..020289a37d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -263,7 +263,7 @@  int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
 BlockDriverState *bdrv_open(const char *filename, const char *reference,
                             QDict *options, int flags, Error **errp);
 BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
-                                       int flags, Error **errp);
+                                       int flags, QDict *options, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs,
                                     QDict *options, int flags);