On 07/04/2011 09:05 PM, James Bottomley wrote:
> On Mon, 2011-07-04 at 20:46 +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.
>>
>> 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
>
> What is the external visible labelling of this topology? It's
> completely weird that the enclosure would burn in non-unique names
> unless there's some reason for it.
I'm not sure what you mean by "external visible labelling". The system has a
SAS expanded backplane. I don't have access to the hardware now, but p7 looked
like this:
$ 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
Element type: SAS connector, subenclosure id: 0
Overall descriptor: ConnectorsInSubEnclsr0
Element 1 descriptor: Connector04
Element 2 descriptor: Connector05
Element 3 descriptor: Connector06
Element 4 descriptor: Connector07
Element 5 descriptor: Connector00
Element 6 descriptor: Connector01
Element 7 descriptor: Connector02
Element 8 descriptor: Connector03
Element 9 descriptor: Connector08
Element 10 descriptor: Connector09
Element 11 descriptor: Connector10
Element 12 descriptor: Connector11
Element type: Temperature sense, subenclosure id: 0
Overall descriptor: TempSensorsInSubEnclsr0
Element 1 descriptor: TempSense01
Element 2 descriptor: TempSense02
Element 3 descriptor: TempSense03
Element type: Voltage sensor, subenclosure id: 0
Overall descriptor: VoltageSensorsInSubEnclsr0
Element 1 descriptor: VoltageSense01
Element 2 descriptor: VoltageSense02
Element 3 descriptor: VoltageSense03
Element 4 descriptor: VoltageSense04
Element type: Current sensor, subenclosure id: 0
Overall descriptor: CurrentSensorsInSubEnclsr0
Element 1 descriptor: CurrentSense01
Element type: Enclosure, subenclosure id: 0
Overall descriptor: EnclosureElementInSubEnclsr0
Element 1 descriptor: EnclosureElement01
Element type: SAS expander, subenclosure id: 0
Overall descriptor: SAS Expander
Element 1 descriptor: Expander0
AFAICS, the spec doesn't seem to say that the names have to be unique.
But, now that I think about it a bit, possibly some of the "array device
slots" might have been vacant, and so descriptors could have been repeated.
This could probably be checked with the status page.
But then I'm not sure, whether sysfs entries should or shouldn't be created
for some cases (eg. status code!=OK).
>> - if (create)
>> + if (create) {
>> + if (enclosure_component_find_by_name(edev, name))
>> + /* name is not unique, already used
>> + * set to NULL, so that enclosure_component_register
>> + * will assign us a new one */
>> + name = NULL;
>
> This just assigns a random name ... if we actually have one, we should
> probably just make it unique.
Agreed. Would something like (below) be acceptable? I'll post the patch
separately, if it is:
---
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 00e5fca..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);
+ else
+ dev_set_name(cdev, "%s", name);
+ } else
dev_set_name(cdev, "%u", number);
cdev->release = enclosure_component_release;
--
Ankit Jain
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
