[dm-devel] [PATCH v2 2/2] multipathd: skip spurious event message for blacklisted paths

[Mauricio Faria de Oliveira]

Ben,

On 12/13/2016 07:50 PM, Benjamin Marzinski wrote:
> [snip] I don't see why the udev_device_ref/unref
> is necessary. While servicing uevents, when we first get the udevice
> from udev_monitor_receive_device() in uevent_listen() it comes with a
> reference. After we finish servicing the uevent, we drop this reference
> in service_uevq().  [snip]

Oh, cool. Thanks for the detailed explanation. I hadn't realized about
that first reference. I'll remove the udev_device_ref/unref() in v3.

 > My second issue is more for Hannes. Is there a reason why we don't check
 > filter_property in pathinfo if we call it with (DI_BLACKLIST | DI_WWID).
 > If we have the information to get the WWID, then we have the information
 > to check filter_property. [snip]

If I may, I'm not sure that filter_property() has to do w/ DI_WWID.

iiuic, it just checks the /name/ of each udev property of udev_device
against the blacklist/blacklist-exceptions entries for property names
in the conf struct (i.e., it doesn't need the path wwid).

And that information about the udev_device is available upfront,
even before we call pathinfo() (BTW, I guess this is the reason
why it's not called from within it, but rather at a point in
path_discover() that only the udev_device is available, like
filter_devnode() too.)

 > [snip]  Looking at the code, I don't see how we could
 > be checking filter_property for paths added via uev_add_path. Am I
 > missing something subtle here, or should we just add a filter_property()
 > check to pathinfo()?

You're correct. If a path is added via uev_add_path() it ignores
the property blacklisting, and end up creating/joining a devmap.
It seems like an unhandled case.

So, I'll submit a v3 w/ your suggestion for this too.

Thanks!


Simple test case w/ change/add uevents:

# cat /etc/multipath.conf
blacklist {
         property "ID_SERIAL"
}

# multipathd -d -s -v3 &
...

# echo change > /sys/block/sdb/uevent
uevent 'change' from '/devices/vio/2000/host0/target0:0:0/0:0:0:1/block/sdb'
Forwarding 1 uevents
sdb: udev property ID_SERIAL blacklisted
sdb: spurious uevent, path is blacklisted

# echo add > /sys/block/sdb/uevent
uevent 'add' from '/devices/vio/2000/host0/target0:0:0/0:0:0:1/block/sdb'
Forwarding 1 uevents
sdb: add path (uevent)
...
0QEMU_QEMU_HARDDISK_drive-scsi0-0-0-1: load table [0 4194304 multipath 1 
retain_attached_hw_handler 0 1 1 service-time 0 1 1 8:16 1]
...
sdb [8:16]: path added to devmap 0QEMU_QEMU_HARDDISK_drive-scsi0-0-0-1





-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center