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