[dm-devel] [PATCH 0/3] consistent behavior of filter_property()

[Benjamin Marzinski]

On Tue, Feb 02, 2021 at 10:27:26PM +0100, mwilck at suse.com wrote:
> From: Martin Wilck <mwilck at suse.com>
> 
> This is a spring-off of the previous discussion under the subject
> "libmultipath: fix NULL dereference in get_be64".
> 
> Repeating part of the text of my last post there:
> 
> pp->uid_attribute may be set from the hwtable, which
> means that vendor/product must be known, which in turn means that
> uid_attribute can't be set correctly before sysfs_pathinfo() is run.
> Thus, in order to be consistent, we need to move the filter_property()
> further down in pathinfo(), after the call to sysfs_pathinfo(). 
> 
> However, that has a side effect. As Ben already stated,
> pp->uid_attribute currently is *never* set the first time the call
> chain pathinfo()->filter_property() is run. After the proposed change,
> it would *always* be set in this call chain, possibly leading to more
> cases where pathinfo returns PATHINFO_SKIPPED.
> 
> This matters e.g. for "multipath -w". When we remove a WWID, we must
> be sure get_refwwid() fills in the wwid, which won't happen if
> PATHINFO_SKIPPED is returned.
> 
> (While looking at this, I discovered that it didn't work in current
> upstream either. I guess it has been broken since 0.6.0, or
> 355291b ("libmultipath: filter for missing property in get_refwwid()"
> This lead to the addition of patch 3/3).
> 
> In order not to break such use cases, we need to make the
> filter_property() test in pathinfo() dependent on DI_BLACKLIST. That
> would make a lot of sense, but it'd cause a semantic change.
> 
> Comments welcome.

Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com>

For the set.
> 
> Regards
> Martin
> 
> 
> Martin Wilck (3):
>   pathinfo: call filter_property() after sysfs_pathinfo()
>   libmultipath: pathinfo: call filter_property only with DI_BLACKLIST
>   multipath -w: allow removing blacklisted paths
> 
>  libmultipath/configure.c |  4 +-
>  libmultipath/discovery.c | 19 +++++++--
>  libmultipath/valid.c     |  4 --
>  tests/Makefile           |  2 +-
>  tests/test-lib.c         | 18 ++++----
>  tests/valid.c            | 91 ++++++++++++++++++++++++++++++++++++----
>  6 files changed, 111 insertions(+), 27 deletions(-)
> 
> -- 
> 2.29.2