multipath-tools 0.14.0 #133
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When libmpathpersist registers a key, It first checks which paths are active, then registers the key on those paths, and then tells multipathd that the key has been registered, and it should start tracking it. If a path comes up after libmpathpersist checks for active paths, but before it tells multipathd that the key has been registered, multipathd will not register a key no that path (since it hasn't been told to that the device has a registered key yet). This can leave the device with a path that is missing a key. To solve this, when multipathd is told that a key has been registered, it checks if there are the same number of registered keys as active paths. If there aren't, it registers keys on all the paths (at least until the number of registered keys and active paths are equal). To avoid doing a bunch of unnecessary PR work, pr_register_active_paths() has a new option to track the number of active paths, and mpath_pr_event_handle() now takes the number of keys to expect. The first time it's called, when there is an unknown number of keys, if the number of keys it finds matches the number of active_paths, it and pr_register_active_paths() exit early, since there is no registering work to do. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
mpathpersist was incorrectly parsing the REPORT CAPABILITES service action output. In reality, the type mask is two bytes where the type information is stored in bits 7, 6, 5, 3, & 1 (0xea) of the first byte and bit 0 (0x01) of the second byte. libmpathpersist was treating these two bytes as a big endian 16 bit number, but mpathpersist was looking for bits in that number as if it was little endian number. Ideally, libmpathpersist would treat prin_capdescr.pr_type_mask as two bytes, like it does for the flags. But we already expose this as a 16 bit number, where we treated the input bytes as a big endian number. There's no great reason to mess with the libmpathpersist API, when we can just make mpathpersist treat this data like libmpathpersist provides it. So, fix mpathpersist to print the data out correctly. Additionally, instead of printing a 1 or a 0 to indicate if a type was supported or not, it was printing the value of the type flag. Also, Persist Through Power Loss Capable (PTPL_C) was being reported if any bit in flags[0] was set. Fix these as well. Reformat all of the capability printing lines, since it is less confusing than only reformatting some of them. Fixes: ae4e8a6 ("mpathpersist: Add new utility for managing persistent reservation on dm multipath device") Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
"opensuse-leap" would use 15.6 already, but make this explicit. Signed-off-by: Martin Wilck <mwilck@suse.com>
GitHub provides arm64 runners now [1]. Use them. The workflows don't seem to run faster than before, but the tests should be more realistic. [1] https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/ Signed-off-by: Martin Wilck <mwilck@suse.com>
This will allow debugging CI failures (to some extent, as we compile with optimization by default). Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
The workflow files were using make's -j flag inconsistently. Fix it. Signed-off-by: Martin Wilck <mwilck@suse.com>
Without it, the workflows will fail. Signed-off-by: Martin Wilck <mwilck@suse.com>
c78cf64 ("multipath-tools: replace FSF licences boilerplate with a SPDX-License-Identifier") updated libdmmp.h. This will cause the multipath build process to update it as well, even though the man page content doesn't change. Avoid that by updating the time stamps. Signed-off-by: Martin Wilck <mwilck@suse.com>
Fix the spelling CI complaints about "0xea01". Signed-off-by: Martin Wilck <mwilck@suse.com>
"nvme" makes "NVMe" redundant. Signed-off-by: Martin Wilck <mwilck@suse.com>
Config from, opensvc#102: https://www.seagate.com/content/dam/seagate/migrated-assets/www-content/support-content/raid-storage-systems/corvault/_shared/files/205042000-01-B_CORVAULT_SMG.pdf 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@lists.linux.dev> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
multipath is also supported by V3500 and V3700 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@lists.linux.dev> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
Currently it seems that urcu would already call pthread_join when calling call_rcu_data_free since a couple of years ago (since version v0.14.0)[0] so calling pthread_join on the just released one is problematic under musl systems. It seems like under glibc this has several checks in place before trying to dereference the thread but under musl it has nothing in place to validate so this causes a coredump on program shutdown. This is currently present in all version when compiled under musl, running multipathd -d and sending a SIGTERM to it, you can see the coredump happening at this point in the code. The patch runs only the old behaviour in urcu older than 0.14.0 to maintain the same bahaviour. In higher versions its not neccesary so we skip it. [0] urcu/userspace-rcu@1cf55ba Signed-off-by: Itxaka <itxaka@kairos.io> Reviewed-by: Martin Wilck <mwilck@suse.com>
If there libmpathpersist failed to create a thread to retry the register and ignore command, mpath_prout_reg should fail. Instead, the code was simply ignoring the failed threads. Fix that. Fixes: 2a4ca25 ("libmpathpersist: change how reservation conflicts are handled") Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
multipathd now has a fixed ordering of the keywords. Specifically, verbs must come first. Fix the man page to reflect the ordering. Fixes: f812466 ("multipathd: more robust command parsing") Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
If prin_do_scsi_ioctl() fails in update_map_pr() for some reason other than Persistent Reservations not being supported, It shouldn't clear the number of registered keys, since there's no reason to think that it has changed. Similarly, if update_map_pr() fails in mpath_pr_event_handle(), don't assume that the nr_keys_needed was cleared. Just return whatever the value is now. This saves multipathd from doing pointless calls to update_map_pr(), if one of the paths is failing. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
When libmpathpersist notifies multipathd that a key has been registered, cli_setprstatus() calls pr_register_active_paths() with a flag to let it know that the paths are likely already registered, and it can skip re-registering them, as long as the number of active paths matches the number of registered keys. This shortcut can fail, causing multipathd to not register needed paths, if either a path becomes usable and another becomes unusable while libmpathpersist is running or if there already were registered keys for I_T Nexus's that don't correspond to path devices. To make this shortcut work in cases like that, this commit adds a new multipathd command "setprstatus map <map> pathlist <pathlist>", where <pathlist> is a quoted, whitespace separated list of scsi path devices. libmpathpersist will send out the list of paths it registered the key on. pr_register_active_paths() will skip calling mpath_pr_event_handle() for paths on that list. In order to deal with the possiblity of a preempt occuring while libmpathpersist was running, the code still needs to check that it has the expected number of keys. Fixes: f7d6cd1 ("multipathd: Fix race while registering PR key") Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
dm_get_multipath() is a static function, not a symbol that can be exported. Clang is strict about not allowing undefined symbols in linker scripts and therefore its presence prevents clang from building libmultipath. It looks like it's a linker error and it's thrown only by lld. lld enabled --no-undefined-version as a default option [0]. Choice of a compiler (gcc, clang) shouldn't matter. One can reproduce the error on any system by specifying lld explicitly: $ make LDFLAGS="-fuse-ld=lld" Or by passing --no-undefined-version (and making the warning fatal), which throws an error wyth any linker: $ make LDFLAGS="-fuse-ld=mold -Wl,--no-undefined-version -Wl,--fatal-warnings". [0] https://reviews.llvm.org/D135402 Fixes: opensvc#132 Fixes: bf3a4ad ("libmultipath: simplify dm_get_maps()") Signed-off-by: Michal Rostecki <vad.sol@proton.me> Reviewed-by: Martin Wilck <mwilck@suse.com>
In ISO C23, memchr, strchr, strpbrk, strrchr and strstr become const-preserving macros [1], meaning that the return value inherits the const qualifier from the function argument. This has turned up a few glitches in our code. [1] https://gustedt.gitlabpages.inria.fr/c23-library/#memchr Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This function is unused. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
It can happen that a path group contains references to paths that are already freed. If we access pp->pgindex in such a case, a use-after-free occurs. In particular, this occurs if we call remove_map() from configure() -> coalesce_paths(), and some paths are freed in orphan_paths() because they are in INIT_PARTIAL or INIT_REMOVED state. The paths of the removed map may still be referenced by maps in the "old" mpvec in configure(). The UAF occurs when configure() calls remove_maps(vecs). This code has been introduced in cd912cf ("libmultipath: fix handling of pp->pgindex"). The commit message stated: "The hunk in group_paths is mostly redundant with the hunk in free_pgvec(), but because we're looping over pg->paths in the former and over pg->pgp in the latter, I think it's better too play safe". It turns out that it would have been better not to "play safe" here. No caller of free_pgvec() requires the pgindex to be reset. For the functions in pgpolicies.c, the reset of pgindex is redundant, as remarked in cd912cf. disassemble_map() will set the pgindex later anyway, and update_multipath_strings() will call disassemble_map(). The other callers remove the struct multipath, in which case the paths will eventually be orphaned or freed (except for the special case of coalesce_paths() mentioned above). Fixes: cd912cf ("libmultipath: fix handling of pp->pgindex") 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>
Move the removal of an mpp from the mpvec into a separate function, remove_map_from_mpvec(). There should be no functional differences so far. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
multipath-tools uses container_of() and similar macros, which imply casts between different types, which isn't stricly compliant with strict aliasing rules. The issue that lead to the previous commit "libmpathutil: use union for bitfield" was one example where this can fail. While that one could be fixed relatively easily, it shows that surprises can happen any time when we compile our code with strict aliasing enabled. This can be seen clearly when we compile with "-fstrict-aliasing -Wstrict-aliasing=1" (note that the bitfield problem is only reported by gcc with "-Wstrict-aliasing=1", other levels of aliasing detection miss it with gcc 15). Use -fno-strict-aliasing to disable it. The kernel does the same. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This comment has been minimized.
This comment has been minimized.
Add a new configuration option 'purge_disconnected' that can be set per multipath device, hardware entry, or globally. This option will be used to control whether multipathd should automatically remove paths that are in a disconnected state. The option is disabled by default (PURGE_DISCONNECTED_OFF). This patch only adds the configuration infrastructure. The actual purge functionality will be implemented in a subsequent patch. Signed-off-by: Brian Bunker <brian@purestorage.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
Implement automatic purging of paths that have been disconnected at the storage target (e.g., LUN unmapped). This builds on the purge_disconnected configuration option added in the previous patch. This adds: - New PATH_DISCONNECTED checker state to signal disconnection - TUR checker support for detecting LUN NOT SUPPORTED (ASC/ASCQ 0x25/0x00) - Purge thread (purgeloop) that removes paths via sysfs delete attribute - State machine to track disconnection and delay purging - Conversion of PATH_DISCONNECTED to PATH_DOWN for normal processing The purge thread runs independently and processes paths that have been marked for purging by the checker thread. Paths are only purged after remaining disconnected for delay_wait_checks intervals to avoid removing paths that are temporarily flapping. Signed-off-by: Brian Bunker <brian@purestorage.com> Signed-off-by: Krishna Kant <krishna.kant@purestorage.com> Reviewed-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@lists.linux.dev> Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
Collaborator
Author
|
Marking as draft because my last patches still need review / ack. |
This comment has been minimized.
This comment has been minimized.
Collaborator
Author
|
The coding style CI failure is just about a comment which this PR doesn't even touch, so please ignore. |
The variable mapname is sometimes allocated and sometimes not. To avoid double-free and still free allocated memory cleanly, introduce a helper pointer for storing possibly allocated memory. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This functionality has been obsolete for a long time. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This isn't necessary in practice, but it fixes a compile warning. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
It's now possible to enable AddressSanitizer by passing the parameter `ASAN=1` on the "make" command line. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Our OPTFLAGS variable contains stack protection options which are intended to be overridable by distribution build scripts. When developers just want to experiment with optimization levels, they often just want to modify the `-O` level. Introduce a variable `OPT` that allows to do just that by passing e.g. `OPT=-O0` to the `make` command line. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The following warning has been observed with gcc 15.2.1 on Fedora Rawhide:
vpd.c: In function 'create_vpd83.constprop':
vpd.c:265:55: error: '%s' directive output truncated writing 64 bytes into a region of size 40 [-Werror=format-truncation=]
265 | len = snprintf((char *)(desc + 4), maxlen, "%s%s",
| ^~
In file included from /usr/include/stdio.h:974,
from vpd.c:8:
In function 'snprintf',
inlined from 'create_scsi_string_desc' at vpd.c:265:8,
inlined from 'create_vpd83.constprop' at vpd.c:313:7:
/usr/include/bits/stdio2.h:68:10: note: '__builtin___snprintf_chk' output 65 or more bytes into a destination of size 40
68 | return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
69 | __glibc_objsize (__s), __fmt,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
70 | __va_arg_pack ());
| ~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Fix it.
Signed-off-by: Martin Wilck <mwilck@suse.com>
This file is formatted in a special way that clang-format won't accept. Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Collaborator
Author
|
Added Reviewed-by: tags from @bmarzins. |
Collaborator
Author
|
More minor NEWS.md updates. |
Signed-off-by: Martin Wilck <mwilck@suse.com>
- addition of purge_disconnected field to multiple data structures - struct bitfield changed to union - addition of udev wrappers Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
multipath-tools 0.14.0, 2026/01
User-visible changes
Automatic removal of disconnected SCSI devices
This release adds support for automatically purging SCSI devices that become
disconnected at the storage target (e.g. because a LUN became unmapped by an
administrator). It's a long-standing problem that such path devices become
"stale" on the Linux side. When they are re-mapped on the storage side,
possibly with different parameters, the kernel may not correctly detect
the changes. By removing such stale devices, the kernel will be forced to
re-probe them, which fixes this problem.
The feature is controlled by a new configuration option
purge_disconnectedthat can be set in
multipath.confper multipath device, hardware entry, orglobally. The feature is switched off by default.
When
purge_disconnectedis enabled, SCSI devices for which the storage arrayreports a "LUN NOT SUPPORTED" status are removed from the system, preventing
stale device entries from accumulating.
As this is a new feature, users are advised to test it in their environment
before enabling it in production.
Bug fixes
mpathpersist registered a key for a map. multipathd could fail to do so in
some cases, e.g. if paths become unavailable or available while the
registration was taking place. Fixes 0.13.0. Commits f7d6cd1, 4f3036b.
mpathpersist --report-capabilitiesoutput. Fixes 0.5.0. Commitc8ed5e6.
reservations commands. Fixes 0.13.0. Commit c971036.
Commit f3ba2e7.
Fixes #132, 0.10.0.
Commit 9f611e2.
Fixes #128, 0.12.0.
Fixes 0.8.6. Commit 29f262b.
Fixes 0.8.9. Commit f1299f2. This fix isn't strictly necessary any
more after adding
-fno-strict-aliasing(see below), but it doesn't hurt tohave it.
once per protocol. Commit 958c826. Fixes 0.9.0.
-fno-strict-aliasing. This turns out to be necessary because our codeuses techniques like
container_of()which don't work well withstrict aliasing rules. Commit 1ef540a.
Fixes #130.
Other changes
from a multi-threaded program. Commits 69f6590, 6cc012f.
Specifically, avoid freeing path objects from code that frees multipath
maps, and vice versa. Treat path orphaning and freeing separately.
Fixes #129
replaced legacy hotplug.
Shortlog
@bmarzins (6):
multipathd: Fix race while registering PR key
mpathpersist: Fix REPORT CAPABILITIES output
libmpathpersist: fix register retry status checking
multipath-tools man pages: fix multipathd commands keyword ordering
multipathd: remember number of registered keys when ioctl fails
libmpathpersist: fix code for skipping multipathd path registration
@brianatpurestorage (2):
libmultipath: add purge_disconnected configuration option
multipathd: implement purge functionality for disconnected paths
@Itxaka (1):
multipathd: Dont pthread_join twice
@mwilck (63):
GitHub Workflows: abi-stable: fix PARENT_TAG logic for pull request
GitHub workflows: Update runners to ubuntu-24.04
GitHub workflows: native.yaml: add fedora-43
GitHub workflows: native.yaml: add openSUSE Leap 16.0 and 15.6
GitHub workflows: run arm64 tests on native runner
GitHub workflows: rolling: upload binaries on error
GitHub workflows: multiarch-stable: upload binaries on error
GitHub workflows: native: upload binaries on error
GitHub workflows: multiarch: upload binaries on error
GitHub workflows: use -j$(nproc) consistently
GitHub workflows: install libmount-dev on Ubuntu
libdmmp: update man page time stamps
GitHub actions: spelling: ignore generic hex number patterns
GitHub actions: spelling: fix warning about ignore expect variant
NEWS.md: describe post-0.13.0 changes
multipath-tools: Fix ISO C23 errors with strchr()
libmultipath: drop drop_multipath
libmultipath: don't access path members in free_pgvec()
libmpathutil: constify find_slot()
libmultipath: don't touch mpvec in remove_map()
libmultipath: export cleanup_multipath()
libmultipath: add cleanup_pathvec_and_free_paths()
libmultipath: don't free paths in orphan_paths()
multipathd: free orphaned paths in checker_finished()
libmultipath: remove free_paths argument from free_pathgroup()
libmultipath: remove free_paths argument from free_pgvec()
libmultipath: remove free_paths argument from free_multipathvec()
libmultipath: remove free_paths argument from free_multipath()
libmultipath: don't free mpp->pg and mpp->paths in remove_map()
multipath: free paths through pathvec in check_usable_paths()
multipathd: add_map_without_path(): don't free paths
libmultipath: remove cleanup_multipath_and_paths()
libmultipath: annotate functions that may free paths
libmultipath: simplify sysfs_get_target_nodename()
multipathd: join the init_unwinder dummy thread
kpartx: fix some memory leaks
libmpathutil: use union for bitfield
libmpathutil: add wrapper code for libudev
multipath-tools: use the libudev wrapper functions
Makefile: add functionality to determine cmocka version
multipath-tools tests: adaptations for cmocka 2.0
libmpathutil, libmultipath: bump library versions
Update NEWS.md
libmultipath: reset pgindex in uninitialize_path()
libmultipath: clarify code in set_path_removed()
libmultipath: nullify pp->mpp before calling free_path()
libmultipath: log error when freeing path that refers to a map
libmultipath: sync_paths(): print message when fixing pp->mpp
libmultipath: don't free mpp->paths in group_paths()
libmultipath: warn only once in scsi_tmo_error_msg()
libmultipath: find_hwe(): fix gcc errors at high optimization levels
multipath-tools: compile with -fno-strict-aliasing
kpartx: avoid double-free of mapname
kpartx: remove "hotplug" mode
multipathd: fix an uninitialized variable warning
Makefiles: add "ASAN=1" make parameter for AddressSanitizer
Makefiles: add "OPT=" make parameter
multipath-tools tests: suppress a compiler warning for vpd.c
.clang-format-ignore: add dict.c
Spelling: Remove unneeded entries in expect.txt
Update NEWS.md
libmultipath: bump library version
libmultipath: bump version to 0.14.0
@vadorovsky (1):
libmultipath: Remove the undefined symbol from the linker script
@xosevp (3):
multipath-tools: add Seagate Exos and Nytro series to hwtable
multipath-tools: identify more Storwize models
multipath-tools: move SPDX-License-Identifier to the top of the file