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

2014-12-04 Thread Harald Hoyer
On 22.09.2014 11:48, Maurizio Lombardi 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.
 
 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 mlomb...@redhat.com


After convincing Kay Sievers, I am going to push this patch, as it fixes
obviously non-predictable symlinks.

To fix already released systemd versions, I would suggest to patch them in a
way, that it outputs both symlinks and add a release note, that the old
symlinks are deprecated.

In the long run, we really should outsource all specialized storage udev helper
code, as Kay mentioned in the various replies to this thread.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2014-09-26 Thread Tomas Henzl
On 09/25/2014 05:07 PM, Kay Sievers wrote:
 On Thu, Sep 25, 2014 at 1:57 PM, Tomas Henzl the...@redhat.com wrote:
 On 09/24/2014 05:03 PM, Kay Sievers wrote:
 Simple as: Don't add new features to systemd/udev, only fix bugs.
 The by-path has been part of udev for a long time, the users of this
 feature are from the same camp as the users of the,
 'enterprise storage'. Everyone has been happily using it,
 until someone found out that the values shown are incorrect.
 The patch posted here corrects the values so it can work as expected.
 You probably were confused by something, but this is _not_ a new feature
 it's a obvious bug fix.

 Please consider again inclusion of this patch.
 Sorry, we are better not changing the current, and possibly used link names.
 They are created that way since a long time and nobody knows who uses
 the current ones.

 Adding additional infrastructure to create more than one by-path link would
 be a new feature, which we will for various mentioned reasons not add to
 the current code, but which needs to happen in an external package,
 maintained/reviewed by someone with an enterprise-storage background
 and not live in the systemd code base.

I haven't noticed a single one technical reason why the by-path should be moved 
out
from udev and not fixed there when it's needed and most importantly how that 
move
would help the users of this feature.
The only point I can understand is that you don't like storage related code in 
udev
because you don't understand it and so you don't want to maintain it. 
I could try to explain that in case of any issues you may seek help in the 
scsi-misc list,
but well ok it's your decision.

This is I think by far not the last storage related code in udev, do you want to
remove the other parts too, like the by-uuid and the other by-* obvious 
candidates ?

Thanks,
Tomas


 I hope you understand the reasoning,
 Kay

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


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

2014-09-26 Thread Kay Sievers
On Fri, Sep 26, 2014 at 6:14 PM, Tomas Henzl the...@redhat.com wrote:
 I haven't noticed a single one technical reason why the by-path should be 
 moved out
 from udev and not fixed there when it's needed and most importantly how that 
 move
 would help the users of this feature.

Because it was changed several times already in the past, and it seems
people cannot make up their mind what is right here.

 The only point I can understand is that you don't like storage related code 
 in udev
 because you don't understand it and so you don't want to maintain it.
 I could try to explain that in case of any issues you may seek help in the 
 scsi-misc list,
 but well ok it's your decision.

Exactly, even the people involved doing the code seem to have problems
to sort it out. This code and use case is just too specific and a
niche, it should not be maintained in systemd/udev but in a separate
package where someone else takes the responsibility for it.

 This is I think by far not the last storage related code in udev, do you want 
 to
 remove the other parts too, like the by-uuid and the other by-* obvious 
 candidates ?

The generic, well-defined and generally useful parts like filesystem,
partition table properties, WWN IDs, ... will stay.

SCSI IDs will move to sg3_util. Specific ATA stuff and the rest of the
overly complex enterprise storage stuff will need a new home.

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


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

2014-09-25 Thread Tomas Henzl
On 09/24/2014 05:03 PM, Kay Sievers wrote:
 - Original Message -
 On Tue, Sep 23, 2014 at 01:55:09PM -0400, Kay Sievers wrote:
 - Original Message -
 We are not applying this patch now. It introduces a complete new
 scheme and we do not want to extend the current SCSI code, we only
 fix obvious bugs.
 Fine with me, however not sure what our story should be for years to come
 regarding SCSI stuff downstream.
 Simple as: Don't add new features to systemd/udev, only fix bugs.

The by-path has been part of udev for a long time, the users of this
feature are from the same camp as the users of the,
'enterprise storage'. Everyone has been happily using it, 
until someone found out that the values shown are incorrect.
The patch posted here corrects the values so it can work as expected.
You probably were confused by something, but this is _not_ a new feature
it's a obvious bug fix.

Please consider again inclusion of this patch.

Tomas 


 SCSI code needs to be maintained by someone who understands it and is
 capable and willing to maintain it in the long run. Systemd/udev
 maintainers lack the experience and cannot do that.

 New features need to happen in an existing or new storage related-package
 and not in the systemd/udev code base.
 Systemd+udev are supposed to be about providing a fairly complete
 basic environment.
 No, it is absolutely not. Things like enterprise storage technology
 is for sure not part of our task. We focus on the basic operating system
 tasks only.

 I don't see why an exception should be made for SAS
 disks. If necessary, we can bring additional people in, or maybe just
 wait for patches.
 No, we don't. We will move this out to an external package.

 This seems like a better solution than waiting for
 a project to be created to create and maintain a tiny amount of code
 and one line of udev rules.
 It is not tiny. It is complicated and has a long history of forth and
 back. Systemd/udev is just the wrong place to maintain it.

 I think that the failure mode is especially ugly here: if you have the
 hardware, but don't install this mythical extra package, systemd/udev
 provides one set of stable names. Then you install the extra
 package, and you get a different set.
 That is how *features* work, they require extra packages. SCSI, SAS, LVM,
 MD,  enterprise storage, ... is _not_ part of systemd's task and should
 be maintained separately.

 Kay, any serious objections to applying only
 downstream for RHEL?
 Well, the currently created links will go away and potentially break
 existing users. Not sure what is acceptable here. I personally would
 not apply it to any released product.
 Right, so applying the patch as is is probably not OK. But we could provide
 the old names as an additional alias. This should be doable.
 Nope, no new features. Upstream systemd/udev will not apply or ship it.
 A new place in an externally maintained package is needed to add/improve
 the current code.

 Kay

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


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

2014-09-24 Thread Zdenek Kabelac

Dne 23.9.2014 v 19:55 Kay Sievers napsal(a):

- Original Message -

We are not applying this patch now. It introduces a complete new
scheme and we do not want to extend the current SCSI code, we only
fix obvious bugs.


Fine with me, however not sure what our story should be for years to come
regarding SCSI stuff downstream.


Simple as: Don't add new features to systemd/udev, only fix bugs.

SCSI code needs to be maintained by someone who understands it and is
capable and willing to maintain it in the long run. Systemd/udev
maintainers lack the experience and cannot do that.



Hi

Nice to see there is finally recognized the need to have separate subsystem 
management.


Now - is there some 'plugin'-like interface so we could get an embedded code 
to be executed right with  udev scanning process ?


I can see a tons of code for i.e. btrfs handling to be wired into udev.
I believe this is also something that should be handled externally from udev
(btrfs-progs  udev plugin ?)

Anyway - we would like to eliminate number of forked dmsetup calls
and even further eliminate rule processing when we detect i.e. internal lvm 
device.


When we activate i.e. 1000LVs there is literally storm of forked processes and 
I think we can do much better.


Since there is already 'blkid' being built-in - we could solve problem 
partially by enhancing blkid detection - but still there are some things we 
can't handle within  blkid (parsing DM  names)


It would be nice to have some plugin API here where we could avoid forking 
process to do rather trivial 'string-manipulation' stuff.




New features need to happen in an existing or new storage related-package
and not in the systemd/udev code base.


So any plans on udev plugin API  (since rules are simply way to slow and 
inefficient)


Zdenek

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


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

2014-09-24 Thread Kay Sievers
- Original Message -
 On Tue, Sep 23, 2014 at 01:55:09PM -0400, Kay Sievers wrote:
  - Original Message -
We are not applying this patch now. It introduces a complete new
scheme and we do not want to extend the current SCSI code, we only
fix obvious bugs.
   
   Fine with me, however not sure what our story should be for years to come
   regarding SCSI stuff downstream.
  
  Simple as: Don't add new features to systemd/udev, only fix bugs.
  
  SCSI code needs to be maintained by someone who understands it and is
  capable and willing to maintain it in the long run. Systemd/udev
  maintainers lack the experience and cannot do that.
  
  New features need to happen in an existing or new storage related-package
  and not in the systemd/udev code base.
 
 Systemd+udev are supposed to be about providing a fairly complete
 basic environment.

No, it is absolutely not. Things like enterprise storage technology
is for sure not part of our task. We focus on the basic operating system
tasks only.

 I don't see why an exception should be made for SAS
 disks. If necessary, we can bring additional people in, or maybe just
 wait for patches.

No, we don't. We will move this out to an external package.

 This seems like a better solution than waiting for
 a project to be created to create and maintain a tiny amount of code
 and one line of udev rules.

It is not tiny. It is complicated and has a long history of forth and
back. Systemd/udev is just the wrong place to maintain it.

 I think that the failure mode is especially ugly here: if you have the
 hardware, but don't install this mythical extra package, systemd/udev
 provides one set of stable names. Then you install the extra
 package, and you get a different set.

That is how *features* work, they require extra packages. SCSI, SAS, LVM,
MD,  enterprise storage, ... is _not_ part of systemd's task and should
be maintained separately.

   Kay, any serious objections to applying only
   downstream for RHEL?
  
  Well, the currently created links will go away and potentially break
  existing users. Not sure what is acceptable here. I personally would
  not apply it to any released product.
 Right, so applying the patch as is is probably not OK. But we could provide
 the old names as an additional alias. This should be doable.

Nope, no new features. Upstream systemd/udev will not apply or ship it.
A new place in an externally maintained package is needed to add/improve
the current code.

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


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

2014-09-24 Thread Kay Sievers
- Original Message -
 Dne 23.9.2014 v 19:55 Kay Sievers napsal(a):
  - Original Message -
  We are not applying this patch now. It introduces a complete new
  scheme and we do not want to extend the current SCSI code, we only
  fix obvious bugs.
 
  Fine with me, however not sure what our story should be for years to come
  regarding SCSI stuff downstream.
 
  Simple as: Don't add new features to systemd/udev, only fix bugs.
 
  SCSI code needs to be maintained by someone who understands it and is
  capable and willing to maintain it in the long run. Systemd/udev
  maintainers lack the experience and cannot do that.
 
 
 Hi
 
 Nice to see there is finally recognized the need to have separate subsystem
 management.
 
 Now - is there some 'plugin'-like interface so we could get an embedded code
 to be executed right with  udev scanning process ?
 
 I can see a tons of code for i.e. btrfs handling to be wired into udev.
 I believe this is also something that should be handled externally from udev
 (btrfs-progs  udev plugin ?)
 
 Anyway - we would like to eliminate number of forked dmsetup calls
 and even further eliminate rule processing when we detect i.e. internal lvm
 device.
 
 When we activate i.e. 1000LVs there is literally storm of forked processes
 and
 I think we can do much better.
 
 Since there is already 'blkid' being built-in - we could solve problem
 partially by enhancing blkid detection - but still there are some things we
 can't handle within  blkid (parsing DM  names)
 
 It would be nice to have some plugin API here where we could avoid forking
 process to do rather trivial 'string-manipulation' stuff.
 
 
  New features need to happen in an existing or new storage related-package
  and not in the systemd/udev code base.
 
 So any plans on udev plugin API  (since rules are simply way to slow and
 inefficient)

No, there are no plans to provide shared code plugins and it is very unlikely
to happen.

External things need to be forked, udev does not want to run external code
in its address space.

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


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

2014-09-24 Thread Zdenek Kabelac

Dne 24.9.2014 v 17:06 Kay Sievers napsal(a):

- Original Message -

Dne 23.9.2014 v 19:55 Kay Sievers napsal(a):

- Original Message -

We are not applying this patch now. It introduces a complete new
scheme and we do not want to extend the current SCSI code, we only
fix obvious bugs.


Fine with me, however not sure what our story should be for years to come
regarding SCSI stuff downstream.


Simple as: Don't add new features to systemd/udev, only fix bugs.

SCSI code needs to be maintained by someone who understands it and is
capable and willing to maintain it in the long run. Systemd/udev
maintainers lack the experience and cannot do that.



Hi

Nice to see there is finally recognized the need to have separate subsystem
management.

Now - is there some 'plugin'-like interface so we could get an embedded code
to be executed right with  udev scanning process ?

I can see a tons of code for i.e. btrfs handling to be wired into udev.
I believe this is also something that should be handled externally from udev
(btrfs-progs  udev plugin ?)

Anyway - we would like to eliminate number of forked dmsetup calls
and even further eliminate rule processing when we detect i.e. internal lvm
device.

When we activate i.e. 1000LVs there is literally storm of forked processes
and
I think we can do much better.

Since there is already 'blkid' being built-in - we could solve problem
partially by enhancing blkid detection - but still there are some things we
can't handle within  blkid (parsing DM  names)

It would be nice to have some plugin API here where we could avoid forking
process to do rather trivial 'string-manipulation' stuff.



New features need to happen in an existing or new storage related-package
and not in the systemd/udev code base.


So any plans on udev plugin API  (since rules are simply way to slow and
inefficient)


No, there are no plans to provide shared code plugins and it is very unlikely
to happen.

External things need to be forked, udev does not want to run external code
in its address space.


It's already being done - udev is using/linking libblkid - thus you run 
'external' code in udev address.


So it's just the view you take on it.

But - we just don't won't link i.e. libdm with libudev to gain knowledge in 
udev process.


So there should be a runtime way how to extend udev for performance reasons.

Of course the other way around is - to 'hack-in' udev code and add another 
chunk of code - like you did for already mentioned btrfs.


But I believe dlopening  runtime object file and using it inside  udev 
scanning process to accelerate udev rules process would be much nicer way.


Zdenek

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


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

2014-09-23 Thread Maurizio Lombardi

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 mlomb...@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-:1c:00.0-sas-0x5000c5001ac1031a-lun-0 - ../../sdc

New scheme:
pci-:1c:00.0-sas-exp0x5003048000201dff-phy2-lun-0 - ../../sdc


Now, what happens if we move the drive to a different slot?


Current scheme:
pci-:1c:00.0-sas-0x5000c5001ac1031a-lun-0 - ../../sdc

New scheme:
pci-: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 mlomb...@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;
  +

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

2014-09-23 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Sep 23, 2014 at 12:33:54PM +0200, Maurizio Lombardi wrote:
 
 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 mlomb...@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-:1c:00.0-sas-0x5000c5001ac1031a-lun-0 - ../../sdc
 
 New scheme:
 pci-:1c:00.0-sas-exp0x5003048000201dff-phy2-lun-0 - ../../sdc
 
 
 Now, what happens if we move the drive to a different slot?
 
 
 Current scheme:
 pci-:1c:00.0-sas-0x5000c5001ac1031a-lun-0 - ../../sdc
 
 New scheme:
 pci-: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.

Patch looks OK. I think we should apply it. It's unfortunate that it
breaks the naming with previous releases, but it seems better to break
it now than to continue with the existing insufficient mechanism.

Zbyszek

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


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

2014-09-23 Thread Kay Sievers
- Original Message -
 On Tue, Sep 23, 2014 at 12:33:54PM +0200, Maurizio Lombardi wrote:
  
  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 mlomb...@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-:1c:00.0-sas-0x5000c5001ac1031a-lun-0 - ../../sdc
  
  New scheme:
  pci-:1c:00.0-sas-exp0x5003048000201dff-phy2-lun-0 - ../../sdc
  
  
  Now, what happens if we move the drive to a different slot?
  
  
  Current scheme:
  pci-:1c:00.0-sas-0x5000c5001ac1031a-lun-0 - ../../sdc
  
  New scheme:
  pci-: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.
 
 Patch looks OK. I think we should apply it. It's unfortunate that it
 breaks the naming with previous releases, but it seems better to break
 it now than to continue with the existing insufficient mechanism.

We are not applying this patch now. It introduces a complete new
scheme and we do not want to extend the current SCSI code, we only
fix obvious bugs.

This is very specific SCSI stuff and it should move out of the
udev/systemd code base. None of us has the slightest idea how it
works, we cannot review or maintain it, we do not know what's
the right thing to do.

Someone with a storage background and willing to maintain the code,
please start to move this code to some other package. Hannes Reinecke
already moved parts of the scsi_id tool to the sg3_utils package.
scsi_id along with the SCSI code in path_id will be removed from
systemd/udev when sg3_util, or any other package, has taken over the
functionality.

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


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

2014-09-23 Thread Kay Sievers
- Original Message -
  We are not applying this patch now. It introduces a complete new
  scheme and we do not want to extend the current SCSI code, we only
  fix obvious bugs.
 
 Fine with me, however not sure what our story should be for years to come
 regarding SCSI stuff downstream.

Simple as: Don't add new features to systemd/udev, only fix bugs.

SCSI code needs to be maintained by someone who understands it and is
capable and willing to maintain it in the long run. Systemd/udev
maintainers lack the experience and cannot do that.

New features need to happen in an existing or new storage related-package
and not in the systemd/udev code base.

 Kay, any serious objections to applying only
 downstream for RHEL?

Well, the currently created links will go away and potentially break
existing users. Not sure what is acceptable here. I personally would
not apply it to any released product.

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


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

2014-09-23 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Sep 23, 2014 at 01:55:09PM -0400, Kay Sievers wrote:
 - Original Message -
   We are not applying this patch now. It introduces a complete new
   scheme and we do not want to extend the current SCSI code, we only
   fix obvious bugs.
  
  Fine with me, however not sure what our story should be for years to come
  regarding SCSI stuff downstream.
 
 Simple as: Don't add new features to systemd/udev, only fix bugs.
 
 SCSI code needs to be maintained by someone who understands it and is
 capable and willing to maintain it in the long run. Systemd/udev
 maintainers lack the experience and cannot do that.
 
 New features need to happen in an existing or new storage related-package
 and not in the systemd/udev code base.

Systemd+udev are supposed to be about providing a fairly complete
basic environment. I don't see why an exception should be made for SAS
disks. If necessary, we can bring additional people in, or maybe just
wait for patches. This seems like a better solution than waiting for
a project to be created to create and maintain a tiny amount of code
and one line of udev rules.

I think that the failure mode is especially ugly here: if you have the
hardware, but don't install this mythical extra package, systemd/udev
provides one set of stable names. Then you install the extra
package, and you get a different set.

  Kay, any serious objections to applying only
  downstream for RHEL?
 
 Well, the currently created links will go away and potentially break
 existing users. Not sure what is acceptable here. I personally would
 not apply it to any released product.
Right, so applying the patch as is is probably not OK. But we could provide
the old names as an additional alias. This should be doable.

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


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

2014-09-22 Thread Tom Gundersen
Hi Maurizio,

On Mon, Sep 22, 2014 at 11:48 AM, Maurizio Lombardi mlomb...@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 mlomb...@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);
 +

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

2014-09-22 Thread Samuli Suominen

On 22/09/14 17:58, Tom Gundersen wrote:
 Hi Maurizio,

 On Mon, Sep 22, 2014 at 11:48 AM, Maurizio Lombardi mlomb...@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?


I wonder if it will solve this one...

https://bugs.freedesktop.org/show_bug.cgi?id=63806

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