OBS test PR from openSUSE #13
Closed
Conversation
This file contains 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
This would normally be set by general system policy. According to systemd-system.conf(5), it is set to infinity by default anyway in systemd. If a user changes this in systemd-system.conf, the setting should also apply to multipathd. Users who don't wont this can still use drop-ins to change the specific settings. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The kernel has had an autoloading mechanism for SCSI device handlers since v4.3. No need to force loading them here again. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This is necessary in the initramfs. With this added, almost no differences between our shipped multipathd.service and the one shipped in dracut remain. The rest should be fixed in dracut. It would be nice to be able to use the original service file from multipath-tools in dracut, in particular for future changes. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
We've had "Before=" dependencies on iscsi.service and iscsid.service since this unit was first created. I'm not sure if we ever needed them. Since c9689b6 ("multipathd: Remove dependency on systemd-udev-settle.service"), we definitely don't. iscsi and iscsid sort themselves after network.target, and multipathd does not, thus these Before= dependencies are redundant. Note: Before c9689b6, iSCSI was actually treated differently than other transports - we forced multipathd to be started after FC device detection, but before iSCSI device detection. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Instead of having multipath allocate its dm_info structure, it should just embed it to save on the extra allocations. This also lets us have only one function that all callers use to fill a dm_info structure. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
When multipathd gets a change event for a multipath device, the dm info may have changed, so update it. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
A mulitpath device can only be reloaded read/write when all paths are read/write. Also, whenever a read-only scsi device is rescanned, the scsi subsystem will first unconditionally issue a uevent with DISK_RO=0 before checking the read-only status, and if it the device is still read-only, issuing another uevent with DISK_RO=1. These uevents cause pointless reloads when read-only paths are rescanned. To avoid this, first check if the path is being changed to the existing multipath read-only state. If the state is the same, do nothing. If it's different, check to see if all paths are read/write before changing a multipath device from read-only to read/write. If the multipath device read-only state is unknown, assume that it needs to be reloaded. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
The libmpathpersist library exports symbols that are not part of the public API. Move the respective code into a separate source file. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
We shouldn't define function prototypes in more than a single place. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
... of FILE_NAME_SIZE. Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This has no effect. If at all, it should have been included before inttypes.h. See https://stackoverflow.com/questions/8132399/how-to-printf-uint64-t-fails-with-spurious-trailing-in-format/8132440#8132440 Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
From now on this header contains only symbols that need to be accessed from multiple sources of libmpathpersist, but not from libmultipath or other source files (those go into libmpathpersist_int.h). Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.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> Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
pp->state should only be set to PATH_PENDING if it is currently in the PATH_UNCHECKED or PATH_WILD states. Otherwise, it should retain its current state. Fix this in pathinfo() calls with DI_NOIO, to match with the rest of the code. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martinn Wilck <mwilck@suse.com>
The only caller of __set_no_path_retry() is set_no_path_retry(), so remove the define and the unneeded arguments from the function. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martinn Wilck <mwilck@suse.com>
set_no_path_retry() could make a multipath device enter recovery mode simply because its paths were still in the PATH_PENDING state. This is possible when a multipath device is first created. After commit 2e61b8f [libmultipath (coverity): Revert "setup_map: wait for pending path checkers to finish"] it has become much more likely. To avoid this, don't enter recovery mode as long as there are some paths in PATH_PENDING. Since path's can only be in this state if they've never had their state checker finish, this change will only effect recently created devices, whose checker hasn't completed yet. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martinn Wilck <mwilck@suse.com>
cc18bab ("libmultipath: embed dm_info in multipath structure") requires a major version bump. Signed-off-by: Martin Wilck <mwilck@suse.com>
This tends to complicate packaging for downstreams as we may choose to not compress man pages at all, use a different tool, or use different options. Most projects don't therefore bother compressing, so this change brings multipath-tool in line with others. Signed-off-by: Sam James <sam@gentoo.org> Reviewed-by: Martin Wilck <mwilck@suse.com>
Minimal clean up done with scripts/checkincludes.pl 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>
On gcc-12 build failed as:
devmapper.c: In function 'dm_simplecmd':
devmapper.c:61:13: error: unused variable 'udev_wait_flag' [-Werror=unused-variable]
61 | int udev_wait_flag = (task == DM_DEVICE_RESUME ||
| ^~~~~~~~~~~~~~
Fix error by hiding it's declaration under #ifdef that uses it.
CC: Martin Wilck <mwilck@suse.com>
CC: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Without the change the build fails as:
devmapper.c:58:69: error: unused parameter 'udev_flags' [-Werror=unused-parameter]
58 | int dm_simplecmd(int task, const char *name, int no_flush, uint16_t udev_flags)
| ~~~~~~~~~^~~~~~~~~~
The change adds __attribute__((used)) annotation to unused function parameter.
CC: Martin Wilck <mwilck@suse.com>
CC: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>Reviewed-by: Martin Wilck <mwilck@suse.com>
On NixOS nothing is installed in /usr/include and instead lives
in it's own prefix. pkg-config variables are expected to be used
for installation discovery:
$ pkg-config --variable=includedir devmapper
/nix/store/c30fr0ahpa285sjkjgiinc2rr68ysmid-lvm2-2.03.14-dev/include
The change switches libdevmapper.h discovery to pkg-config provided path.
CC: Martin Wilck <mwilck@suse.com>
CC: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
On NixOS nothing is installed in /usr/include and instead lives
in it's own prefix. pkg-config variables are expected to be used
for installation discovery:
$ pkg-config --variable=includedir libudev
/nix/store/27mwkz5zhzw0gip8y7pvjyma5r0hzzaw-systemd-249.7-dev/include
The change switches libudev.h discovery to pkg-config provided path.
CC: Martin Wilck <mwilck@suse.com>
CC: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
On NixOS nothing is installed in /usr/include and instead lives in it's own prefix. The change switches linux/nvme_ioctl.h discovery to user provided path. CC: Martin Wilck <mwilck@suse.com> CC: Benjamin Marzinski <bmarzins@redhat.com> Signed-off-by: Sergei Trofimovich <slyich@gmail.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
`gzip` supports (deprecated) `GZIP` environment variable. If it's
already present Makefile would override it and pass it through causing
thre breakage:
$ dev>GZIP=-n make
gzip -9 -c mpath_persistent_reserve_in.3 > mpath_persistent_reserve_in.3.gz
gzip: -c: option not valid in GZIP environment variable
Try `gzip --help' for more information.
Fix build by renaming GZIP variable to GZIP_PROG to avoid collision.
mwilck: most of this is obsoleted by a220dd1 ("build: don't compress man pages")
CC: Martin Wilck <mwilck@suse.com>
CC: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
The historical-service-time path selector prints out 2 path group status arguments. This is the only path selector that uses the group status arguments. All the others only have path status arguments. disassemble_status() was expecting the number of group status arguments to always be zero, causing it to fail at disassembling the status of devices that use historical-service-time path selector. Now multipath actually checks the number of group arguments, and skips them. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
Pull the code that checks if a path needs to trigger a uevent, and triggers, out of trigger_paths_udev_change() and into a new function, trigger_path_udev_change(). This function will be used separately by a future patch. No functional changes. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
When a multipath device is created for the first time, there is a window where some path devices way be added to the multipath device, but never claimed in udev. This can allow other device owners, like lvm, to think they can use the device. When a multipath device is first created, all the existing paths that are not claimed by multipath have a uevent triggered so that they can get claimed. After that, multipath assumes all future paths added to the multipath device will have been claimed by multipath, since the device's WWID is now in the wwids file. This doesn't work for any paths that have already been processed by the multipath.rules udev rules before the multipath device was created. To close this window, when path device is added, and a matching multipath device already exists, multipathd now checks if the device is claimed by multipath, and if not, triggers a uevent to claim it. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
gcc 12.0.1 failed building libmultipath due to a format-overflow false
positive on 32-bit architectures. This isn't so surprising as
format-overflow=2 is very aggressive in the assumptions it makes about
the arguments. Here, it assumes that mpp->wwid could take up all the
space that a pointer could point to, even if I add code to this function
to explicitly null terminate mpp->wwid to fit in WWID_SIZE.
To avoid this and simplify the function, switch from using calloc() and
sprintf() to just using asprintf().
For reference, the gcc build error that this fixes is:
devmapper.c: In function 'dm_addmap.constprop.0':
devmapper.h:27:21: error: '%s' directive writing up to 2147483644 bytes into a region of size 2147483641 [-Werror=format-overflow=]
27 | #define UUID_PREFIX "mpath-"
| ^~~~~~~~
devmapper.c:484:53: note: format string is defined here
484 | sprintf(prefixed_uuid, UUID_PREFIX "%s", mpp->wwid);
| ^~
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
|
Failed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.