[libvirt] [PATCH] storage: Don't do wait loops from VolLookupByPath

Cole Robinson crobinso at redhat.com
Sun Oct 21 17:11:50 UTC 2012


virStorageVolLookupByPath is an API call that virt-manager uses
quite a bit when dealing with storage. This call use BackendStablePath
which has several usleep() heuristics that can be tripped up
and hang virt-manager for a while.

Current example: an empty mpath pool pointing to /dev/mapper makes
_any_ calls to virStorageVolLookupByPath take 5 seconds.

The sleep heuristics are actually only needed in certain cases
when we are waiting for new storage to appear, so let's skip the
timeout steps when calling from LookupByPath.
---
 src/storage/storage_backend.c      | 12 ++++++++----
 src/storage/storage_backend.h      |  3 ++-
 src/storage/storage_backend_disk.c |  2 +-
 src/storage/storage_backend_scsi.c |  3 ++-
 src/storage/storage_driver.c       |  3 ++-
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index aea70e2..85b8287 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1338,10 +1338,14 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target,
  *
  * Typically target.path is one of the /dev/disk/by-XXX dirs
  * with stable paths.
+ *
+ * If 'wait' is true, we use a timeout loop to give dynamic paths
+ * a change to appear.
  */
 char *
 virStorageBackendStablePath(virStoragePoolObjPtr pool,
-                            const char *devpath)
+                            const char *devpath,
+                            bool wait)
 {
     DIR *dh;
     struct dirent *dent;
@@ -1372,7 +1376,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
  reopen:
     if ((dh = opendir(pool->def->target.path)) == NULL) {
         opentries++;
-        if (errno == ENOENT && opentries < 50) {
+        if (wait && errno == ENOENT && opentries < 50) {
             usleep(100 * 1000);
             goto reopen;
         }
@@ -1387,7 +1391,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
      * the target directory and figure out which one points
      * to this device node.
      *
-     * And it might need some time till the stabe path shows
+     * And it might need some time till the stable path shows
      * up, so add timeout to retry here.
      */
  retry:
@@ -1411,7 +1415,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
         VIR_FREE(stablepath);
     }
 
-    if (++retry < 100) {
+    if (wait && ++retry < 100) {
         usleep(100 * 1000);
         goto retry;
     }
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index a67eeb7..71935a7 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -130,7 +130,8 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target,
                                         int fd);
 
 char *virStorageBackendStablePath(virStoragePoolObjPtr pool,
-                                  const char *devpath);
+                                  const char *devpath,
+                                  bool wait);
 
 typedef int (*virStorageBackendListVolRegexFunc)(virStoragePoolObjPtr pool,
                                                  char **const groups,
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 5d9e72f..3cd4362 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -83,7 +83,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
          * dir every time its run. Should figure out a more efficient
          * way of doing this...
          */
-        vol->target.path = virStorageBackendStablePath(pool, devpath);
+        vol->target.path = virStorageBackendStablePath(pool, devpath, true);
         VIR_FREE(devpath);
         if (vol->target.path == NULL)
             return -1;
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 27dcbb6..4e832eb 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -246,7 +246,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
      * way of doing this...
      */
     if ((vol->target.path = virStorageBackendStablePath(pool,
-                                                        devpath)) == NULL) {
+                                                        devpath,
+                                                        true)) == NULL) {
         retval = -1;
         goto free_vol;
     }
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 28829d3..4fbc0c0 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1318,7 +1318,8 @@ storageVolumeLookupByPath(virConnectPtr conn,
             const char *stable_path;
 
             stable_path = virStorageBackendStablePath(driver->pools.objs[i],
-                                                      cleanpath);
+                                                      cleanpath,
+                                                      false);
             if (stable_path == NULL) {
                 /* Don't break the whole lookup process if it fails on
                  * getting the stable path for some of the pools.
-- 
1.7.11.7




More information about the libvir-list mailing list