[Qemu-devel] [PATCH v2 1/2] quorum: Simplify quorum_open()

[Max Reitz]

Although it may not look like it, this patch simplifies quorum_open().
qdict_array_split() is now able to return QLists with different objects
than only QDicts, therefore it will now do all the work and
quorum_open() does not have to handle reference strings by itself.

This allows mixing full option dicts and reference strings for
specifying the child block devices of quorum; furthermore, it improves
handling of malformed specifications.

Signed-off-by: Max Reitz <address@hidden>
Reviewed-by: Benoit Canet <address@hidden>
---
 block/quorum.c | 66 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 73dd45b..6c28239 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -720,7 +720,6 @@ static int quorum_open(BlockDriverState *bs, QDict 
*options, int flags,
     QDict *sub = NULL;
     QList *list = NULL;
     const QListEntry *lentry;
-    const QDictEntry *dentry;
     int i;
     int ret = 0;
 
@@ -728,10 +727,15 @@ static int quorum_open(BlockDriverState *bs, QDict 
*options, int flags,
     qdict_extract_subqdict(options, &sub, "children.");
     qdict_array_split(sub, &list);
 
-    /* count how many different children are present and validate
-     * qdict_size(sub) address the open by reference case
-     */
-    s->num_children = !qlist_size(list) ? qdict_size(sub) : qlist_size(list);
+    if (qdict_size(sub)) {
+        error_setg(&local_err, "Invalid option children.%s",
+                   qdict_first(sub)->key);
+        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");
@@ -767,30 +771,38 @@ 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);
 
-    /* Open by file name or options dict (command line or QMP) */
-    if (s->num_children == qlist_size(list)) {
-        for (i = 0, lentry = qlist_first(list); lentry;
-            lentry = qlist_next(lentry), i++) {
-            QDict *d = qobject_to_qdict(lentry->value);
-            QINCREF(d);
-            ret = bdrv_open(&s->bs[i], NULL, NULL, d, flags, NULL, &local_err);
-            if (ret < 0) {
-                goto close_exit;
-            }
-            opened[i] = true;
+    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;
         }
-    /* Open by QMP references */
-    } else {
-        for (i = 0, dentry = qdict_first(sub); dentry;
-             dentry = qdict_next(sub, dentry), i++) {
-            QString *string = qobject_to_qstring(dentry->value);
-            ret = bdrv_open(&s->bs[i], NULL, qstring_get_str(string), NULL,
-                            flags, NULL, &local_err);
-            if (ret < 0) {
-                goto close_exit;
-            }
-            opened[i] = true;
+
+        if (ret < 0) {
+            goto close_exit;
         }
+        opened[i] = true;
     }
 
     g_free(opened);
-- 
1.9.0