Re: [PATCH v2] ses: Handle non-unique element descriptors

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Fri, 2011-07-29 at 16:32 +0530, Ankit Jain wrote:
> Some SES devices give non-unique Element Descriptors as part of the
> Element Descriptor diag page. Since we use these for creating sysfs
> entries, they need to be unique. The specification doesn't require
> these to be unique.
> 
> Eg:
> $ sg_ses -p 7 /dev/sg0
>   FTS CORP  TXS6_SAS20BPX12   0500
>     enclosure services device
> Element descriptor In diagnostic page:
>   generation code: 0x0
>   element descriptor by type list
>     Element type: Array device, subenclosure id: 0
>       Overall descriptor: ArrayDevicesInSubEnclsr0
>       Element 1 descriptor: ArrayDevice00
>       Element 2 descriptor: ArrayDevice01
>       Element 3 descriptor: ArrayDevice02
>       Element 4 descriptor: ArrayDevice03
>       Element 5 descriptor: ArrayDevice03
>       Element 6 descriptor: ArrayDevice03
>       Element 7 descriptor: ArrayDevice03
>       Element 8 descriptor: ArrayDevice03
>       Element 9 descriptor: ArrayDevice03
>       Element 10 descriptor: ArrayDevice03
>       Element 11 descriptor: ArrayDevice03
>       Element 12 descriptor: ArrayDevice03
> 
> Changes in v2:
> - Based on James' comments, in case of a non-unique name, it gets
>   named <elem_desc>_<elem_number>.
> 
> Based on scsi-misc
> 
> Signed-off-by: Ankit Jain <jankit@suse.de>
> 
> ---
> diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
> index 00e5fcac8..f4b53fd 100644
> --- a/drivers/misc/enclosure.c
> +++ b/drivers/misc/enclosure.c
> @@ -239,6 +239,23 @@ static void enclosure_component_release(struct device *dev)
>  	put_device(dev->parent);
>  }
>  
> +static struct enclosure_component *
> +enclosure_component_find_by_name(struct enclosure_device *edev,
> +				const char *name)
> +{
> +	int i;
> +	if (!edev || !name || !name[0])
> +		return NULL;
> +
> +	for (i=0; i<edev->components; i++) {
> +		struct enclosure_component *ecomp = &edev->component[i];
> +		if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name))
> +			return ecomp;
> +	}
> +
> +	return NULL;
> +}
> +
>  static const struct attribute_group *enclosure_groups[];
>  
>  /**
> @@ -276,9 +293,13 @@ enclosure_component_register(struct enclosure_device *edev,
>  	ecomp->number = number;
>  	cdev = &ecomp->cdev;
>  	cdev->parent = get_device(&edev->edev);
> -	if (name && name[0])
> -		dev_set_name(cdev, "%s", name);
> -	else
> +
> +	if (name && name[0]) {
> +		if (enclosure_component_find_by_name (edev, name))
> +			dev_set_name(cdev, "%s_%u", name, number);

Dash is probably more consistent with the rest of sysfs than underscore.

Also, you want to check for just in case clashes with the new name, so
something like:

char newname[64];
i=1;
ssprintf(newname, "%s", name)
while (enclosure_component_find_by_name (edev, newname))
	ssprintf(newname, "%s-%s", name, i++);
dev_set_name(cdev, "%s", newname);

This also counts from one rather than using the random component number,
so if you have 15 slots all called "slot", they appear as "slot",
"slot-1" "slot-2" etc.

James