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);