[Qemu-devel] [RFC PATCH 27/41] block: Add bdrv_new_open_driver()

Kevin Wolf posted 41 patches 8 years, 12 months ago
[Qemu-devel] [RFC PATCH 27/41] block: Add bdrv_new_open_driver()
Posted by Kevin Wolf 8 years, 12 months ago
This function allows to create more or less normal BlockDriverStates
even for BlockDrivers that aren't globally registered (e.g. helper
filters for block jobs).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 31 ++++++++++++++++++++++++++++++-
 include/block/block.h |  2 ++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 9c80cba..7d84d43 100644
--- a/block.c
+++ b/block.c
@@ -948,13 +948,16 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
     }
 
     bs->drv = drv;
+    bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
     bs->opaque = g_malloc0(drv->instance_size);
 
     if (drv->bdrv_file_open) {
         assert(!drv->bdrv_needs_filename || bs->filename[0]);
         ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
-    } else {
+    } else if (drv->bdrv_open) {
         ret = drv->bdrv_open(bs, options, open_flags, &local_err);
+    } else {
+        ret = 0;
     }
 
     if (ret < 0) {
@@ -995,6 +998,32 @@ free_and_fail:
     return ret;
 }
 
+int bdrv_new_open_driver(BlockDriver *drv, BlockDriverState **result,
+                         const char *node_name, int flags, Error **errp)
+{
+    BlockDriverState *bs;
+    int ret;
+
+    bs = bdrv_new();
+    bs->open_flags = flags;
+    bs->explicit_options = qdict_new();
+    bs->options = qdict_new();
+    bs->opaque = NULL;
+
+    update_options_from_flags(bs->options, flags);
+
+    ret = bdrv_open_driver(bs, drv, node_name, bs->options, flags, errp);
+    if (ret < 0) {
+        QDECREF(bs->explicit_options);
+        QDECREF(bs->options);
+        bdrv_unref(bs);
+        return ret;
+    }
+
+    *result = bs;
+    return 0;
+}
+
 QemuOptsList bdrv_runtime_opts = {
     .name = "bdrv_common",
     .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
diff --git a/include/block/block.h b/include/block/block.h
index 93812df..3238850 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -215,6 +215,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
                            const char *bdref_key, Error **errp);
 BlockDriverState *bdrv_open(const char *filename, const char *reference,
                             QDict *options, int flags, Error **errp);
+int bdrv_new_open_driver(BlockDriver *drv, BlockDriverState **result,
+                         const char *node_name, int flags, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs,
                                     QDict *options, int flags);
-- 
1.8.3.1


Re: [Qemu-devel] [RFC PATCH 27/41] block: Add bdrv_new_open_driver()
Posted by Max Reitz 8 years, 11 months ago
On 13.02.2017 18:22, Kevin Wolf wrote:
> This function allows to create more or less normal BlockDriverStates
> even for BlockDrivers that aren't globally registered (e.g. helper
> filters for block jobs).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c               | 31 ++++++++++++++++++++++++++++++-
>  include/block/block.h |  2 ++
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 9c80cba..7d84d43 100644
> --- a/block.c
> +++ b/block.c
> @@ -948,13 +948,16 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
>      }
>  
>      bs->drv = drv;
> +    bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
>      bs->opaque = g_malloc0(drv->instance_size);
>  
>      if (drv->bdrv_file_open) {
>          assert(!drv->bdrv_needs_filename || bs->filename[0]);
>          ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
> -    } else {
> +    } else if (drv->bdrv_open) {
>          ret = drv->bdrv_open(bs, options, open_flags, &local_err);
> +    } else {
> +        ret = 0;
>      }
>  
>      if (ret < 0) {
> @@ -995,6 +998,32 @@ free_and_fail:
>      return ret;
>  }
>  
> +int bdrv_new_open_driver(BlockDriver *drv, BlockDriverState **result,

Really? After we put much work into getting bdrv_open() to return a BDS
pointer? :-/

Do you need to distinguish based on -EPERM somewhere? I'd much rather
have the BDS pointer returned, personally. If you need the integer
return value somewhere, couldn't that be a parameter (which can
optionally be NULL if the caller doesn't need it)?

Max

> +                         const char *node_name, int flags, Error **errp)
> +{
> +    BlockDriverState *bs;
> +    int ret;
> +
> +    bs = bdrv_new();
> +    bs->open_flags = flags;
> +    bs->explicit_options = qdict_new();
> +    bs->options = qdict_new();
> +    bs->opaque = NULL;
> +
> +    update_options_from_flags(bs->options, flags);
> +
> +    ret = bdrv_open_driver(bs, drv, node_name, bs->options, flags, errp);
> +    if (ret < 0) {
> +        QDECREF(bs->explicit_options);
> +        QDECREF(bs->options);
> +        bdrv_unref(bs);
> +        return ret;
> +    }
> +
> +    *result = bs;
> +    return 0;
> +}
> +
>  QemuOptsList bdrv_runtime_opts = {
>      .name = "bdrv_common",
>      .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
> diff --git a/include/block/block.h b/include/block/block.h
> index 93812df..3238850 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -215,6 +215,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
>                             const char *bdref_key, Error **errp);
>  BlockDriverState *bdrv_open(const char *filename, const char *reference,
>                              QDict *options, int flags, Error **errp);
> +int bdrv_new_open_driver(BlockDriver *drv, BlockDriverState **result,
> +                         const char *node_name, int flags, Error **errp);
>  BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
>                                      BlockDriverState *bs,
>                                      QDict *options, int flags);
>