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.4: Makefile overhaul, and some bug fixes #53

Merged
merged 65 commits into from Dec 19, 2022

Conversation

mwilck
Copy link
Contributor

@mwilck mwilck commented Oct 28, 2022

Hello @cvaroqui, hello @bmarzins,

here is yet another PR. The recent build issues reported (#50, #52) made me
want to cleanup the Makefiles. While at it, I made a few changes that I'd wanted to make for some time.

The major changes are:

  • small config files config.mk and autoconfig.h are generated early during build and reused. This way we don't need to test certain system properties over and over again. This is sort of a "poor-man's autoconf", or "poor man's meson", if you prefer. But I think it serves us quite well for now. autoconf is slow and over-engineered for our purposes, and switching to meson would be a major project; I'm not sure if it'd be worth the time, given that our Makefiiles contain quite a bit of complex logic that we'd need to recreate in meson. With this in place, we can consider switching to some other configuration mechanism in the future if we want.
  • We have a "quiet build" now by default, like the kernel and many other projects. Normal verbosity is reinstated using make V=1 (still less verbose than before, because lots of -D options have been moved to autoconfig.h)
  • Variables to pass on the make command line have changed. SYSTEMDPATH and libdir are no more, instead we have now systemd_prefix and plugindir. In general, the paths are now set up more cleanly IMO. I added also some documentation in README.md about this.
  • Duplicated code has been moved from Makefiles into a common rules.mk file. A large part of the complex functions we used in make code has been moved to a separate Makefile, which is only used for creating config.mk. Makefiles have decreased in size.

I believe that this PR is rather uninteresting for the general public on dm-devel (again, because it's just trivial fixes and Makefile stuff). Therefore I'm pusing it here. If anyone wishes that I post to dm-devel as well, please speak up.

Update 2022-12-03: I have added another bunch of small patches on top of the previous PR. There was a small cleanup series by @bmarzins, some documentation fixes, and yet another round of Makefile improvements. There are also rather big changes for the GitHub workflow CI.
We have a much larger matrix of test beds for compilation and unit tests now. You don't see these new workflows here yet, because by GitHub policy the workflows of the target repository are run for PRs. Check https://github.com/openSUSE/multipath-tools/actions to see how the
new test matrix looks like. The containers for the test beds come from my ghcr.io repo now.

Update 2022-12-19: added 3 more patches from @bmarzins and one from @standby24x7,
and bump version to 0.9.4

Regards
Martin

@bmarzins (8):
libmultipath: impove add_feature() variable names
multipathd: don't initialize the field width in show_path()
libmultipath: improve remove_feature() variable names
multipath.conf(5): remove io-affinity information
libmpathpersist: fix command keyword ordering
libmpathutil: simplify set_value and validate_config_strvec
libmultipath: don't leak memory on invalid strings
libmutipath: validate the argument count of config strings

@mwilck (48):
fixup! Makefile.inc: fix man and include paths (fix regression introduced by #50)
multipath-tools: Makefile.inc: Fix paths for systemd (fix #52)
multipath-tools: Makefile.inc: don't take values from environment
multipath-tools: Makefile.inc: get rid of RUN
multipath-tools: Makefile.inc: more compact code for LIB
multipath-tools: Makefiles: simplify code for include dirs
multipath-tools: Makefiles: use $(mandir)
multipath-tools: Makefile.inc: simplify expression for SYSTEMD
multipath-tools: Makefile.inc: untangle paths and source directories
multipath-tools: Makefiles: replace $(libdir) by $(plugindir)
multipath-tools: Makefile.inc: use simple make variables consistently
multipath-tools: Makefile.inc: fix a log message
multipath-tools: Makefile.inc: set systemd-specific flags
multipathd: Makefile: fix compilation flags for libedit
multipath-tools Makefiles: clean up executable Makefiles
multipath-tools: Makefiles: use common code for libraries
multipath-tools: Makefiles: move common code to rules.mk
multipath-tools Makefiles: create config.mk
multipath-tools Makefiles: enable quiet build
multipath-tools: quiet build support for top-level Makefile
GitHub workflows: use make -j8 -Orecurse
README.md: Move section about libedit and libreadline
README.md: Fix indentation in paragraph about device handlers
README.md: document options for paths and optimization
README.md: document how to customize build
libmultipath: avoid -Warray-bounds error with gcc 12 and musl libc
multipath: avoid NULL string in released_to_systemd()
libmultipath: avoid NULL string in is_udev_ready()
(2nd bunch)
multipath.conf(5): remove deprecated note for san_path_err algorithm
multipath.conf(5): improve documentation of dev_loss_tmo
multipath-tools tests: Makefile: dump output in case of failure
multipath-tools Makefile: fix compilation on Ubuntu trusty
libdmmp: allow installing without perl
multipath-tools Makefiles: fix variable passing to sub-make
multipath-tools: create-config.mk: Test _FORTIFY_SOURCE=2, too
multipath-tools: create-config.mk: fix logic for building autoconfig.h
multipath-tools: README.md: document passing CFLAGS etc to the build process
multipath-tools: tests/directio: fix failure wuith MUSL libc on s390x
multipathd: fix compilation error on Centos 7
libmpmathutil: fix compilation issue for clock_gettime()
libmultipath: fix call of udev_device_unref() for old libudev
multipath-tools: detect presence of libjson-c
GitHub workflows: use new build containers from ghcr.io
GitHub workflows: disable fail-fast strategy
GitHub workflows: multiarch.yaml: add i386
GitHub workflows: run tests for rolling distros on schedule
fixup! multipath-tools Makefile: fix compilation on Ubuntu trusty
GitHub workflows: multiarch.yaml: add i386
GitHub workflows: run tests for rolling distros on schedule
fixup! multipath-tools Makefile: fix compilation on Ubuntu trusty
libmultipath: bump version to 0.9.4

@standby24x7 (1):
kpartx_id: Add missing 3rd option in usage (fixes #57)

@pkoetsier (1):
libmultipath: fix 'show paths format' failure (fixes #49)

@xosevp (7):
multipath-tools: add EF to rdac info
multipath-tools: multipath.conf man page housekeeping
multipath-tools: update hwtable text/info/comments
multipath-tools: add PowerMax NVMe to hwtable
multipath-tools: add more info for NetApp ontap prio
multipath-tools: fix c&p error in install_keyword for deprecated pg_timeout
multipath-tools: mailing list url was changed

pkoetsier and others added 29 commits October 28, 2022 00:31
Prevent 'multipathd show paths format "%c"' from failing on orphan paths.
For orphan paths the checker class isn't set, which caused
snprint_path_checker() to fail which in turn caused 'show paths format' to fail
when the format string contained "%c".

Reviewed-by: Martin Wilck <mwilck@suse.com>
Paths would now be wrong with the default (empty) prefix.
Fix it.

Fixes: 2b2885c ("Makefile.inc: fix man and include paths")
Signed-off-by: Martin Wilck <mwilck@suse.com>
With prefix=/usr, systemd files were installed under /usr/usr/lib,
which is bogus. Clean this up, and document how to handle systemd's
"rootprefix" options. Remove the previous SYSTEMDPATH option.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Don't use environment variables to initialize LIB, RUN, SYSTEMD, SYSTEMDPATH,
DEVMAPPER_INCDIR, LIBUDEV_INCDIR, and LINUX_HEADERS_INCDIR. Taking
such variables from the environment is generally discouraged
(see https://www.gnu.org/software/make/manual/html_node/Environment.html).

Overriding variables from the commandline is still possible
(https://www.gnu.org/software/make/manual/html_node/Overriding.html).
So now, when building multipath-tools, rather then running
"RUN=/somedir make", users need to run "make RUN=/somedir".

This simplifies the Makefile without losing important functionality.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Just use $(runtimedir). Also, make the code more compact by using
the "if" function instead of a conditional.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Use make's "if" function instead of a conditional.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Use make's if function, and use lower-case latters for make variable
names that represent directories, such as elsewhere.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Simply use $(mandir) rather than 3 different variables.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Use a shell "or" function here. Note the use of "strip" to remove
the whitespace that are caused by the line break.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Only the installation paths can be customized. Move the definitions
of $(multipathdir) etc. further down.

Signed-off-by: Martin Wilck <mwilck@suse.com>
The make variables $(libdir) and $(plugindir) are redundant.
I overlooked that in af15832 ("multipath-tools: make multipath_dir a
compiled-in option"). While libdir has existed longer, I think
plugindir describes better what this path is used for, so replace
libdir by plugindir.

Signed-off-by: Martin Wilck <mwilck@suse.com>
"Simply expanded" make variables are generally preferred over "recursively
expanded" make variables, unless they reference other variables that are
defined over overwritten further down in the Makefile (see
https://www.gnu.org/software/make/manual/html_node/Flavors.html).
Using them makes the code easier to read and even somewhat faster.

We've been adding simply expanded variables over time, but most older
code still uses recursively expanded ones. Try to be consistent, at least
in Makefile.inc.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Define SYSTEMD_CPPFLAGS and SYSTEMD_LIBDEPS, and use them in Makefiles.

Signed-off-by: Martin Wilck <mwilck@suse.com>
The workaround for the wrong prototype in older libedit versions
had ended up in the code for libreadline. Fix it, and use simple
make variables.

Fixes: 2bd80f6 ("multipathd: fix incompatible pointer type error with libedit")
Signed-off-by: Martin Wilck <mwilck@suse.com>
Move the EXEC definition to the top, and use simple make variables
where possible.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Move shared code to Makefile.inc as far as possible. Use simple
make variables where possible.

Signed-off-by: Martin Wilck <mwilck@suse.com>
The library Makefiles contain a lot of similar rules. Just
maintain them in a single file.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Rather than running the test scripts for certain system features
every time "make" is called, save the configuration in "config.mk"
and "libmultipath/autoconfig.h", and reuse it later. This reduces
build time, especially in subsequent builds, and the build output is
less garbled by compiler options.

It works by invoking the separate make file "create-config.mk" at
the beginning of the build process. Most of the complex makefile
functions are moved to "create-config.mk".

Signed-off-by: Martin Wilck <mwilck@suse.com>
Like many other projects, make it possible to print much less
output during build. Verbose output is enabled with "make V=1", as
usual.

Signed-off-by: Martin Wilck <mwilck@suse.com>
This requires including "Makefile.inc" in the top-level Makefile,
too.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Without -Orecurse, quiet make output is confusing in parallel mode
because make prints "Entering directory" and "Leaving directory"
messages before and after every target.

Use parallel compilation also in native.yaml and foreign.yaml.

Signed-off-by: Martin Wilck <mwilck@suse.com>
This paragraph belongs in the "Customiting build" section.

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 following error is observed with gcc 12, strangely only with
MUSL libc:

In function ‘__uatomic_inc’,
     inlined from ‘lock’ at ../libmultipath/lock.h:24:2,
     inlined from ‘child’ at main.c:3523:3,
     inlined from ‘main’ at main.c:3755:11:
/usr/include/urcu/uatomic/x86.h:439:17: error: array subscript ‘struct __uatomic_dummy[0]’ is partly outside array bounds of ‘unsigned char[72]’ [-Werror=array-bounds]

The problem is that &(vecs->lock.waiters) is casted to a pointer to
struct { long[10]; } which goes beyond the "struct vectors".
We don't read or write from/to that memory, but the compiler complains either
way.

latest liburcu has a patch for it:

http://git.liburcu.org/?p=userspace-rcu.git;a=commitdiff;h=835b9ab3ca3777fe42e37e92096226ebd19ca75b

For now, just disable the warning in lock.h, using a pragma.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reported-by: Xose Vasquez Perez <xose.vasquez@gmail.com>
Fixes: b28c406 ("multipath -u: don't grab devices already passed to system")
Signed-off-by: Martin Wilck <mwilck@suse.com>
Fixes: 2b25a9e ("libmultipath: select_action(): force udev reload for uninitialized maps")
Signed-off-by: Martin Wilck <mwilck@suse.com>
@mwilck
Copy link
Contributor Author

mwilck commented Oct 28, 2022

@xosevp for information.

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

Signed-off-by: Martin Wilck <mwilck@suse.com>
This test fails because of an unexpected sign extension. See
comments in the code.

Signed-off-by: Martin Wilck <mwilck@suse.com>
On Centos 7 / aarch64, we observe the following compilation error:

In file included from /usr/include/scsi/scsi_netlink.h:26:0,
                 from /usr/include/scsi/scsi_netlink_fc.h:25,
                 from fpin_handlers.c:7:
fpin_handlers.c: In function 'fpin_fabric_notification_receiver':
fpin_handlers.c:527:25: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   if (!NLMSG_OK((struct nlmsghdr *)buf, (unsigned int)ret)) {
                         ^
fpin_handlers.c:527:25: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   if (!NLMSG_OK((struct nlmsghdr *)buf, (unsigned int)ret)) {
                         ^
fpin_handlers.c:532:32: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   plen = NLMSG_PAYLOAD((struct nlmsghdr *)buf, 0);

Fix it using a separate pointer.

Signed-off-by: Martin Wilck <mwilck@suse.com>
some older versions of glibc need to add -lrt for clock_gettime()

Signed-off-by: Martin Wilck <mwilck@suse.com>
In old libudev versions, udev_device_unref() had void type.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Don't build libdmmp if libjson-c is unavailable. Setting ENABLE_LIBDMMP
manually will override the autodetection.

Signed-off-by: Martin Wilck <mwilck@suse.com>
mosteo/actions@v1 now supports --pull-params and --params, making
it possible to support true multiplatform containers.

Thus use the new, enlarged set set of multipath-build containers
from ghcr.io/mwilck. This enables a larger set of build environments
and architectures than before. Unfortunately, arm/v7 builds fail
on all distros except Alpine
(see https://bugzilla.kernel.org/show_bug.cgi?id=205957)

"foreign.yaml" now only contains the cross-build / run setup
(Debian bullseye and sid), whereas for other platforms we compile
and build in the emulated environment (qemu-linux-user + container).
These jobs are moved to the new yaml file "multiarch.yaml".

Signed-off-by: Martin Wilck <mwilck@suse.com>
Switch off fail-fast, so that we can see which builds failed.

Signed-off-by: Martin Wilck <mwilck@suse.com>
We don't run this from "native.yaml" any more, so add it here.
mwilck and others added 5 commits December 3, 2022 01:07
This way we will find regressions with new compilers or libraries
faster.
Add Alletra 5000, FAS/AFF and E/EF Series info.
Compact some info.
Delete trivial/redundant comments.
Reformat LIO.

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>
No official config, just a "multipath -ll" output:
https://bugzilla.redhat.com/1686708#c0

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>
and format fixes.

Cc: George Martin <Martin.George@netapp.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>
…imeout

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>
@mwilck
Copy link
Contributor Author

mwilck commented Dec 3, 2022

Sorry for the force-push, I made a stupid mistake.

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>
@mwilck
Copy link
Contributor Author

mwilck commented Dec 5, 2022

@xosevp reported an issue with my patch "multipath-tools Makefile: fix compilation on Ubuntu trusty". (a file -.o was created). Posting a fix.

@xosevp
Copy link
Contributor

xosevp commented Dec 5, 2022 via email

standby24x7 and others added 5 commits December 19, 2022 08:38
This patch adds missing 3rd option in kpatx_id usage.

Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
alloc_strvec() will never create a strvec with multiple tokens between
the quote tokens.  Verify this in validate_config_strvec(), and simplify
set_value() by only reading one value after a quote token.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
If set_path() or set_str_noslash() are called with a bad value, they
ignore it and continue to use the old value. But they weren't freeing
the bad value, causing a memory leak.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
The features, path_selector, and hardware_handler config options pass
their strings directly into the kernel.  If users omit the argument
counts from these strings, or use the wrong value, the kernel's table
parsing gets completely messed up, and the error messages it prints
don't reflect what actully went wrong. To avoid messing up the
kernel table parsing, verify that these strings correctly set the
argument count to the number of arguments they have.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
@mwilck mwilck changed the title Makefile overhaul, and some bug fixes multipath-tools 0.9.4: Makefile overhaul, and some bug fixes Dec 19, 2022
@cvaroqui cvaroqui merged commit db4804b into opensvc:master Dec 19, 2022
66 checks passed
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.

None yet

6 participants