[libvirt] [PATCH] storage: Do not break the whole vol lookup process in the middle

[Eric Blake]

On 09/21/2011 04:36 AM, Osier Yang wrote:
> * src/storage/storage_driver.c: As virStorageVolLookupByPath lookups
> all the pool objs of the drivers, breaking when failing on getting
> the stable path of the pool will just breaks the lookup process, it
> can cause the API fails even if the vol exists indeed. It won't get
> any benifit. This patch is to fix it.

s/benifit/benefit/

> ---
>   src/storage/storage_driver.c |   14 ++++++--------
>   1 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index c05b74e..8a3b5be 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1247,14 +1247,13 @@ storageVolumeLookupByPath(virConnectPtr conn,
>
>               stable_path = virStorageBackendStablePath(driver->pools.objs[i],
>                                                         cleanpath);
> -            /*
> -             * virStorageBackendStablePath already does
> -             * virStorageReportError if it fails; we just need to keep
> -             * propagating the return code
> -             */
>               if (stable_path == NULL) {
> -                virStoragePoolObjUnlock(driver->pools.objs[i]);
> -                goto cleanup;
> +                /* Don't break the whole lookup process if fails on

s/if fails/if it fails/

> +                 * getting the stabe path for some of the pool.

s/stabe/stable/ s/pool./pools./

> +                 */
> +                VIR_WARN("Failed to get stable path for pool '%s'",
> +                         driver->pools.objs[i]->def->name);
> +                continue;

You still need to call virStoragePoolObjUnlock prior to continue, 
otherwise you have a resource leak.

>               }
>
>               vol = virStorageVolDefFindByPath(driver->pools.objs[i],
> @@ -1274,7 +1273,6 @@ storageVolumeLookupByPath(virConnectPtr conn,
>           virStorageReportError(VIR_ERR_NO_STORAGE_VOL,
>                                 "%s", _("no storage vol with matching path"));
>
> -cleanup:
>       VIR_FREE(cleanpath);
>       storageDriverUnlock(driver);
>       return ret;

I like the idea, but it would be worth seeing a v2.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org