Re: [libvirt] [PATCH v4 2/4] scsi: Adjust return value for virStorageBackendSCSINewLun

On 04/21/2015 03:13 AM, Ján Tomko wrote: > On Mon, Apr 20, 2015 at 12:50:07PM -0400, John Ferlan wrote: >> >> ... >> >>>> + /* Check if the pool is using a stable target path. The call to >>>> + * virStorageBackendStablePath will fail if the pool target path >>>> + * isn't stable and just return the strdup'd 'devpath' anyway. >>>> + * This would be indistinguishable to failing to find the stable >>>> + * path to the device if the virDirRead loop to search the >>>> + * target pool path for our devpath had failed. >>>> + */ >>>> + if (!virStorageBackendPoolPathIsStable(pool->def->target.path)) { >>>> + virReportError(VIR_ERR_INVALID_ARG, >>>> + _("unable to use target path '%s' for dev '%s'"), >>>> + NULLSTR(pool->def->target.path), dev); >>>> + goto cleanup; >>>> + } >>> >>> /dev is a valid non-stable pool target path. >>> >>>> + >>>> if (VIR_ALLOC(vol) < 0) >>>> goto cleanup; >>>> >>>> @@ -187,13 +211,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, >>>> true)) == NULL) >>>> goto cleanup; >>>> >>>> - if (STREQ(devpath, vol->target.path) && >>>> - !(STREQ(pool->def->target.path, "/dev") || >>>> - STREQ(pool->def->target.path, "/dev/"))) { >>>> + if (STREQ(devpath, vol->target.path)) { >>>> >>> >>> Before, when virStorageBackendStablePath returned the same devpath because >>> the pool path was "/dev", we continued with processing the volume. >>> >>> After this patch, we won't even get here because of the first check. >>> >>> Failure to stabilize the path should be expected here, if the pool >>> target path is not stable. >>> >> >> OK, but because virStorageBackendStablePath won't process the >> pool->target.path of "/dev", we'll return the duplicated 'devpath' and >> return -2 for every volume in the pool thus making it look empty. >> >> What good is that? >> > > None. We should process the volume instead of returning -2. > Does the following squashed in work for you? Essentially restoring the /dev || /dev/ check after virStorageBackendStablePath and adding it to the virStorageBackendPoolPathIsStable ? iff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index ae3cd9a..b97b2b0 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -175,7 +175,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, * path to the device if the virDirRead loop to search the * target pool path for our devpath had failed. */ - if (!virStorageBackendPoolPathIsStable(pool->def->target.path)) { + if (!virStorageBackendPoolPathIsStable(pool->def->target.path) && + !(STREQ(pool->def->target.path, "/dev") || + STREQ(pool->def->target.path, "/dev/"))) { virReportError(VIR_ERR_INVALID_ARG, _("unable to use target path '%s' for dev '%s'"), NULLSTR(pool->def->target.path), dev); @@ -211,7 +213,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, true)) == NULL) goto cleanup; - if (STREQ(devpath, vol->target.path)) { + if (STREQ(devpath, vol->target.path) && + !(STREQ(pool->def->target.path, "/dev") || + STREQ(pool->def->target.path, "/dev/"))) { VIR_DEBUG("No stable path found for '%s' in '%s'", devpath, pool->def->target.path);