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

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



On 2012年10月22日 01:11, Cole Robinson wrote:
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.

Makes sense. ACK


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