Re: [systemd-devel] [PATCH] udev: build by-path identifiers for ATA

2015-09-08 Thread David Milburn


Hi Lennart,

On 09/06/2015 05:48 AM, Lennart Poettering wrote:

On Fri, 04.09.15 10:59, David Milburn (dmilb...@redhat.com) wrote:

Please file this issue as github PR:

https://github.com/systemd/systemd


Ok, did that.





+static struct udev_device *handle_scsi_ata(struct udev_device *parent, char 
**path)
+{


Please reading CODING_STYLE. We tend to place the opening bracket on
the same line as the function name.


+   struct udev *udev  = udev_device_get_udev(parent);
+   struct udev_device *targetdev;
+   struct udev_device *target_parent;
+   struct udev_device *atadev;
+   const char *port_no;
+
+   targetdev = udev_device_get_parent_with_subsystem_devtype(parent, "scsi", 
"scsi_host");
+   if (targetdev == NULL)
+   return NULL;


Usually, we don' compare explicitly with NULL.


Ok, fixed that.




+
+   target_parent = udev_device_get_parent(targetdev);
+   if (target_parent == NULL)
+   return NULL;
+
+   atadev = udev_device_new_from_subsystem_sysname(udev, "ata_port",
+   
udev_device_get_sysname(target_parent));
+   if (atadev == NULL)
+   return NULL;
+
+   port_no = udev_device_get_sysattr_value(atadev, "port_no");
+   if (port_no == NULL) {
+   parent = NULL;
+   goto out;
+   }
+   path_prepend(path, "ata-%s", port_no);
+out:
+udev_device_unref(atadev);
+return parent;


Indentation is borked... Needs to be 8ch all the way.


Sorry, usually use tabs, fixed that.




+}
+
  static struct udev_device *handle_scsi_default(struct udev_device *parent, 
char **path)
  {


This is indication that the patch is not against git master, as this
has been reindented a while back to put the bracket on the same line
as the function name (see above). Please file these things as PRs,
which will catch these issues right away, as the patch is then checked
and test-built just by submitting it.


Ok, PR is against master.

Thanks for reviewing.

David

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] udev: build by-path identifiers for ATA

2015-09-04 Thread David Milburn
Patch to build by-path identifiers for ATA devices.

# pwd
/dev/disk/by-path

# ll
total 0
lrwxrwxrwx. 1 root root  9 Sep  4 10:02 pci-:00:1f.2-ata-2 -> ../../sr0
lrwxrwxrwx. 1 root root  9 Sep  4 10:02 pci-:00:1f.2-ata-3 -> ../../sdd
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:00:1f.2-ata-3-part1 -> 
../../sdd1
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:00:1f.2-ata-3-part2 -> 
../../sdd2
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:00:1f.2-ata-3-part3 -> 
../../sdd3
lrwxrwxrwx. 1 root root  9 Sep  4 10:02 pci-:03:00.0-ata-4 -> ../../sda
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:03:00.0-ata-4-part1 -> 
../../sda1
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:03:00.0-ata-4-part2 -> 
../../sda2
lrwxrwxrwx. 1 root root  9 Sep  4 10:02 pci-:08:00.0-ata-1 -> ../../sdc
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:08:00.0-ata-1-part1 -> 
../../sdc1
lrwxrwxrwx. 1 root root 10 Sep  4 10:02 pci-:08:00.0-ata-1-part2 -> 
../../sdc2

--- src/udev/udev-builtin-path_id.c.systemd-208-20.el7  2015-09-03 
10:19:20.385434297 -0500
+++ src/udev/udev-builtin-path_id.c 2015-09-04 10:05:22.039260808 -0500
@@ -217,6 +217,38 @@ out:
 return parent;
 }
 
+static struct udev_device *handle_scsi_ata(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 *atadev;
+   const char *port_no;
+
+   targetdev = udev_device_get_parent_with_subsystem_devtype(parent, 
"scsi", "scsi_host");
+   if (targetdev == NULL)
+   return NULL;
+
+   target_parent = udev_device_get_parent(targetdev);
+   if (target_parent == NULL)
+   return NULL;
+
+   atadev = udev_device_new_from_subsystem_sysname(udev, "ata_port",
+   
udev_device_get_sysname(target_parent));
+   if (atadev == NULL)
+   return NULL;
+
+   port_no = udev_device_get_sysattr_value(atadev, "port_no");
+   if (port_no == NULL) {
+   parent = NULL;
+   goto out;
+   }
+   path_prepend(path, "ata-%s", port_no);
+out:
+udev_device_unref(atadev);
+return parent;
+}
+
 static struct udev_device *handle_scsi_default(struct udev_device *parent, 
char **path)
 {
 struct udev_device *hostdev;
@@ -374,20 +406,9 @@ static struct udev_device *handle_scsi(s
 goto out;
 }
 
-/*
- * We do not support the ATA transport class, it uses global counters
- * to name the ata devices which numbers spread across multiple
- * controllers.
- *
- * The real link numbers are not exported. Also, possible chains of 
ports
- * behind port multipliers cannot be composed that way.
- *
- * Until all that is solved at the kernel level, there are no by-path/
- * links for ATA devices.
- */
 if (strstr(name, "/ata") != NULL) {
-parent = NULL;
-goto out;
+   parent = handle_scsi_ata(parent, path);
+   goto out;
 }
 
 if (strstr(name, "/vmbus_") != NULL) {
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel