From: mwilck@suse.com
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@lists.linux.dev, Martin Wilck <mwilck@suse.com>
Subject: [PATCH v2 09/14] libmpathutil: remove systemd_service_enabled()
Date: Thu, 26 Oct 2023 19:41:48 +0200 [thread overview]
Message-ID: <20231026174153.1133-10-mwilck@suse.com> (raw)
In-Reply-To: <20231026174153.1133-1-mwilck@suse.com>
From: Martin Wilck <mwilck@suse.com>
Since c203929 ("multipathd.service: add dependency on
systemd-udevd-kernel.socket"), multipathd will start early during boot.
Moreover, we recommend using socket activation for multipathd,
and if multipathd.socket is enabled, the __mpath_connect()
check will succeed anyway.
The systemd_service_enabled() test was just "good enough" for
standard situations but never robust; it checked for multipathd.wants in
"some" directory, which might or might not be the current target,
and it would return true even of multipathd.service was masked.
Remove this test.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathutil/libmpathutil.version | 17 +++------
libmpathutil/util.c | 58 -------------------------------
libmpathutil/util.h | 1 -
libmultipath/valid.c | 16 ++-------
tests/valid.c | 24 ++-----------
5 files changed, 9 insertions(+), 107 deletions(-)
diff --git a/libmpathutil/libmpathutil.version b/libmpathutil/libmpathutil.version
index 6ebb718..15ff467 100644
--- a/libmpathutil/libmpathutil.version
+++ b/libmpathutil/libmpathutil.version
@@ -93,12 +93,15 @@ local:
};
/* symbols referenced internally by libmultipath */
-LIBMPATHUTIL_1.0 {
+LIBMPATHUTIL_2.0 {
alloc_bitfield;
__append_strbuf_str;
append_strbuf_quoted;
basenamecpy;
+ cleanup_fd_ptr;
cleanup_free_ptr;
+ cleanup_vector_free;
+ cleanup_fclose;
filepresent;
find_keyword;
free_keywords;
@@ -120,21 +123,9 @@ LIBMPATHUTIL_1.0 {
snprint_keyword;
steal_strbuf_str;
strlcat;
- systemd_service_enabled;
validate_config_strvec;
vector_find_or_add_slot;
vector_insert_slot;
vector_move_up;
vector_sort;
};
-
-LIBMPATHUTIL_1.1 {
-global:
- cleanup_fd_ptr;
-} LIBMPATHUTIL_1.0;
-
-LIBMPATHUTIL_1.2 {
-global:
- cleanup_vector_free;
- cleanup_fclose;
-} LIBMPATHUTIL_1.0;
diff --git a/libmpathutil/util.c b/libmpathutil/util.c
index 92f25a5..9d147fc 100644
--- a/libmpathutil/util.c
+++ b/libmpathutil/util.c
@@ -213,64 +213,6 @@ setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached)
}
}
-int systemd_service_enabled_in(const char *dev, const char *prefix)
-{
- static const char service[] = "multipathd.service";
- char path[PATH_MAX], file[PATH_MAX];
- DIR *dirfd;
- struct dirent *d;
- int found = 0;
-
- if (safe_sprintf(path, "%s/systemd/system", prefix))
- return 0;
-
- condlog(3, "%s: checking for %s in %s", dev, service, path);
-
- dirfd = opendir(path);
- if (dirfd == NULL)
- return 0;
-
- while ((d = readdir(dirfd)) != NULL) {
- char *p;
- struct stat stbuf;
-
- if ((strcmp(d->d_name,".") == 0) ||
- (strcmp(d->d_name,"..") == 0))
- continue;
-
- if (strlen(d->d_name) < 6)
- continue;
-
- p = d->d_name + strlen(d->d_name) - 6;
- if (strcmp(p, ".wants"))
- continue;
- if (!safe_sprintf(file, "%s/%s/%s",
- path, d->d_name, service)
- && stat(file, &stbuf) == 0) {
- condlog(3, "%s: found %s", dev, file);
- found++;
- break;
- }
- }
- closedir(dirfd);
-
- return found;
-}
-
-int systemd_service_enabled(const char *dev)
-{
- int found = 0;
-
- found = systemd_service_enabled_in(dev, "/etc");
- if (!found)
- found = systemd_service_enabled_in(dev, "/usr/lib");
- if (!found)
- found = systemd_service_enabled_in(dev, "/lib");
- if (!found)
- found = systemd_service_enabled_in(dev, "/run");
- return found;
-}
-
static int _linux_version_code;
static pthread_once_t _lvc_initialized = PTHREAD_ONCE_INIT;
diff --git a/libmpathutil/util.h b/libmpathutil/util.h
index 99a471d..de9fcfd 100644
--- a/libmpathutil/util.h
+++ b/libmpathutil/util.h
@@ -21,7 +21,6 @@ size_t strlcat(char * restrict dst, const char * restrict src, size_t size);
dev_t parse_devt(const char *dev_t);
char *convert_dev(char *dev, int is_path_device);
void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
-int systemd_service_enabled(const char *dev);
int get_linux_version_code(void);
int safe_write(int fd, const void *buf, size_t count);
void set_max_fds(rlim_t max_fds);
diff --git a/libmultipath/valid.c b/libmultipath/valid.c
index d4dae3e..f223778 100644
--- a/libmultipath/valid.c
+++ b/libmultipath/valid.c
@@ -314,23 +314,11 @@ is_path_valid(const char *name, struct config *conf, struct path *pp,
return PATH_IS_VALID_NO_CHECK;
}
- /*
- * "multipath -u" may be run before the daemon is started. In this
- * case, systemd might own the socket but might delay multipathd
- * startup until some other unit (udev settle!) has finished
- * starting. With many LUNs, the listen backlog may be exceeded, which
- * would cause connect() to block. This causes udev workers calling
- * "multipath -u" to hang, and thus creates a deadlock, until "udev
- * settle" times out. To avoid this, call connect() in non-blocking
- * mode here, and take EAGAIN as indication for a filled-up systemd
- * backlog.
- */
-
if (check_multipathd) {
fd = __mpath_connect(1);
if (fd < 0) {
- if (errno != EAGAIN && !systemd_service_enabled(name)) {
- condlog(3, "multipathd not running or enabled");
+ if (errno != EAGAIN) {
+ condlog(3, "multipathd not running");
return PATH_IS_NOT_VALID;
}
} else
diff --git a/tests/valid.c b/tests/valid.c
index 7032932..18a5a7b 100644
--- a/tests/valid.c
+++ b/tests/valid.c
@@ -62,11 +62,6 @@ int __wrap___mpath_connect(int nonblocking)
return -1;
}
-int __wrap_systemd_service_enabled(const char *dev)
-{
- return (int)mock_type(bool);
-}
-
/* There's no point in checking the return value here */
int __wrap_mpath_disconnect(int fd)
{
@@ -216,7 +211,6 @@ enum {
enum {
CHECK_MPATHD_RUNNING,
CHECK_MPATHD_EAGAIN,
- CHECK_MPATHD_ENABLED,
CHECK_MPATHD_SKIP,
};
@@ -232,11 +226,8 @@ static void setup_passing(char *name, char *wwid, unsigned int check_multipathd,
else if (check_multipathd == CHECK_MPATHD_EAGAIN) {
will_return(__wrap___mpath_connect, false);
will_return(__wrap___mpath_connect, EAGAIN);
- } else if (check_multipathd == CHECK_MPATHD_ENABLED) {
- will_return(__wrap___mpath_connect, false);
- will_return(__wrap___mpath_connect, ECONNREFUSED);
- will_return(__wrap_systemd_service_enabled, true);
}
+
/* nothing for CHECK_MPATHD_SKIP */
if (stage == STAGE_CHECK_MULTIPATHD)
return;
@@ -342,19 +333,10 @@ static void test_check_multipathd(void **state)
will_return(__wrap_sysfs_is_multipathed, false);
will_return(__wrap___mpath_connect, false);
will_return(__wrap___mpath_connect, ECONNREFUSED);
- will_return(__wrap_systemd_service_enabled, false);
+
assert_int_equal(is_path_valid(name, &conf, &pp, true),
PATH_IS_NOT_VALID);
assert_string_equal(pp.dev, name);
- /* test pass because service is enabled. fail getting udev */
- memset(&pp, 0, sizeof(pp));
- setup_passing(name, NULL, CHECK_MPATHD_ENABLED, STAGE_CHECK_MULTIPATHD);
- will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
- will_return(__wrap_udev_device_new_from_subsystem_sysname,
- name);
- assert_int_equal(is_path_valid(name, &conf, &pp, true),
- PATH_IS_ERROR);
- assert_string_equal(pp.dev, name);
/* test pass because connect returned EAGAIN. fail getting udev */
setup_passing(name, NULL, CHECK_MPATHD_EAGAIN, STAGE_CHECK_MULTIPATHD);
will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
@@ -533,7 +515,7 @@ static void test_check_uuid_present(void **state)
memset(&pp, 0, sizeof(pp));
conf.find_multipaths = FIND_MULTIPATHS_STRICT;
- setup_passing(name, wwid, CHECK_MPATHD_ENABLED, STAGE_CHECK_WWIDS);
+ setup_passing(name, wwid, CHECK_MPATHD_RUNNING, STAGE_CHECK_WWIDS);
will_return(__wrap_dm_map_present_by_uuid, 1);
will_return(__wrap_dm_map_present_by_uuid, wwid);
assert_int_equal(is_path_valid(name, &conf, &pp, true),
--
2.42.0
next prev parent reply other threads:[~2023-10-26 17:42 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-26 17:41 [PATCH v2 00/14] multipath: aio, systemd, and documentation improvements mwilck
2023-10-26 17:41 ` [PATCH v2 01/14] libmultipath: reduce log level of directio messages mwilck
2023-10-26 17:41 ` [PATCH v2 02/14] libmultipath: directio: don't reset ct->running after io_cancel() mwilck
2023-10-27 18:54 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 03/14] libmultipath: directio: fix error handling mwilck
2023-10-27 18:54 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 04/14] libmultipath: io_err_stat: don't free aio memory before completion mwilck
2023-10-27 18:55 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 05/14] libmultipath: io_err_stat: call io_destroy() inside free_io_err_pathvec() mwilck
2023-10-27 18:55 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 06/14] libmultipath: io_err_stat: use higher number of aio slots mwilck
2023-10-27 18:56 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 07/14] libmultipath: io_err_stat: fix error handling mwilck
2023-10-27 19:09 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 08/14] multipathd.service: require modprobe@dm_multipath.service if available mwilck
2023-10-27 19:09 ` Benjamin Marzinski
2023-10-27 20:05 ` Benjamin Marzinski
2023-10-30 11:45 ` Martin Wilck
2023-10-26 17:41 ` mwilck [this message]
2023-10-26 17:41 ` [PATCH v2 10/14] multipath.conf.5: fix typo mwilck
2023-10-27 19:10 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 11/14] Makefile.inc, README.md: fix docs for prefix in split-usr case mwilck
2023-10-27 19:27 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 12/14] README.md: update mailing list and contributing information mwilck
2023-10-27 19:29 ` Benjamin Marzinski
2023-10-30 11:41 ` Martin Wilck
2023-10-26 17:41 ` [PATCH v2 13/14] README.md: Extend the section about NVMe mwilck
2023-10-27 19:34 ` Benjamin Marzinski
2023-10-26 17:41 ` [PATCH v2 14/14] README.md: fix formatting of Changelog section mwilck
2023-10-27 19:35 ` Benjamin Marzinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231026174153.1133-10-mwilck@suse.com \
--to=mwilck@suse.com \
--cc=bmarzins@redhat.com \
--cc=christophe.varoqui@opensvc.com \
--cc=dm-devel@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.