On Tue, Apr 21, 2015 at 06:18:56AM -0400, John Ferlan wrote:
>
>
> 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);
>
ACK
Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list