[v2,0/8] multipath-tools fixes

Message

Martin Wilck Nov. 27, 2024, 11:04 p.m. UTC

The first patch is an attempt to fix
https://github.com/opensvc/multipath-tools/issues/105.

Patch 2-5 provide a re-implementation of pgcmp() without relying on
pgp->id, as discussed during the review of v1 of this series.

Patch 6 and 7 change the way multipathd triggers uevents when
it detects configuration changes, motivated by
https://github.com/opensvc/multipath-tools/issues/105.

Up to now, multipathd had done this basically only if the "failed" state of a
map had changed. This would miss cases where e.g. with find_multipaths strict,
a path had been added to the WWIDs file and "multipathd reconfigure" had been
run. In such cases, the paths would be added to the map, but udev would still
not see them.  The same holds for changes of the blacklist.

With this set, when a map is created, or when map creation fails, multipathd
checks whether udev already sees the correct properties for the device, and
triggers an uevent otherwise.

When maps are to be removed because of configuration changes in
coalesce_maps() (for example if a WWID has been removed from the WWIDs file),
multipathd now also triggers uevents for the respective path devices. This
is only done in coalesce_maps(), where current configuration changes are
reflected. We can't do it after a "remove map" CLI command, for example,
because that is by definition a temporary change. If we triggered an uevent
for that, multipathd would recreate the map it just removed as soone as it
receives the uevents.

Patch 8 is a minor log message change.

Martin Wilck (8):
  libmultipath: fix handling of pp->pgindex
  libmpathutil: change STATIC_BITFIELD to BITFIELD
  libmpathutil: add cleanup_bitfield()
  libmultipath: move pathcmp() to configure.c
  libmultipath: re-implement pgcmp without the pathgroup id
  libmultipath: trigger uevents upon map creation in domap()
  multipathd: trigger uevents upon map removal in coalesce_maps()
  multipathd: improve a log message in coalesce_maps()

 libmpathutil/libmpathutil.version |   1 +
 libmpathutil/util.c               |   5 ++
 libmpathutil/util.h               |  10 +--
 libmultipath/configure.c          | 124 ++++++++++++++++++++----------
 libmultipath/devmapper.c          |   9 +--
 libmultipath/discovery.c          |   2 +-
 libmultipath/dmparser.c           |   1 -
 libmultipath/pgpolicies.c         |   6 ++
 libmultipath/structs.c            |  31 +++-----
 libmultipath/structs.h            |   3 -
 libmultipath/structs_vec.c        |  15 ++++
 multipathd/main.c                 |   6 +-
 12 files changed, 135 insertions(+), 78 deletions(-)

Comments

Benjamin Marzinski Dec. 3, 2024, 8:11 p.m. UTC | #1

On Thu, Nov 28, 2024 at 12:04:22AM +0100, Martin Wilck wrote:

For all but patch 5/8:
Reviewed-by: Benjamin Marzinski <[e-mail]>

> The first patch is an attempt to fix
> https://github.com/opensvc/multipath-tools/issues/105.
> 
> Patch 2-5 provide a re-implementation of pgcmp() without relying on
> pgp->id, as discussed during the review of v1 of this series.
> 
> Patch 6 and 7 change the way multipathd triggers uevents when
> it detects configuration changes, motivated by
> https://github.com/opensvc/multipath-tools/issues/105.
> 
> Up to now, multipathd had done this basically only if the "failed" state of a
> map had changed. This would miss cases where e.g. with find_multipaths strict,
> a path had been added to the WWIDs file and "multipathd reconfigure" had been
> run. In such cases, the paths would be added to the map, but udev would still
> not see them.  The same holds for changes of the blacklist.
> 
> With this set, when a map is created, or when map creation fails, multipathd
> checks whether udev already sees the correct properties for the device, and
> triggers an uevent otherwise.
> 
> When maps are to be removed because of configuration changes in
> coalesce_maps() (for example if a WWID has been removed from the WWIDs file),
> multipathd now also triggers uevents for the respective path devices. This
> is only done in coalesce_maps(), where current configuration changes are
> reflected. We can't do it after a "remove map" CLI command, for example,
> because that is by definition a temporary change. If we triggered an uevent
> for that, multipathd would recreate the map it just removed as soone as it
> receives the uevents.
> 
> Patch 8 is a minor log message change.
> 
> Martin Wilck (8):
>   libmultipath: fix handling of pp->pgindex
>   libmpathutil: change STATIC_BITFIELD to BITFIELD
>   libmpathutil: add cleanup_bitfield()
>   libmultipath: move pathcmp() to configure.c
>   libmultipath: re-implement pgcmp without the pathgroup id
>   libmultipath: trigger uevents upon map creation in domap()
>   multipathd: trigger uevents upon map removal in coalesce_maps()
>   multipathd: improve a log message in coalesce_maps()
> 
>  libmpathutil/libmpathutil.version |   1 +
>  libmpathutil/util.c               |   5 ++
>  libmpathutil/util.h               |  10 +--
>  libmultipath/configure.c          | 124 ++++++++++++++++++++----------
>  libmultipath/devmapper.c          |   9 +--
>  libmultipath/discovery.c          |   2 +-
>  libmultipath/dmparser.c           |   1 -
>  libmultipath/pgpolicies.c         |   6 ++
>  libmultipath/structs.c            |  31 +++-----
>  libmultipath/structs.h            |   3 -
>  libmultipath/structs_vec.c        |  15 ++++
>  multipathd/main.c                 |   6 +-
>  12 files changed, 135 insertions(+), 78 deletions(-)
> 
> -- 
> 2.47.0