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@xxxxxxxxxx>
> ---
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@xxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
