util: Add a return value to void hash iterators

Our existing virHashForEach method iterates through all items disregarding the
fact, that some of the iterators might have actually failed. Errors are usually
dispatched through an error element in opaque data which then causes the
original caller of virHashForEach to return -1. In that case, virHashForEach
could return as soon as one of the iterators fail. This patch changes the
iterator return type and adjusts all of its instances accordingly, so the
actual refactor of virHashForEach method can be dealt with later.

Signed-off-by: Erik Skultety <eskultet@redhat.com>

src/conf/network_conf.c

@@ -4399,7 +4399,7 @@ struct virNetworkObjListData {
     bool error;
 };
 
-static void
+static int
 virNetworkObjListPopulate(void *payload,
                           const void *name ATTRIBUTE_UNUSED,
                           void *opaque)
@@ -4409,7 +4409,7 @@ virNetworkObjListPopulate(void *payload,
     virNetworkPtr net = NULL;
 
     if (data->error)
-        return;
+        return 0;
 
     virObjectLock(obj);
 
@@ -4434,6 +4434,7 @@ virNetworkObjListPopulate(void *payload,
 
  cleanup:
     virObjectUnlock(obj);
+    return 0;
 }
 
 int
@@ -4478,7 +4479,7 @@ struct virNetworkObjListForEachHelperData {
     int ret;
 };
 
-static void
+static int
 virNetworkObjListForEachHelper(void *payload,
                                const void *name ATTRIBUTE_UNUSED,
                                void *opaque)
@@ -4487,6 +4488,7 @@ virNetworkObjListForEachHelper(void *payload,
 
     if (data->callback(payload, data->opaque) < 0)
         data->ret = -1;
+    return 0;
 }
 
 /**
@@ -4524,7 +4526,7 @@ struct virNetworkObjListGetHelperData {
     bool error;
 };
 
-static void
+static int
 virNetworkObjListGetHelper(void *payload,
                            const void *name ATTRIBUTE_UNUSED,
                            void *opaque)
@@ -4533,11 +4535,11 @@ virNetworkObjListGetHelper(void *payload,
     virNetworkObjPtr obj = payload;
 
     if (data->error)
-        return;
+        return 0;
 
     if (data->nnames >= 0 &&
         data->got == data->nnames)
-        return;
+        return 0;
 
     virObjectLock(obj);
 
@@ -4557,6 +4559,7 @@ virNetworkObjListGetHelper(void *payload,
 
  cleanup:
     virObjectUnlock(obj);
+    return 0;
 }
 
 int

src/conf/nwfilter_params.c

@@ -709,19 +709,19 @@ struct addToTableStruct {
 };
 
 
-static void
+static int
 addToTable(void *payload, const void *name, void *data)
 {
     struct addToTableStruct *atts = (struct addToTableStruct *)data;
     virNWFilterVarValuePtr val;
 
     if (atts->errOccurred)
-        return;
+        return 0;
 
     val = virNWFilterVarValueCopy((virNWFilterVarValuePtr)payload);
     if (!val) {
         atts->errOccurred = 1;
-        return;
+        return 0;
     }
 
     if (virNWFilterHashTablePut(atts->target, (const char *)name, val) < 0) {
@@ -731,6 +731,8 @@ addToTable(void *payload, const void *name, void *data)
         atts->errOccurred = 1;
         virNWFilterVarValueFree(val);
     }
+
+    return 0;
 }
 
 

src/conf/snapshot_conf.c

@@ -844,7 +844,7 @@ struct virDomainSnapshotNameData {
     bool error;
 };
 
-static void virDomainSnapshotObjListCopyNames(void *payload,
+static int virDomainSnapshotObjListCopyNames(void *payload,
                                              const void *name ATTRIBUTE_UNUSED,
                                              void *opaque)
 {
@@ -852,40 +852,41 @@ static void virDomainSnapshotObjListCopyNames(void *payload,
     struct virDomainSnapshotNameData *data = opaque;
 
     if (data->error)
-        return;
+        return 0;
     /* Caller already sanitized flags.  Filtering on DESCENDANTS was
      * done by choice of iteration in the caller.  */
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_LEAVES) && obj->nchildren)
-        return;
+        return 0;
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES) && !obj->nchildren)
-        return;
+        return 0;
 
     if (data->flags & VIR_DOMAIN_SNAPSHOT_FILTERS_STATUS) {
         if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE) &&
             obj->def->state == VIR_DOMAIN_SHUTOFF)
-            return;
+            return 0;
         if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY) &&
             obj->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
-            return;
+            return 0;
         if (!(data->flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE) &&
             obj->def->state != VIR_DOMAIN_SHUTOFF &&
             obj->def->state != VIR_DOMAIN_DISK_SNAPSHOT)
-            return;
+            return 0;
     }
 
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL) &&
         virDomainSnapshotIsExternal(obj))
-        return;
+        return 0;
     if ((data->flags & VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) &&
         !virDomainSnapshotIsExternal(obj))
-        return;
+        return 0;
 
     if (data->names && data->count < data->maxnames &&
         VIR_STRDUP(data->names[data->count], obj->def->name) < 0) {
         data->error = true;
-        return;
+        return 0;
     }
     data->count++;
+    return 0;
 }
 
 int
@@ -1012,7 +1013,7 @@ struct snapshot_act_on_descendant {
     void *data;
 };
 
-static void
+static int
 virDomainSnapshotActOnDescendant(void *payload,
                                  const void *name,
                                  void *data)
@@ -1024,6 +1025,7 @@ virDomainSnapshotActOnDescendant(void *payload,
                                                            curr->iter,
                                                            curr->data);
     (curr->iter)(payload, name, curr->data);
+    return 0;
 }
 
 /* Run iter(data) on all descendants of snapshot, while ignoring all
@@ -1055,7 +1057,7 @@ struct snapshot_set_relation {
     virDomainSnapshotObjListPtr snapshots;
     int err;
 };
-static void
+static int
 virDomainSnapshotSetRelations(void *payload,
                               const void *name ATTRIBUTE_UNUSED,
                               void *data)
@@ -1085,6 +1087,7 @@ virDomainSnapshotSetRelations(void *payload,
     obj->parent->nchildren++;
     obj->sibling = obj->parent->first_child;
     obj->parent->first_child = obj;
+    return 0;
 }
 
 /* Populate parent link and child count of all snapshots, with all

src/conf/virchrdev.c

@@ -292,13 +292,14 @@ virChrdevsPtr virChrdevAlloc(void)
 /**
  * Helper to clear stream callbacks when freeing the hash
  */
-static void virChrdevFreeClearCallbacks(void *payload,
+static int virChrdevFreeClearCallbacks(void *payload,
                                        const void *name ATTRIBUTE_UNUSED,
                                        void *data ATTRIBUTE_UNUSED)
 {
     virStreamPtr st = payload;
 
     virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL);
+    return 0;
 }
 
 /**

src/conf/virdomainobjlist.c

@@ -607,7 +607,7 @@ struct virDomainObjListData {
 };
 
 
-static void
+static int
 virDomainObjListCount(void *payload,
                       const void *name ATTRIBUTE_UNUSED,
                       void *opaque)
@@ -627,6 +627,7 @@ virDomainObjListCount(void *payload,
     }
  cleanup:
     virObjectUnlock(obj);
+    return 0;
 }
 
 
@@ -653,7 +654,7 @@ struct virDomainIDData {
 };
 
 
-static void
+static int
 virDomainObjListCopyActiveIDs(void *payload,
                               const void *name ATTRIBUTE_UNUSED,
                               void *opaque)
@@ -668,6 +669,7 @@ virDomainObjListCopyActiveIDs(void *payload,
         data->ids[data->numids++] = obj->def->id;
  cleanup:
     virObjectUnlock(obj);
+    return 0;
 }
 
 
@@ -697,7 +699,7 @@ struct virDomainNameData {
 };
 
 
-static void
+static int
 virDomainObjListCopyInactiveNames(void *payload,
                                   const void *name ATTRIBUTE_UNUSED,
                                   void *opaque)
@@ -706,7 +708,7 @@ virDomainObjListCopyInactiveNames(void *payload,
     struct virDomainNameData *data = opaque;
 
     if (data->oom)
-        return;
+        return 0;
 
     virObjectLock(obj);
     if (data->filter &&
@@ -720,6 +722,7 @@ virDomainObjListCopyInactiveNames(void *payload,
     }
  cleanup:
     virObjectUnlock(obj);
+    return 0;
 }
 
 
@@ -753,7 +756,7 @@ struct virDomainListIterData {
 };
 
 
-static void
+static int
 virDomainObjListHelper(void *payload,
                        const void *name ATTRIBUTE_UNUSED,
                        void *opaque)
@@ -762,6 +765,7 @@ virDomainObjListHelper(void *payload,
 
     if (data->callback(payload, data->opaque) < 0)
         data->ret = -1;
+    return 0;
 }
 
 
@@ -850,7 +854,7 @@ struct virDomainListData {
 };
 
 
-static void
+static int
 virDomainObjListCollectIterator(void *payload,
                                 const void *name ATTRIBUTE_UNUSED,
                                 void *opaque)
@@ -858,6 +862,7 @@ virDomainObjListCollectIterator(void *payload,
     struct virDomainListData *data = opaque;
 
     data->vms[data->nvms++] = virObjectRef(payload);
+    return 0;
 }
 
 

src/locking/lock_daemon.c

@@ -649,7 +649,7 @@ struct virLockDaemonClientReleaseData {
     bool gotError;
 };
 
-static void
+static int
 virLockDaemonClientReleaseLockspace(void *payload,
                                     const void *name ATTRIBUTE_UNUSED,
                                     void *opaque)
@@ -664,6 +664,7 @@ virLockDaemonClientReleaseLockspace(void *payload,
         data->hadSomeLeases = true;
     else if (rc < 0)
         data->gotError = true;
+    return 0;
 }
 
 

src/nwfilter/nwfilter_dhcpsnoop.c

@@ -1864,7 +1864,7 @@ virNWFilterSnoopPruneIter(const void *payload,
  * Iterator to write all leases of a single request to a file.
  * Call this function with the SnoopLock held.
  */
-static void
+static int
 virNWFilterSnoopSaveIter(void *payload,
                          const void *name ATTRIBUTE_UNUSED,
                          void *data)
@@ -1880,6 +1880,7 @@ virNWFilterSnoopSaveIter(void *payload,
         ignore_value(virNWFilterSnoopLeaseFileWrite(tfd, req->ifkey, ipl));
 
     virNWFilterSnoopReqUnlock(req);
+    return 0;
 }
 
 /*

src/nwfilter/nwfilter_gentech_driver.c

@@ -219,19 +219,20 @@ struct printString
 };
 
 
-static void
+static int
 printString(void *payload ATTRIBUTE_UNUSED, const void *name, void *data)
 {
     struct printString *ps = data;
 
     if ((STREQ((char *)name, NWFILTER_STD_VAR_IP) && !ps->reportIP) ||
         (STREQ((char *)name, NWFILTER_STD_VAR_MAC) && !ps->reportMAC))
-        return;
+        return 0;
 
     if (virBufferUse(&ps->buf) && ps->separator)
         virBufferAdd(&ps->buf, ps->separator, -1);
 
     virBufferAdd(&ps->buf, name, -1);
+    return 0;
 }
 
 /**

src/qemu/qemu_domain.c

@@ -2836,7 +2836,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
 }
 
 /* Hash iterator callback to discard multiple snapshots.  */
-void qemuDomainSnapshotDiscardAll(void *payload,
+int qemuDomainSnapshotDiscardAll(void *payload,
                                  const void *name ATTRIBUTE_UNUSED,
                                  void *data)
 {
@@ -2850,6 +2850,7 @@ void qemuDomainSnapshotDiscardAll(void *payload,
                                     curr->metadata_only);
     if (err && !curr->err)
         curr->err = err;
+    return 0;
 }
 
 int

src/qemu/qemu_domain.h

@@ -401,7 +401,7 @@ struct _virQEMUSnapRemove {
     bool current;
 };
 
-void qemuDomainSnapshotDiscardAll(void *payload,
+int qemuDomainSnapshotDiscardAll(void *payload,
                                  const void *name,
                                  void *data);
 

src/qemu/qemu_driver.c

@@ -2240,7 +2240,7 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags)
 
 
 /* Count how many snapshots in a set are external snapshots or checkpoints.  */
-static void
+static int
 qemuDomainSnapshotCountExternal(void *payload,
                                 const void *name ATTRIBUTE_UNUSED,
                                 void *data)
@@ -2250,6 +2250,7 @@ qemuDomainSnapshotCountExternal(void *payload,
 
     if (virDomainSnapshotIsExternal(snap))
         (*count)++;
+    return 0;
 }
 
 static int
@@ -10770,7 +10771,7 @@ qemuDomainBlockResize(virDomainPtr dom,
 }
 
 
-static void
+static int
 qemuDomainBlockStatsGatherTotals(void *payload,
                                  const void *name ATTRIBUTE_UNUSED,
                                  void *opaque)
@@ -10791,6 +10792,7 @@ qemuDomainBlockStatsGatherTotals(void *payload,
     QEMU_BLOCK_STAT_TOTAL(rd_total_times);
     QEMU_BLOCK_STAT_TOTAL(flush_total_times);
 #undef QEMU_BLOCK_STAT_TOTAL
+    return 0;
 }
 
 
@@ -15548,7 +15550,7 @@ struct _virQEMUSnapReparent {
 };
 
 
-static void
+static int
 qemuDomainSnapshotReparentChildren(void *payload,
                                    const void *name ATTRIBUTE_UNUSED,
                                    void *data)
@@ -15557,7 +15559,7 @@ qemuDomainSnapshotReparentChildren(void *payload,
     virQEMUSnapReparentPtr rep = data;
 
     if (rep->err < 0)
-        return;
+        return 0;
 
     VIR_FREE(snap->def->parent);
     snap->parent = rep->parent;
@@ -15565,7 +15567,7 @@ qemuDomainSnapshotReparentChildren(void *payload,
     if (rep->parent->def &&
         VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) {
         rep->err = -1;
-        return;
+        return 0;
     }
 
     if (!snap->sibling)
@@ -15573,6 +15575,7 @@ qemuDomainSnapshotReparentChildren(void *payload,
 
     rep->err = qemuDomainSnapshotWriteMetadata(rep->vm, snap, rep->caps,
                                                rep->cfg->snapshotDir);
+    return 0;
 }
 
 

src/test/test_driver.c

@@ -6295,7 +6295,7 @@ struct _testSnapRemoveData {
     bool current;
 };
 
-static void
+static int
 testDomainSnapshotDiscardAll(void *payload,
                              const void *name ATTRIBUTE_UNUSED,
                              void *data)
@@ -6306,6 +6306,7 @@ testDomainSnapshotDiscardAll(void *payload,
     if (snap->def->current)
         curr->current = true;
     virDomainSnapshotObjListRemove(curr->vm->snapshots, snap);
+    return 0;
 }
 
 typedef struct _testSnapReparentData testSnapReparentData;
@@ -6317,7 +6318,7 @@ struct _testSnapReparentData {
     virDomainSnapshotObjPtr last;
 };
 
-static void
+static int
 testDomainSnapshotReparentChildren(void *payload,
                                    const void *name ATTRIBUTE_UNUSED,
                                    void *data)
@@ -6326,7 +6327,7 @@ testDomainSnapshotReparentChildren(void *payload,
     testSnapReparentDataPtr rep = data;
 
     if (rep->err < 0)
-        return;
+        return 0;
 
     VIR_FREE(snap->def->parent);
     snap->parent = rep->parent;
@@ -6334,11 +6335,12 @@ testDomainSnapshotReparentChildren(void *payload,
     if (rep->parent->def &&
         VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) {
         rep->err = -1;
-        return;
+        return 0;
     }
 
     if (!snap->sibling)
         rep->last = snap;
+    return 0;
 }
 
 static int

src/uml/uml_driver.c

@@ -724,7 +724,7 @@ struct umlProcessAutoDestroyData {
     virConnectPtr conn;
 };
 
-static void umlProcessAutoDestroyDom(void *payload,
+static int umlProcessAutoDestroyDom(void *payload,
                                     const void *name,
                                     void *opaque)
 {
@@ -738,17 +738,17 @@ static void umlProcessAutoDestroyDom(void *payload,
     VIR_DEBUG("conn=%p uuidstr=%s thisconn=%p", conn, uuidstr, data->conn);
 
     if (data->conn != conn)
-        return;
+        return 0;
 
     if (virUUIDParse(uuidstr, uuid) < 0) {
         VIR_WARN("Failed to parse %s", uuidstr);
-        return;
+        return 0;
     }
 
     if (!(dom = virDomainObjListFindByUUID(data->driver->domains,
                                            uuid))) {
         VIR_DEBUG("No domain object to kill");
-        return;
+        return 0;
     }
 
     VIR_DEBUG("Killing domain");
@@ -766,6 +766,7 @@ static void umlProcessAutoDestroyDom(void *payload,
     if (event)
         umlDomainEventQueue(data->driver, event);
     virHashRemoveEntry(data->driver->autodestroy, uuidstr);
+    return 0;
 }
 
 /*

src/util/virclosecallbacks.c

@@ -252,7 +252,7 @@ struct virCloseCallbacksData {
     bool oom;
 };
 
-static void
+static int
 virCloseCallbacksGetOne(void *payload,
                         const void *key,
                         void *opaque)
@@ -263,23 +263,24 @@ virCloseCallbacksGetOne(void *payload,
     unsigned char uuid[VIR_UUID_BUFLEN];
 
     if (virUUIDParse(uuidstr, uuid) < 0)
-        return;
+        return 0;
 
     VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p",
               closeDef->conn, data->conn, uuidstr, closeDef->cb);
 
     if (data->conn != closeDef->conn || !closeDef->cb)
-        return;
+        return 0;
 
     if (VIR_EXPAND_N(data->list->entries,
                      data->list->nentries, 1) < 0) {
         data->oom = true;
-        return;
+        return 0;
     }
 
     memcpy(data->list->entries[data->list->nentries - 1].uuid,
            uuid, VIR_UUID_BUFLEN);
     data->list->entries[data->list->nentries - 1].callback = closeDef->cb;
+    return 0;
 }
 
 static virCloseCallbacksListPtr

src/util/virhash.c

@@ -738,7 +738,7 @@ struct getKeysIter
     size_t arrayIdx;
 };
 
-static void virHashGetKeysIterator(void *payload,
+static int virHashGetKeysIterator(void *payload,
                                   const void *key, void *data)
 {
     struct getKeysIter *iter = data;
@@ -747,6 +747,7 @@ static void virHashGetKeysIterator(void *payload,
     iter->sortArray[iter->arrayIdx].value = payload;
 
     iter->arrayIdx++;
+    return 0;
 }
 
 typedef int (*qsort_comp)(const void *, const void *);

src/util/virhash.h

@@ -43,8 +43,10 @@ typedef void (*virHashDataFree) (void *payload, const void *name);
  * @data: user supplied data blob
  *
  * Callback to process a hash entry during iteration
+ *
+ * Returns -1 to stop the iteration, e.g. in case of an error
  */
-typedef void (*virHashIterator) (void *payload, const void *name, void *data);
+typedef int (*virHashIterator) (void *payload, const void *name, void *data);
 /**
  * virHashSearcher:
  * @payload: the data in the hash

src/xen/xm_internal.c

@@ -1118,7 +1118,7 @@ struct xenXMListIteratorContext {
     char ** names;
 };
 
-static void
+static int
 xenXMListIterator(void *payload ATTRIBUTE_UNUSED,
                   const void *name,
                   void *data)
@@ -1127,10 +1127,10 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED,
     virDomainDefPtr def = NULL;
 
     if (ctx->oom)
-        return;
+        return 0;
 
     if (ctx->count == ctx->max)
-        return;
+        return 0;
 
     def = xenDaemonLookupByName(ctx->conn, name);
     if (!def) {
@@ -1141,6 +1141,7 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED,
     } else {
         virDomainDefFree(def);
     }
+    return 0;
 }
 
 

tests/virhashtest.c

@@ -56,11 +56,12 @@ testHashInit(int size)
     return hash;
 }
 
-static void
+static int
 testHashCheckForEachCount(void *payload ATTRIBUTE_UNUSED,
                           const void *name ATTRIBUTE_UNUSED,
                           void *data ATTRIBUTE_UNUSED)
 {
+    return 0;
 }
 
 static int
@@ -183,7 +184,7 @@ testHashRemove(const void *data ATTRIBUTE_UNUSED)
 const int testHashCountRemoveForEachSome =
     ARRAY_CARDINALITY(uuids) - ARRAY_CARDINALITY(uuids_subset);
 
-static void
+static int
 testHashRemoveForEachSome(void *payload ATTRIBUTE_UNUSED,
                           const void *name,
                           void *data)
@@ -200,12 +201,13 @@ testHashRemoveForEachSome(void *payload ATTRIBUTE_UNUSED,
             break;
         }
     }
+    return 0;
 }
 
 
 const int testHashCountRemoveForEachAll = 0;
 
-static void
+static int
 testHashRemoveForEachAll(void *payload ATTRIBUTE_UNUSED,
                          const void *name,
                          void *data)
@@ -213,12 +215,13 @@ testHashRemoveForEachAll(void *payload ATTRIBUTE_UNUSED,
     virHashTablePtr hash = data;
 
     virHashRemoveEntry(hash, name);
+    return 0;
 }
 
 
 const int testHashCountRemoveForEachForbidden = ARRAY_CARDINALITY(uuids);
 
-static void
+static int
 testHashRemoveForEachForbidden(void *payload ATTRIBUTE_UNUSED,
                                const void *name,
                                void *data)
@@ -238,6 +241,7 @@ testHashRemoveForEachForbidden(void *payload ATTRIBUTE_UNUSED,
             break;
         }
     }
+    return 0;
 }
 
 
@@ -302,15 +306,15 @@ testHashSteal(const void *data ATTRIBUTE_UNUSED)
 }
 
 
-static void
+static int
 testHashIter(void *payload ATTRIBUTE_UNUSED,
              const void *name ATTRIBUTE_UNUSED,
              void *data ATTRIBUTE_UNUSED)
 {
-    return;
+    return 0;
 }
 
-static void
+static int
 testHashForEachIter(void *payload ATTRIBUTE_UNUSED,
                     const void *name ATTRIBUTE_UNUSED,
                     void *data)
@@ -332,6 +336,7 @@ testHashForEachIter(void *payload ATTRIBUTE_UNUSED,
     if (virHashForEach(hash, testHashIter, NULL) >= 0)
         VIR_TEST_VERBOSE("\niterating through hash in ForEach"
                 " should be forbidden");
+    return 0;
 }
 
 static int