From: Martin Wilck <mwi...@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 <mwi...@suse.com>
Reviewed-by: Benjamin Marzinski <bmarz...@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


Reply via email to