[PATCH v1 1/1] [SCSI] enclosure: Handle non-unique element descriptors

[Markus Stockhausen]

[SCSI] enclosure: Handle non-unique element descriptors

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

Based on http://thread.gmane.org/gmane.linux.scsi/69289. This
version implements James' ideas about the naming convention

Signed-off-by: Markus Stockhausen <***@collogia.de>

diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 2cf2bbc..824a8d7 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -187,6 +187,7 @@ void enclosure_unregister(struct enclosure_device *edev)
EXPORT_SYMBOL_GPL(enclosure_unregister);

#define ENCLOSURE_NAME_SIZE 64
+#define COMPONENT_NAME_SIZE 64

static void enclosure_link_name(struct enclosure_component *cdev, char *name)
{
@@ -246,6 +247,29 @@ static void enclosure_component_release(struct device *dev)
put_device(dev->parent);
}

+struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *edev,
+ const char *name)
+{
+ int i;
+ const char *cname;
+ struct enclosure_component *ecomp;
+
+ if (!edev || !name || !name[0])
+ return NULL;
+
+ for (i=0; i<edev->components; i++) {
+ ecomp = &edev->component[i];
+ cname = dev_name(&ecomp->cdev);
+ if (ecomp->number != -1 &&
+ cname && cname[0] &&
+ !strcmp(cname, name))
+ return ecomp;
+ }
+
+ return NULL;
+}
+
static const struct attribute_group *enclosure_component_groups[];

/**
@@ -269,7 +293,8 @@ enclosure_component_register(struct enclosure_device *edev,
{
struct enclosure_component *ecomp;
struct device *cdev;
- int err;
+ int err, i;
+ char newname[COMPONENT_NAME_SIZE];

if (number >= edev->components)
return ERR_PTR(-EINVAL);
@@ -283,9 +308,21 @@ 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]) {
+ /* Some hardware (e.g. enclosure in RX300 S6) has components
+ * with non unique names. Registering duplicates in sysfs
+ * will lead to warnings during bootup. So make the names
+ * unique by appending consecutive numbers -1, -2, ... */
+ i = 1;
+ snprintf(newname, COMPONENT_NAME_SIZE,
+ "%s", name);
+ while (i < 255 &&
+ enclosure_component_find_by_name (edev, newname))
+ snprintf(newname, COMPONENT_NAME_SIZE,
+ "%s-%i", name, i++);
+ dev_set_name(cdev, "%s", newname);
+ } else
dev_set_name(cdev, "%u", number);

cdev->release = enclosure_component_release;

[James Bottomley]

Post by Markus Stockhausen
[SCSI] enclosure: Handle non-unique element descriptors
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.
$ sg_ses -p 7 /dev/sg0
FTS CORP TXS6_SAS20BPX12 0500
enclosure services device
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
Based on http://thread.gmane.org/gmane.linux.scsi/69289. This
version implements James' ideas about the naming convention
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 2cf2bbc..824a8d7 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -187,6 +187,7 @@ void enclosure_unregister(struct enclosure_device *edev)
EXPORT_SYMBOL_GPL(enclosure_unregister);
#define ENCLOSURE_NAME_SIZE 64
+#define COMPONENT_NAME_SIZE 64
static void enclosure_link_name(struct enclosure_component *cdev, char *name)
{
@@ -246,6 +247,29 @@ static void enclosure_component_release(struct device *dev)
put_device(dev->parent);
}
+struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *edev,
+ const char *name)
+{
+ int i;
+ const char *cname;
+ struct enclosure_component *ecomp;
+
+ if (!edev || !name || !name[0])
+ return NULL;
+
+ for (i=0; i<edev->components; i++) {
+ ecomp = &edev->component[i];
+ cname = dev_name(&ecomp->cdev);
+ if (ecomp->number != -1 &&
+ cname && cname[0] &&
+ !strcmp(cname, name))
+ return ecomp;
+ }
+
+ return NULL;
+}
+
static const struct attribute_group *enclosure_component_groups[];
/**
@@ -269,7 +293,8 @@ enclosure_component_register(struct enclosure_device *edev,
{
struct enclosure_component *ecomp;
struct device *cdev;
- int err;
+ int err, i;
+ char newname[COMPONENT_NAME_SIZE];
if (number >= edev->components)
return ERR_PTR(-EINVAL);
@@ -283,9 +308,21 @@ 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]) {
+ /* Some hardware (e.g. enclosure in RX300 S6) has components
+ * with non unique names. Registering duplicates in sysfs
+ * will lead to warnings during bootup. So make the names
+ * unique by appending consecutive numbers -1, -2, ... */
+ i = 1;
+ snprintf(newname, COMPONENT_NAME_SIZE,
+ "%s", name);
+ while (i < 255 &&
+ enclosure_component_find_by_name (edev, newname))
+ snprintf(newname, COMPONENT_NAME_SIZE,
+ "%s-%i", name, i++);

We need error handling here (what happens when i becomes 255). Since
it's iterating through a unique name space, I think you can just let the
loop be unbounded because if it's not, the system will run out of memory
before we get into an apparently endless loop.

After fixing this, you can add my acked-by.

James

Post by Markus Stockhausen
+ dev_set_name(cdev, "%s", newname);
+ } else
dev_set_name(cdev, "%u", number);
cdev->release = enclosure_component_release;

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Markus Stockhausen]

Gesendet: Mittwoch, 1. Oktober 2014 17:31
An: Markus Stockhausen
Betreff: Re: [PATCH v1 1/1] [SCSI] enclosure: Handle non-unique element descriptors

Post by Markus Stockhausen
[SCSI] enclosure: Handle non-unique element descriptors

...

Post by Markus Stockhausen
+
+ if (name && name[0]) {
+ /* Some hardware (e.g. enclosure in RX300 S6) has components
+ * with non unique names. Registering duplicates in sysfs
+ * will lead to warnings during bootup. So make the names
+ * unique by appending consecutive numbers -1, -2, ... */
+ i = 1;
+ snprintf(newname, COMPONENT_NAME_SIZE,
+ "%s", name);
+ while (i < 255 &&
+ enclosure_component_find_by_name (edev, newname))
+ snprintf(newname, COMPONENT_NAME_SIZE,
+ "%s-%i", name, i++);

We need error handling here (what happens when i becomes 255). Since
it's iterating through a unique name space, I think you can just let the
loop be unbounded because if it's not, the system will run out of memory
before we get into an apparently endless loop.
After fixing this, you can add my acked-by.
James

Hi James.

Just to make sure I get it right. My intention was to ensure that we will
not loop endlessly. Thinking about the corner cases I was unsure if we
might encounter component names with up to 63 chars. If yes appending
the numbers will not not change the generated names throughout the
loop. So ensure that we will stop somewhere by giving it an upper bound.
In the error case we will have the same behaviour as now. Creating an
already existing filename.

You are more experienced with that. So if i drop the "i<255" it will be
fine and can be implemented?

Markus