commit 8a87f3d72279acb89f3d09b28d285d2fb6a7decf
author Max Reitz <mreitz@redhat.com> Fri Feb 21 22:30:37 2014 +0100
committer Kevin Wolf <kwolf@redhat.com> Fri Feb 21 22:32:24 2014 +0100
tree 30e695b2eb3e99c4277c8da9de8dcf768219da17
parent c7fc5bc2a4d89ccdb1ffabc720e7c87558c9aaef

quorum: Simplify quorum_open()

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 <mreitz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

block/quorum.c

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 @@
     QDict *sub = NULL;
     QList *list = NULL;
     const QListEntry *lentry;
-    const QDictEntry *dentry;
     int i;
     int ret = 0;
 
@@ -728,10 +727,15 @@
     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 @@
     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);