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.
Jan
> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list