[PATCH] path_id: Re-introduce SAS phy enumeration of devices

[Nils Carlson]

When path_id was converted to C code the enumeration of SAS
devices by phy disappeared. This patch reintroduces enumeration
of the form

pci-0000:05:00.0-sas-phy0:1-0x500000e114de2b42:0-lun0

where phy0:1 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected and 1 is the number of phys on the port.

Please test this patch thoroughly as it has only been tested
with an older version of udev.

Signed-off-by: Nils Carlson <***@ericsson.com>
---
src/udev-builtin-path_id.c | 80 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/src/udev-builtin-path_id.c b/src/udev-builtin-path_id.c
index a8559d2..c575fd4 100644
--- a/src/udev-builtin-path_id.c
+++ b/src/udev-builtin-path_id.c
@@ -121,13 +121,67 @@ out:
return parent;
}

+static int get_sas_port_phy_id(struct udev *udev, const char *phy_name)
+{
+ struct udev_device *phydev;
+ const char *phy_id_str;
+ int phy_id = -1;
+
+ phydev = udev_device_new_from_subsystem_sysname(udev, "sas_phy",
+ phy_name);
+ if (phydev == NULL) {
+ return -1;
+ }
+
+ phy_id_str = udev_device_get_sysattr_value(phydev, "phy_identifier");
+ if (phy_id_str == NULL) {
+ goto out;
+ }
+
+ phy_id = atoi(phy_id_str);
+
+out:
+ udev_device_unref(phydev);
+
+ return phy_id;
+}
+
+static int get_sas_port_num_phys(struct udev *udev, const char *port_name)
+{
+ struct udev_device *portdev;
+ const char *num_phys_str;
+ int num_phys = -1;
+
+ portdev = udev_device_new_from_subsystem_sysname(udev, "sas_port",
+ port_name);
+ if (portdev == NULL) {
+ return -1;
+ }
+
+ num_phys_str = udev_device_get_sysattr_value(portdev, "num_phys");
+ if (num_phys_str == NULL) {
+ goto out;
+ }
+
+ num_phys = atoi(num_phys_str);
+
+out:
+ udev_device_unref(portdev);
+
+ return num_phys;
+}
+
static struct udev_device *handle_scsi_sas(struct udev_device *parent, char **path)
{
struct udev *udev = udev_device_get_udev(parent);
struct udev_device *targetdev;
struct udev_device *target_parent;
struct udev_device *sasdev;
+ struct udev_device *portdev;
+ struct udev_list_entry *list_entry;
const char *sas_address;
+ int tmp_phy_id, phy_id = 255;
+ int num_phys;
char *lun = NULL;

targetdev = udev_device_get_parent_with_subsystem_devtype(parent, "scsi", "scsi_target");
@@ -138,6 +192,29 @@ static struct udev_device *handle_scsi_sas(struct udev_device *parent, char **pa
if (target_parent == NULL)
return NULL;

+ portdev = udev_device_get_parent(target_parent);
+ if (target_parent == NULL)
+ return NULL;
+
+ udev_list_entry_foreach(list_entry,
+ udev_device_get_sysattr_list_entry(portdev)) {
+ const char *name = udev_list_entry_get_name(list_entry);
+
+ if (strncmp(name, "phy", 3) != 0)
+ continue;
+
+ tmp_phy_id = get_sas_port_phy_id(udev, name);
+ if (tmp_phy_id >= 0 && tmp_phy_id < phy_id)
+ phy_id = tmp_phy_id;
+ }
+ if (phy_id == 255)
+ return NULL;
+
+ num_phys = get_sas_port_num_phys(udev,
+ udev_device_get_sysname(portdev));
+ if (num_phys < 0)
+ return NULL;
+
sasdev = udev_device_new_from_subsystem_sysname(udev, "sas_device",
udev_device_get_sysname(target_parent));
if (sasdev == NULL)
@@ -150,7 +227,8 @@ static struct udev_device *handle_scsi_sas(struct udev_device *parent, char **pa
}

format_lun_number(parent, &lun);
- path_prepend(path, "sas-%s-%s", sas_address, lun);
+ path_prepend(path, "sas-phy%d:%d-%s-%s", phy_id, num_phys,
+ sas_address, lun);
if (lun)
free(lun);
out:

[Hannes Reinecke]

Hi Nils,

Thanks a lot for this. It definitely a step in the correct

Post by Nils Carlson
When path_id was converted to C code the enumeration of SAS
devices by phy disappeared. This patch reintroduces enumeration
of the form
=20
pci-0000:05:00.0-sas-phy0:1-0x500000e114de2b42:0-lun0
=20
where phy0:1 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected and 1 is the number of phys on the port.
=20

I would rather not do this.
=46irst of all, I doubt we need the overall number of phys here.
Secondly, James B. assured me that the phy enumeration is pretty much
stable, so we should be able to use that number as-is.

Also, there is a 1:1 match between the 'phy_identifier' sysfs attribute
and the second number of the phy name itself (the first number is the
SCSI host number), so we could as well just parse the phy name and get
the number from there.
But, of course, reading the phy_identifier is okay, too.

So I would propose just to insert a 'phy1' there.

But again, thanks for doing it.

Cheers,

Hannes
--=20
Dr. Hannes Reinecke zSeries & Storage
***@suse.de [phone]
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg
GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug=
" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Nils Carlson]

Hi Hannes,

Post by Hannes Reinecke
Hi Nils,
Thanks a lot for this. It definitely a step in the correct

Post by Nils Carlson
where phy0:1 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected and 1 is the number of phys on the port.

I would rather not do this.
First of all, I doubt we need the overall number of phys here.
Secondly, James B. assured me that the phy enumeration is pretty much
stable, so we should be able to use that number as-is.

I agree this would be clean, the problem is that it represents an API
change; one could argue though that that API change has already been
made through the breakage though.

Post by Hannes Reinecke
Also, there is a 1:1 match between the 'phy_identifier' sysfs attribute
and the second number of the phy name itself (the first number is the
SCSI host number), so we could as well just parse the phy name and get
the number from there.
But, of course, reading the phy_identifier is okay, too

Yes, this would of course also work, though I think reading the
phy_identifier is cleaner.
.

Post by Hannes Reinecke
So I would propose just to insert a 'phy1' there.

I will need to check that nobody has coded the phyX:Y format into any
scripts, hopefully the answer is no.

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