Merge ~rafaeldtinoco/ubuntu/+source/sg3-utils:lp1833618-bionic into ubuntu/+source/sg3-utils:ubuntu/bionic-devel

Proposed by Rafael David Tinoco on 2019-10-01
Status: Needs review
Proposed branch: ~rafaeldtinoco/ubuntu/+source/sg3-utils:lp1833618-bionic
Merge into: ubuntu/+source/sg3-utils:ubuntu/bionic-devel
Diff against target: 156 lines (+130/-0)
4 files modified
debian/changelog (+8/-0)
debian/patches/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch (+85/-0)
debian/patches/new-location-for-major-and-minor.patch (+35/-0)
debian/patches/series (+2/-0)
Reviewer Review Type Date Requested Status
Rafael David Tinoco Approve on 2019-10-22
Ryan Harper (community) 2019-10-11 Approve on 2019-10-18
Ubuntu Core Development Team 2019-10-01 Pending
Review via email: mp+373440@code.launchpad.net
To post a comment you must log in.
Rafael David Tinoco (rafaeldtinoco) wrote :

I'll wait for feedback in bug:

https://bugs.launchpad.net/ubuntu/+source/sg3-utils/+bug/1833618

Before asking for this to be sponsored (fyio).

d8fe484... by Rafael David Tinoco on 2019-10-09

55-scsi-sg3_id.rules: ID_SERIAL fix for USB SPC-only devices

This upstream commit changes the udev rules logic for USB block devices
that only supports SCSI PRIMARY COMMANDS, not supporting SCSI INQ 0x80
nor 0x83.

Despite a possible delay, still existing in this version, whenever a
SPC-only block device (pendrive ?) is attached, this version fixes the
main issue for (LP: 1833618), wrong ID_SERIAL for those devices.

507c4d7... by Rafael David Tinoco on 2019-10-09

Fix for FTBFS in Ubuntu Disco due to header changes.

- new location for major() and minor()

0161dca... by Rafael David Tinoco on 2019-10-09

changelog

Ryan Harper (raharper) wrote :

One comment for clarification. I believe this is OK as-is due to sg3-utils not being installed in the bionic server image by default.

Rafael David Tinoco (rafaeldtinoco) wrote :

you mean leaving Bionic as won't fix for this ?

I'm okay with that, public bug is already documenting the issue and has a patch if anyone wants (which is just changing udev rules).

Rafael David Tinoco (rafaeldtinoco) wrote :

sorry, ignore previous comment, checking inline comments..

Ryan Harper (raharper) wrote :

Thanks for the clarification.

review: Approve
Rafael David Tinoco (rafaeldtinoco) wrote :

Thx, I'll move on with this fix then (also based on Christian's review of the Disco version):

https://code.launchpad.net/~rafaeldtinoco/ubuntu/+source/sg3-utils/+git/sg3-utils/+merge/373439

review: Approve

Unmerged commits

0161dca... by Rafael David Tinoco on 2019-10-09

changelog

507c4d7... by Rafael David Tinoco on 2019-10-09

Fix for FTBFS in Ubuntu Disco due to header changes.

- new location for major() and minor()

d8fe484... by Rafael David Tinoco on 2019-10-09

55-scsi-sg3_id.rules: ID_SERIAL fix for USB SPC-only devices

This upstream commit changes the udev rules logic for USB block devices
that only supports SCSI PRIMARY COMMANDS, not supporting SCSI INQ 0x80
nor 0x83.

Despite a possible delay, still existing in this version, whenever a
SPC-only block device (pendrive ?) is attached, this version fixes the
main issue for (LP: 1833618), wrong ID_SERIAL for those devices.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 15acda8..be1c47c 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+sg3-utils (1.42-2ubuntu2~18.04~1) bionic; urgency=medium
7+
8+ * d/p/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch: ID_SERIAL fix for USB
9+ SPC-only devices in 55-scsi-sg3_id.rules (LP: #1833618)
10+ * d/p/new-location-for-major-and-minor.patch: Fix FTBS header issue
11+
12+ -- Rafael David Tinoco <rafaeldtinoco@ubuntu.com> Wed, 09 Oct 2019 06:38:32 +0000
13+
14 sg3-utils (1.42-2ubuntu1) yakkety; urgency=medium
15
16 * debian/control, debian/rules: build a sg3-udeb installer package again.
17diff --git a/debian/patches/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch b/debian/patches/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch
18new file mode 100644
19index 0000000..7eddde3
20--- /dev/null
21+++ b/debian/patches/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch
22@@ -0,0 +1,85 @@
23+Description: 55-scsi-sg3_id.rules: ID_SERIAL fix for USB SPC-only devices
24+
25+Upstream commit had changes for:
26+
27+git-svn-id: svn://localhost/trunk@703 6180dd3e-e324-4e3e-922d-17de1ae2f315
28+---
29+ ChangeLog | 4 +++-
30+ scripts/55-scsi-sg3_id.rules | 15 ++++++++----
31+ scripts/rescan-scsi-bus.sh | 46 ++++++++++++++++++++----------------
32+ 3 files changed, 38 insertions(+), 27 deletions(-)
33+
34+But, for this SRU, I'm backporting ONLY the changes in:
35+
36+ 55-scsi-sg3_id.rules | 15 ++++++++++-----
37+ 1 file changed, 10 insertions(+), 5 deletions(-)
38+
39+Because I'm only interested in the fix for USB SPC-only devices ID_SERIAL.
40+
41+This upstream commit changes the udev rules logic for USB block devices
42+that only supports SCSI PRIMARY COMMANDS, not supporting SCSI INQ 0x80
43+nor 0x83.
44+
45+Despite a possible delay, still existing in this version, whenever a
46+SPC-only block device (pendrive ?) is attached, this version fixes the
47+main issue for LP: 1833618, wrong ID_SERIAL for those devices.
48+
49+Udev rules from sg3-utils will, still, execute SCSI INQ 0X80 trying to use VPDs
50+from the kernel, and those will fail fast. Unfortunately, the subsequent SCSI
51+INQ 0x83 command, also trying to use VPDs from kernel, will take much longer
52+time (sometimes up to 1 minute) to fail when it sees there is no existing
53+vpd_pg83 file in sysfs and send an INQ to the USB block device, adding a delay
54+in the device attachment. This is not being addressed in this fix as this is
55+still present in upstream version.
56+
57+This last issue has to be addressed in a future sg3-utils-merge and
58+that is why is being documented here (when this patch will likely be
59+dropped).
60+
61+Signed-off-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
62+
63+Author: Douglas Gilbert <dgilbert@interlog.com>
64+Origin: upstream, https://github.com/hreinecke/sg3_utils/commit/988e967513f
65+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1833618
66+Last-Update: 2019-10-09
67+---
68+ scripts/55-scsi-sg3_id.rules | 15 ++++++++++-----
69+ 1 file changed, 10 insertions(+), 5 deletions(-)
70+
71+diff --git a/scripts/55-scsi-sg3_id.rules b/scripts/55-scsi-sg3_id.rules
72+index e914a5d..8c7356b 100644
73+--- a/scripts/55-scsi-sg3_id.rules
74++++ b/scripts/55-scsi-sg3_id.rules
75+@@ -7,7 +7,12 @@ SUBSYSTEM!="block", GOTO="sg3_utils_id_end"
76+ # Import values for partitions
77+ ENV{DEVTYPE}=="partition", IMPORT{parent}="SCSI_*", ENV{ID_SCSI}="1"
78+ # SCSI INQUIRY values
79+-KERNEL=="sd*[!0-9]|sr*", IMPORT{program}="/usr/bin/sg_inq --export $tempnode", ENV{ID_SCSI}="1"
80++# If the 'inquiry' sysfs attribute is present the kernel will already
81++# have scanned for VPD pages, so if the vpd page attribute is not
82++# present it is not supported (or deemed unsafe to access).
83++# Hence we can skip the call to sg_inq and avoid I/O altogether.
84++KERNEL=="sd*[!0-9]|sr*", IMPORT{program}="/usr/bin/sg_inq --export --inhex=/sys/block/$kernel/device/inquiry --raw", ENV{ID_SCSI}="1", ENV{ID_SCSI_SN}="1", ENV{ID_SCSI_DI}="1"
85++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}!="1", IMPORT{program}="/usr/bin/sg_inq --export $tempnode", ENV{ID_SCSI}="1"
86+ # scsi_id compat mappings
87+ ENV{SCSI_VENDOR}=="?*", ENV{ID_VENDOR}="$env{SCSI_VENDOR}"
88+ ENV{SCSI_VENDOR_ENC}=="?*", ENV{ID_VENDOR_ENC}="$env{SCSI_VENDOR_ENC}"
89+@@ -16,11 +21,11 @@ ENV{SCSI_MODEL_ENC}=="?*", ENV{ID_MODEL_ENC}="$env{SCSI_MODEL_ENC}"
90+ ENV{SCSI_REVISION}=="?*", ENV{ID_REVISION}="$env{SCSI_REVISION}"
91+ ENV{SCSI_TYPE}=="?*", ENV{ID_TYPE}="$env{SCSI_TYPE}"
92+ # SCSI EVPD page 0x80 values
93+-KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", IMPORT{program}="/usr/bin/sg_inq --export --inhex=/sys/block/$kernel/device/vpd_pg80 --raw", ENV{ID_SCSI_SN}="1"
94+-KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_SN}!="1", IMPORT{program}="/usr/bin/sg_inq --export --page=sn $tempnode", ENV{ID_SCSI_SN}="1"
95++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_SN}=="1", IMPORT{program}="/usr/bin/sg_inq --export --inhex=/sys/block/$kernel/device/vpd_pg80 --raw"
96++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_SN}!="1", IMPORT{program}="/usr/bin/sg_inq --export --page=sn $tempnode"
97+ # SCSI EVPD page 0x83 values
98+-KERNEL=="sd*[!0-9]", ENV{ID_SCSI}=="1", IMPORT{program}="/usr/bin/sg_inq --export --inhex=/sys/block/$kernel/device/vpd_pg83 --raw", ENV{ID_SCSI_DI}="1"
99+-KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_DI}!="1", IMPORT{program}="/usr/bin/sg_inq --export --page=di $tempnode", ENV{ID_SCSI_DI}="1"
100++KERNEL=="sd*[!0-9]", ENV{ID_SCSI}=="1", ENV{ID_SCSI_DI}=="1", IMPORT{program}="/usr/bin/sg_inq --export --inhex=/sys/block/$kernel/device/vpd_pg83 --raw"
101++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_DI}!="1", IMPORT{program}="/usr/bin/sg_inq --export --page=di $tempnode"
102+
103+ # ID_WWN compat mapping
104+ ENV{SCSI_IDENT_LUN_NAA_REGEXT}=="?*", ENV{ID_WWN}!="?*", ENV{ID_WWN}="0x$env{SCSI_IDENT_LUN_NAA_REGEXT}"
105+--
106+2.20.1
107+
108diff --git a/debian/patches/new-location-for-major-and-minor.patch b/debian/patches/new-location-for-major-and-minor.patch
109new file mode 100644
110index 0000000..4196aa7
111--- /dev/null
112+++ b/debian/patches/new-location-for-major-and-minor.patch
113@@ -0,0 +1,35 @@
114+Description: new location for major() and minor()
115+
116+[Backport]
117+ Only backported the header location change.
118+
119+Signed-off-by: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
120+
121+Author: Rafael David Tinoco <rafaeldtinoco@ubuntu.com>
122+Origin: backport, https://github.com/hreinecke/sg3_utils/commit/3e50c99af4a4
123+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sg3-utils/+bug/1833618
124+Last-Update: 2019-10-01
125+---
126+ src/sg_map26.c | 5 ++++-
127+ 1 file changed, 4 insertions(+), 1 deletion(-)
128+
129+diff --git a/src/sg_map26.c b/src/sg_map26.c
130+index 1a1301a..92f6de9 100644
131+--- a/src/sg_map26.c
132++++ b/src/sg_map26.c
133+@@ -31,8 +31,11 @@
134+ #include <dirent.h>
135+ #include <libgen.h>
136+ #include <sys/ioctl.h>
137+-#include <sys/types.h>
138+ #include <sys/stat.h>
139++#include <sys/sysmacros.h> /* new location for major + minor */
140++#ifndef major
141++#include <sys/types.h>
142++#endif
143+ #include <linux/major.h>
144+
145+ #ifdef HAVE_CONFIG_H
146+--
147+2.20.1
148+
149diff --git a/debian/patches/series b/debian/patches/series
150index 30d2fb2..fa0b7a4 100644
151--- a/debian/patches/series
152+++ b/debian/patches/series
153@@ -1 +1,3 @@
154 ship-rescan-script.sh
155+55-scsi-sg3_id.rules-ID_SERIAL-fix.patch
156+new-location-for-major-and-minor.patch

Subscribers

People subscribed via source and target branches