Discussion:
Re-enable by-path links for ata device
Robert Milasan
2012-08-02 18:31:26 UTC
Permalink
Hello, just created a patch for openSUSE to re-enable by-path links for
ata devices. The patch is based on handle_scsi_default function, but
instead of using "host{NUMBER}", but "ata{NUMBER}" and it works almost
the same as for scsi devices.

The patch is not the best or the cleanest, but it works.

Index: udev-182/src/udev-builtin-path_id.c
===================================================================
--- udev-182.orig/src/udev-builtin-path_id.c
+++ udev-182/src/udev-builtin-path_id.c
@@ -286,6 +286,85 @@ out:
return hostdev;
}

+static struct udev_device *handle_ata(struct udev_device *parent, char **path)
+{
+ struct udev_device *hostdev;
+ int host, bus, target, lun;
+ const char *name;
+ char *base;
+ char *pos;
+ DIR *dir;
+ struct dirent *dent;
+ int basenum, len;
+
+ hostdev = udev_device_get_parent_with_subsystem_devtype(parent, "scsi", "scsi_host");
+ if (hostdev == NULL)
+ return NULL;
+
+ name = udev_device_get_sysname(parent);
+ if (sscanf(name, "%d:%d:%d:%d", &host, &bus, &target, &lun) != 4)
+ return NULL;
+
+ /* rebase ata offset to get the local relative number */
+ basenum = -1;
+ base = strdup(udev_device_get_syspath(hostdev));
+ if (base == NULL)
+ return NULL;
+ pos = strrchr(base, '/');
+ if (pos == NULL) {
+ parent = NULL;
+ goto out;
+ }
+ pos[0] = '\0';
+ len = strlen(base) - 5;
+ if (len <= 0) {
+ parent = NULL;
+ goto out;
+ }
+ base[len] = '\0';
+ dir = opendir(base);
+ if (dir == NULL) {
+ parent = NULL;
+ goto out;
+ }
+ for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) {
+ char *rest;
+ int i;
+
+ if (dent->d_name[0] == '.')
+ continue;
+ if (dent->d_type != DT_DIR && dent->d_type != DT_LNK)
+ continue;
+ if (strncmp(dent->d_name, "ata", 3) != 0)
+ continue;
+ i = strtoul(&dent->d_name[3], &rest, 10);
+
+ /* ata devices start with 1, so decrease by 1 if i is bigger then 0 */
+ if (i > 0)
+ i--;
+ if (rest[0] != '\0')
+ continue;
+ /*
+ * find the smallest number; the host really needs to export its
+ * own instance number per parent device; relying on the global host
+ * enumeration and plainly rebasing the numbers sounds unreliable
+ */
+ if (basenum == -1 || i < basenum)
+ basenum = i;
+ }
+ closedir(dir);
+ if (basenum == -1) {
+ parent = NULL;
+ goto out;
+ }
+ host -= basenum;
+
+ path_prepend(path, "scsi-%u:%u:%u:%u", host, bus, target, lun);
+out:
+ free(base);
+ return hostdev;
+}
+
static struct udev_device *handle_scsi(struct udev_device *parent, char **path)
{
const char *devtype;
@@ -322,16 +401,8 @@ static struct udev_device *handle_scsi(s
goto out;
}

- /*
- * We do not support the ATA transport class, it creates duplicated link
- * names as the fake SCSI host adapters are all separated, they are all
- * re-based as host == 0. ATA should just stop faking two duplicated
- * hierarchies for a single topology and leave the SCSI stuff alone;
- * until that happens, there are no by-path/ links for ATA devices behind
- * an ATA transport class.
- */
if (strstr(name, "/ata") != NULL) {
- parent = NULL;
+ parent = handle_ata(parent, path);
goto out;
}

NOTE: The patch is for udev 182, but its easy to adapt this to systemd
187.
--
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
2012-08-03 08:44:11 UTC
Permalink
Post by Robert Milasan
Hello, just created a patch for openSUSE to re-enable by-path links for
ata devices. The patch is based on handle_scsi_default function, but
instead of using "host{NUMBER}", but "ata{NUMBER}" and it works almost
the same as for scsi devices.
The patch is not the best or the cleanest, but it works.
We can not play these games for any new stuff, like SCSI path_id is
doing here, it's a legacy from a time we had we no good idea how
things should work. What SCSI path_id is doing here is "I hope it
works, let's try ..." software, which is a model that we don't support
anymore. :)

We need the ATA transport class to export the local port number of the
host adapter, and use that.

Rebasing in userspace by making assumptions about a global in-kernel
instance counter is racy, unreliable and can only work if all things
would always happen in strict order, and they don't. Manual driver
unbind/bind, parallel hotplug gets into the way of this too simple
logic.

The proper fix is to teach the kernel to export the numbers directly,
or get the numbers in some way, but not by doing readdir() and
starting to count and make a guess.

Hope you understand that we can not do this for any new stuff, it just
does not meet the requirements we have today.

Thanks,
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
Robert Milasan
2012-08-03 09:31:45 UTC
Permalink
On Fri, 3 Aug 2012 10:44:11 +0200
Post by Kay Sievers
Post by Robert Milasan
Hello, just created a patch for openSUSE to re-enable by-path links
for ata devices. The patch is based on handle_scsi_default
function, but instead of using "host{NUMBER}", but "ata{NUMBER}"
and it works almost the same as for scsi devices.
The patch is not the best or the cleanest, but it works.
We can not play these games for any new stuff, like SCSI path_id is
doing here, it's a legacy from a time we had we no good idea how
things should work. What SCSI path_id is doing here is "I hope it
works, let's try ..." software, which is a model that we don't support
anymore. :)
We need the ATA transport class to export the local port number of the
host adapter, and use that.
Rebasing in userspace by making assumptions about a global in-kernel
instance counter is racy, unreliable and can only work if all things
would always happen in strict order, and they don't. Manual driver
unbind/bind, parallel hotplug gets into the way of this too simple
logic.
The proper fix is to teach the kernel to export the numbers directly,
or get the numbers in some way, but not by doing readdir() and
starting to count and make a guess.
Hope you understand that we can not do this for any new stuff, it just
does not meet the requirements we have today.
Thanks,
Kay
It's fine with me, just wanted to make sure people know, maybe someone
needs it or wants it.

Thanks,
Robert
--
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
Loading...