[v2,00/49] multipath-tools: devmapper API refactored

Message

Martin Wilck July 12, 2024, 5:14 p.m. UTC

Patch 1-9 are generic cleanups, partly targeting the previously reported
issues with multipath-tools installation on NixOS by removing hard-coded
paths as much as possible.

Patch 10-18 are basic cleanups for the devmapper code.

Patch 19 is the heart of this patch set, introducing the function
libmp_mapinfo(), which is supposed to serve as general API for fetching
information about maps from libdevmapper. It can work with arbitrary
input for identifying a map, and produce any combination of desired
outputs, with the most lightweight (combination of) ioctls possible.
This part of the set removes some code that had been changed
in patch 10-16. This is sort of suboptimal, but IMO it improves
the readability of the set as  a whole.

Patch 20 adds unit tests for this function.

Patch 21-45 change the libmultipath code to use the new API everywhere,
and add some additional minor improvements and fixes.

Patch 46-49 have been added in v2.

* 46: fix a long-standing bug I accidentally discovered during testing
* 47: simplify and fix the logic for determining whether a map is in use
* 48: implement Ben's idea from the comment of #31 in the old series.
* 49: unrelated, fixes for the directio test

The code is pushed to https://github.com/openSUSE/multipath-tools/tree/tip.

Comments and reviews welcome.

I'm posting the entire series, because some of those patches that Ben had
already reviewed are changed.

# Changes v1->v2:

* 1: add fix for libaio
* 6: make sure kpartx.rules is built (Ben)
* 16: split this large patch between the changes that are just related
  to cleanup_dm_task(), and the other code modifications
* 17: new, used to be part of #16
* 18: new, used to be part of #17. Changed the treatment of DMP_ERR /
  DM_IS_MPATH_ERR (Ben)

(From here on, patch number n maps to (n-2) in the v1 set).

* 19 (was 17):
      - Allow passing a dev_t in mapid_t (Ben).
      - Remove the tgt_type field in mapinfo_t. Instead, introduce
        flags MAPINFO_MPATH_ONLY and MAPINFO_PART_ONLY to make
	libmp_mapinfo__() filter either multipath or partition devices,
	inspired by Ben's comment on patch #31 of the v1 series.
	See also #48.
      These changes require changes in users of the API in patch 30ff.
      I have not removed the Reviewed-by: tags in the followup patches.
* 20 (18): Adapt tests to #19.
* 25 (23):
      - dm_get_wwid() won't touch the output unless it returns DMP_OK
      - adjusted callers to make sure the uuid is not accessed unless
        DMP_OK was returned
      - distinguish between "multipath map is present" and "other map
        is present" in domap() (Ben)
      - Change handling of DMP_ERR in alias_already_taken() (Ben)
* 28 (26): Fixed bug in the return code handling in dm_dev_t()
* 30 (28):
      - Change return code handling in dm_type_match(), to distinguish
        between "not multipath" and "generic error" (Ben)
      - adapt to #19
* 31 (29): adapt to #19
* 32 (30): adapt to #19
* 33 (31): adapt to #18. is_mpath_uuid() will be unexported in #48
* 36 (34):
      - don't check the UUID in do_foreach_partmaps(). This would break
        the partmap_in_use detection (but see #47 below).
      - adapt to #19
* 37 (35): fetch mpp->size (Ben). Print a warning if the size has changed.
* 41 (39):
      - fetch mpp->size (Ben).
      - adapt to #19
* 42 (40): - remove redundant sync_map_state() call (Ben)
      - fetch mpp->size (Ben)
      - check return code of dm_get_wwid()
      - fix segfault caused by passing NULL to update_map()
* n/a (42): dropped (Ben)

(From here on, n maps to (n - 1) in the v1 set).

* 44 (43): adapt to #19

Patch 46-49 are new.

Martin Wilck (49):
  multipath-tools CI: more fixes for arm/v7
  multipath-tools CI: fix dmevents test for Debian Sid, arm/v7
  create-config.mk: use printf instead of /bin/echo
  multipathd.service.in: use @BINDIR@ instead of /sbin
  Makefile.inc: replace @BINDIR@ with $(TGTDIR)/$(bindir)
  kpartx.rules: use @BINDIR@ to locate kpartx
  multipath-tools: Remove hard-coded paths to executables
  multipath-tools: compile_commands.json fixes
  multipath-tools: .gitignore: ignore o.wrap files for CI helpers
  libmultipath: remove unused includes in devmapper.h
  libmultipath: use DM_DEVICE_INFO in dm_mapname()
  multipath-tools: don't call dm_task_no_open_count()
  libmpathutil: export cleanup_udev_device()
  libmpathutil: add cleanup_vector()
  libmultipath: add cleanup helpers for struct multipath
  libmultipath: add cleanup_dm_task(), and use it in devmapper.c
  libmultipath: rename dm_type()->dm_type_match() and use symbolic
    values
  libmultipath: Use symbolic return values for dm_is_mpath()
  libmultipath: add libmp_mapinfo()
  libmultipath tests: add tests for libmp_mapinfo()
  libmultipath: implement dm_get_info() and dm_map_present() with new
    API
  libmultipath: remove dm_get_prefixed_uuid()
  libmultipath: is_mpath_part(): improve parsing
  libmultipath: rename dm_get_uuid() -> dm_get_wwid()
  libmultipath: improve dm_get_wwid() return value logic
  libmultipath: reimplement dm_map_name() with new API
  libmultipath: reimplement dm_map_present_by_uuid()
  libmultipath: reimplement dm_get_opencount() with new API
  libmpathpersist: skip redundant dm_map_present() call
  libmultipath: implement dm_is_mpath() with new API
  libmultipath: implement dm_get_multipath() with new API
  libmultipath: use libmp_mapinfo() in _dm_flush_map()
  libmultipath: add is_mpath_uuid() helper
  libmultipath: add is_mpath_part_uuid() helper
  libmultipath: add dmp_errstr() helper
  libmultipath: use libmp_mapinfo() in do_foreach_partmaps()
  libmultipath: use libmp_pathinfo() in update_multipath_table()
  libmultipath: update mpp->dmi in update_multipath_table()
  libmultipath: drop extra call to dm_map_present() in domap()
  libmultipath: split off update_multipath_table__()
  multipath: implement check_usable_paths() with libmp_pathinfo()
  multipathd: implement add_map_without_path() with libmp_mapinfo()
  libmultipath: simplify dm_get_maps()
  libmpathpersist: use libmp_mapinfo() in mpath_get_map()
  libmpathpersist: use mpp->alias in do_mpath_persistent_reserve_out()
  libmultipath: fix deferred_remove logic in remove_partmap()
  libmultipath: don't call do_foreach_partmaps() recursively
  libmultipath: Move UUID check into libmp_pathinfo__()
  multipath-tools tests: fix directio test with real device

 .gitignore                               |    4 +
 Makefile                                 |    4 +-
 Makefile.inc                             |   29 +-
 create-config.mk                         |    4 +-
 kpartx/Makefile                          |    6 +-
 kpartx/devmapper.c                       |   15 -
 kpartx/{kpartx.rules => kpartx.rules.in} |    2 +-
 kpartx/kpartx_id                         |    8 +-
 libmpathpersist/mpath_persist_int.c      |   81 +-
 libmpathutil/libmpathutil.version        |    5 +
 libmpathutil/util.c                      |    6 +
 libmpathutil/util.h                      |    3 +
 libmpathutil/vector.c                    |    6 +
 libmpathutil/vector.h                    |    1 +
 libmultipath/alias.c                     |   11 +-
 libmultipath/configure.c                 |   31 +-
 libmultipath/devmapper.c                 | 1052 +++++++---------
 libmultipath/devmapper.h                 |  109 +-
 libmultipath/libmultipath.version        |   11 +-
 libmultipath/print.c                     |    6 -
 libmultipath/structs.c                   |   12 +
 libmultipath/structs.h                   |    2 +
 libmultipath/structs_vec.c               |   52 +-
 libmultipath/structs_vec.h               |    2 +
 libmultipath/valid.c                     |    2 +-
 libmultipath/wwids.c                     |    2 +-
 multipath/11-dm-mpath.rules.in           |    4 +-
 multipath/main.c                         |   57 +-
 multipath/multipath.rules.in             |    6 +-
 multipathd/dmevents.c                    |   21 +-
 multipathd/main.c                        |   77 +-
 multipathd/multipathd.service.in         |    4 +-
 multipathd/waiter.c                      |    2 -
 tests/Makefile                           |   13 +-
 tests/README.md                          |   29 +-
 tests/alias.c                            |   32 +-
 tests/directio.c                         |  182 +--
 tests/dmevents.c                         |   34 +-
 tests/mapinfo.c                          | 1401 ++++++++++++++++++++++
 tests/valid.c                            |   10 +-
 tests/wrap64.h                           |   11 +-
 41 files changed, 2370 insertions(+), 979 deletions(-)
 rename kpartx/{kpartx.rules => kpartx.rules.in} (96%)
 create mode 100644 tests/mapinfo.c