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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]