http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c1 --- Comment #1 from Thomas Blume --- Created attachment 734741 --> http://bugzilla.suse.com/attachment.cgi?id=734741&action=edit readd-format-lenght-parameter.patch -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c3 Thomas Blume changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jthumshirn@suse.com Flags| |needinfo?(jthumshirn@suse.c | |om) --- Comment #3 from Thomas Blume --- With the attached patches, I can get by-id nvme symlinks from pure sysfs values that are identical to the ones created by the scsi_id call. That way we can probably obsolete scsi_id. Testpackages for SLE12SP3 are available at: http://download.suse.de/ibs/home:/tsaupe:/branches:/SUSE:/SLE-12:/SP3:/GA:/s... I've tested on blueshark4 and saw it working correctly, but I'd highly appreciate some more testing (unfortunately all orthos machines that have nvme devices are in use). Johannes, could you take a look and probably give it a try? -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c5 Franck Bui changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(fbui@suse.com) | --- Comment #5 from Franck Bui --- (In reply to Thomas Blume from comment #1)

Created attachment 734741 [details] readd-format-lenght-parameter.patch

can you submit this patch upstream ? -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c7 Johannes Thumshirn changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jthumshirn@suse.c | |om) | --- Comment #7 from Johannes Thumshirn --- (In reply to Thomas Blume from comment #3)

With the attached patches, I can get by-id nvme symlinks from pure sysfs values that are identical to the ones created by the scsi_id call. That way we can probably obsolete scsi_id.

Testpackages for SLE12SP3 are available at:

http://download.suse.de/ibs/home:/tsaupe:/branches:/SUSE:/SLE-12:/SP3:/GA:/ systemd-bsc1051465/standard/

I've tested on blueshark4 and saw it working correctly, but I'd highly appreciate some more testing (unfortunately all orthos machines that have nvme devices are in use). Johannes, could you take a look and probably give it a try?

Yes I have an Intel NVMe here as well which "just" needs to find a new home in a machine and I can/will test on qemu as well. Thanks for looking into this. -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 Johannes Thumshirn changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(jthumshirn@suse.c | |om) -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c10 Thomas Blume changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #734743|0 |1 is obsolete| | --- Comment #10 from Thomas Blume --- Created attachment 734940 --> http://bugzilla.suse.com/attachment.cgi?id=734940&action=edit 0001-rules-use-truncated-sysfs-value-for-serial-id.patch Sorry, there was a mistake in the udev rules. Here is an updated patch. I have also updated the test packages. -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c12 Thomas Blume changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(francoisvalenduc@ | |gmail.com) --- Comment #12 from Thomas Blume --- (In reply to François Valenduc from comment #11)

Still no success with this new version.

Please attach the output of the commands: grep nvme /usr/lib/udev/rules.d/61-persistent-storage-compat.rules udevadm test /block/$NVME where $NVME is the devicename from: /sys/block/ For example, if you have: /sys/block/nvme0n1 the command is: udevadm test /block/nvme0n1 -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c14 --- Comment #14 from Thomas Blume --- (In reply to François Valenduc from comment #13)

My problem is probably different since I am not using nvme. The fact is that if I upgrade to the latest version of udev available in opensuse 42.3, the system doesn't boot any more because apparently, it doesn't find the lvm volumes. From what I can see, it finds the harddisk and all the partitions, but after that, it is completely stuck.

Indeed, the change in the udev rules is for nvme devices only. Other devices won't be affected. Seems we better continue processing your issue in bug 1051354. Please attach: /run/initramfs/rdsosreport.txt from a failed boot there (I assume that your system boot ends in the dracut shell when the failure appears). -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c16 --- Comment #16 from Thomas Blume --- Created attachment 735115 --> http://bugzilla.suse.com/attachment.cgi?id=735115&action=edit udevadm test with original scsi_id based rules -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c18 --- Comment #18 from Franck Bui --- Hi Thomas, (In reply to Thomas Blume from comment #15)

Attaching the full test output below. Are the symlinks ok like that?

You shouldn't see any difference in the generated symlinks, should you ? Also please make sure to run the latest version of systemd, especially the version containing 8ea065d44c161675df2a01542889d58bbb4f850d commit. Did you already submit your patch to upstream ? Thanks. -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c20 --- Comment #20 from Franck Bui --- (In reply to Thomas Blume from comment #19)

The patch provides symlinks with the fractional serial id, like it was shown with scsi_id used. Still, I see a fractional serial id as kind of fault, because it might be non uniqe. Therefore I've also created an additional symlink that provides the full serial id. It looks like this is alo what upstream does, and I think this is the one we should use long term. We should keep the symlink with the fractional serial id only for backward compatibility.

Well normally we already provide the "upstream" symlink so I'm not sure to understand why you needed to do it again.

It seems that this feature has been removed by purpose, see:

https://github.com/systemd/systemd/commit/ a0ee5a05bb3a9a838c35e07ff7a0bb7bbd2d0c9b

But it is unclear why.

And sadly there weren't any explications provided when this feature was simply removed... Let's see... -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c22 Thomas Blume changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jthumshirn@suse.c |needinfo?(fbui@suse.com) |om), | |needinfo?(francoisvalenduc@ | |gmail.com) | --- Comment #22 from Thomas Blume --- (In reply to Thomas Blume from comment #21)

Sorry, my fault, I don't add the upstream symlinks again. The diff of the udev rule is here:

https://github.com/tblume/systemd-suse-devel/commit/ 78e7459c1d4daecbd4dba8b327957d05ecdc7d56

Upstream declined re-adding the format lenght parameter, but it's rather for design reasons: --> I don't remember any specific reasons about this one, just that we removed a whole bunch of magic string mangling; we did not want udev provide shell-like features. People asked for more of them, and we thought they should use a shell, if they need to do more complicated things. [...] I think we should have less string mangling support in udev rules, not more. hence, let's not add this, I see little point in restoring something that was removed 8 years ago and nobody noticed or missed so far. Closing. I hope this makes sense. --< Question is where we go from here. Should we add a suse-only patch or move the funbctionality to an external script/binary? Franck, any thoughts? -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c24 --- Comment #24 from Thomas Blume --- (In reply to Franck Bui from comment #23)

Couldn't we try to do the string mangling in the rule file directly instead, something like that instead ?

...

IMPORT{program}="/bin/sh -c 'ID_SERIAL=$env{ID_SERIAL}; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}'

The shell expression looks simple enough to be embedded in the rule.

That doesn't work because you can't expand ID_SERIAL within the shell. e.g. the following rule: KERNEL=="nvme*", ENV{DEVTYPE}=="disk", IMPORT{program}="/bin/sh -c 'ID_SERIAL=$env{ID_SERIAL}; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}'" leaves ID_SERIAL_TRUNCATED empty: --> IMPORT '/bin/sh -c 'ID_SERIAL=; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}'' /usr/lib/udev/rules.d/60-persistent-storage-compat.rules:47 starting '/bin/sh -c 'ID_SERIAL=; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}'' '/bin/sh -c 'ID_SERIAL=; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}''(out) 'ID_SERIAL_TRUNCATED=' Process '/bin/sh -c 'ID_SERIAL=; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}'' succeeded. --< because ID_SERIAL is not present in the shell environment. But passing an udev variable/attribute to a shell script works, e.g. the rule: KERNEL=="nvme*", ENV{DEVTYPE}=="disk", ATTRS{model}=="?*", IMPORT{program}="/tmp/echo_args.sh $attr{model}" using a script like: --> # cat /tmp/echo_args.sh #!/bin/sh IN=${@/ /_} OUT=$(echo ${IN/ /_}) echo ID_MODEL_TRUNCATED=${OUT:0:16} --< produces the correct output: --> IMPORT '/tmp/echo_args.sh INTEL SSDPEDMD800G4' /usr/lib/udev/rules.d/60-persistent-storage-compat.rules:50 starting '/tmp/echo_args.sh INTEL SSDPEDMD800G4' '/tmp/echo_args.sh INTEL SSDPEDMD800G4'(out) 'ID_MODEL_TRUNCATED=INTEL_SSDPEDMD80' Process '/tmp/echo_args.sh INTEL SSDPEDMD800G4' succeeded. --< -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c26 --- Comment #26 from Thomas Blume --- (In reply to Franck Bui from comment #25)

(In reply to Thomas Blume from comment #24)

...

KERNEL=="nvme*", ENV{DEVTYPE}=="disk", IMPORT{program}="/bin/sh -c 'ID_SERIAL=$env{ID_SERIAL}; echo ID_SERIAL_TRUNCATED=${ID_SERIAL:0:16}'"

leaves ID_SERIAL_TRUNCATED empty:

because ID_SERIAL is not present in the shell environment.

Well that was an example which was supposed to show that string mangling could be inlined in the rule file.

Upstream seems to prefer less string mangling in the rules and to favour using shell scripts instead (see comment#22). I would follow them in this case.

ID_SERIAL or whatever that needs to be used, is supposed to be retrieved from sysfs by using the "$attr{xxx}" syntax.

Yes, that's what the rule is doing.

Please note that we should make sure to not pollute the environment that is already setup by the upstream rules. Therefore setting ID_SERIAL or ID_MODEL in 61-xxx is not a good idea.

Agreed, but can we be sure that the upstream rules always run before the compat rules? I see it as a kind of security measure to also set ID_SERIAL in the compat rules. Since it takes the data from sysfs (just like the upstream rules do), we shouldn't overwrite something with wrong values. But that way we can be sure that the compat rules work, even if the upstream rules fail or are executed later. Does that make sense? -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c28 --- Comment #28 from Thomas Blume --- (In reply to Franck Bui from comment #27)

(In reply to Thomas Blume from comment #26)

...

Upstream seems to prefer less string mangling in the rules and to favour using shell scripts instead (see comment#22). I would follow them in this case.

I'm not sure upstream claims that but I really prefer have the oneline shell command embedded in the rule file. It doesn't really make sense to carry a shell script that has a single command.

Ok, so I will try to make this work without a shell script. Let's see if I can find a way.

Currently upstream rule file is named 60-xxx and ours is 61-xxx, so that should be ok.

Ok, so I will rely on this.

Well if upstream doesn't set them we shouldn't either and if they set it then we can reuse them without doing any modifications.

Agreed -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 Kresten P. Vester changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kresten@vester.org -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c31 --- Comment #31 from Franck Bui --- Hi Thomas, sorry for the delay... Could you post your patch to @systemd-maintainers ? Thanks. -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c33 --- Comment #33 from Franck Bui --- Really strange, I can't find your email... could you send it again ? -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 SMASH SMASH changed: What |Removed |Added ---------------------------------------------------------------------------- Whiteboard| | maint:planned:update -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 Swamp Workflow Management changed: What |Removed |Added ---------------------------------------------------------------------------- Whiteboard|maint:planned:update |ibs:running:7054:moderate |ibs:running:7054:moderate | -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 Swamp Workflow Management changed: What |Removed |Added ---------------------------------------------------------------------------- Whiteboard|ibs:running:7054:moderate | -- You are receiving this mail because: You are on the CC list for the bug.

http://bugzilla.suse.com/show_bug.cgi?id=1051465 http://bugzilla.suse.com/show_bug.cgi?id=1051465#c45 --- Comment #45 from Swamp Workflow Management --- openSUSE-RU-2018:1433-1: An update that has 9 recommended fixes can now be installed. Category: recommended (moderate) Bug References: 1045092,1051465,1066422,1075804,1082485,1084626,1085062,1086785,1087323 CVE References: Sources used: openSUSE Leap 42.3 (src): systemd-228-50.1, systemd-mini-228-50.1 -- You are receiving this mail because: You are on the CC list for the bug.