complete boot failure in 4.5-rc1 caused by nvme: make SG_IO support optional
James Bottomley
maintainerThe reason is fairly obvious: the default for the new option
BLK_DEV_NVME_SCSI is N and all the distribution kernels (and me when
testing) take the default options (I checked in the OBS kernel builds and this is true).
The net result is that scsi_id from udev no longer works on nvme disks and that means that the /dev/disk/by-id links are all gone in 4.5-rc1.
If this happens to be how you name your disks in fstab or crypttab,
you can't boot.
If you're going to break an ABI in this way, you really have to plan for it in userspace. How are NVMe disk ids supposed to be exported now? Does udev need a nvme_id program to do this? Until there's an infrastructure ready to work in this way, we need to unconditionally enable BLK_DEV_NVME_SCSI.
James
Christoph Hellwig
maintainerKeith said it should be on by default, and I promised him to change it once we run into problems, which I guess this counts as.
But just curious: what distro are you using? Upstream systemd explicitly rejected using scsi_id for NVMe here:
https://github.com/systemd/systemd/issues/1453
and all my test systems don't do this either.
James Bottomley
maintainerQuoted 9 lines · Christoph Hellwig
Keith said it should be on by default, and I promised him to change it once we run into problems, which I guess this counts as.
But just curious: what distro are you using? Upstream systemd explicitly rejected using
scsi_idfor NVMe here:https://github.com/systemd/systemd/issues/1453and all my test systems don't do this either.
This was SUSE (in my case, openSUSE Leap). I just checked the source package; they patch the by-id rules back in for NVME:
# PATCH-FIX-SUSE 1101-rules-persistent-device-names-for-NVMe-devices.patch (bsc#944132)
Patch1101: 1101-rules-persistent-device-names-for-NVMe-devices.patch
The bugzilla is giving access denied for bug id 944132, so it's likely some proprietary vendor problem. The patch has no preamble, so it's hard to tell what they were thinking.
James
Jens Axboe
maintainerQuoted 14 lines · James Bottomley
Quoted 9 lines · Christoph Hellwig
Keith said it should be on by default, and I promised him to change it once we run into problems, which I guess this counts as.
But just curious: what distro are you using? Upstream systemd explicitly rejected using
scsi_idfor NVMe here:https://github.com/systemd/systemd/issues/1453and all my test systems don't do this either.
This was SUSE (in my case, openSUSE Leap). I just checked the source package; they patch the by-id rules back in for NVME:
Quoted 6 lines
# PATCH-FIX-SUSE 1101-rules-persistent-device-names-for-NVMe-devices.patch (bsc#944132)
Patch1101: 1101-rules-persistent-device-names-for-NVMe-devices.patchThe bugzilla is giving access denied for bug id 944132, so it's likely some proprietary vendor problem. The patch has no preamble, so it's hard to tell what they were thinking.
I run root-on-nvme on my laptop, and I haven't observed any problems. Generally I hate for options to default y unless absolutely necessary, it's a sure fire way to feature creep your kernel without noticing. I don't think getting all hot about this issue is fair, if the only known case is suse.
If anything, let's make the description better. It's trying to be funny, it'd be better if it was descriptive and covered this case as well.
Signature · 1 line
Jens Axboe
James Bottomley
maintainerQuoted 22 lines · Jens Axboe
Quoted 21 lines · James Bottomley
Quoted 10 lines · Christoph Hellwig
Keith said it should be on by default, and I promised him to change
it once we run into problems, which I guess this counts as.But just curious: what distro are you using? Upstream systemd explicitly rejected using
scsi_idfor NVMe here:https://github.com/systemd/systemd/issues/1453and all my test systems don't do this either.
This was SUSE (in my case, openSUSE Leap). I just checked the source
package; they patch the by-id rules back in for NVME:# PATCH-FIX-SUSE 1101-rules-persistent-device-names-for-NVMe -devices.patch (bsc#944132)
Patch1101: 1101-rules-persistent-device-names-for-NVMe
-devices.patch
Quoted 7 lines
Quoted 5 lines
The bugzilla is giving access denied for bug id 944132, so it's likely
some proprietary vendor problem. The patch has no preamble, so it's
hard to tell what they were thinking.I run root-on-nvme on my laptop, and I haven't observed any problems.
Me too apparently. It looks like this problem may be SUSE specific unless another distro has enabled this. I can see why they would: you do need persistent names for devices, even NVMe ones.
Quoted 4 lines
Generally I hate for options to default y unless absolutely necessary, it's a sure fire way to feature creep your kernel without noticing. I don't think getting all hot about this issue is fair, if the only known case is suse.
Well, OK, I'm annoyed because it was a systemd system which means debugging boot failures are excruciatingly difficult so it took me a week and a half to find out what the problem was. Perhaps I was a bit rash to label this as an easily foreseen problem.
I opened a bug against SUSE to tell them to turn it on:
https://bugzilla.opensuse.org/show_bug.cgi?id=965497
The second problem is that there's currently no way to transition to using the serial attribute the way the udev 60-persistent-storage.rules are written, so if distros have some by-id hack, it will have to be maintained for a while. I annotated the already closed bug on this in systemd with the rules that work for me.
Quoted 3 lines
If anything, let's make the description better. It's trying to be funny, it'd be better if it was descriptive and covered this case as well.
The problem with this is that when moving to new kernels, distro maintainers don't read the new option help texts, they just take the defaults. However, I checked the only other distribution I use (debian) and they don't have a nvme persistent ID hack, so if someone checked ubuntu and Red Hat, I think all the majors are now covered and perhaps there's no need to do anything more.
James
Christoph Hellwig
maintainerQuoted 5 lines · James Bottomley
Quoted 1 line
I run root-on-nvme on my laptop, and I haven't observed any problems.
Me too apparently. It looks like this problem may be SUSE specific unless another distro has enabled this. I can see why they would: you do need persistent names for devices, even NVMe ones.
I don't have root on nvme, just my xfstests device, but I still didn't see the problem, neither did my various nvme development setups.
Quoted 9 lines
I opened a bug against SUSE to tell them to turn it on:
https://bugzilla.opensuse.org/show_bug.cgi?id=965497
The second problem is that there's currently no way to transition to using the serial attribute the way the udev 60-persistent-storage.rules are written, so if distros have some by-id hack, it will have to be maintained for a while. I annotated the already closed bug on this in systemd with the rules that work for me.
We now expose the NVMe serial and NGUI, out of which the evpd page is mangled depending on the NVMe spec version that the device supports as sysfs attributes, distros can do the same mangling if they want to support their old ids.
Quoted 47 lines · James Bottomley
Quoted 22 lines · Jens Axboe
Quoted 21 lines · James Bottomley
Quoted 10 lines · Christoph Hellwig
Keith said it should be on by default, and I promised him to change
it once we run into problems, which I guess this counts as.But just curious: what distro are you using? Upstream systemd explicitly rejected using
scsi_idfor NVMe here:https://github.com/systemd/systemd/issues/1453and all my test systems don't do this either.
This was SUSE (in my case, openSUSE Leap). I just checked the source
package; they patch the by-id rules back in for NVME:# PATCH-FIX-SUSE 1101-rules-persistent-device-names-for-NVMe -devices.patch (bsc#944132)
Patch1101: 1101-rules-persistent-device-names-for-NVMe
-devices.patchQuoted 7 lines
Quoted 5 lines
The bugzilla is giving access denied for bug id 944132, so it's likely
some proprietary vendor problem. The patch has no preamble, so it's
hard to tell what they were thinking.I run root-on-nvme on my laptop, and I haven't observed any problems.
Me too apparently. It looks like this problem may be SUSE specific unless another distro has enabled this. I can see why they would: you do need persistent names for devices, even NVMe ones.
Quoted 4 lines
Generally I hate for options to default y unless absolutely necessary, it's a sure fire way to feature creep your kernel without noticing. I don't think getting all hot about this issue is fair, if the only known case is suse.
Well, OK, I'm annoyed because it was a systemd system which means debugging boot failures are excruciatingly difficult so it took me a week and a half to find out what the problem was. Perhaps I was a bit rash to label this as an easily foreseen problem.
I opened a bug against SUSE to tell them to turn it on:
Quoted 8 lines
https://bugzilla.opensuse.org/show_bug.cgi?id=965497
The second problem is that there's currently no way to transition to using the serial attribute the way the udev 60-persistent-storage.rules are written, so if distros have some by-id hack, it will have to be maintained for a while. I annotated the already closed bug on this in systemd with the rules that work for me.
Why, but you can.
That's precisely what I did with the transition to sg_inq; I've added a new set of rules (55-sg_inq.rules and 59-sg-symlinks.rules) which will override the values from 60-persistent-storage.rules.
Do we have defined sysfs attributes for NVMe devices nowadays? If so I'd be willing to create/send some sysfs rules for them.
Cheers,
Hannes
Signature · 5 lines
Dr. Hannes Reinecke Teamlead Storage & Networking hare at suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N?rnberg)
Quoted 2 lines
Do we have defined sysfs attributes for NVMe devices nowadays?
/sys/block/nvme0n1/uuid
Quoted 1 line
If so I'd be willing to create/send some sysfs rules for them.
That'd be great!
Christoph Hellwig
maintainerQuoted 4 lines · Sagi Grimberg
Quoted 2 lines
Do we have defined sysfs attributes for NVMe devices nowadays?
/sys/block/nvme0n1/uuid
That's only supported for NVMe 1.1 and higher devices, and optional. For older or stupid devices we need to support the algorithm based on the serial attribute from nvme_fill_device_id_scsi_string() in drivers/nvme/host/scsi.c.
Which btw is ?ncorrect as it doesn't identify namespaces properly, and thus the associatation of 0 is an actively harmful lie.
Quoted 10 lines · Christoph Hellwig
Quoted 4 lines · Sagi Grimberg
Quoted 2 lines
Do we have defined sysfs attributes for NVMe devices nowadays?
/sys/block/nvme0n1/uuidThat's only supported for NVMe 1.1 and higher devices, and optional. For older or stupid devices we need to support the algorithm based on the serial attribute from
nvme_fill_device_id_scsi_string()indrivers/nvme/host/scsi.c.
It's even worse. NGUID was defined for 1.2 devices and higher. 1.1 devices should have EUI-64 at:
/sys/block/nvmeXnY/eui
1.2 devices will have either uuid or eui (or both).
The majority of devices in circulation today are 1.0, and need to concat these three entries to make a unique identifier:
/sys/block/nvmeXnY/device/serial
/sys/block/nvmeXnY/device/model
/sys/block/nvmeXnY/nsid
Quoted 17 lines · Keith Busch
Quoted 10 lines · Christoph Hellwig
Quoted 4 lines · Sagi Grimberg
Quoted 2 lines
Do we have defined sysfs attributes for NVMe devices nowadays?
/sys/block/nvme0n1/uuidThat's only supported for NVMe 1.1 and higher devices, and optional. For older or stupid devices we need to support the algorithm based on the serial attribute from
nvme_fill_device_id_scsi_string()indrivers/nvme/host/scsi.c.It's even worse. NGUID was defined for 1.2 devices and higher. 1.1 devices should have EUI-64 at:
/sys/block/nvmeXnY/eui
Quoted 8 lines
1.2 devices will have either uuid or eui (or both).
The majority of devices in circulation today are 1.0, and need to concat these three entries to make a unique identifier:
/sys/block/nvmeXnY/device/serial
/sys/block/nvmeXnY/device/model
/sys/block/nvmeXnY/nsid
Ok, so what about having a 'wwid' attribute which provides combined information (like scsi has)?
Cheers,
Hannes
Signature · 5 lines
Dr. Hannes Reinecke Teamlead Storage & Networking hare at suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N?rnberg)
Quoted 2 lines · Hannes Reinecke
Ok, so what about having a 'wwid' attribute which provides combined information (like scsi has)?
That looks like the sensible thing to do. Thanks for pointer.
Going forward, I will solicite more feedback from scsi developers so NVMe's external attributes better align with storage that already solved our issues. I agree with Christoph that we never should have relied on SCSI translations for NVMe, but we don't want to reinvent generic solutions either.
James Bottomley
maintainerQuoted 66 lines · Hannes Reinecke
Quoted 56 lines · James Bottomley
Quoted 23 lines · Jens Axboe
Quoted 22 lines · James Bottomley
Quoted 11 lines · Christoph Hellwig
Keith said it should be on by default, and I promised him to change
it once we run into problems, which I guess this counts as.But just curious: what distro are you using? Upstream systemd
explicitly rejected usingscsi_idfor NVMe here:https://github.com/systemd/systemd/issues/1453and all my test systems don't do this either.
This was SUSE (in my case, openSUSE Leap). I just checked the source
package; they patch the by-id rules back in for NVME:# PATCH-FIX-SUSE 1101-rules-persistent-device-names-for-NVMe -devices.patch (bsc#944132)
Patch1101: 1101-rules-persistent-device-names-for-NVMe
-devices.patchQuoted 8 lines
Quoted 5 lines
The bugzilla is giving access denied for bug id 944132, so it's likely
some proprietary vendor problem. The patch has no preamble, so it's
hard to tell what they were thinking.I run root-on-nvme on my laptop, and I haven't observed any problems.
Me too apparently. It looks like this problem may be SUSE specific unless another distro has enabled this. I can see why they would:
you
do need persistent names for devices, even NVMe ones.Quoted 6 lines
Generally I hate for options to default y unless absolutely necessary, it's a sure fire way to feature creep your kernel without
noticing. I don't think getting all hot about this issue is fair, if
the only known case is suse.Well, OK, I'm annoyed because it was a systemd system which means debugging boot failures are excruciatingly difficult so it took me a
week and a half to find out what the problem was. Perhaps I was a bit
rash to label this as an easily foreseen problem.I opened a bug against SUSE to tell them to turn it on:
Quoted 9 lines
The second problem is that there's currently no way to transition to
using the serial attribute the way the udev 60-persistent -storage.rules
are written, so if distros have some by-id hack, it will have to be maintained for a while. I annotated the already closed bug on this in
systemd with the rules that work for me.
Quoted 7 lines
Why, but you can.
That's precisely what I did with the transition tosg_inq; I've added a new set of rules (55-sg_inq.rules and 59-sg-symlinks.rules) which will override the values from 60-persistent-storage.rules.Do we have defined sysfs attributes for NVMe devices nowadays? If so I'd be willing to create/send some sysfs rules for them.
This is the one I finally settled on (for 60-persistent-storage) for SUSE. It keeps everything working across old and new kernels.
# nvme
SUBSYSTEMS=="nvme", ATTRS{serial}=="?*", PROGRAM="/usr/bin/echo $attr{serial}", ENV{ID_SERIAL}="%c", ENV{ID_BUS}="nvme"I already complained to Keith that the echo is required to strip the leading whitespace which I don't think should be in the serial attribute.
James
Christoph Hellwig
maintainerJens,
do you want a 'default y' patch or just a better description? I'd be happy to send either one.
Jens Axboe
maintainerQuoted 5 lines · Christoph Hellwig
Jens,
do you want a 'default y' patch or just a better description? I'd be happy to send either one.
A better description
Signature · 1 line
Jens Axboe
Christoph Hellwig
maintainerDoes this looks reasonable?
---
Quoted 1 line
From
7843fae979df3fc14007735f54cc6bb2f6f66dc5Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Tue, 9 Feb 2016 18:11:32 +0100
Subject: nvme: fix Kconfig description for BLK_DEV_NVME_SCSI
--- drivers/nvme/host/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/nvme/host/Kconfig +2 -2
@@ -17,8 +17,8 @@ config BLK_DEV_NVME_SCSI and block devices nodes, as well a a translation for a small number of selected SCSI commands to NVMe commands to the NVMe driver. If you don't know what this means you probably want - to say N here, and if you know what it means you probably - want to say N as well. + to say N here, unless you run a distro that abuses this for + stable device names like some OpenSuSE and SLES versions. config NVME_FABRICS tristate
Signature · 1 line
2.1.4
Jens Axboe
maintainerQuoted 18 lines · Christoph Hellwig
Does this looks reasonable?
---
From7843fae979df3fc14007735f54cc6bb2f6f66dc5Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch at lst.de>
Date: Tue, 9 Feb 2016 18:11:32 +0100
Subject: nvme: fixKconfigdescription forBLK_DEV_NVME_SCSISigned-off-by Christoph Hellwig <hch at lst.de>--- drivers/nvme/host/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/nvme/host/Kconfig +0
-0
drivers/nvme/host/Kconfig config BLK_DEV_NVME_SCSI · line 21
Quoted 7 lines
drivers/nvme/host/Kconfig +2
-2@@ -17,8 +17,8 @@ config BLK_DEV_NVME_SCSI and block devices nodes, as well a a translation for a small number of selected SCSI commands to NVMe commands to the NVMe driver. If you don't know what this means you probably want - to say N here, and if you know what it means you probably - want to say N as well. + to say N here, unless you run a distro that abuses this for + stable device names like some OpenSuSE and SLES versions.
Yep, that looks a lot more reasonable to me. Might be wort including that it impacts the mount-by-id on those distros.
Signature · 1 line
Jens Axboe
Christoph Hellwig
maintainerUpdated version below:
---
Quoted 1 line
From
d63251560cf2670badbc86c83502502f29c087e0Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <[email protected]>
Date: Tue, 9 Feb 2016 18:11:32 +0100
Subject: nvme: fix Kconfig description for BLK_DEV_NVME_SCSI
--- drivers/nvme/host/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/nvme/host/Kconfig +3 -2
@@ -17,8 +17,9 @@ config BLK_DEV_NVME_SCSI and block devices nodes, as well a a translation for a small number of selected SCSI commands to NVMe commands to the NVMe driver. If you don't know what this means you probably want - to say N here, and if you know what it means you probably - want to say N as well. + to say N here, unless you run a distro that abuses the SCSI + emulation to provide stable device names for mount by id like + some OpenSuSE and SLES versions. config NVME_FABRICS tristate
Signature · 1 line
2.1.4
Jens Axboe
maintainerQuoted 18 lines · Christoph Hellwig
Updated version below:
---
Fromd63251560cf2670badbc86c83502502f29c087e0Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch at lst.de>
Date: Tue, 9 Feb 2016 18:11:32 +0100
Subject: nvme: fixKconfigdescription forBLK_DEV_NVME_SCSISigned-off-by Christoph Hellwig <hch at lst.de>--- drivers/nvme/host/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/nvme/host/Kconfig +0
-0
drivers/nvme/host/Kconfig config BLK_DEV_NVME_SCSI · line 22
Quoted 8 lines
drivers/nvme/host/Kconfig +3
-2@@ -17,8 +17,9 @@ config BLK_DEV_NVME_SCSI and block devices nodes, as well a a translation for a small number of selected SCSI commands to NVMe commands to the NVMe driver. If you don't know what this means you probably want - to say N here, and if you know what it means you probably - want to say N as well. + to say N here, unless you run a distro that abuses the SCSI + emulation to provide stable device names for mount by id like + some OpenSuSE and SLES versions.
Thanks, looks good to me, I'll fold it in for the current series.
Signature · 1 line
Jens Axboe
James Bottomley
maintainerQuoted 4 lines · Christoph Hellwig
Jens,
do you want a 'default y' patch or just a better description? I'd be happy to send either one.
Since it only appears to be SUSE and they've now been told, better description is fine.
James
Quoted 14 lines · James Bottomley
Quoted 9 lines · Christoph Hellwig
Keith said it should be on by default, and I promised him to change it once we run into problems, which I guess this counts as.
But just curious: what distro are you using? Upstream systemd explicitly rejected using
scsi_idfor NVMe here:https://github.com/systemd/systemd/issues/1453and all my test systems don't do this either.
This was SUSE (in my case, openSUSE Leap). I just checked the source package; they patch the by-id rules back in for NVME:
Quoted 7 lines
# PATCH-FIX-SUSE 1101-rules-persistent-device-names-for-NVMe-devices.patch (bsc#944132)
Patch1101: 1101-rules-persistent-device-names-for-NVMe-devices.patchThe bugzilla is giving access denied for bug id 944132, so it's likely some proprietary vendor problem. The patch has no preamble, so it's hard to tell what they were thinking.
They didn't think at all. That abovementioned bug just states 'by-id symlinks for NVMe drives are missing'.
And they fixed it by add the respective rules (using sg_inq) to udev.
There's no mentioning of any NVMe specific sysfs attributes whatsoever.
Cheers,
Hannes
Signature · 5 lines
Dr. Hannes Reinecke Teamlead Storage & Networking hare at suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG N?rnberg)