[systemd-devel] [PATCH] udevd: SAS: use SAS addr + PHY id in by-path whenever possible.
Maurizio Lombardi
mlombard at redhat.com
Tue Sep 23 03:33:54 PDT 2014
On Mon, 2014-09-22 at 16:58 +0200, Tom Gundersen wrote:
> 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?
Suppose you have the following configuration:
pci card --> SAS expander --> hard drive
you can think to an expander as a box with N slots, each slot can be connected to 1 hard drive.
The problem with the current scheme is that it uses the SAS addr of the
hard drives in by-path names and this is not sufficient to identify its physical
position, which is what by-path is supposed to do.
Example:
If we add an hard disk to the SAS expander, what will we find in by-path?
------------
Current scheme:
pci-0000:1c:00.0-sas-0x5000c5001ac1031a-lun-0 -> ../../sdc
New scheme:
pci-0000:1c:00.0-sas-exp0x5003048000201dff-phy2-lun-0 -> ../../sdc
------------
Now, what happens if we move the drive to a different slot?
------------
Current scheme:
pci-0000:1c:00.0-sas-0x5000c5001ac1031a-lun-0 -> ../../sdc
New scheme:
pci-0000:1c:00.0-sas-exp0x5003048000201dff-phy4-lun-0 -> ../../sdc
------------
As you can see, with the current scheme by-path is not functioning as intended because
it does not show the change of the physical position of the drive.
So, it would be better to use the SAS addr of the nearest expander plus the
PHY id the disk is connected to;
this way, when you move the drive, the change will be reflected in the by-path
link, as shown in the example before.
Note: it is not always possible get the PHY id the drive is connected to (it happens when SAS wide ports are used);
in this case, we'll keep the old by-path format.
>
> 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
More information about the systemd-devel
mailing list