Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multipath-tools 0.9.1 #43

Merged
merged 85 commits into from
Sep 7, 2022
Merged

multipath-tools 0.9.1 #43

merged 85 commits into from
Sep 7, 2022

Conversation

mwilck
Copy link
Collaborator

@mwilck mwilck commented Sep 7, 2022

Hi @cvaroqui , here is my PR for multipath-tools 0.9.1

  • Further changes related Please license remaining GPL-2.0-only files as 2-or-later (or LGPL?) #36
    • By default, multipath-tools doesn't use a readline library any more.
    • This can be changed by passing READLINE=libreadline or READLINE=libedit to make when building, selecting libreadline or libedit, respectively.
    • The readline library is only used by the new helper program multipathc, which is shipped under GPL 3.0 or later, and doesn't link against libmultipath. This should avoid the licensing problem, even if libreadline is used (IANAL).
  • Avoid checker thread blocking uevents or other requests for an extended amount of time with a huge amount of path devices
  • Improve startup time for very large multipath.conf files
  • Bug fixes
  • Fixes for Spelling #37
  • Documentation fixes

Detailed patch breakdown

  • (*) below indicates a minor patch that hasn't undergone formal review on dm-devel. This follows our practice that changes for GitHub workflows and library versioning don't require review. (@bmarzins, perhaps ec5b60b may be worth looking at, the others are only mentioned for completeness).
  • (+) below indicates a small change wrt the last version reviewed upstream in the respective patch (a build error fix and a minor adjustment in libmpathutil.version).

@bmarzins (13):

  libmultipath: fix find_multipaths_timeout for unknown hardware
  libmultipath: unset detect_checker for clariion / Unity arrays
  multipathd: factor out the code to flush a map with no paths
  libmultipath: return success if we raced to remove a map and lost
  multipathd: Handle losing all path in update_map
  multipath: fix systemd timers in the initramfs
  multipathd: Use regular pthread_mutex_t for waiter_lock
  multipathd: track waiters for mutex_lock
  multipathd: Occasionally allow waiters to interrupt checking paths
  multipathd: allow uxlsnr clients to interrupt checking paths
  multipathd: fix uxlsnr timeout
  multipathd: Don't check if timespec.tv_sec is zero
  multipathd: check for command overflow

@hreinecke (1):

  multipathd: replace libreadline with getline(). 

Fixes #41.

@jsoref (4):

  libmultipath: spelling: cplusplus
  libmultipath: spelling: ascii
  libmultipath: spelling: progress
  multipath-tools: spelling fixes

Fixes #37.

@mwilck (58):

  github workflows: switch to fedora 36 (*)
  libmultipath: alua: remove get_sysfs_pg83()
  libmultipath: remove sysfs_get_binary()
  libmultipath: sysfs_bin_attr_get_value(): no error if buffer is filled
  libmultipath: common code path for sysfs_attr_get_value()
  libmultipath: sanitize error checking in sysfs accessors
  libmultipath: get rid of PATH_SIZE
  libmultipath: sysfs_attr_get_value(): don't return 0 if buffer too small
  libmultipath: sysfs_attr_set_value(): don't return 0 on partial write
  libmultipath: sysfs: cleanup file descriptors on pthread_cancel()
  libmultipath, multipathd: log failure setting sysfs attributes
  multipath tests: expect_condlog: skip depending on verbosity
  multipath tests: __wrap_dlog: print log message
  multipath tests: add sysfs test
  libmultipath.version: bump version for sysfs accessors
  Merge branch 'licensing-fixes' into queue
  Merge branch 'master' into queue
  libmultipath: add msort.c from glibc
  libmultipath: modifications for msort.c
  libmultipath: merge_mptable(): sort table by wwid
  libmultipath: check_alias_settings(): pre-sort mptable by alias
  multipath: optimize program startup for frequent invocations
  .gitignore: ignore generated ABI files
  libmultipath: move all reservation key functions to prkey.c
  libmultipath: always set _GNU_SOURCE
  multipath-tools: Makefile: fix dependencies for "install" target
  libmultipath checkers/prioritizers: search for includes in libmultipath (+)
  libmultipath: remove weak attribute for {get,put}_multipath_config
  libmultipath.version: bump before library split (*)
  libmultipath: split off libmpathutil (+)
  multipathc: add new interactive client program
  multipathd: exec multipathc in interactive mode
  multipathd: fix incompatible pointer type error with libedit
  multipathd: fix use-after-free in handle_path_wwid_change()
  GitHub workflows: foreign.yaml: adapt to library split (*)
  multipath-tools: Makefile: remove useless .PHONY targets
  multipath-tools: Makefile: fix install->all dependency
  multipath-tools: Makefile: remove dependency test -> test-progs
  multipath-tools: Makefile: run abidiff with --redundant flag
  libdmmp: Makefile: create man3dir
  tests/Makefile: use $(multipathdir)
  tests/Makefile: add library dependencies
  tests/Makefile: use symbolic links under tests/lib
  tests/Makefile: redirect program output into output file
  GitHub workflows: package shared objects in artifact
  libmultipath: replace close_fd() with cleanup_fd_ptr()
  libmultipath: cleanup_free_ptr(): avoid double free
  multipath: find_multipaths_check_timeout(): no need for pthread cleanup
  multipathd: fix segfault in cli_list_map_fmt()
  multipathd: fix broken pthread cleanup in fpin_fabric_notification_receiver()
  multipathd: Fix command completion in interactive mode
  multipathd: add multipathc.8 manual page
  multipathd.8: remove misleading paragraph about multipath
  multipathd.8: Fix "SEE ALSO" section
  multipathd: install multipathc.8 Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
  kpartx_id: remove bashism (*)
  GitHub workflows: test different values of READLINE (*)
  libmultipath: bump version to 0.9.1 (*)

@xosevp (8):

  multipath-tools: update Huawei OceanStor NVMe vendor id
  multipath-tools: update "Generic NVMe" vendor regex in hwtable
  multipath-tools: update devel repo info in README.md
  multipath-tools: delete README.alua
  multipath-tools: add ALUA info to README.md
  multipath-tools: remove list of rebranded arrays vendors from man page
  multipath-tools: correct CLARiiON info from multipath.conf man page
  multipath-tools: add basic info on how to use multipath-tools with NVMe devices

chengjike.cheng AT huawei.com (1)

  multipath-tools: fix "multipath -ll" bug for Native NVME Multipath devices

mwilck and others added 30 commits June 10, 2022 22:44
…vices

After "Native NVME Multipath" is configured,
the content displayed is incorrect when you run "multipath -ll" command.
Each NVME devices have the same path name. For example:

[root@localhost home]# multipath -ll
eui.710032e8fb22a86c24a52c1000000db8 [nvme]:nvme1n1 NVMe,Huawei-XSG1,1000001
size=10485760 features='n/a' hwhandler='ANA' wp=rw
|-+- policy='n/a' prio=50 status=optimized
| `- 1:4:1   nvme1c4n1 0:0 n/a   optimized live
`-+- policy='n/a' prio=50 status=optimized
  `- 1:9:1   nvme1c9n1 0:0 n/a   optimized live
eui.710032e8fb22a86b24a52c7c00000db7 [nvme]:nvme1n2 NVMe,Huawei-XSG1,1000001
size=10485760 features='n/a' hwhandler='ANA' wp=rw
|-+- policy='n/a' prio=50 status=optimized
| `- 1:4:1   nvme1c4n1 0:0 n/a   optimized live
`-+- policy='n/a' prio=50 status=optimized
  `- 1:9:1   nvme1c9n1 0:0 n/a   optimized live
[root@localhost home]#

The logical paths of "nvme1n1" and "nvme1n2" are both "nvme1c4n1" and "nvme1c9n1".
So when multipath-tools aggregates disks, use "nvme_ns_head->instance" for matching.
such as ,Use "b" in "nvmeanb" string to match "z" in "nvmexcynz"(a,b,x,y,z can be any number),
and if "b" and "z" are the same, they are related.

Signed-off-by: chengjike <chengjike.cheng@huawei.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
"NVME" in the doc, but "NVMe" is the real output:
(page 61-62): https://drive.google.com/file/d/1c5RK4GXX7ofZBFxTtZ_IN1qHyIjw5eR1
https://marc.info/?l=dm-devel&m=163393151312418
Use both, just in case.

Cc: <chengjike.cheng@huawei.com>
Cc: <sunao.sun@huawei.com>
Cc: <jiangtao62@huawei.com>
Cc: Zhouweigang (Jack) <zhouweigang09@huawei.com>
Cc: Zou Ming <zouming.zouming@huawei.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
to accept NVME and NVMe

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
pp->hwe is now a vector that will always be allocated for all path
devices. Instead of checking if it is NULL, check if it is empty.

Fixes: f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe")
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>
Since b72e753 ("libmultipath: alua: try to retrieve inquiry data from sysfs"),
we fetch inquiry and VPD data from sysfs anyway. No need to do this twice.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This function adds no value on top of sysfs_bin_attr_get_value().
Remove it.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
sysfs_bin_attr_get_value() sets the length of bytes read to 0
when the provided buffer was too small, truncating potentially
useful data. This is harmful e.g. in do_inquiry(), if the "inquiry"
sysfs attribute contains more than 96 bytes (which is possible).

Actually, binary attributes don't need to be 0-terminated. Thus,
unlike for string attributes, it's not an error if the requested number of
bytes is exactly equal to the number of bytes read. We expect that
the caller knows how many bytes it needs to read.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The code for sysfs_attr_get_value and sysfs_bin_attr_get_value() was
almost identical. Use a common code path.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
udev_device_get_syspath() may return NULL; check for it, and check
for pathname overflow. Disallow a zero buffer length. The fstat()
calls were superfluous, as a read() or write() on the fd would
return the respective error codes depending on file type or permissions,
the extra system call and code complexity adds no value.

Log levels should be moderate in sysfs.c, because it depends
on the caller whether errors getting/setting certain attributes are
fatal.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
replace PATH_SIZE with the system limit PATH_MAX. In some places,
PATH_SIZE was used for file names. Use FILE_NAME_SIZE in these cases.
Also, use a constant for "multipathd.service" in systemd_service_enabled_in().

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
If the passed read buffer is too small to hold the value read plus
terminating 0 byte, return the given size value rather than 0.

This way we get similar semantics as for sysfs_bin_attr_get_get_value(),
except that sysfs_attr_get_value() must always 0-terminate the value;
thus a return value equal to the length parameter is an error for
the non-binary case.

Provide a helper macro to test this "overflow" condition.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
sysfs_attr_set_value() returned 0 if not all requested bytes were written.
Change this to return the number of bytes written. Error checking is now
somewhat more involved; provide a helper macro for it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Failure to set a sysfs attribute is worth noting, normally.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
otherwise we'll get failures if verbosity level is low.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This makes it easier to analyze errors from __wrap_dlog().

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Formally, the ABI is still the same, but the semantics of the
return value have changed.

Signed-off-by: Martin Wilck <mwilck@suse.com>

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Dell EMC would like to always use the emc_clariion checker. Currently
detect_checker will switch the checker to TUR for Unity arrays.
This can cause problems on some setups with replicated Unity LUNs,
which are handled correctly the the emc_checker, but not the TUR
checker.

Cc: vincent.chen1@dell.com
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Reviewed-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Spelling errors detected by the GitHub check-spelling action:
https://github.com/marketplace/actions/check-spelling

Signed-off-by: Josh Soref <2119212+jsoref@users.noreply.github.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
It does not provide useful info, and it is incomplete.

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Remove "Unity" from emc prio and hardware_handler, because
Unity does not support PNR mode, just ALUA (page 113 and 153):
https://www.delltechnologies.com/asset/en-us/products/storage/technical-support/docu5128.pdf
And add PNR info.

Cc: Yanfei Chen <vincent.chen1@dell.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
…Me devices

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
.PHONY is only necessary for targets that may match actually existing
files.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Rather than enforcing a "make all; make install" in the top directory,
make sure that the $DIR.install correctly depends on $DIR for all
subdirs. This speeds up compilation without breaking dependencies.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This dependency is already established in tests/Makefile. "test"
must depend on "all" though.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
With --redundant, abidiff lists all changed functions, which may be
handy.
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Without this "make install" may fail.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
.. instead of hard-coded ../libmultipath.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The tests need to be re-run if changes have been made to libmultipath
and other libraries. Add dependencies to take care of that.
Also, the libmultipath.so.0 copy under tests must be rebuilt if anything
changes under libmultipath.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Hardlinks would not update if the .so files were rebuilt. Symlinks
work better for this purpose.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The test program output is too verbose to read every time.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
As we are now using symlinks under tests/lib, the .so files under
checkers, prioritizers, and foreign need to be packaged, too.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This is a nicer API without ugly casts, and less likely to close
valid file descriptors accidentally. Also, it can be used for
both pthread_cleanup_push and __attribute__((cleanup)).

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
... by nullifying the passed pointer after freeing it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
multipath is not a multithreaded program, no pthread-cancel complexity
is necesssary here.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
get_multipath_layout() must be passed an mpvec, not a pathvec.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
…ceiver()

We were returning from a phtread-cancel block. That should be avoided.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The command completion never worked, because the handlers
array wasn't initialized in client mode.

The handlers array is now also required in multipathc,
but it doesn't need the actual handler functions. To keep
multipathc as small as possible, we just add a handler table
with NULL handler functions.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Document multipathc, and update the man page of multipathd to
describe the command mode correctly.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This has been wrong for a long time. Remove it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Fix the systemd reference, and he ugly line break on small terminals.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Do this only in build-and-unittest for now, otherwise our
matrix grows too excessively.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
@cvaroqui cvaroqui merged commit 3db6872 into opensvc:master Sep 7, 2022
@mwilck mwilck requested a review from bmarzins September 7, 2022 11:20
@mwilck
Copy link
Collaborator Author

mwilck commented Sep 7, 2022

Wow, that was fast, thanks!

@mwilck mwilck removed the request for review from bmarzins September 7, 2022 11:21
@mwilck
Copy link
Collaborator Author

mwilck commented Sep 7, 2022

@cvaroqui, could you please tag 3db6872 as 0.9.1 ?

@cvaroqui
Copy link
Member

cvaroqui commented Sep 7, 2022

Tagged.
Thank you for your great work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants