diff mbox

[06/34] block: Use QemuOpts in bdrv_open_common()

Message ID 1431105726-3682-7-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf May 8, 2015, 5:21 p.m. UTC
Instead of manually parsing options and then deleting them from the
options QDict, just use QemuOpts like most other places that deal with
block device options.

More options will be added there and then QemuOpts is a lot more
managable than open-coding everything.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

Comments

Eric Blake May 8, 2015, 10:57 p.m. UTC | #1
On 05/08/2015 11:21 AM, Kevin Wolf wrote:
> Instead of manually parsing options and then deleting them from the
> options QDict, just use QemuOpts like most other places that deal with
> block device options.
> 
> More options will be added there and then QemuOpts is a lot more
> managable than open-coding everything.

s/managable/manageable/

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz May 11, 2015, 2:57 p.m. UTC | #2
On 08.05.2015 19:21, Kevin Wolf wrote:
> Instead of manually parsing options and then deleting them from the
> options QDict, just use QemuOpts like most other places that deal with
> block device options.
>
> More options will be added there and then QemuOpts is a lot more
> managable than open-coding everything.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c | 37 ++++++++++++++++++++++++++++++++-----
>   1 file changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 7904098..cea022f 100644
> --- a/block.c
> +++ b/block.c
> @@ -757,6 +757,19 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
>       QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
>   }
>   
> +static QemuOptsList bdrv_runtime_opts = {
> +    .name = "bdrv_common",
> +    .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
> +    .desc = {
> +        {
> +            .name = "node-name",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Node name of the block device node",
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>   /*
>    * Common part for opening disk images and files
>    *
> @@ -768,6 +781,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>       int ret, open_flags;
>       const char *filename;
>       const char *node_name = NULL;
> +    QemuOpts *opts;
>       Error *local_err = NULL;
>   
>       assert(drv != NULL);
> @@ -788,19 +802,28 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>   
>       trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
>   
> -    node_name = qdict_get_try_str(options, "node-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;
> +    }
> +
> +    node_name = qemu_opt_get(opts, "node-name");
>       bdrv_assign_node_name(bs, node_name, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
> -        return -EINVAL;
> +        ret = -EINVAL;
> +        goto fail_opts;
>       }
> -    qdict_del(options, "node-name");
>   
>       /* bdrv_open() with directly using a protocol as drv. This layer is already
>        * opened, so assign it to bs (while file becomes a closed BlockDriverState)
>        * and return immediately. */
>       if (file != NULL && drv->bdrv_file_open) {
>           bdrv_swap(file, bs);
> +        qemu_opts_del(opts);
>           return 0;
>       }
>   
> @@ -817,7 +840,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>                           ? "Driver '%s' can only be used for read-only devices"
>                           : "Driver '%s' is not whitelisted",
>                      drv->format_name);
> -        return -ENOTSUP;
> +        ret = -ENOTSUP;
> +        goto fail_opts;
>       }
>   
>       assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
> @@ -826,7 +850,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
>               bdrv_enable_copy_on_read(bs);
>           } else {
>               error_setg(errp, "Can't use copy-on-read on read-only device");
> -            return -EINVAL;
> +            ret = -EINVAL;
> +            goto fail_opts;
>           }
>       }
>   
> @@ -898,6 +923,8 @@ free_and_fail:

"opts" seems to be leaked in case of success.

Max

>       g_free(bs->opaque);
>       bs->opaque = NULL;
>       bs->drv = NULL;
> +fail_opts:
> +    qemu_opts_del(opts);
>       return ret;
>   }
>
diff mbox

Patch

diff --git a/block.c b/block.c
index 7904098..cea022f 100644
--- a/block.c
+++ b/block.c
@@ -757,6 +757,19 @@  static void bdrv_assign_node_name(BlockDriverState *bs,
     QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
 }
 
+static QemuOptsList bdrv_runtime_opts = {
+    .name = "bdrv_common",
+    .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
+    .desc = {
+        {
+            .name = "node-name",
+            .type = QEMU_OPT_STRING,
+            .help = "Node name of the block device node",
+        },
+        { /* end of list */ }
+    },
+};
+
 /*
  * Common part for opening disk images and files
  *
@@ -768,6 +781,7 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     int ret, open_flags;
     const char *filename;
     const char *node_name = NULL;
+    QemuOpts *opts;
     Error *local_err = NULL;
 
     assert(drv != NULL);
@@ -788,19 +802,28 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
 
     trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
 
-    node_name = qdict_get_try_str(options, "node-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;
+    }
+
+    node_name = qemu_opt_get(opts, "node-name");
     bdrv_assign_node_name(bs, node_name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        return -EINVAL;
+        ret = -EINVAL;
+        goto fail_opts;
     }
-    qdict_del(options, "node-name");
 
     /* bdrv_open() with directly using a protocol as drv. This layer is already
      * opened, so assign it to bs (while file becomes a closed BlockDriverState)
      * and return immediately. */
     if (file != NULL && drv->bdrv_file_open) {
         bdrv_swap(file, bs);
+        qemu_opts_del(opts);
         return 0;
     }
 
@@ -817,7 +840,8 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
                         ? "Driver '%s' can only be used for read-only devices"
                         : "Driver '%s' is not whitelisted",
                    drv->format_name);
-        return -ENOTSUP;
+        ret = -ENOTSUP;
+        goto fail_opts;
     }
 
     assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
@@ -826,7 +850,8 @@  static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
             bdrv_enable_copy_on_read(bs);
         } else {
             error_setg(errp, "Can't use copy-on-read on read-only device");
-            return -EINVAL;
+            ret = -EINVAL;
+            goto fail_opts;
         }
     }
 
@@ -898,6 +923,8 @@  free_and_fail:
     g_free(bs->opaque);
     bs->opaque = NULL;
     bs->drv = NULL;
+fail_opts:
+    qemu_opts_del(opts);
     return ret;
 }