Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Control: severity -1 important HI Mauricio, On Mon, 2015-06-22 at 08:33 -0300, Mauricio Faria de Oliveira wrote: On 06/21/2015 06:16 AM, Ritesh Raj Sarraf wrote: Given that mulitpath doesn't do a release very often, I plan on rebasing on the latest git repo (which includes many of Hannes' fixes). They only reason I'm procrastinating is because neither of our patches have been accepted/pushed by Christoph. BTW, Christophe recently mentioned release plans on the mailing list ([dm-devel] Next release plans?), catching up with inbox around the summer holidays, but I don't know which timeframe that actually is. Maybe those patches get in too. I'll look into this soon. But lowering the severity because, from the description, it is again specific to one family of controllers. This week, I intend to work on both the patches. -- Ritesh Raj Sarraf | http://people.debian.org/~rrs Debian - The Universal Operating System signature.asc Description: This is a digitally signed message part
Processed: Re: Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Processing control commands: severity -1 important Bug #782400 [src:multipath-tools] multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute Severity set to 'important' from 'serious' -- 782400: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=782400 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Hi Ritesh, On 06/21/2015 06:16 AM, Ritesh Raj Sarraf wrote: Given that mulitpath doesn't do a release very often, I plan on rebasing on the latest git repo (which includes many of Hannes' fixes). They only reason I'm procrastinating is because neither of our patches have been accepted/pushed by Christoph. BTW, Christophe recently mentioned release plans on the mailing list ([dm-devel] Next release plans?), catching up with inbox around the summer holidays, but I don't know which timeframe that actually is. Maybe those patches get in too. -- Mauricio Faria de Oliveira IBM Linux Technology Center -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
On Thursday 28 May 2015 05:31 PM, Mauricio Faria de Oliveira wrote: These days I don't work on storage, i.e. don't have a team that spends the time on these tools. So I keep the upstream first policy, for now. Even my patch (kpartx) is very trivial but 've put it on hold until Christoph accepts it. If this really doesn't work in the long run, we may re-visit it. Ok, got it. Thanks for clarifying, I had misunderstood that. Given that mulitpath doesn't do a release very often, I plan on rebasing on the latest git repo (which includes many of Hannes' fixes). They only reason I'm procrastinating is because neither of our patches have been accepted/pushed by Christoph. -- Ritesh Raj Sarraf RESEARCHUT - http://www.researchut.com Necessity is the mother of invention. signature.asc Description: OpenPGP digital signature
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
On Thursday 28 May 2015 12:37 AM, Mauricio Faria de Oliveira wrote: Thanks! Do you think it would be a problem to include the patch? As explained the commit message, I think it's an obvious fix for the regression introduced (with 'chomp', or similar name) for the case when the sysfs attribute 'rev' contains only spaces -- because the 'chomp' (or similarly named) function will remove all spaces from the string, so its size will be zero (so, the check for size greater than, but not 'or equal to', will surely fail). No. It has nothing to do with the patch. These days I don't work on storage, i.e. don't have a team that spends the time on these tools. So I keep the upstream first policy, for now. Even my patch (kpartx) is very trivial but 've put it on hold until Christoph accepts it. If this really doesn't work in the long run, we may re-visit it. -- Ritesh Raj Sarraf RESEARCHUT - http://www.researchut.com Necessity is the mother of invention. signature.asc Description: OpenPGP digital signature
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
On Thu, 2015-05-28 at 12:21 +0530, Ritesh Raj Sarraf wrote: No. It has nothing to do with the patch. These days I don't work on storage, i.e. don't have a team that spends the time on these tools. So I keep the upstream first policy, for now. Even my patch (kpartx) is very trivial but 've put it on hold until Christoph accepts it. If this really doesn't work in the long run, we may re-visit it. Ok, got it. Thanks for clarifying, I had misunderstood that. Regards, Mauricio -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
On Wednesday 22 April 2015 05:12 PM, Mauricio Faria de Oliveira wrote: Hey Ritesh, On 04/21/2015 10:23 AM, Ritesh Raj Sarraf wrote: Is this been merged upstream ? No, I haven't seen any reply to it on dm-devel nor commits. I have the impression there's some delay between those 2 points. Yeah. I'm not sure what the workflow Christoph has. I have submitted some patches for kpartx, and haven't had any response. I just pinged Hannes on the mailing list, in case he has some time to check/acknowledge it, so we may have an idea if that's eventually going upstream or requires more changes, before including in Debian. Thanks! OKay!! THanks. I'll put this one on hold then. The other one, is already ready for Unstable. And as I write, I'm also preparing the same for Jessie+U1. -- Ritesh Raj Sarraf RESEARCHUT - http://www.researchut.com Necessity is the mother of invention. signature.asc Description: OpenPGP digital signature
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Hi Ritesh, On Wed, 2015-05-27 at 23:40 +0530, Ritesh Raj Sarraf wrote: OKay!! THanks. I'll put this one on hold then. The other one, is already ready for Unstable. And as I write, I'm also preparing the same for Jessie+U1. Thanks! Do you think it would be a problem to include the patch? As explained the commit message, I think it's an obvious fix for the regression introduced (with 'chomp', or similar name) for the case when the sysfs attribute 'rev' contains only spaces -- because the 'chomp' (or similarly named) function will remove all spaces from the string, so its size will be zero (so, the check for size greater than, but not 'or equal to', will surely fail). Kind regards, Mauricio -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Hey Ritesh, On 04/21/2015 10:23 AM, Ritesh Raj Sarraf wrote: Is this been merged upstream ? No, I haven't seen any reply to it on dm-devel nor commits. I have the impression there's some delay between those 2 points. I just pinged Hannes on the mailing list, in case he has some time to check/acknowledge it, so we may have an idea if that's eventually going upstream or requires more changes, before including in Debian. Thanks! -- Mauricio Faria de Oliveira IBM Linux Technology Center -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
On Monday 13 April 2015 05:52 AM, Mauricio Faria de Oliveira wrote: Oops, please use this version (-v2). When the forwarded patch is applied in the source package, the change ends up in cciss_sysfs_pathinfo() rather than scsi_sysfs_pathinfo(): Hunk #1 succeeded at 783 (offset -312 lines). This version has the change in the right place, plus quilt refresh. Hi Mauricio, Is this been merged upstream ? -- Ritesh Raj Sarraf RESEARCHUT - http://www.researchut.com Necessity is the mother of invention. signature.asc Description: OpenPGP digital signature
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Oops, please use this version (-v2).
When the forwarded patch is applied in the source package, the change
ends up in cciss_sysfs_pathinfo() rather than scsi_sysfs_pathinfo():
Hunk #1 succeeded at 783 (offset -312 lines).
This version has the change in the right place, plus quilt refresh.
Thanks!
--
Mauricio Faria de Oliveira
IBM Linux Technology Center
diff -Nru multipath-tools-0.5.0/debian/changelog
multipath-tools-0.5.0/debian/changelog
--- multipath-tools-0.5.0/debian/changelog 2015-04-13 05:26:21.0
+0530
+++ multipath-tools-0.5.0/debian/changelog 2015-04-13 05:30:14.0
+0530
@@ -1,3 +1,9 @@
+multipath-tools (0.5.0-6pathselector1blankrev1) UNRELEASED; urgency=medium
+
+ * 0014-libmultipath-discovery-blank-rev-attr.patch: fix discovery of devices
with blank rev.
+
+ -- Mauricio Faria de Oliveira mauri...@linux.vnet.ibm.com Sat, 11 Apr 2015
12:40:19 -0300
+
multipath-tools (0.5.0-6pathselector1) UNRELEASED; urgency=medium
* initramfs/hooks: include dm-service-time in initramfs (new default path
selector).
diff -Nru
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
---
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
1970-01-01 05:30:00.0 +0530
+++
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
2015-04-13 05:31:34.0 +0530
@@ -0,0 +1,37 @@
+Forwarded: https://www.redhat.com/archives/dm-devel/2015-April/msg00049.html
+From b7ec63e727880aa709c2aa7b6029dba9ad7d9d74 Mon Sep 17 00:00:00 2001
+From: Mauricio Faria de Oliveira mauri...@linux.vnet.ibm.com
+Date: Sat, 11 Apr 2015 12:02:29 -0300
+Subject: [PATCH] libmultipath: fix discovery of devices with sysfs attr
containing only spaces
+
+commit 43c61e979521b17d9f50ea8e722682c33d0b7d69 causes sysfs_get_attr() to
+return 0 if attr contains only spaces, and scsi_sysfs_pathinfo() to return 1
+in 'if (sysfs_get_attr() = 0)'; this breaks the device's discovery.
+
+Changing the comparison operator '=' to '' allows discovery to continue.
+
+This affects some IPR controllers (rev sysfs attr):
+
+ # grep . /sys/block/sda/device/{vendor,model,rev} | tr ' ' '#'
+ /sys/block/sda/device/vendor:IBM#
+ /sys/block/sda/device/model:IPR-0###5EC99A00
+ /sys/block/sda/device/rev:
+
+Signed-off-by: Mauricio Faria de Oliveira mauri...@linux.vnet.ibm.com
+---
+ libmultipath/discovery.c |2 +-
+ 1 files changed, 1 insertions(+), 1 deletions(-)
+
+Index: multipath-tools-0.5.0/libmultipath/discovery.c
+===
+--- multipath-tools-0.5.0.orig/libmultipath/discovery.c
multipath-tools-0.5.0/libmultipath/discovery.c
+@@ -661,7 +661,7 @@ scsi_sysfs_pathinfo (struct path * pp)
+
+ condlog(3, %s: product = %s, pp-dev, pp-product_id);
+
+- if (sysfs_get_rev(parent, pp-rev, SCSI_REV_SIZE) = 0)
++ if (sysfs_get_rev(parent, pp-rev, SCSI_REV_SIZE) 0)
+ return 1;
+
+ condlog(3, %s: rev = %s, pp-dev, pp-rev);
diff -Nru multipath-tools-0.5.0/debian/patches/series
multipath-tools-0.5.0/debian/patches/series
--- multipath-tools-0.5.0/debian/patches/series 2014-09-05 17:42:48.0
+0530
+++ multipath-tools-0.5.0/debian/patches/series 2015-04-13 05:30:14.0
+0530
@@ -4,3 +4,4 @@
0011-patch-udev-rule.patch
0012-format-security-build-fixes.patch
0013-fix-spelling-error.patch
+0014-libmultipath-discovery-blank-rev-attr.patch
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
Package: src:multipath-tools
Version: 0.5.0-6
Severity: serious
Tags: patch
Forwarded: https://www.redhat.com/archives/dm-devel/2015-April/msg00049.html
Hi Ritesh,
I noticed some IPR controllers with multipath support may have only
spaces in the rev sysfs attribute, which breaks a device's discovery
(example below got no paths at all).
May you please consider this (forwarded) patch for Jessie?
Thank you.
Example:
---
The disks in this IPR controller:
# grep . /sys/block/sda/device/{vendor,model,rev} | tr ' ' '#'
/sys/block/sda/device/vendor:IBM#
/sys/block/sda/device/model:IPR-0###5EC99A00
/sys/block/sda/device/rev:
All of them (6 disks, 2 paths per disk):
# grep . /sys/block/sd*/device/rev | tr ' ' '#'
/sys/block/sda/device/rev:
/sys/block/sdb/device/rev:
/sys/block/sdc/device/rev:
/sys/block/sdd/device/rev:
/sys/block/sde/device/rev:
/sys/block/sdf/device/rev:
/sys/block/sdg/device/rev:
/sys/block/sdh/device/rev:
/sys/block/sdi/device/rev:
/sys/block/sdj/device/rev:
/sys/block/sdk/device/rev:
/sys/block/sdl/device/rev:
Comparison of 'multipath -v3' without/with the patch
(notice 'no paths' vs. 'paths list')
# diff -u multipath-v3.no-timestamp.{old,new}
...
sda: vendor = IBM
sda: product = IPR-0 5EC99A00
+ sda: rev =
+ sda: h:b:t:l = 0:2:0:0
+ sda: tgt_node_name =
+ sda: path state = running
+ sda: 4252 cyl, 128 heads, 32 sectors/track, start at 0
...
+ sda: alua prio = 50
sdb: ...
...
-= no paths =
+= paths list =
+uuidhcildev dev_t pri dm_st chk_st
vend/prod/rev
+1IBM_IPR-0_5EC99A000410 0:2:0:0 sda 8:0 50 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A0003F0 0:2:1:0 sdb 8:16 50 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A0003D0 0:2:2:0 sdc 8:32 50 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A000430 0:2:3:0 sdd 8:48 50 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC94760 0:2:4:0 sde 8:64 50 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC94780 0:2:5:0 sdf 8:80 50 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A000410 1:2:0:0 sdg 8:96 10 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A0003F0 1:2:1:0 sdh 8:112 10 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A0003D0 1:2:2:0 sdi 8:128 10 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC94780 1:2:3:0 sdj 8:144 10 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC94760 1:2:4:0 sdk 8:160 10 undef ready
IBM,IPR-0 5EC
+1IBM_IPR-0_5EC99A000430 1:2:5:0 sdl 8:176 10 undef ready
IBM,IPR-0 5EC
...
--
Mauricio Faria de Oliveira
IBM Linux Technology Center
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#782400: multipath-tools: libmultipath: fix discovery of devices with empty rev sysfs attribute
On 04/11/2015 01:00 PM, Mauricio Faria de Oliveira wrote:
Tags: patch
It helps to attach the patch :)
--
Mauricio Faria de Oliveira
IBM Linux Technology Center
diff -Nru multipath-tools-0.5.0/debian/changelog
multipath-tools-0.5.0/debian/changelog
--- multipath-tools-0.5.0/debian/changelog 2015-03-04 05:53:28.0
-0300
+++ multipath-tools-0.5.0/debian/changelog 2015-04-11 12:48:01.0
-0300
@@ -1,3 +1,9 @@
+multipath-tools (0.5.0-6pathselector1blankrev1) UNRELEASED; urgency=medium
+
+ * 0014-libmultipath-discovery-blank-rev-attr.patch: fix discovery of devices
with blank rev.
+
+ -- Mauricio Faria de Oliveira mauri...@linux.vnet.ibm.com Sat, 11 Apr 2015
12:40:19 -0300
+
multipath-tools (0.5.0-6pathselector1) UNRELEASED; urgency=medium
* initramfs/hooks: include dm-service-time in initramfs (new default path
selector).
diff -Nru
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
---
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
1969-12-31 21:00:00.0 -0300
+++
multipath-tools-0.5.0/debian/patches/0014-libmultipath-discovery-blank-rev-attr.patch
2015-04-11 12:48:06.0 -0300
@@ -0,0 +1,40 @@
+Forwarded: https://www.redhat.com/archives/dm-devel/2015-April/msg00049.html
+From b7ec63e727880aa709c2aa7b6029dba9ad7d9d74 Mon Sep 17 00:00:00 2001
+From: Mauricio Faria de Oliveira mauri...@linux.vnet.ibm.com
+Date: Sat, 11 Apr 2015 12:02:29 -0300
+Subject: [PATCH] libmultipath: fix discovery of devices with sysfs attr
containing only spaces
+
+commit 43c61e979521b17d9f50ea8e722682c33d0b7d69 causes sysfs_get_attr() to
+return 0 if attr contains only spaces, and scsi_sysfs_pathinfo() to return 1
+in 'if (sysfs_get_attr() = 0)'; this breaks the device's discovery.
+
+Changing the comparison operator '=' to '' allows discovery to continue.
+
+This affects some IPR controllers (rev sysfs attr):
+
+ # grep . /sys/block/sda/device/{vendor,model,rev} | tr ' ' '#'
+ /sys/block/sda/device/vendor:IBM#
+ /sys/block/sda/device/model:IPR-0###5EC99A00
+ /sys/block/sda/device/rev:
+
+Signed-off-by: Mauricio Faria de Oliveira mauri...@linux.vnet.ibm.com
+---
+ libmultipath/discovery.c |2 +-
+ 1 files changed, 1 insertions(+), 1 deletions(-)
+
+diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
+index 4582a20..dde57c0 100644
+--- a/libmultipath/discovery.c
b/libmultipath/discovery.c
+@@ -1095,7 +1095,7 @@ scsi_sysfs_pathinfo (struct path * pp)
+
+ condlog(3, %s: product = %s, pp-dev, pp-product_id);
+
+- if (sysfs_get_rev(parent, pp-rev, SCSI_REV_SIZE) = 0)
++ if (sysfs_get_rev(parent, pp-rev, SCSI_REV_SIZE) 0)
+ return 1;
+
+ condlog(3, %s: rev = %s, pp-dev, pp-rev);
+--
+1.7.1
+
diff -Nru multipath-tools-0.5.0/debian/patches/series
multipath-tools-0.5.0/debian/patches/series
--- multipath-tools-0.5.0/debian/patches/series 2014-09-05 09:12:48.0
-0300
+++ multipath-tools-0.5.0/debian/patches/series 2015-04-11 12:48:06.0
-0300
@@ -4,3 +4,4 @@
0011-patch-udev-rule.patch
0012-format-security-build-fixes.patch
0013-fix-spelling-error.patch
+0014-libmultipath-discovery-blank-rev-attr.patch
