[systemd-devel] [PATCH] udevd: SAS: use SAS addr + PHY id in by-path whenever possible.

[Tom Gundersen]

Hi Maurizio,

On Mon, Sep 22, 2014 at 11:48 AM, Maurizio Lombardi <mlombard at redhat.com> wrote:
> This patch changes the naming scheme for sas disks. The original names used
> disk's sas address and lun, the new scheme uses sas address of the
> nearest expander (if available) and a phy id of the used connection.
> If no expander is used, the phy id of hba phy is used.
> Note that names that refer to RAID or other abstract devices are
> unchanged.

What's the problem with the current scheme, what's the benefit of the
new one? Will this break backwards compatibility, if so, why is this
justifiable?

Cheers,

Tom

> Name in raid configuration:
> hba_pci_address-sas-raid_sas_address-lunY-partZ
>
> Name in expander bare disk configuration:
> hba_pci_address-sas-expander_sas_address-phyX-lunY-partZ
>
> Name format without expanders:
> hba_pci_address-sas-phyX-lunY-partZ
>
> Signed-off-by: Maurizio Lombardi <mlombard at redhat.com>
> ---
>  src/udev/udev-builtin-path_id.c | 96 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c
> index 073f05a..7a97411 100644
> --- a/src/udev/udev-builtin-path_id.c
> +++ b/src/udev/udev-builtin-path_id.c
> @@ -118,7 +118,7 @@ out:
>          return parent;
>  }
>
> -static struct udev_device *handle_scsi_sas(struct udev_device *parent, char **path) {
> +static struct udev_device *handle_scsi_sas_wide_port(struct udev_device *parent, char **path) {
>          struct udev *udev  = udev_device_get_udev(parent);
>          struct udev_device *targetdev;
>          struct udev_device *target_parent;
> @@ -154,6 +154,100 @@ out:
>          return parent;
>  }
>
> +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 *port;
> +        struct udev_device *expander;
> +        struct udev_device *target_sasdev = NULL;
> +        struct udev_device *expander_sasdev = NULL;
> +        struct udev_device *port_sasdev = NULL;
> +        const char *sas_address = NULL;
> +        const char *phy_id;
> +        const char *phy_count;
> +        char *lun = NULL;
> +
> +        targetdev = udev_device_get_parent_with_subsystem_devtype(parent, "scsi", "scsi_target");
> +        if (targetdev == NULL)
> +                return NULL;
> +
> +        target_parent = udev_device_get_parent(targetdev);
> +        if (target_parent == NULL)
> +                return NULL;
> +
> +        /* Get sas device */
> +        target_sasdev = udev_device_new_from_subsystem_sysname(udev,
> +                          "sas_device", udev_device_get_sysname(target_parent));
> +        if (target_sasdev == NULL)
> +                return NULL;
> +
> +        /* The next parent is sas port */
> +        port = udev_device_get_parent(target_parent);
> +        if (port == NULL) {
> +                parent = NULL;
> +                goto out;
> +        }
> +
> +        /* Get port device */
> +        port_sasdev = udev_device_new_from_subsystem_sysname(udev,
> +                          "sas_port", udev_device_get_sysname(port));
> +
> +        phy_count = udev_device_get_sysattr_value(port_sasdev, "num_phys");
> +        if (phy_count == NULL) {
> +               parent = NULL;
> +               goto out;
> +        }
> +
> +        /* Check if we are simple disk */
> +        if (strncmp(phy_count, "1", 2) != 0) {
> +                 parent = handle_scsi_sas_wide_port(parent, path);
> +                 goto out;
> +        }
> +
> +        /* Get connected phy */
> +        phy_id = udev_device_get_sysattr_value(target_sasdev, "phy_identifier");
> +        if (phy_id == NULL) {
> +                parent = NULL;
> +                goto out;
> +        }
> +
> +        /* The port's parent is either hba or expander */
> +        expander = udev_device_get_parent(port);
> +        if (expander == NULL) {
> +                parent = NULL;
> +                goto out;
> +        }
> +
> +        /* Get expander device */
> +        expander_sasdev = udev_device_new_from_subsystem_sysname(udev,
> +                          "sas_device", udev_device_get_sysname(expander));
> +        if (expander_sasdev != NULL) {
> +                 /* Get expander's address */
> +                 sas_address = udev_device_get_sysattr_value(expander_sasdev,
> +                                                    "sas_address");
> +                 if (sas_address == NULL) {
> +                        parent = NULL;
> +                        goto out;
> +                 }
> +        }
> +
> +        format_lun_number(parent, &lun);
> +        if (sas_address)
> +                 path_prepend(path, "sas-exp%s-phy%s-%s", sas_address, phy_id, lun);
> +        else
> +                 path_prepend(path, "sas-phy%s-%s", phy_id, lun);
> +
> +        if (lun)
> +                free(lun);
> +out:
> +        udev_device_unref(target_sasdev);
> +        udev_device_unref(expander_sasdev);
> +        udev_device_unref(port_sasdev);
> +        return parent;
> +}
> +
>  static struct udev_device *handle_scsi_iscsi(struct udev_device *parent, char **path) {
>          struct udev *udev  = udev_device_get_udev(parent);
>          struct udev_device *transportdev;
> --
> Maurizio Lombardi
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel