[libvirt] [PATCH v2 6/6] iscsi: Add checks for non standard stable target.path

[Ján Tomko]

On Wed, Apr 01, 2015 at 01:29:11PM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1171933
> 
> If a non stable path is provided for the pool's target path, check to
> see if the directory exists before allowing pool startup; otherwise,

The only non-stable path that can possibly result in a non-empty pool is '/dev/'.

> later in the processLU calls to find LUN's all that happens is the
> volume target.path will get the strdup'd value of the pool target.path
> (which doesn't exist), so attempts to find the LU are unsuccessful
> resulting in a started pool with no devices listed even though the
> block devices for the iSCSI LU's do exist.
> 
> Additionally if the non stable path does exist and it's determined no
> targets are found, then force failure in the refresh path.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_iscsi.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
> index fba037f..b5a15b1 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -149,6 +149,15 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
>      if (virStorageBackendSCSIFindLUs(pool, host) < 0)
>          goto cleanup;
>  
> +    if (pool->volumes.count == 0 &&

This is always true if the path does not start with /dev.
https://www.redhat.com/archives/libvir-list/2015-March/msg01578.html

> +        !STRPREFIX(pool->def->target.path, "/dev")) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Non stable target path '%s' for pool '%s' "
> +                         "found no target volumes"),
> +                       pool->def->target.path, pool->def->name);
> +        return -1;
> +    }
> +
>      retval = 0;
>  
>   cleanup:

> @@ -393,6 +402,15 @@ virStorageBackendISCSIStartPool(virConnectPtr conn,
>          return -1;
>      }
>  
> +    if (!STRPREFIX(pool->def->target.path, "/dev") &&
> +        !virFileExists(pool->def->target.path)) {

If you remove the virFileExists condition, you don't need the error
check in the first hunk.

Jan

> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Non stable target path '%s' not found for pool '%s'"),
> +                       pool->def->target.path, pool->def->name);
> +        return -1;
> +    }
> +
> +
>      if ((session = virStorageBackendISCSISession(pool, true)) == NULL) {
>          if ((portal = virStorageBackendISCSIPortal(&pool->def->source)) == NULL)
>              goto cleanup;
> -- 
> 2.1.0
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150402/869c3395/attachment-0001.sig>