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

... >> + /* 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?

Wouldn't it be better to tell the user that "/dev" is not a valid stable path name... The path really needs to be more specific... I suppose one could change virStorageBackendStablePath to accept "/dev" and do the search, but that "could" take a while depending on the size of the "/dev" file system. John -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list