multipath-tools 0.13.1 #134
Open
+664
−214
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> (cherry picked from commit f7d6cd1)
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> (cherry picked from commit c8ed5e6)
Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 866f343)
Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 541e36e)
Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 1dbc6ff)
"opensuse-leap" would use 15.6 already, but make this explicit. Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 6e09ca1)
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> (cherry picked from commit 00cdcbc)
This will allow debugging CI failures (to some extent, as we compile with optimization by default). Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit aee2310)
Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 34746cd)
Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit f3bdfef)
Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit b854c7b)
The workflow files were using make's -j flag inconsistently. Fix it. Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit f24aca3)
Without it, the workflows will fail. Signed-off-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 2c872a7)
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> (cherry picked from commit 196b3d1)
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> (cherry picked from commit 9889fae)
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> (cherry picked from commit 5bdafa5)
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> (cherry picked from commit 5835dca)
Fix the spelling CI complaints about "0xea01". Signed-off-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> (cherry picked from commit c971036)
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> (cherry picked from commit f3ba2e7)
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> (cherry picked from commit c46a4f2)
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> (cherry picked from commit 4f3036b)
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> (cherry picked from commit a298603)
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> (cherry picked from commit 9f611e2)
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> (cherry picked from commit 7a07023)
This closes a minor memory leak (thread-local storage of the dummy thread is never freed). Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> (cherry picked from commit 29f262b)
These leaks were reported by LeakSanitizer. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> (cherry picked from commit 8c39e60)
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> (cherry picked from commit 02e0933)
The intention of this code was to warn only once for each unsupported protocol. But the condition statement is missing. Fixes: 6ad77db ("libmultipath: Set the scsi timeout parameters by path") Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> (cherry picked from commit 958c826)
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> (cherry picked from commit 1ef540a)
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> (cherry picked from commit c032ba1)
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> (cherry picked from commit 5e5d210)
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> (cherry picked from commit c94c65c)
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>
(cherry picked from commit 888187e)
cmocka 2.0.0 introduces a couple of changes that require version-dependent code to be used. Add macros to determine the cmocka version. If the version can't be determined, assume 1.1.0. Use a different approach here than in the stable branch, because the fix there is rather large. Instead of replacing all deprecated macros, just treat the respective warnings as non-fatal by adding the compiler flag -Wno-error=deprecated-declarations for the CI code. cmocka 2.0.1 added a macro to suppress these warnings altogether, but we should also be able to build against 2.0.0. Fixes: opensvc#129 Signed-off-by: Martin Wilck <mwilck@suse.com>
This comment has been minimized.
This comment has been minimized.
"nvme" makes "NVMe" redundant. Signed-off-by: Martin Wilck <mwilck@suse.com>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
If a path in a mulitpath device is offline while multipathd is reconfigured, it will get added to the updated multipath device, just like it was in the old multipath device. However the device will still be in the INIT_NEW state because it can't get initilized while offline. This is different than the INIT_PARTIAL state because the path was discovered in path_discovery(). INIT_PARTIAL is for paths that multipathd did not discover in path_discovery() or receive a uevent for, but are part of a multipath device that was added, and which should receive a uevent shortly. There is no reason to expect a uevent for these offline paths. When the path comes back online, multipathd will run the checker and prioritizer on it. The only two pathinfo checks that won't happen are the DI_WWID and DI_IOCTL ones. Modify pathinfo() to make sure that if DI_IOCTL was skipped for offline paths, it gets called the next time pathinfo() is called after the fd can be opened. Also, make sure that when one of these offline paths becomes usable, its WWID is rechecked. With those changes, all the DI_ALL checks will be accounted for, and the path can be marked INIT_OK. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 1942fb1)
If a path has a checker selected and is offline, multipathd will print a "path offline" message. However if the checker isn't selected, for instance because multipathd was started or reconfigured while the path was offline, multipathd was not printing the "path offline" message. Fix that. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com> (cherry picked from commit 1a364a1)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.13.1, 2026/01
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.
mpathpersist --report-capabilitiesoutput. Fixes 0.5.0.reservations commands. Fixes 0.13.0.
Fixes #132, 0.10.0.
Fixes #128, 0.12.0.
Fixes 0.8.6.
once per protocol. 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.
Fixes #130.
Other changes
-Wno-error=deprecated-declarationscompiler flag.
Fixes #129