On 10/22/2012 03:25 AM, Osier Yang wrote:
> 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
Thanks Osier, pushed now.
- Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list