[libvirt] [PATCH 3/4] storage_conf: Fix the scsi_host.name comparison

[Ján Tomko]

On 09/30/2014 11:35 PM, John Ferlan wrote:
> Since the 'scsi_host#' concept was introduced in commit id '9f781da6' it
> is possible to provide either "name='host#'" or "name='scsi_host#'" to
> define/name the scsi_host adapter source address.  The concept being
> that 'name#' is/was legacy and 'scsi_host#' is what's seen in the
> 'virsh nodedev-list [--cap scsi_host]' output.  However, in doing the
> comparison for a to be defined scsi_host, a latent bug allows the same
> source to be defined twice, e.g. "name='host5'" is not distiguished
> from "name='scsi_host5'", although in reality they eventually become
> the same thing in commit id 'c1f63a9b' with the introduction of the
> getHostNumber() API.
> 
> So this change will ensure that no one can use 'host5' and 'scsi_host5'
> (or whatever #) to resolve to the same source adapter when going through
> the define the source adapter processing and checking for duplicates. Doing
> so will now result in an error (assuming that an existing pool is using
> 'host5' and the to be defined pool tries 'scsi_host5'):
> 
> $ virsh pool-define scsi-pool-use-scsi_host.xml
> error: Failed to define pool from scsi-pool-use-scsi_host.xml
> error: operation failed: Storage source conflict with pool: 'scsi-pool-use-host
> $
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/storage_conf.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index ac41307..74267bd 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -2063,6 +2063,27 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
>  }
>  
>  static bool
> +matchSCSIAdapterName(const char *pool_name,
> +                     const char *def_name)
> +{
> +    /* Names can be either "scsi_host#" or just "host#", where
> +     * "host#" is the back-compat format, but both equate to
> +     * the same source adapter.  First check if both pool and def
> +     * are using same format (easier) - if so, then compare
> +     */
> +    if ((STRPREFIX(pool_name, "scsi_") && STRPREFIX(def_name, "scsi")) ||
> +        (STRPREFIX(pool_name, "host") && STRPREFIX(def_name, "host")))
> +        return STREQ(pool_name, def_name);
> +
> +    /* If pool uses "scsi_host#" and def uses "host#", deal with that here */
> +    if (STRPREFIX(pool_name, "scsi_"))
> +        return STREQ(&pool_name[5], def_name);
> +
> +    /* Otherwise we have pool with "host#" and def with "scsi_host#" */
> +    return STREQ(pool_name, &def_name[5]);
> +}

fc_host prefix is not handled here, but getHostNumber will allow it. Maybe the
checks should be shared? (as long as we don't error out on unknown prefixes,
since we didn't validate the adapter name in the past).

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141003/a62ee30e/attachment-0001.sig>