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

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.