diff mbox series

[1/2] block/rbd: pull out qemu_rbd_convert_options

Message ID 78d2f47b07325fce34a03b115159830f662b7f90.1536642773.git.jcody@redhat.com (mailing list archive)
State New, archived
Headers show
Series block/rbd: enable filename parsing on open | expand

Commit Message

Jeff Cody Sept. 11, 2018, 5:15 a.m. UTC 
Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
into a helper function.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/rbd.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

Comments

Eric Blake Sept. 11, 2018, 5:50 p.m. UTC | #1 
On 9/11/18 12:15 AM, Jeff Cody wrote:
> Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
> into a helper function.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/rbd.c | 36 ++++++++++++++++++++++++------------
>   1 file changed, 24 insertions(+), 12 deletions(-)
> 

> -
> +    r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
> -        r = -EINVAL;
>           goto out;
>       }

You could simplify this to:

r = qemu_rbd_convert_options(bs, options, &opts, errp);
if (r < 0) {
     goto out;
}

Either way, this refactoring looks correct.

Reviewed-by: Eric Blake <eblake@redhat.com>
John Snow Sept. 11, 2018, 6:06 p.m. UTC | #2 
On 09/11/2018 01:15 AM, Jeff Cody wrote:
> Code movement to pull the conversion from Qdict to BlockdevOptionsRbd
> into a helper function.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/rbd.c | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index ca8e5bbace..a8e79d01d2 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -655,12 +655,34 @@ failed_opts:
>      return r;
>  }
>  
> +static int qemu_rbd_convert_options(BlockDriverState *bs, QDict *options,
> +                                    BlockdevOptionsRbd **opts, Error **errp)
> +{
> +    Visitor *v;
> +    Error *local_err = NULL;
> +
> +    /* Convert the remaining options into a QAPI object */
> +    v = qobject_input_visitor_new_flat_confused(options, errp);
> +    if (!v) {
> +        return -EINVAL;
> +    }
> +
> +    visit_type_BlockdevOptionsRbd(v, NULL, opts, &local_err);
> +    visit_free(v);
> +
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
>  static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>                           Error **errp)
>  {
>      BDRVRBDState *s = bs->opaque;
>      BlockdevOptionsRbd *opts = NULL;
> -    Visitor *v;
>      const QDictEntry *e;
>      Error *local_err = NULL;
>      char *keypairs, *secretid;
> @@ -676,19 +698,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
>          qdict_del(options, "password-secret");
>      }
>  
> -    /* Convert the remaining options into a QAPI object */
> -    v = qobject_input_visitor_new_flat_confused(options, errp);
> -    if (!v) {
> -        r = -EINVAL;
> -        goto out;
> -    }
> -
> -    visit_type_BlockdevOptionsRbd(v, NULL, &opts, &local_err);
> -    visit_free(v);
> -
> +    r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> -        r = -EINVAL;
>          goto out;
>      }
>  
> 

Reviewed-by: John Snow <jsnow@redhat.com>
diff mbox series

Patch

diff --git a/block/rbd.c b/block/rbd.c
index ca8e5bbace..a8e79d01d2 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -655,12 +655,34 @@  failed_opts:
     return r;
 }
 
+static int qemu_rbd_convert_options(BlockDriverState *bs, QDict *options,
+                                    BlockdevOptionsRbd **opts, Error **errp)
+{
+    Visitor *v;
+    Error *local_err = NULL;
+
+    /* Convert the remaining options into a QAPI object */
+    v = qobject_input_visitor_new_flat_confused(options, errp);
+    if (!v) {
+        return -EINVAL;
+    }
+
+    visit_type_BlockdevOptionsRbd(v, NULL, opts, &local_err);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
                          Error **errp)
 {
     BDRVRBDState *s = bs->opaque;
     BlockdevOptionsRbd *opts = NULL;
-    Visitor *v;
     const QDictEntry *e;
     Error *local_err = NULL;
     char *keypairs, *secretid;
@@ -676,19 +698,9 @@  static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
         qdict_del(options, "password-secret");
     }
 
-    /* Convert the remaining options into a QAPI object */
-    v = qobject_input_visitor_new_flat_confused(options, errp);
-    if (!v) {
-        r = -EINVAL;
-        goto out;
-    }
-
-    visit_type_BlockdevOptionsRbd(v, NULL, &opts, &local_err);
-    visit_free(v);
-
+    r = qemu_rbd_convert_options(bs, options, &opts, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        r = -EINVAL;
         goto out;
     }