diff mbox

[03/34] quorum: Use bdrv_open_image()

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

Commit Message

Kevin Wolf May 8, 2015, 5:21 p.m. UTC
Besides standardising on a single interface for opening child nodes,
this simplifies the .bdrv_open() implementation of the quorum block
driver by using block layer functionality for handling BlockdevRefs.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/quorum.c | 51 +++++++++++----------------------------------------
 1 file changed, 11 insertions(+), 40 deletions(-)

Comments

Eric Blake May 8, 2015, 9:33 p.m. UTC | #1
On 05/08/2015 11:21 AM, Kevin Wolf wrote:
> Besides standardising on a single interface for opening child nodes,
> this simplifies the .bdrv_open() implementation of the quorum block
> driver by using block layer functionality for handling BlockdevRefs.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/quorum.c | 51 +++++++++++----------------------------------------
>  1 file changed, 11 insertions(+), 40 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz May 11, 2015, 2:27 p.m. UTC | #2
On 08.05.2015 19:21, Kevin Wolf wrote:
> Besides standardising on a single interface for opening child nodes,
> this simplifies the .bdrv_open() implementation of the quorum block
> driver by using block layer functionality for handling BlockdevRefs.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/quorum.c | 51 +++++++++++----------------------------------------
>   1 file changed, 11 insertions(+), 40 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Jeff Cody May 12, 2015, 7:07 p.m. UTC | #3
On Fri, May 08, 2015 at 07:21:35PM +0200, Kevin Wolf wrote:
> Besides standardising on a single interface for opening child nodes,
> this simplifies the .bdrv_open() implementation of the quorum block
> driver by using block layer functionality for handling BlockdevRefs.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/quorum.c | 51 +++++++++++----------------------------------------
>  1 file changed, 11 insertions(+), 40 deletions(-)
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index f91ef75..a33881a 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -866,25 +866,18 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>      Error *local_err = NULL;
>      QemuOpts *opts = NULL;
>      bool *opened;
> -    QDict *sub = NULL;
> -    QList *list = NULL;
> -    const QListEntry *lentry;
>      int i;
>      int ret = 0;
>  
>      qdict_flatten(options);
> -    qdict_extract_subqdict(options, &sub, "children.");
> -    qdict_array_split(sub, &list);
>  
> -    if (qdict_size(sub)) {
> -        error_setg(&local_err, "Invalid option children.%s",
> -                   qdict_first(sub)->key);
> +    /* count how many different children are present */
> +    s->num_children = qdict_array_entries(options, "children.");
> +    if (s->num_children < 0) {
> +        error_setg(&local_err, "Option children is not a valid array");
>          ret = -EINVAL;
>          goto exit;
>      }
> -
> -    /* count how many different children are present */
> -    s->num_children = qlist_size(list);
>      if (s->num_children < 2) {
>          error_setg(&local_err,
>                     "Number of provided children must be greater than 1");
> @@ -937,37 +930,17 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>      s->bs = g_new0(BlockDriverState *, s->num_children);
>      opened = g_new0(bool, s->num_children);
>  
> -    for (i = 0, lentry = qlist_first(list); lentry;
> -         lentry = qlist_next(lentry), i++) {
> -        QDict *d;
> -        QString *string;
> -
> -        switch (qobject_type(lentry->value))
> -        {
> -            /* List of options */
> -            case QTYPE_QDICT:
> -                d = qobject_to_qdict(lentry->value);
> -                QINCREF(d);
> -                ret = bdrv_open(&s->bs[i], NULL, NULL, d, flags, NULL,
> -                                &local_err);
> -                break;
> -
> -            /* QMP reference */
> -            case QTYPE_QSTRING:
> -                string = qobject_to_qstring(lentry->value);
> -                ret = bdrv_open(&s->bs[i], NULL, qstring_get_str(string), NULL,
> -                                flags, NULL, &local_err);
> -                break;
> -
> -            default:
> -                error_setg(&local_err, "Specification of child block device %i "
> -                           "is invalid", i);
> -                ret = -EINVAL;
> -        }
> +    for (i = 0; i < s->num_children; i++) {
> +        char indexstr[32];
> +        ret = snprintf(indexstr, 32, "children.%d", i);
> +        assert(ret < 32);
>  
> +        ret = bdrv_open_image(&s->bs[i], NULL, options, indexstr, flags,
> +                              false, &local_err);
>          if (ret < 0) {
>              goto close_exit;
>          }
> +
>          opened[i] = true;
>      }
>  
> @@ -990,8 +963,6 @@ exit:
>      if (local_err) {
>          error_propagate(errp, local_err);
>      }
> -    QDECREF(list);
> -    QDECREF(sub);
>      return ret;
>  }
>  
> -- 
> 1.8.3.1
> 
>

Reviewed-by: Jeff Cody <jcody@redhat.com>
Alberto Garcia May 20, 2015, 2:46 p.m. UTC | #4
On Fri 08 May 2015 07:21:35 PM CEST, Kevin Wolf wrote:

> Besides standardising on a single interface for opening child nodes,
> this simplifies the .bdrv_open() implementation of the quorum block
> driver by using block layer functionality for handling BlockdevRefs.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
diff mbox

Patch

diff --git a/block/quorum.c b/block/quorum.c
index f91ef75..a33881a 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -866,25 +866,18 @@  static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     Error *local_err = NULL;
     QemuOpts *opts = NULL;
     bool *opened;
-    QDict *sub = NULL;
-    QList *list = NULL;
-    const QListEntry *lentry;
     int i;
     int ret = 0;
 
     qdict_flatten(options);
-    qdict_extract_subqdict(options, &sub, "children.");
-    qdict_array_split(sub, &list);
 
-    if (qdict_size(sub)) {
-        error_setg(&local_err, "Invalid option children.%s",
-                   qdict_first(sub)->key);
+    /* count how many different children are present */
+    s->num_children = qdict_array_entries(options, "children.");
+    if (s->num_children < 0) {
+        error_setg(&local_err, "Option children is not a valid array");
         ret = -EINVAL;
         goto exit;
     }
-
-    /* count how many different children are present */
-    s->num_children = qlist_size(list);
     if (s->num_children < 2) {
         error_setg(&local_err,
                    "Number of provided children must be greater than 1");
@@ -937,37 +930,17 @@  static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     s->bs = g_new0(BlockDriverState *, s->num_children);
     opened = g_new0(bool, s->num_children);
 
-    for (i = 0, lentry = qlist_first(list); lentry;
-         lentry = qlist_next(lentry), i++) {
-        QDict *d;
-        QString *string;
-
-        switch (qobject_type(lentry->value))
-        {
-            /* List of options */
-            case QTYPE_QDICT:
-                d = qobject_to_qdict(lentry->value);
-                QINCREF(d);
-                ret = bdrv_open(&s->bs[i], NULL, NULL, d, flags, NULL,
-                                &local_err);
-                break;
-
-            /* QMP reference */
-            case QTYPE_QSTRING:
-                string = qobject_to_qstring(lentry->value);
-                ret = bdrv_open(&s->bs[i], NULL, qstring_get_str(string), NULL,
-                                flags, NULL, &local_err);
-                break;
-
-            default:
-                error_setg(&local_err, "Specification of child block device %i "
-                           "is invalid", i);
-                ret = -EINVAL;
-        }
+    for (i = 0; i < s->num_children; i++) {
+        char indexstr[32];
+        ret = snprintf(indexstr, 32, "children.%d", i);
+        assert(ret < 32);
 
+        ret = bdrv_open_image(&s->bs[i], NULL, options, indexstr, flags,
+                              false, &local_err);
         if (ret < 0) {
             goto close_exit;
         }
+
         opened[i] = true;
     }
 
@@ -990,8 +963,6 @@  exit:
     if (local_err) {
         error_propagate(errp, local_err);
     }
-    QDECREF(list);
-    QDECREF(sub);
     return ret;
 }