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

Proposed by Rafael David Tinoco on 2019-10-01
Status: Merged
Approved by: Christian Ehrhardt  on 2019-10-23
Approved revision: 78092a6a38994f2dd17bcd21b1bc1892302e33d2
Merged at revision: 78092a6a38994f2dd17bcd21b1bc1892302e33d2
Proposed branch: ~rafaeldtinoco/ubuntu/+source/sg3-utils:lp1833618-disco
Merge into: ubuntu/+source/sg3-utils:ubuntu/disco-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 (community) Approve on 2019-10-22
Christian Ehrhardt  2019-10-01 Approve on 2019-10-15
Ryan Harper 2019-10-11 Pending
Canonical Server Team 2019-10-09 Pending
Review via email:
To post a comment you must log in.
Rafael David Tinoco (rafaeldtinoco) wrote :

I have to re-submit Disco MR and a new PPA package, as the compiler in Disco is likely more recent and treating lots of "new warnings" as errors (-Wimplicit-fallthrough, etc...). I'll do it tomorrow!

review: Needs Fixing
Rafael David Tinoco (rafaeldtinoco) wrote :

I'll wait for feedback in bug:

Before asking for this to be sponsored (fyio).

review: Approve
Christian Ehrhardt  (paelzer) wrote :

The patch looks correctly ported form upstream (the needed snippet applies as-is), I'll leave the effect it has on the system to Ryan's review who is deeper into the tech side of this case already.

Minor fixups needed, therefore "Need Fixing"

review: Needs Fixing
Christian Ehrhardt  (paelzer) wrote :

Minor comments on the FTBFS fixes as well

Christian Ehrhardt  (paelzer) wrote :

Other than the stated patches LGTM, as I said functionally we might wait for Ryan to comment.
You might cleanup the style issue until then to be ready to upload.

Rafael David Tinoco (rafaeldtinoco) wrote :

Fixed, re-pushing.

Christian Ehrhardt  (paelzer) wrote :

LGTM now, thanks for the fixups.

One nit pick inline, just to bother you :-)

review: Approve
Rafael David Tinoco (rafaeldtinoco) wrote :
review: Approve
Rafael David Tinoco (rafaeldtinoco) wrote :

> I'll go on with this fix.

Christian, would you mind uploading this for me ? sg3-utils needs core-dev!

Thanks a lot!

PS: next one is the Bionic SRU (that will fix that original MAAS case).

Christian Ehrhardt  (paelzer) wrote :

I've checked the other MPs comments.
Marking as approved and sponsoring as requested.

Christian Ehrhardt  (paelzer) wrote :

Umm - only now I see a bad version number.
I either missed it on the initial review or it wasn't there back then.

Anyway - let me explain and fix it up on upload:

First of all "double ~" seems wrong and ~ means "before what is in front"
This is disco, so 19.04, which means before the 19.04 - that seems odd.

Current versions:
 sg3-utils | 1.42-2ubuntu1 | bionic | source, amd64, arm64, armhf, i386, ppc64el, s390x
 sg3-utils | 1.42-2ubuntu1 | disco | source, amd64, arm64, armhf, i386, ppc64el, s390x
 sg3-utils | 1.44-1ubuntu1 | eoan | source, amd64, arm64, armhf, i386, ppc64el, s390x
 sg3-utils | 1.44-1ubuntu1 | focal | source, amd64, arm64, armhf, i386, ppc64el, s390x

E+F are fixe. So we need new versions vor B+D and since they are the same now we need the release in the version - so far so good.

But that would usually be:
B: 1.42-2ubuntu1.18.04.1
D: 1.42-2ubuntu1.19.04.1


Christian Ehrhardt  (paelzer) wrote :

$ git push pkg upload/1.42-2ubuntu1.19.04.1
Enumerating objects: 26, done.
Counting objects: 100% (26/26), done.
Delta compression using up to 4 threads
Compressing objects: 100% (21/21), done.
Writing objects: 100% (21/21), 5.25 KiB | 1.31 MiB/s, done.
Total 21 (delta 11), reused 0 (delta 0)
To ssh://
 * [new tag] upload/1.42-2ubuntu1.19.04.1 -> upload/1.42-2ubuntu1.19.04.1

$ dput ubuntu ../sg3-utils_1.42-2ubuntu1.19.04.1_source.changes
Checking signature on .changes
gpg: ../sg3-utils_1.42-2ubuntu1.19.04.1_source.changes: Error checking signature from BA3E29338280B242: SignatureVerifyError: 0
Checking signature on .dsc
gpg: ../sg3-utils_1.42-2ubuntu1.19.04.1.dsc: Error checking signature from BA3E29338280B242: SignatureVerifyError: 0
Uploading to ubuntu (via ftp to
  Uploading sg3-utils_1.42-2ubuntu1.19.04.1.dsc: done.
  Uploading sg3-utils_1.42-2ubuntu1.19.04.1.debian.tar.xz: done.
  Uploading sg3-utils_1.42-2ubuntu1.19.04.1_source.buildinfo: done.
  Uploading sg3-utils_1.42-2ubuntu1.19.04.1_source.changes: done.
Successfully uploaded packages.

Now in the SRU Teams queue

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..e53287a 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+sg3-utils (1.42-2ubuntu2~19.04~1) disco; urgency=medium
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
12+ -- Rafael David Tinoco <> Wed, 09 Oct 2019 06:13:54 +0000
14 sg3-utils (1.42-2ubuntu1) yakkety; urgency=medium
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
25+Upstream commit had changes for:
27+git-svn-id: svn://localhost/trunk@703 6180dd3e-e324-4e3e-922d-17de1ae2f315
29+ ChangeLog | 4 +++-
30+ scripts/55-scsi-sg3_id.rules | 15 ++++++++----
31+ scripts/ | 46 ++++++++++++++++++++----------------
32+ 3 files changed, 38 insertions(+), 27 deletions(-)
34+But, for this SRU, I'm backporting ONLY the changes in:
36+ 55-scsi-sg3_id.rules | 15 ++++++++++-----
37+ 1 file changed, 10 insertions(+), 5 deletions(-)
39+Because I'm only interested in the fix for USB SPC-only devices ID_SERIAL.
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.
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.
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.
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
61+Signed-off-by: Rafael David Tinoco <>
63+Author: Douglas Gilbert <>
64+Origin: upstream,
66+Last-Update: 2019-10-09
68+ scripts/55-scsi-sg3_id.rules | 15 ++++++++++-----
69+ 1 file changed, 10 insertions(+), 5 deletions(-)
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
89+@@ -16,11 +21,11 @@ ENV{SCSI_MODEL_ENC}=="?*", ENV{ID_MODEL_ENC}="$env{SCSI_MODEL_ENC}"
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"
103+ # ID_WWN compat mapping
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()
117+ Only backported the header location change.
119+Signed-off-by: Rafael David Tinoco <>
121+Author: Rafael David Tinoco <>
122+Origin: backport,
124+Last-Update: 2019-10-01
126+ src/sg_map26.c | 5 ++++-
127+ 1 file changed, 4 insertions(+), 1 deletion(-)
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>
143+ #include <linux/major.h>
145+ #ifdef HAVE_CONFIG_H
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 @@


People subscribed via source and target branches