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

[Rafael David Tinoco]

Bug: https://bugs.launchpad.net/ubuntu/+source/sg3-utils/+bug/1833618
PPA: https://launchpad.net/~rafaeldtinoco/+archive/ubuntu/lp1833618

[Rafael David Tinoco (rafaeldtinoco)]

https://bugs.launchpad.net/curtin/+bug/1833618/comments/45:

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)]

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).

[Rafael David Tinoco (rafaeldtinoco)]

review: Approve

[Christian Ehrhardt  (paelzer)]

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)]

Minor comments on the FTBFS fixes as well

[Christian Ehrhardt  (paelzer)]

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)]

Fixed, re-pushing.

[Christian Ehrhardt  (paelzer)]

LGTM now, thanks for the fixups.

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

review: Approve

[Rafael David Tinoco (rafaeldtinoco)]

Okay so based on the 2 approvals, including:

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

I'll go on with this fix.

review: Approve

[Rafael David Tinoco (rafaeldtinoco)]

> 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)]

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

[Christian Ehrhardt  (paelzer)]

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:
1.42-2ubuntu2~19.04~1

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

See https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

[Christian Ehrhardt  (paelzer)]

$ 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://git.launchpad.net/~usd-import-team/ubuntu/+source/sg3-utils
 * [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 upload.ubuntu.com):
  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

diff --git a/debian/changelog b/debian/changelog
index 15acda8..e53287a 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+sg3-utils (1.42-2ubuntu2~19.04~1) disco; urgency=medium
+
+  * d/p/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch: ID_SERIAL fix for USB
+    SPC-only devices in 55-scsi-sg3_id.rules (LP: #1833618)
+  * d/p/new-location-for-major-and-minor.patch: Fix FTBS header issue
+
+ -- Rafael David Tinoco <[e-mail]>  Wed, 09 Oct 2019 06:13:54 +0000
+
 sg3-utils (1.42-2ubuntu1) yakkety; urgency=medium
 
   * debian/control, debian/rules: build a sg3-udeb installer package again.
diff --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
new file mode 100644
index 0000000..7eddde3
--- /dev/null
+++ b/debian/patches/55-scsi-sg3_id.rules-ID_SERIAL-fix.patch
@@ -0,0 +1,85 @@
+Description: 55-scsi-sg3_id.rules: ID_SERIAL fix for USB SPC-only devices
+
+Upstream commit had changes for:
+
+git-svn-id: svn://localhost/trunk@703 6180dd3e-e324-4e3e-922d-17de1ae2f315
+---
+ ChangeLog                    |  4 +++-
+ scripts/55-scsi-sg3_id.rules | 15 ++++++++----
+ scripts/rescan-scsi-bus.sh   | 46 ++++++++++++++++++++----------------
+ 3 files changed, 38 insertions(+), 27 deletions(-)
+
+But, for this SRU, I'm backporting ONLY the changes in:
+
+ 55-scsi-sg3_id.rules |   15 ++++++++++-----
+ 1 file changed, 10 insertions(+), 5 deletions(-)
+
+Because I'm only interested in the fix for USB SPC-only devices ID_SERIAL.
+
+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.
+
+Udev rules from sg3-utils will, still, execute SCSI INQ 0X80 trying to use VPDs
+from the kernel, and those will fail fast. Unfortunately, the subsequent SCSI
+INQ 0x83 command, also trying to use VPDs from kernel, will take much longer
+time (sometimes up to 1 minute) to fail when it sees there is no existing
+vpd_pg83 file in sysfs and send an INQ to the USB block device, adding a delay
+in the device attachment. This is not being addressed in this fix as this is
+still present in upstream version.
+
+This last issue has to be addressed in a future sg3-utils-merge and
+that is why is being documented here (when this patch will likely be
+dropped).
+
+Signed-off-by: Rafael David Tinoco <[e-mail]>
+
+Author: Douglas Gilbert <[e-mail]>
+Origin: upstream, https://github.com/hreinecke/sg3_utils/commit/988e967513f
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1833618
+Last-Update: 2019-10-09
+---
+ scripts/55-scsi-sg3_id.rules | 15 ++++++++++-----
+ 1 file changed, 10 insertions(+), 5 deletions(-)
+
+diff --git a/scripts/55-scsi-sg3_id.rules b/scripts/55-scsi-sg3_id.rules
+index e914a5d..8c7356b 100644
+--- a/scripts/55-scsi-sg3_id.rules
++++ b/scripts/55-scsi-sg3_id.rules
+@@ -7,7 +7,12 @@ SUBSYSTEM!="block", GOTO="sg3_utils_id_end"
+ # Import values for partitions
+ ENV{DEVTYPE}=="partition", IMPORT{parent}="SCSI_*", ENV{ID_SCSI}="1"
+ # SCSI INQUIRY values
+-KERNEL=="sd*[!0-9]|sr*", IMPORT{program}="/usr/bin/sg_inq --export $tempnode", ENV{ID_SCSI}="1"
++# If the 'inquiry' sysfs attribute is present the kernel will already
++# have scanned for VPD pages, so if the vpd page attribute is not
++# present it is not supported (or deemed unsafe to access).
++# Hence we can skip the call to sg_inq and avoid I/O altogether.
++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"
++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}!="1", IMPORT{program}="/usr/bin/sg_inq --export $tempnode", ENV{ID_SCSI}="1"
+ # scsi_id compat mappings
+ ENV{SCSI_VENDOR}=="?*", ENV{ID_VENDOR}="$env{SCSI_VENDOR}"
+ ENV{SCSI_VENDOR_ENC}=="?*", ENV{ID_VENDOR_ENC}="$env{SCSI_VENDOR_ENC}"
+@@ -16,11 +21,11 @@ ENV{SCSI_MODEL_ENC}=="?*", ENV{ID_MODEL_ENC}="$env{SCSI_MODEL_ENC}"
+ ENV{SCSI_REVISION}=="?*", ENV{ID_REVISION}="$env{SCSI_REVISION}"
+ ENV{SCSI_TYPE}=="?*", ENV{ID_TYPE}="$env{SCSI_TYPE}"
+ # SCSI EVPD page 0x80 values
+-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"
+-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"
++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"
++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_SN}!="1", IMPORT{program}="/usr/bin/sg_inq --export --page=sn $tempnode"
+ # SCSI EVPD page 0x83 values
+-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"
+-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"
++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"
++KERNEL=="sd*[!0-9]|sr*", ENV{ID_SCSI}=="1", ENV{ID_SCSI_DI}!="1", IMPORT{program}="/usr/bin/sg_inq --export --page=di $tempnode"
+ 
+ # ID_WWN compat mapping
+ ENV{SCSI_IDENT_LUN_NAA_REGEXT}=="?*", ENV{ID_WWN}!="?*", ENV{ID_WWN}="0x$env{SCSI_IDENT_LUN_NAA_REGEXT}"
+-- 
+2.20.1
+
diff --git a/debian/patches/new-location-for-major-and-minor.patch b/debian/patches/new-location-for-major-and-minor.patch
new file mode 100644
index 0000000..4196aa7
--- /dev/null
+++ b/debian/patches/new-location-for-major-and-minor.patch
@@ -0,0 +1,35 @@
+Description: new location for major() and minor()
+
+[Backport]
+    Only backported the header location change.
+
+Signed-off-by: Rafael David Tinoco <[e-mail]>
+
+Author: Rafael David Tinoco <[e-mail]>
+Origin: backport, https://github.com/hreinecke/sg3_utils/commit/3e50c99af4a4
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sg3-utils/+bug/1833618
+Last-Update: 2019-10-01
+---
+ src/sg_map26.c | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/src/sg_map26.c b/src/sg_map26.c
+index 1a1301a..92f6de9 100644
+--- a/src/sg_map26.c
++++ b/src/sg_map26.c
+@@ -31,8 +31,11 @@
+ #include <dirent.h>
+ #include <libgen.h>
+ #include <sys/ioctl.h>
+-#include <sys/types.h>
+ #include <sys/stat.h>
++#include <sys/sysmacros.h>      /* new location for major + minor */
++#ifndef major
++#include <sys/types.h>
++#endif
+ #include <linux/major.h>
+ 
+ #ifdef HAVE_CONFIG_H
+-- 
+2.20.1
+
diff --git a/debian/patches/series b/debian/patches/series
index 30d2fb2..fa0b7a4 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,3 @@
 ship-rescan-script.sh
+55-scsi-sg3_id.rules-ID_SERIAL-fix.patch
+new-location-for-major-and-minor.patch