Multipath v. 0.4.7 trips over USB flash drive

* Multipath v. 0.4.7 trips over USB flash drive
@ 2007-11-19 20:43 usvyatsky_ilya
  2007-11-19 21:32 ` Mike Anderson
  2007-11-22 12:49 ` Guido Guenther
  0 siblings, 2 replies; 5+ messages in thread
From: usvyatsky_ilya @ 2007-11-19 20:43 UTC (permalink / raw)
  To: dm-devel

[-- Attachment #1: Type: text/plain, Size: 2518 bytes --]

Hi there,

I am using DM MPIO for quite a while now and in general it works.
Lately I encountered a problem with one of my boxes that is equipped
with USB flash drive and is connected to a disk array.
Namely, the flash drive has an empty bcdDevice entry, which translates
into empty /sys/block/sdXX/device/rev file.
As a result, the 'multipath -ll' command trips over this device and
bails out without listing any array mpathXX devices.
The devices are created in /dev/disk/by-name/ directory, so it's just
the userland tool issue.

I've built debug version of multipath and found the following:

1. The function sysfs_get_rev() (which is generated from macro
declare_sysfs_get_str()) assumes that the rev has to have at least 2
characters. Since in the case of my device this assumption is false, the
function returns 1.
2. The caller function is scsi_sysfs_pathinfo() and it returns 1 as
well.
3. The caller function is sysfs_pathinfo(), which returns 1 as well.
4. The caller function is pathinfo(), which also returns 1.
5. The caller function is store_pathinfo(). Here the pathinfo is not
stored and NULL is returned to the caller.
6. The caller is path_discover() that returns 1.
7. The caller is path_discovery() that returns non-zero result.
8. The caller is configure() that bails out on this result.

I have tried to work around this issue by blacklisting the device, but
unfortunately the only blacklist check that is done prior to call to
store_pathinfo() in the path_discover() is based on the devnode. 
This means that effectively I cannot use this workaround since with USB
device I cannot guarantee that it will always be mapped to the same
devnode.

I'd like to propose a couple of options for a fix for this issue:

1. Re-implement sysfs_get_rev() in such a way that rev filed minimal
length is no longer required. 
   Please see attached a patch that is supposed to take care of this.
2. Re-implement pathinfo() to check whether vendor and/or product are
blacklisted. 
   This way non-compliant devices may be blacklisted based on vendor
and/or product name.

I am not sure, which option is preferable, but I clearly need at least
one of them in the product.

Your comments are greatly appreciated. 

Please note that my patch is against v. 0.4.7 of the multipath tool. 
I realize that upstream may differ significantly, so if option (1) makes
sense I'll revalidate with the latest version and submit the patch if
necessary.

Thanks,

Ilya.

[-- Attachment #2: multipath-tools-0.4.7-empty-rev.diff --]
[-- Type: application/octet-stream, Size: 1079 bytes --]

diff -r multipath-tools-0.4.7-patched/libmultipath/discovery.c multipath-tools-0.4.7/libmultipath/discovery.c
145c145
< #define declare_sysfs_get_str(fname, fmt, min_len) \
---
> #define declare_sysfs_get_str(fname, fmt) \
164c164
< 	if (attr->len < min_len || attr->len - 1 > len) \
---
> 	if (attr->len < 2 || attr->len - 1 > len) \
176,181c176,181
< declare_sysfs_get_str(devtype, "%s/block/%s/device/devtype", 2);
< declare_sysfs_get_str(cutype, "%s/block/%s/device/cutype", 2);
< declare_sysfs_get_str(vendor, "%s/block/%s/device/vendor", 2);
< declare_sysfs_get_str(model, "%s/block/%s/device/model", 2);
< declare_sysfs_get_str(rev, "%s/block/%s/device/rev", 0);
< declare_sysfs_get_str(dev, "%s/block/%s/dev", 2);
---
> declare_sysfs_get_str(devtype, "%s/block/%s/device/devtype");
> declare_sysfs_get_str(cutype, "%s/block/%s/device/cutype");
> declare_sysfs_get_str(vendor, "%s/block/%s/device/vendor");
> declare_sysfs_get_str(model, "%s/block/%s/device/model");
> declare_sysfs_get_str(rev, "%s/block/%s/device/rev");
> declare_sysfs_get_str(dev, "%s/block/%s/dev");

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread