Re: [libvirt] [PATCH 3/4] storage: Introduce virStoragePoolObjListSearch
On Thu, Nov 16, 2017 at 11:58:04AM -0500, John Ferlan wrote:
Create an API to search through the storage pool objects looking for
a specific truism from a callback API in order to return the specific
storage pool object that is desired.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
As for the searcher that is a copy-paste of the 'for each iterator' with a
single difference, I assume the list is going to be converted to a hash table
further on, right?
...
+
+static bool
+storagePoolLookupByTargetPathCallback(virStoragePoolObjPtr obj,
+ const void *opaque)
+{
+ const char *path = opaque;
+ virStoragePoolDefPtr def;
+
+ if (!virStoragePoolObjIsActive(obj))
+ return false;
+
+ def = virStoragePoolObjGetDef(obj);
+ return STREQ(path, def->target.path);
+}
+
+
virStoragePoolPtr
storagePoolLookupByTargetPath(virConnectPtr conn,
const char *path)
{
- size_t i;
+ virStoragePoolObjPtr obj;
+ virStoragePoolDefPtr def;
virStoragePoolPtr pool = NULL;
char *cleanpath;
cleanpath = virFileSanitizePath(path);
if (!cleanpath)
return NULL;
+ VIR_FREE(cleanpath);
Existing prior your patch, but I'm clearly missing the point of running
virFileSanitizePath here, since it can only return NULL on a failed allocation
and the way we're treating it now is "it either failed or some sanitization
may or may not have happened" - my question therefore is, why don't we care
about the result of sanitizing the path but we have to run it anyway?
storageDriverLock();
- for (i = 0; i < driver->pools.count && !pool; i++) {
- virStoragePoolObjPtr obj = driver->pools.objs[i];
- virStoragePoolDefPtr def;
-
- virStoragePoolObjLock(obj);
+ if ((obj == virStoragePoolObjListSearch(&driver->pools,
+ storagePoolLookupByTargetPathCallback,
+ path))) {
def = virStoragePoolObjGetDef(obj);
-
- if (!virStoragePoolObjIsActive(obj)) {
- virStoragePoolObjEndAPI(&obj);
- continue;
- }
-
- if (STREQ(path, def->target.path))
- pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
-
+ pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
virStoragePoolObjEndAPI(&obj);
}
storageDriverUnlock();
@@ -1672,7 +1689,6 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
path);
}
- VIR_FREE(cleanpath);
return pool;
}
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>