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

[Nils Carlson]

Changes since v1:
* Remove the number of phys from the output
* Extract the phy identifier from the symlink, less code.

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-0x500000e114de2b42:0-lun0

where phy0 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected.

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 | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/udev-builtin-path_id.c b/src/udev-builtin-path_id.c
index a8559d2..54b22c4 100644
--- a/src/udev-builtin-path_id.c
+++ b/src/udev-builtin-path_id.c
@@ -127,7 +127,10 @@ static struct udev_device *handle_scsi_sas(struct udev_device *parent, char **pa
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;
char *lun = NULL;

targetdev = udev_device_get_parent_with_subsystem_devtype(parent, "scsi", "scsi_target");
@@ -138,6 +141,31 @@ 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);
+ char *phy_id_str;
+
+ if (strncmp(name, "phy", 3) != 0)
+ continue;
+
+ phy_id_str = strstr(name, ":");
+ if (phy_id_str == NULL)
+ continue;
+
+ phy_id_str++;
+
+ tmp_phy_id = atoi(phy_id_str);
+ if (tmp_phy_id >= 0 && tmp_phy_id < phy_id)
+ phy_id = tmp_phy_id;
+ }
+ if (phy_id == 255)
+ return NULL;
+
sasdev = udev_device_new_from_subsystem_sysname(udev, "sas_device",
udev_device_get_sysname(target_parent));
if (sasdev == NULL)
@@ -150,7 +178,7 @@ 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-%s-%s", phy_id, sas_address, lun);
if (lun)
free(lun);
out:

[Kay Sievers]

Post by Nils Carlson
This patch reintroduces enumeration
of the form
pci-0000:05:00.0-sas-phy0-0x500000e114de2b42:0-lun0
where phy0 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected.

Without having any real insight knowledge, a quick check:

We usually refuse any "find the lowest number approach" in new code,
and require the kernel to export a stable instance number per parent
device, which is entirely disconnected from any global device
enumeration counter or prober ordering.

Almost all existing examples doing that are broken, and some even
needed to be removed because they just blindly bet on luck that things
are always in the same probe order.

Many device can individually unbound and rebound in the kernel, so
after the rebind the same device will no longer have a smaller number
like it had before. Kernel enumeration is only in a very few cases
useful to build persistent paths.

The approach in this case is safe against individual pieces being
unregistered and registered again?

Kay
--
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

[Hannes Reinecke]

Post by Kay Sievers
=20

Post by Nils Carlson
This patch reintroduces enumeration
of the form
pci-0000:05:00.0-sas-phy0-0x500000e114de2b42:0-lun0
where phy0 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected.

=20
=20
We usually refuse any "find the lowest number approach" in new code,
and require the kernel to export a stable instance number per parent
device, which is entirely disconnected from any global device
enumeration counter or prober ordering.
=20
Almost all existing examples doing that are broken, and some even
needed to be removed because they just blindly bet on luck that thing=

s

Post by Kay Sievers
are always in the same probe order.
=20
Many device can individually unbound and rebound in the kernel, so
after the rebind the same device will no longer have a smaller number
like it had before. Kernel enumeration is only in a very few cases
useful to build persistent paths.
=20
The approach in this case is safe against individual pieces being
unregistered and registered again?
=20

Note, this is just a first step towards real configuration.

This is the SAS PHY issue we've talked about earlier; eventually we'll
have to export paths for all phys.

I'm working on a patch (on top of this one), but it'll be quite a chang=
e
to path_id. So I've asked for this patch to be sent now, rather than
wait for the full blown solution.

Cheers,

Hannes
--=20
Dr. Hannes Reinecke zSeries & Storage
***@suse.de [phone]
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg
GF: J. Hawn, J. Guild, F. Imend=C3=B6rffer, HRB 16746 (AG N=C3=BCrnberg=
)
--
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

[Kay Sievers]

Post by Hannes Reinecke

Post by Kay Sievers

Post by Nils Carlson
This patch reintroduces enumeration
of the form
pci-0000:05:00.0-sas-phy0-0x500000e114de2b42:0-lun0
where phy0 is the reintroduced substring where 0 corresponds
to the lowest phy identifier on the port to which the device
is connected.

We usually refuse any "find the lowest number approach" in new code,
and require the kernel to export a stable instance number per parent
device, which is entirely disconnected from any global device
enumeration counter or prober ordering.
Almost all existing examples doing that are broken, and some even
needed to be removed because they just blindly bet on luck that things
are always in the same probe order.
Many device can individually unbound and rebound in the kernel, so
after the rebind the same device will no longer have a smaller number
like it had before. Kernel enumeration is only in a very few cases
useful to build persistent paths.
The approach in this case is safe against individual pieces being
unregistered and registered again?

Note, this is just a first step towards real configuration.
This is the SAS PHY issue we've talked about earlier; eventually we'll
have to export paths for all phys.
I'm working on a patch (on top of this one), but it'll be quite a change
to path_id. So I've asked for this patch to be sent now, rather than
wait for the full blown solution.

Yeah, sure, I just need to make sure we do not do things like this any more:
http://git.kernel.org/?p=linux/hotplug/udev.git;a=blob;f=src/udev-builtin-path_id.c;h=a8559d2dd4118ac89feb03b7db8ef364caa73a0b;hb=HEAD#l255

It is just broken, and the kernel needs to export that if we really
want to use it in userspace. The kernel device name has zero meaning
and we must not derive any order from it, never. Re-registering the
same device will probably just break that too-simple logic.

I just asked, because the requirement for new stuff is much higher
today, we do not fiddle around with broken kernel interfaces anymore.
If the stuff does not export the proper things, we just don't support
it.

The time to fiddle with symptoms is over. We either get it fixed for
real, or it will just not show up in new things in higher levels in
userspace, including new symlinks udev creates.

And the transport classes in sysfs are just a nightmare; but I know, I
don't need to tell you. :)

Kay
--
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