[PATCH v3 00/10] multipath-tools: provide pathname and abstract sockets

[PATCH v3 00/10] multipath-tools: provide pathname and abstract sockets

[Martin Wilck]

This set is an attempt to fix https://github.com/opensvc/multipath-tools/issues/111

Changes v2 -> v3:

Many, as a result of further discussions on GitHub. multipathd will now open
both the abstract socket and the pathname socket /run/multipathd.socket by
default, and client programs will try both sockets before giving up.  The
"use_regular_socket=1" build parameter is not necessary any more.

Note that we'd switched from the regular socket "/var/run/multipath.sock" to
an abstract socket in bb89077 ("multipathd: switch to abstract sockets for CLI
commands") in multipath-tools 0.5.0. The rationale back then was that creating
a pathname socket might fail if the directory structure isn't set up yet. As
we now create both, and fail only if neither could be created, and as there is
the early /run directory on modern systems, this argument shouldn't apply to
this patch set. The name of the socket has been changed in order to indicate
that the new approach is different from the old one.

Changes v1 -> v2:

- Introduced the mpath_fill_sockaddr() helper to cleanup struct sockaddr_un handling,
in response to comments from Ben Marzinski on GitHub.
- Fixed double remove of multipathd.socket during "make clean" (Ben).

Martin Wilck (10):
  multipath-tools: move DEFAULT_SOCKET definition into Makefile.inc
  multipath-tools: add helper mpath_fill_sockaddr__()
  libmpathutil: add support for Unix pathname sockets
  libmpathutil: move systemd_listen_fds() support into multipathd
  multipathd: make uxsock_listen() take a pointer to fd
  multipathd: allow receiving two socket fds from systemd
  multipathd: listen on pathname and abstract socket by default
  libmpathcmd: try both abstract and pathname sockets
  libmpathcmd: honor MULTIPATH_SOCKET_NAME environment variable
  multipathd: honor MULTIPATH_SOCKET_NAME environment variable

 .gitignore                                    |  1 +
 Makefile.inc                                  | 10 +++-
 create-config.mk                              |  1 +
 libmpathcmd/mpath_cmd.c                       | 25 ++++++----
 libmpathcmd/mpath_cmd.h                       |  1 -
 libmpathcmd/mpath_fill_sockaddr.c             | 32 +++++++++++++
 libmpathutil/uxsock.c                         | 41 +++++++---------
 libmultipath/defaults.h                       |  1 -
 multipathd/Makefile                           |  4 +-
 multipathd/main.c                             | 48 +++++++++++++++++--
 ...multipathd.socket => multipathd.socket.in} |  3 +-
 multipathd/uxlsnr.c                           | 34 +++++++++----
 multipathd/uxlsnr.h                           |  3 +-
 13 files changed, 149 insertions(+), 55 deletions(-)
 create mode 100644 libmpathcmd/mpath_fill_sockaddr.c
 rename multipathd/{multipathd.socket => multipathd.socket.in} (84%)

-- 
2.48.1

[PATCH v3 01/10] multipath-tools: move DEFAULT_SOCKET definition into Makefile.inc

[Martin Wilck]

This enables configuring the socket name. Follow up patches will
add more flexibility for configuring the sockets.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile.inc            | 5 ++++-
 libmpathcmd/mpath_cmd.h | 1 -
 libmultipath/defaults.h | 1 -
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index 65f6efc..76efeb9 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -79,6 +79,8 @@ libudev_incdir	:= $(or $(shell $(PKG_CONFIG) --variable=includedir libudev),/usr
 kernel_incdir	:= /usr/include
 sysdir_bin      := $(sys_execprefix)bin
 
+abstract_socket := /org/kernel/linux/storage/multipathd
+
 ifeq ($(V),)
 Q		:= @
 # make's "Entering directory" messages are confusing in parallel mode
@@ -114,7 +116,8 @@ CPPFLAGS	:= $(FORTIFY_OPT) $(CPPFLAGS) $(D_URCU_VERSION) \
 		   -DBIN_DIR=\"$(bindir)\" -DMULTIPATH_DIR=\"$(TGTDIR)$(plugindir)\" \
 		   -DRUNTIME_DIR=\"$(runtimedir)\" -DCONFIG_DIR=\"$(TGTDIR)$(configdir)\" \
 		   -DDEFAULT_CONFIGFILE=\"$(TGTDIR)$(configfile)\" -DSTATE_DIR=\"$(TGTDIR)$(statedir)\" \
-		   -DEXTRAVERSION=\"$(EXTRAVERSION)\" -MMD -MP
+		   -DEXTRAVERSION=\"$(EXTRAVERSION)\" \
+		   -DDEFAULT_SOCKET=\"$(abstract_socket)\" -MMD -MP
 CFLAGS		:= -std=$(C_STD) $(CFLAGS) $(OPTFLAGS) $(WARNFLAGS) -pipe \
 		   -fexceptions
 BIN_CFLAGS	:= -fPIE -DPIE
diff --git a/libmpathcmd/mpath_cmd.h b/libmpathcmd/mpath_cmd.h
index bf30c1e..c96fabc 100644
--- a/libmpathcmd/mpath_cmd.h
+++ b/libmpathcmd/mpath_cmd.h
@@ -30,7 +30,6 @@
 extern "C" {
 #endif
 
-#define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
 #define DEFAULT_REPLY_TIMEOUT	4000
 
 
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 02f7e57..134b690 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -67,7 +67,6 @@
 #define DEV_LOSS_TMO_UNSET	0U
 #define MAX_DEV_LOSS_TMO	UINT_MAX
 #define DEFAULT_PIDFILE		RUNTIME_DIR "/multipathd.pid"
-#define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
 #define DEFAULT_BINDINGS_FILE	STATE_DIR "/bindings"
 #define DEFAULT_WWIDS_FILE	STATE_DIR "/wwids"
 #define DEFAULT_PRKEYS_FILE	STATE_DIR "/prkeys"
-- 
2.48.1

[PATCH v3 02/10] multipath-tools: add helper mpath_fill_sockaddr__()

[Martin Wilck]

Create a static new helper function which is used by both libmpathcmd
and libmpathutil and fills in the socket address name from the compile-time
default. The function is able to handle both abstract and pathname sockets,
but more changes are needed to make the latter actually work.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 .gitignore                                    |  1 +
 Makefile.inc                                  |  7 ++--
 create-config.mk                              |  1 +
 libmpathcmd/mpath_cmd.c                       | 11 ++-----
 libmpathcmd/mpath_fill_sockaddr.c             | 32 +++++++++++++++++++
 libmpathutil/uxsock.c                         | 15 ++++-----
 multipathd/Makefile                           |  4 +--
 ...multipathd.socket => multipathd.socket.in} |  2 +-
 8 files changed, 51 insertions(+), 22 deletions(-)
 create mode 100644 libmpathcmd/mpath_fill_sockaddr.c
 rename multipathd/{multipathd.socket => multipathd.socket.in} (87%)

diff --git a/.gitignore b/.gitignore
index 4548cfb..6a1f6fc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -24,6 +24,7 @@ multipathd/multipathd
 multipathd/multipathd.8
 multipathd/multipathc
 multipathd/multipathd.service
+multipathd/multipathd.socket
 mpathpersist/mpathpersist
 mpathpersist/mpathpersist.8
 abi.tar.gz
diff --git a/Makefile.inc b/Makefile.inc
index 76efeb9..4015006 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -79,7 +79,7 @@ libudev_incdir	:= $(or $(shell $(PKG_CONFIG) --variable=includedir libudev),/usr
 kernel_incdir	:= /usr/include
 sysdir_bin      := $(sys_execprefix)bin
 
-abstract_socket := /org/kernel/linux/storage/multipathd
+abstract_socket := @/org/kernel/linux/storage/multipathd
 
 ifeq ($(V),)
 Q		:= @
@@ -117,7 +117,9 @@ CPPFLAGS	:= $(FORTIFY_OPT) $(CPPFLAGS) $(D_URCU_VERSION) \
 		   -DRUNTIME_DIR=\"$(runtimedir)\" -DCONFIG_DIR=\"$(TGTDIR)$(configdir)\" \
 		   -DDEFAULT_CONFIGFILE=\"$(TGTDIR)$(configfile)\" -DSTATE_DIR=\"$(TGTDIR)$(statedir)\" \
 		   -DEXTRAVERSION=\"$(EXTRAVERSION)\" \
-		   -DDEFAULT_SOCKET=\"$(abstract_socket)\" -MMD -MP
+		   -DDEFAULT_SOCKET=\"$(abstract_socket)\" \
+		   -DWSTRINGOP_TRUNCATION=$(if $(WSTRINGOP_TRUNCATION),1,0) \
+		   -MMD -MP
 CFLAGS		:= -std=$(C_STD) $(CFLAGS) $(OPTFLAGS) $(WARNFLAGS) -pipe \
 		   -fexceptions
 BIN_CFLAGS	:= -fPIE -DPIE
@@ -170,4 +172,5 @@ NV_VERSION_SCRIPT = $(DEVLIB:%.so=%-nv.version)
 		-e 's:@SYSDIR_BIN@:'$(sysdir_bin)': g' \
 		-e 's:@RUNTIME_DIR@:'$(runtimedir)':g' \
 		-e 's/@MODPROBE_UNIT@/'$(MODPROBE_UNIT)'/g' \
+		-e 's,@MPATH_SOCKET@,'$(abstract_socket)',g' \
 		$< >$@
diff --git a/create-config.mk b/create-config.mk
index ab163ed..f05fdab 100644
--- a/create-config.mk
+++ b/create-config.mk
@@ -191,6 +191,7 @@ $(TOPDIR)/config.mk:	$(multipathdir)/autoconfig.h
 	@echo "ERROR_DISCARDED_QUALIFIERS := $(call TEST_CC_OPTION,-Werror=discarded-qualifiers,)" >>$@
 	@echo "WNOCLOBBERED := $(call TEST_CC_OPTION,-Wno-clobbered -Wno-error=clobbered,)" >>$@
 	@echo "WFORMATOVERFLOW := $(call TEST_CC_OPTION,-Wformat-overflow=2,)" >>$@
+	@echo "WSTRINGOP_TRUNCATION := $(call TEST_CC_OPTION,-Wstringop-truncation)" >>$@
 	@echo "W_MISSING_INITIALIZERS := $(call TEST_MISSING_INITIALIZERS)" >>$@
 	@echo "W_URCU_TYPE_LIMITS := $(call TEST_URCU_TYPE_LIMITS)" >>$@
 	@echo "ENABLE_LIBDMMP := $(ENABLE_LIBDMMP)" >>$@
diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index a38e8b6..5a39471 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -24,11 +24,13 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <poll.h>
+#include <stddef.h>
 #include <string.h>
 #include <errno.h>
 #include <fcntl.h>
 
 #include "mpath_cmd.h"
+#include "mpath_fill_sockaddr.c"
 
 /*
  * keep reading until its all read
@@ -101,14 +103,6 @@ int mpath_connect__(int nonblocking)
 	struct sockaddr_un addr;
 	int flags = 0;
 
-	memset(&addr, 0, sizeof(addr));
-	addr.sun_family = AF_LOCAL;
-	addr.sun_path[0] = '\0';
-	strncpy(&addr.sun_path[1], DEFAULT_SOCKET, sizeof(addr.sun_path) - 1);
-	len = strlen(DEFAULT_SOCKET) + 1 + sizeof(sa_family_t);
-	if (len > sizeof(struct sockaddr_un))
-		len = sizeof(struct sockaddr_un);
-
 	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
 	if (fd == -1)
 		return -1;
@@ -119,6 +113,7 @@ int mpath_connect__(int nonblocking)
 			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
 	}
 
+	len = mpath_fill_sockaddr__(&addr, DEFAULT_SOCKET);
 	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
 		int err = errno;
 
diff --git a/libmpathcmd/mpath_fill_sockaddr.c b/libmpathcmd/mpath_fill_sockaddr.c
new file mode 100644
index 0000000..750ef3e
--- /dev/null
+++ b/libmpathcmd/mpath_fill_sockaddr.c
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2025 SUSE LLC
+ * SPDX-license-identifier: LGPL-2.1-or-newer
+ */
+
+static size_t mpath_fill_sockaddr__(struct sockaddr_un *addr, const char *name)
+{
+	size_t len;
+
+	addr->sun_family = AF_LOCAL;
+
+	if (name[0] != '@') {
+		/* Pathname socket. This should be NULL-terminated. */
+		strncpy(&addr->sun_path[0], name, sizeof(addr->sun_path) - 1);
+		addr->sun_path[sizeof(addr->sun_path) - 1] = '\0';
+		len = offsetof(struct sockaddr_un, sun_path) + strlen(name) + 1;
+	} else {
+		addr->sun_path[0] = '\0';
+		/*
+		 * The abstract socket's name doesn't need to be NULL terminated.
+		 * Actually, a trailing NULL would be considered part of the socket name.
+		 */
+#pragma GCC diagnostic push
+#if WSTRINGOP_TRUNCATION
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
+		strncpy(&addr->sun_path[1], &name[1], sizeof(addr->sun_path) - 1);
+#pragma GCC diagnostic pop
+		len = offsetof(struct sockaddr_un, sun_path) + strlen(name);
+	}
+	return len > sizeof(*addr) ? sizeof(*addr) : len;
+}
diff --git a/libmpathutil/uxsock.c b/libmpathutil/uxsock.c
index 2135476..12c4608 100644
--- a/libmpathutil/uxsock.c
+++ b/libmpathutil/uxsock.c
@@ -6,12 +6,15 @@
  */
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 #include <stdarg.h>
+#include <stddef.h>
 #include <fcntl.h>
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/un.h>
 #include <poll.h>
 #include <signal.h>
@@ -33,6 +36,8 @@
 static int _recv_packet(int fd, char **buf, unsigned int timeout,
 			ssize_t limit);
 
+#include "../libmpathcmd/mpath_fill_sockaddr.c"
+
 /*
  * create a unix domain socket and start listening on it
  * return a file descriptor open on the socket
@@ -63,15 +68,7 @@ int ux_socket_listen(const char *name)
 		return -1;
 	}
 
-	memset(&addr, 0, sizeof(addr));
-	addr.sun_family = AF_LOCAL;
-	addr.sun_path[0] = '\0';
-	len = strlen(name) + 1;
-	if (len >= sizeof(addr.sun_path))
-		len = sizeof(addr.sun_path) - 1;
-	memcpy(&addr.sun_path[1], name, len);
-
-	len += sizeof(sa_family_t);
+	len = mpath_fill_sockaddr__(&addr, name);
 	if (bind(fd, (struct sockaddr *)&addr, len) == -1) {
 		condlog(3, "Couldn't bind to ux_socket, error %d", errno);
 		close(fd);
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 997b40c..61cf1af 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -41,7 +41,7 @@ ifeq ($(FPIN_SUPPORT),1)
 OBJS += fpin_handlers.o
 endif
 
-all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
+all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service $(EXEC).socket
 
 $(EXEC): $(OBJS) $(multipathdir)/libmultipath.so $(mpathcmddir)/libmpathcmd.so
 	@echo building $@ because of $?
@@ -78,7 +78,7 @@ uninstall:
 	$(Q)$(RM) $(DESTDIR)$(unitdir)/$(EXEC).socket
 
 clean: dep_clean
-	$(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
+	$(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service $(EXEC).socket
 
 include $(wildcard $(OBJS:.o=.d) $(CLI_OBJS:.o=.d))
 
diff --git a/multipathd/multipathd.socket b/multipathd/multipathd.socket.in
similarity index 87%
rename from multipathd/multipathd.socket
rename to multipathd/multipathd.socket.in
index 6a62f5f..c0e86c3 100644
--- a/multipathd/multipathd.socket
+++ b/multipathd/multipathd.socket.in
@@ -7,7 +7,7 @@ ConditionVirtualization=!container
 Before=sockets.target
 
 [Socket]
-ListenStream=@/org/kernel/linux/storage/multipathd
+ListenStream=@MPATH_SOCKET@
 
 [Install]
 # Socket activation for multipathd is disabled by default.
-- 
2.48.1

Re: [PATCH v3 02/10] multipath-tools: add helper mpath_fill_sockaddr__()

[Benjamin Marzinski]

On Fri, Feb 14, 2025 at 11:10:03PM +0100, Martin Wilck wrote:
> Create a static new helper function which is used by both libmpathcmd
> and libmpathutil and fills in the socket address name from the compile-time
> default. The function is able to handle both abstract and pathname sockets,
> but more changes are needed to make the latter actually work.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  .gitignore                                    |  1 +
>  Makefile.inc                                  |  7 ++--
>  create-config.mk                              |  1 +
>  libmpathcmd/mpath_cmd.c                       | 11 ++-----
>  libmpathcmd/mpath_fill_sockaddr.c             | 32 +++++++++++++++++++
>  libmpathutil/uxsock.c                         | 15 ++++-----
>  multipathd/Makefile                           |  4 +--
>  ...multipathd.socket => multipathd.socket.in} |  2 +-
>  8 files changed, 51 insertions(+), 22 deletions(-)
>  create mode 100644 libmpathcmd/mpath_fill_sockaddr.c
>  rename multipathd/{multipathd.socket => multipathd.socket.in} (87%)
> 
> diff --git a/.gitignore b/.gitignore
> index 4548cfb..6a1f6fc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -24,6 +24,7 @@ multipathd/multipathd
>  multipathd/multipathd.8
>  multipathd/multipathc
>  multipathd/multipathd.service
> +multipathd/multipathd.socket
>  mpathpersist/mpathpersist
>  mpathpersist/mpathpersist.8
>  abi.tar.gz
> diff --git a/Makefile.inc b/Makefile.inc
> index 76efeb9..4015006 100644
> --- a/Makefile.inc
> +++ b/Makefile.inc
> @@ -79,7 +79,7 @@ libudev_incdir	:= $(or $(shell $(PKG_CONFIG) --variable=includedir libudev),/usr
>  kernel_incdir	:= /usr/include
>  sysdir_bin      := $(sys_execprefix)bin
>  
> -abstract_socket := /org/kernel/linux/storage/multipathd
> +abstract_socket := @/org/kernel/linux/storage/multipathd
>  
>  ifeq ($(V),)
>  Q		:= @
> @@ -117,7 +117,9 @@ CPPFLAGS	:= $(FORTIFY_OPT) $(CPPFLAGS) $(D_URCU_VERSION) \
>  		   -DRUNTIME_DIR=\"$(runtimedir)\" -DCONFIG_DIR=\"$(TGTDIR)$(configdir)\" \
>  		   -DDEFAULT_CONFIGFILE=\"$(TGTDIR)$(configfile)\" -DSTATE_DIR=\"$(TGTDIR)$(statedir)\" \
>  		   -DEXTRAVERSION=\"$(EXTRAVERSION)\" \
> -		   -DDEFAULT_SOCKET=\"$(abstract_socket)\" -MMD -MP
> +		   -DDEFAULT_SOCKET=\"$(abstract_socket)\" \
> +		   -DWSTRINGOP_TRUNCATION=$(if $(WSTRINGOP_TRUNCATION),1,0) \
> +		   -MMD -MP
>  CFLAGS		:= -std=$(C_STD) $(CFLAGS) $(OPTFLAGS) $(WARNFLAGS) -pipe \
>  		   -fexceptions
>  BIN_CFLAGS	:= -fPIE -DPIE
> @@ -170,4 +172,5 @@ NV_VERSION_SCRIPT = $(DEVLIB:%.so=%-nv.version)
>  		-e 's:@SYSDIR_BIN@:'$(sysdir_bin)': g' \
>  		-e 's:@RUNTIME_DIR@:'$(runtimedir)':g' \
>  		-e 's/@MODPROBE_UNIT@/'$(MODPROBE_UNIT)'/g' \
> +		-e 's,@MPATH_SOCKET@,'$(abstract_socket)',g' \
>  		$< >$@
> diff --git a/create-config.mk b/create-config.mk
> index ab163ed..f05fdab 100644
> --- a/create-config.mk
> +++ b/create-config.mk
> @@ -191,6 +191,7 @@ $(TOPDIR)/config.mk:	$(multipathdir)/autoconfig.h
>  	@echo "ERROR_DISCARDED_QUALIFIERS := $(call TEST_CC_OPTION,-Werror=discarded-qualifiers,)" >>$@
>  	@echo "WNOCLOBBERED := $(call TEST_CC_OPTION,-Wno-clobbered -Wno-error=clobbered,)" >>$@
>  	@echo "WFORMATOVERFLOW := $(call TEST_CC_OPTION,-Wformat-overflow=2,)" >>$@
> +	@echo "WSTRINGOP_TRUNCATION := $(call TEST_CC_OPTION,-Wstringop-truncation)" >>$@
>  	@echo "W_MISSING_INITIALIZERS := $(call TEST_MISSING_INITIALIZERS)" >>$@
>  	@echo "W_URCU_TYPE_LIMITS := $(call TEST_URCU_TYPE_LIMITS)" >>$@
>  	@echo "ENABLE_LIBDMMP := $(ENABLE_LIBDMMP)" >>$@
> diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> index a38e8b6..5a39471 100644
> --- a/libmpathcmd/mpath_cmd.c
> +++ b/libmpathcmd/mpath_cmd.c
> @@ -24,11 +24,13 @@
>  #include <sys/socket.h>
>  #include <sys/un.h>
>  #include <poll.h>
> +#include <stddef.h>
>  #include <string.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  
>  #include "mpath_cmd.h"
> +#include "mpath_fill_sockaddr.c"
>  
>  /*
>   * keep reading until its all read
> @@ -101,14 +103,6 @@ int mpath_connect__(int nonblocking)
>  	struct sockaddr_un addr;
>  	int flags = 0;
>  
> -	memset(&addr, 0, sizeof(addr));
> -	addr.sun_family = AF_LOCAL;
> -	addr.sun_path[0] = '\0';
> -	strncpy(&addr.sun_path[1], DEFAULT_SOCKET, sizeof(addr.sun_path) - 1);
> -	len = strlen(DEFAULT_SOCKET) + 1 + sizeof(sa_family_t);
> -	if (len > sizeof(struct sockaddr_un))
> -		len = sizeof(struct sockaddr_un);
> -
>  	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
>  	if (fd == -1)
>  		return -1;
> @@ -119,6 +113,7 @@ int mpath_connect__(int nonblocking)
>  			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
>  	}
>  
> +	len = mpath_fill_sockaddr__(&addr, DEFAULT_SOCKET);
>  	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
>  		int err = errno;
>  
> diff --git a/libmpathcmd/mpath_fill_sockaddr.c b/libmpathcmd/mpath_fill_sockaddr.c
> new file mode 100644
> index 0000000..750ef3e
> --- /dev/null
> +++ b/libmpathcmd/mpath_fill_sockaddr.c
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright (c) 2025 SUSE LLC
> + * SPDX-license-identifier: LGPL-2.1-or-newer
> + */
> +
> +static size_t mpath_fill_sockaddr__(struct sockaddr_un *addr, const char *name)
> +{
> +	size_t len;
> +
> +	addr->sun_family = AF_LOCAL;
> +
> +	if (name[0] != '@') {
> +		/* Pathname socket. This should be NULL-terminated. */
> +		strncpy(&addr->sun_path[0], name, sizeof(addr->sun_path) - 1);
> +		addr->sun_path[sizeof(addr->sun_path) - 1] = '\0';
> +		len = offsetof(struct sockaddr_un, sun_path) + strlen(name) + 1;
> +	} else {
> +		addr->sun_path[0] = '\0';
> +		/*

Nitpick: These comment lines are needlessly longer than 80 characters,
but otherwise, everything's fine.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> +		 * The abstract socket's name doesn't need to be NULL terminated.
> +		 * Actually, a trailing NULL would be considered part of the socket name.
> +		 */
> +#pragma GCC diagnostic push
> +#if WSTRINGOP_TRUNCATION
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> +#endif
> +		strncpy(&addr->sun_path[1], &name[1], sizeof(addr->sun_path) - 1);
> +#pragma GCC diagnostic pop
> +		len = offsetof(struct sockaddr_un, sun_path) + strlen(name);
> +	}
> +	return len > sizeof(*addr) ? sizeof(*addr) : len;
> +}
> diff --git a/libmpathutil/uxsock.c b/libmpathutil/uxsock.c
> index 2135476..12c4608 100644
> --- a/libmpathutil/uxsock.c
> +++ b/libmpathutil/uxsock.c
> @@ -6,12 +6,15 @@
>   */
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <unistd.h>
>  #include <stdarg.h>
> +#include <stddef.h>
>  #include <fcntl.h>
>  #include <sys/ioctl.h>
>  #include <sys/types.h>
>  #include <sys/socket.h>
> +#include <sys/stat.h>
>  #include <sys/un.h>
>  #include <poll.h>
>  #include <signal.h>
> @@ -33,6 +36,8 @@
>  static int _recv_packet(int fd, char **buf, unsigned int timeout,
>  			ssize_t limit);
>  
> +#include "../libmpathcmd/mpath_fill_sockaddr.c"
> +
>  /*
>   * create a unix domain socket and start listening on it
>   * return a file descriptor open on the socket
> @@ -63,15 +68,7 @@ int ux_socket_listen(const char *name)
>  		return -1;
>  	}
>  
> -	memset(&addr, 0, sizeof(addr));
> -	addr.sun_family = AF_LOCAL;
> -	addr.sun_path[0] = '\0';
> -	len = strlen(name) + 1;
> -	if (len >= sizeof(addr.sun_path))
> -		len = sizeof(addr.sun_path) - 1;
> -	memcpy(&addr.sun_path[1], name, len);
> -
> -	len += sizeof(sa_family_t);
> +	len = mpath_fill_sockaddr__(&addr, name);
>  	if (bind(fd, (struct sockaddr *)&addr, len) == -1) {
>  		condlog(3, "Couldn't bind to ux_socket, error %d", errno);
>  		close(fd);
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 997b40c..61cf1af 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -41,7 +41,7 @@ ifeq ($(FPIN_SUPPORT),1)
>  OBJS += fpin_handlers.o
>  endif
>  
> -all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
> +all : $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service $(EXEC).socket
>  
>  $(EXEC): $(OBJS) $(multipathdir)/libmultipath.so $(mpathcmddir)/libmpathcmd.so
>  	@echo building $@ because of $?
> @@ -78,7 +78,7 @@ uninstall:
>  	$(Q)$(RM) $(DESTDIR)$(unitdir)/$(EXEC).socket
>  
>  clean: dep_clean
> -	$(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service
> +	$(Q)$(RM) core *.o $(EXEC) $(CLI) $(MANPAGES) $(EXEC).service $(EXEC).socket
>  
>  include $(wildcard $(OBJS:.o=.d) $(CLI_OBJS:.o=.d))
>  
> diff --git a/multipathd/multipathd.socket b/multipathd/multipathd.socket.in
> similarity index 87%
> rename from multipathd/multipathd.socket
> rename to multipathd/multipathd.socket.in
> index 6a62f5f..c0e86c3 100644
> --- a/multipathd/multipathd.socket
> +++ b/multipathd/multipathd.socket.in
> @@ -7,7 +7,7 @@ ConditionVirtualization=!container
>  Before=sockets.target
>  
>  [Socket]
> -ListenStream=@/org/kernel/linux/storage/multipathd
> +ListenStream=@MPATH_SOCKET@
>  
>  [Install]
>  # Socket activation for multipathd is disabled by default.
> -- 
> 2.48.1

[PATCH v3 03/10] libmpathutil: add support for Unix pathname sockets

[Martin Wilck]

Pathname sockets need to be world read/writable in order to allow regular
users to read information from multipathd. Our SO_PEERCRED permission check
will make sure that they can't make configuration changes. Also, SO_REUSEADDR
doesn't work for pathname sockets as it does for abstract Unix sockets. A
possibly pre-existing socket file must be removed before trying to recreate it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/uxsock.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/libmpathutil/uxsock.c b/libmpathutil/uxsock.c
index 12c4608..889d7a1 100644
--- a/libmpathutil/uxsock.c
+++ b/libmpathutil/uxsock.c
@@ -62,6 +62,11 @@ int ux_socket_listen(const char *name)
 		return fd;
 	}
 #endif
+
+	/* This is after the PID check, so unlinking should be fine */
+	if (name[0] != '@' && unlink(name) == -1 && errno != ENOENT)
+		condlog(1, "Failed to unlink %s", name);
+
 	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
 	if (fd == -1) {
 		condlog(3, "Couldn't create ux_socket, error %d", errno);
@@ -75,6 +80,14 @@ int ux_socket_listen(const char *name)
 		return -1;
 	}
 
+	/*
+	 * Socket needs to have rw permissions for everone.
+	 * SO_PEERCRED makes sure that only root can modify things.
+	 */
+	if (name[0] != '@' &&
+	    chmod(name, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) == -1)
+		condlog(3, "failed to set permissions on %s: %s", name, strerror(errno));
+
 	if (listen(fd, 10) == -1) {
 		condlog(3, "Couldn't listen to ux_socket, error %d", errno);
 		close(fd);
-- 
2.48.1

[PATCH v3 04/10] libmpathutil: move systemd_listen_fds() support into multipathd

[Martin Wilck]

This feature is only used by multipathd.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/uxsock.c | 15 ---------------
 multipathd/main.c     | 28 +++++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/libmpathutil/uxsock.c b/libmpathutil/uxsock.c
index 889d7a1..59c4717 100644
--- a/libmpathutil/uxsock.c
+++ b/libmpathutil/uxsock.c
@@ -46,23 +46,8 @@ int ux_socket_listen(const char *name)
 {
 	int fd;
 	size_t len;
-#ifdef USE_SYSTEMD
-	int num;
-#endif
 	struct sockaddr_un addr;
 
-#ifdef USE_SYSTEMD
-	num = sd_listen_fds(0);
-	if (num > 1) {
-		condlog(3, "sd_listen_fds returned %d fds", num);
-		return -1;
-	} else if (num == 1) {
-		fd = SD_LISTEN_FDS_START + 0;
-		condlog(3, "using fd %d from sd_listen_fds", fd);
-		return fd;
-	}
-#endif
-
 	/* This is after the PID check, so unlinking should be fine */
 	if (name[0] != '@' && unlink(name) == -1 && errno != ENOENT)
 		condlog(1, "Failed to unlink %s", name);
diff --git a/multipathd/main.c b/multipathd/main.c
index 52630f7..5455bdf 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1863,15 +1863,41 @@ uevqloop (void * ap)
 	pthread_cleanup_pop(1);
 	return NULL;
 }
+
+#ifdef USE_SYSTEMD
+static int get_systemd_sockets(long *ux_sock)
+{
+	int num = sd_listen_fds(0);
+
+	if (num > 1) {
+		condlog(3, "sd_listen_fds returned %d fds", num);
+		return -1;
+	} else if (num == 1) {
+		ux_sock[0] = SD_LISTEN_FDS_START + 0;
+		condlog(3, "using fd %ld from sd_listen_fds", ux_sock[0]);
+	}
+	return num;
+}
+#else
+static int get_systemd_sockets(long *ux_sock __attribute__((unused)))
+{
+	return 0;
+}
+#endif
+
+
 static void *
 uxlsnrloop (void * ap)
 {
 	long ux_sock;
+	int num;
 
 	pthread_cleanup_push(rcu_unregister, NULL);
 	rcu_register_thread();
 
-	ux_sock = ux_socket_listen(DEFAULT_SOCKET);
+	num = get_systemd_sockets(&ux_sock);
+	if (num < 1)
+		ux_sock = ux_socket_listen(DEFAULT_SOCKET);
 	if (ux_sock == -1) {
 		condlog(1, "could not create uxsock: %d", errno);
 		exit_daemon();
-- 
2.48.1

Re: [PATCH v3 04/10] libmpathutil: move systemd_listen_fds() support into multipathd

[Benjamin Marzinski]

On Fri, Feb 14, 2025 at 11:10:05PM +0100, Martin Wilck wrote:
> This feature is only used by multipathd.

We should probably move all of ux_socket_listen() to multipathd, since
it's only used there. But I see why you split out this code, and moving
ux_socket_listen() can happen whenever.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmpathutil/uxsock.c | 15 ---------------
>  multipathd/main.c     | 28 +++++++++++++++++++++++++++-
>  2 files changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/libmpathutil/uxsock.c b/libmpathutil/uxsock.c
> index 889d7a1..59c4717 100644
> --- a/libmpathutil/uxsock.c
> +++ b/libmpathutil/uxsock.c
> @@ -46,23 +46,8 @@ int ux_socket_listen(const char *name)
>  {
>  	int fd;
>  	size_t len;
> -#ifdef USE_SYSTEMD
> -	int num;
> -#endif
>  	struct sockaddr_un addr;
>  
> -#ifdef USE_SYSTEMD
> -	num = sd_listen_fds(0);
> -	if (num > 1) {
> -		condlog(3, "sd_listen_fds returned %d fds", num);
> -		return -1;
> -	} else if (num == 1) {
> -		fd = SD_LISTEN_FDS_START + 0;
> -		condlog(3, "using fd %d from sd_listen_fds", fd);
> -		return fd;
> -	}
> -#endif
> -
>  	/* This is after the PID check, so unlinking should be fine */
>  	if (name[0] != '@' && unlink(name) == -1 && errno != ENOENT)
>  		condlog(1, "Failed to unlink %s", name);
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 52630f7..5455bdf 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1863,15 +1863,41 @@ uevqloop (void * ap)
>  	pthread_cleanup_pop(1);
>  	return NULL;
>  }
> +
> +#ifdef USE_SYSTEMD
> +static int get_systemd_sockets(long *ux_sock)
> +{
> +	int num = sd_listen_fds(0);
> +
> +	if (num > 1) {
> +		condlog(3, "sd_listen_fds returned %d fds", num);
> +		return -1;
> +	} else if (num == 1) {
> +		ux_sock[0] = SD_LISTEN_FDS_START + 0;
> +		condlog(3, "using fd %ld from sd_listen_fds", ux_sock[0]);
> +	}
> +	return num;
> +}
> +#else
> +static int get_systemd_sockets(long *ux_sock __attribute__((unused)))
> +{
> +	return 0;
> +}
> +#endif
> +
> +
>  static void *
>  uxlsnrloop (void * ap)
>  {
>  	long ux_sock;
> +	int num;
>  
>  	pthread_cleanup_push(rcu_unregister, NULL);
>  	rcu_register_thread();
>  
> -	ux_sock = ux_socket_listen(DEFAULT_SOCKET);
> +	num = get_systemd_sockets(&ux_sock);
> +	if (num < 1)
> +		ux_sock = ux_socket_listen(DEFAULT_SOCKET);
>  	if (ux_sock == -1) {
>  		condlog(1, "could not create uxsock: %d", errno);
>  		exit_daemon();
> -- 
> 2.48.1

[PATCH v3 05/10] multipathd: make uxsock_listen() take a pointer to fd

[Martin Wilck]

This prepares being able to pass multiple socket fds.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c   |  2 +-
 multipathd/uxlsnr.c | 11 ++++++++---
 multipathd/uxlsnr.h |  3 +--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 5455bdf..602215c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1924,7 +1924,7 @@ uxlsnrloop (void * ap)
 	       == DAEMON_CONFIGURE)
 		handle_signals(false);
 
-	uxsock_listen(ux_sock, ap);
+	uxsock_listen(1, &ux_sock, ap);
 
 out_sock:
 	pthread_cleanup_pop(1); /* uxsock_cleanup */
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index d74bb35..79e28e4 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -614,7 +614,7 @@ static void handle_client(struct client *c, struct vectors *vecs, short revents)
 /*
  * entry point
  */
-void *uxsock_listen(long ux_sock, void *trigger_data)
+void *uxsock_listen(int n_socks, long *ux_sock, void *trigger_data)
 {
 	sigset_t mask;
 	int max_pfds = MIN_POLLS + POLLFDS_BASE;
@@ -623,6 +623,11 @@ void *uxsock_listen(long ux_sock, void *trigger_data)
 	struct watch_descriptors wds = { .conf_wd = -1, .dir_wd = -1, .mp_wd = -1, };
 	struct vectors *vecs = trigger_data;
 
+	if (n_socks != 1) {
+		condlog(0, "uxsock: no socket fds");
+		exit_daemon();
+		return NULL;
+	}
 	condlog(3, "uxsock: startup listener");
 	polls = calloc(1, max_pfds * sizeof(*polls));
 	if (!polls) {
@@ -673,7 +678,7 @@ void *uxsock_listen(long ux_sock, void *trigger_data)
 			}
 		}
 		if (num_clients < MAX_CLIENTS) {
-			polls[POLLFD_UX].fd = ux_sock;
+			polls[POLLFD_UX].fd = ux_sock[0];
 			polls[POLLFD_UX].events = POLLIN;
 		} else {
 			/*
@@ -767,7 +772,7 @@ void *uxsock_listen(long ux_sock, void *trigger_data)
 
 		/* see if we got a new client */
 		if (polls[POLLFD_UX].revents & POLLIN) {
-			new_client(ux_sock);
+			new_client(ux_sock[0]);
 		}
 
 		/* handle inotify events on config files */
diff --git a/multipathd/uxlsnr.h b/multipathd/uxlsnr.h
index a14a667..a331e23 100644
--- a/multipathd/uxlsnr.h
+++ b/multipathd/uxlsnr.h
@@ -5,7 +5,6 @@
 
 bool waiting_clients(void);
 void uxsock_cleanup(void *arg);
-void *uxsock_listen(long ux_sock,
-		    void * trigger_data);
+void *uxsock_listen(int n_socks, long *ux_sock, void *trigger_data);
 
 #endif
-- 
2.48.1

[PATCH v3 06/10] multipathd: allow receiving two socket fds from systemd

[Martin Wilck]

Add another ListenStream directive in multipathd.socket for a Unix pathname
socket. In multipathd, read both socket fds from systemd, and open both
when they are defined.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile.inc                    |  4 +++-
 multipathd/main.c               | 18 ++++++++++++------
 multipathd/multipathd.socket.in |  3 ++-
 multipathd/uxlsnr.c             | 33 ++++++++++++++++++++++-----------
 4 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index 4015006..d0fecc3 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -80,6 +80,7 @@ kernel_incdir	:= /usr/include
 sysdir_bin      := $(sys_execprefix)bin
 
 abstract_socket := @/org/kernel/linux/storage/multipathd
+pathname_socket := /run/multipathd.socket
 
 ifeq ($(V),)
 Q		:= @
@@ -172,5 +173,6 @@ NV_VERSION_SCRIPT = $(DEVLIB:%.so=%-nv.version)
 		-e 's:@SYSDIR_BIN@:'$(sysdir_bin)': g' \
 		-e 's:@RUNTIME_DIR@:'$(runtimedir)':g' \
 		-e 's/@MODPROBE_UNIT@/'$(MODPROBE_UNIT)'/g' \
-		-e 's,@MPATH_SOCKET@,'$(abstract_socket)',g' \
+		-e 's,@ABSTRACT_SOCKET@,'$(abstract_socket)',g' \
+		-e 's,@PATHNAME_SOCKET@,'$(pathname_socket)',g' \
 		$< >$@
diff --git a/multipathd/main.c b/multipathd/main.c
index 602215c..ac204b2 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1869,9 +1869,13 @@ static int get_systemd_sockets(long *ux_sock)
 {
 	int num = sd_listen_fds(0);
 
-	if (num > 1) {
+	if (num > 2) {
 		condlog(3, "sd_listen_fds returned %d fds", num);
 		return -1;
+	} else if (num == 2) {
+		ux_sock[0] = SD_LISTEN_FDS_START + 0;
+		ux_sock[1] = SD_LISTEN_FDS_START + 1;
+		condlog(3, "using fd %ld and %ld from sd_listen_fds", ux_sock[0], ux_sock[1]);
 	} else if (num == 1) {
 		ux_sock[0] = SD_LISTEN_FDS_START + 0;
 		condlog(3, "using fd %ld from sd_listen_fds", ux_sock[0]);
@@ -1889,16 +1893,18 @@ static int get_systemd_sockets(long *ux_sock __attribute__((unused)))
 static void *
 uxlsnrloop (void * ap)
 {
-	long ux_sock;
+	long ux_sock[2] = {-1, -1};
 	int num;
 
 	pthread_cleanup_push(rcu_unregister, NULL);
 	rcu_register_thread();
 
 	num = get_systemd_sockets(&ux_sock);
-	if (num < 1)
-		ux_sock = ux_socket_listen(DEFAULT_SOCKET);
-	if (ux_sock == -1) {
+	if (num < 1) {
+		ux_sock[0] = ux_socket_listen(DEFAULT_SOCKET);
+		num = 1;
+	}
+	if (ux_sock[0] == -1) {
 		condlog(1, "could not create uxsock: %d", errno);
 		exit_daemon();
 		goto out;
@@ -1924,7 +1930,7 @@ uxlsnrloop (void * ap)
 	       == DAEMON_CONFIGURE)
 		handle_signals(false);
 
-	uxsock_listen(1, &ux_sock, ap);
+	uxsock_listen(num, ux_sock, ap);
 
 out_sock:
 	pthread_cleanup_pop(1); /* uxsock_cleanup */
diff --git a/multipathd/multipathd.socket.in b/multipathd/multipathd.socket.in
index c0e86c3..11002fc 100644
--- a/multipathd/multipathd.socket.in
+++ b/multipathd/multipathd.socket.in
@@ -7,7 +7,8 @@ ConditionVirtualization=!container
 Before=sockets.target
 
 [Socket]
-ListenStream=@MPATH_SOCKET@
+ListenStream=@ABSTRACT_SOCKET@
+ListenStream=@PATHNAME_SOCKET@
 
 [Install]
 # Socket activation for multipathd is disabled by default.
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index 79e28e4..aa6aae7 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -69,7 +69,8 @@ struct client {
 
 /* Indices for array of poll fds */
 enum {
-	POLLFD_UX = 0,
+	POLLFD_UX1 = 0,
+	POLLFD_UX2,
 	POLLFD_NOTIFY,
 	POLLFD_IDLE,
 	POLLFDS_BASE,
@@ -164,9 +165,10 @@ void uxsock_cleanup(void *arg)
 {
 	struct client *client_loop;
 	struct client *client_tmp;
-	long ux_sock = (long)arg;
+	long *ux_sock = (long *)arg;
 
-	close(ux_sock);
+	close(ux_sock[0]);
+	close(ux_sock[1]);
 	close(notify_fd);
 
 	list_for_each_entry_safe(client_loop, client_tmp, &clients, node) {
@@ -614,20 +616,24 @@ static void handle_client(struct client *c, struct vectors *vecs, short revents)
 /*
  * entry point
  */
-void *uxsock_listen(int n_socks, long *ux_sock, void *trigger_data)
+void *uxsock_listen(int n_socks, long *ux_sock_in, void *trigger_data)
 {
 	sigset_t mask;
 	int max_pfds = MIN_POLLS + POLLFDS_BASE;
+	long ux_sock[2] = {-1, -1};
 	/* conf->sequence_nr will be 1 when uxsock_listen is first called */
 	unsigned int sequence_nr = 0;
 	struct watch_descriptors wds = { .conf_wd = -1, .dir_wd = -1, .mp_wd = -1, };
 	struct vectors *vecs = trigger_data;
 
-	if (n_socks != 1) {
-		condlog(0, "uxsock: no socket fds");
+	if (n_socks < 1 || n_socks > 2) {
+		condlog(0, "uxsock: unsupported number of socket fds");
 		exit_daemon();
 		return NULL;
-	}
+	} else if (n_socks == 2)
+		ux_sock[1] = ux_sock_in[1];
+	ux_sock[0] = ux_sock_in[0];
+
 	condlog(3, "uxsock: startup listener");
 	polls = calloc(1, max_pfds * sizeof(*polls));
 	if (!polls) {
@@ -678,8 +684,10 @@ void *uxsock_listen(int n_socks, long *ux_sock, void *trigger_data)
 			}
 		}
 		if (num_clients < MAX_CLIENTS) {
-			polls[POLLFD_UX].fd = ux_sock[0];
-			polls[POLLFD_UX].events = POLLIN;
+			polls[POLLFD_UX1].fd = ux_sock[0];
+			polls[POLLFD_UX1].events = POLLIN;
+			polls[POLLFD_UX2].fd = ux_sock[1];
+			polls[POLLFD_UX2].events = POLLIN;
 		} else {
 			/*
 			 * New clients can't connect, num_clients won't grow
@@ -687,7 +695,7 @@ void *uxsock_listen(int n_socks, long *ux_sock, void *trigger_data)
 			 */
 			condlog(1, "%s: max client connections reached, pausing polling",
 				__func__);
-			polls[POLLFD_UX].fd = -1;
+			polls[POLLFD_UX1].fd = polls[POLLFD_UX2].fd = -1;
 		}
 
 		reset_watch(notify_fd, &wds, &sequence_nr);
@@ -771,9 +779,12 @@ void *uxsock_listen(int n_socks, long *ux_sock, void *trigger_data)
 		handle_signals(true);
 
 		/* see if we got a new client */
-		if (polls[POLLFD_UX].revents & POLLIN) {
+		if (polls[POLLFD_UX1].revents & POLLIN) {
 			new_client(ux_sock[0]);
 		}
+		if (polls[POLLFD_UX2].revents & POLLIN) {
+			new_client(ux_sock[1]);
+		}
 
 		/* handle inotify events on config files */
 		if (polls[POLLFD_NOTIFY].revents & POLLIN)
-- 
2.48.1

[PATCH v3 07/10] multipathd: listen on pathname and abstract socket by default

[Martin Wilck]

Pass both ABSTRACT_SOCKET and PATHNAME_SOCKET to the compiler at
build time, and listen on both sockets by default.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile.inc            |  2 +-
 libmpathcmd/mpath_cmd.c |  2 +-
 multipathd/main.c       | 11 ++++++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index d0fecc3..9e3dc46 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -118,7 +118,7 @@ CPPFLAGS	:= $(FORTIFY_OPT) $(CPPFLAGS) $(D_URCU_VERSION) \
 		   -DRUNTIME_DIR=\"$(runtimedir)\" -DCONFIG_DIR=\"$(TGTDIR)$(configdir)\" \
 		   -DDEFAULT_CONFIGFILE=\"$(TGTDIR)$(configfile)\" -DSTATE_DIR=\"$(TGTDIR)$(statedir)\" \
 		   -DEXTRAVERSION=\"$(EXTRAVERSION)\" \
-		   -DDEFAULT_SOCKET=\"$(abstract_socket)\" \
+		   -DABSTRACT_SOCKET=\"$(abstract_socket)\" -DPATHNAME_SOCKET=\"$(pathname_socket)\" \
 		   -DWSTRINGOP_TRUNCATION=$(if $(WSTRINGOP_TRUNCATION),1,0) \
 		   -MMD -MP
 CFLAGS		:= -std=$(C_STD) $(CFLAGS) $(OPTFLAGS) $(WARNFLAGS) -pipe \
diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index 5a39471..c7cf954 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -113,7 +113,7 @@ int mpath_connect__(int nonblocking)
 			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
 	}
 
-	len = mpath_fill_sockaddr__(&addr, DEFAULT_SOCKET);
+	len = mpath_fill_sockaddr__(&addr, ABSTRACT_SOCKET);
 	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
 		int err = errno;
 
diff --git a/multipathd/main.c b/multipathd/main.c
index ac204b2..b41c181 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1899,13 +1899,14 @@ uxlsnrloop (void * ap)
 	pthread_cleanup_push(rcu_unregister, NULL);
 	rcu_register_thread();
 
-	num = get_systemd_sockets(&ux_sock);
+	num = get_systemd_sockets(ux_sock);
 	if (num < 1) {
-		ux_sock[0] = ux_socket_listen(DEFAULT_SOCKET);
-		num = 1;
+		ux_sock[0] = ux_socket_listen(ABSTRACT_SOCKET);
+		ux_sock[1] = ux_socket_listen(PATHNAME_SOCKET);
+		num = 2;
 	}
-	if (ux_sock[0] == -1) {
-		condlog(1, "could not create uxsock: %d", errno);
+	if (ux_sock[0] == -1 && ux_sock[1] == -1) {
+		condlog(1, "could not create sockets: %d", errno);
 		exit_daemon();
 		goto out;
 	}
-- 
2.48.1

[PATCH v3 08/10] libmpathcmd: try both abstract and pathname sockets

[Martin Wilck]

When connecting to the multipathd socket, try the pathname socket
first, then the abstract socket. Fail only if both connection attempts
fail.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathcmd/mpath_cmd.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index c7cf954..ba5bb31 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -102,7 +102,10 @@ int mpath_connect__(int nonblocking)
 	size_t len;
 	struct sockaddr_un addr;
 	int flags = 0;
+	const char *names[2] = {PATHNAME_SOCKET, ABSTRACT_SOCKET};
+	int name_idx = 0;
 
+retry:
 	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
 	if (fd == -1)
 		return -1;
@@ -113,13 +116,17 @@ int mpath_connect__(int nonblocking)
 			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
 	}
 
-	len = mpath_fill_sockaddr__(&addr, ABSTRACT_SOCKET);
+	len = mpath_fill_sockaddr__(&addr, names[name_idx]);
 	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
 		int err = errno;
 
 		close(fd);
-		errno = err;
-		return -1;
+		if (err == ECONNREFUSED && ++name_idx == 1)
+			goto retry;
+		else {
+			errno = err;
+			return -1;
+		}
 	}
 
 	if (nonblocking && flags != -1)
-- 
2.48.1

Re: [PATCH v3 08/10] libmpathcmd: try both abstract and pathname sockets

[Benjamin Marzinski]

On Fri, Feb 14, 2025 at 11:10:09PM +0100, Martin Wilck wrote:
> When connecting to the multipathd socket, try the pathname socket
> first, then the abstract socket. Fail only if both connection attempts
> fail.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmpathcmd/mpath_cmd.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> index c7cf954..ba5bb31 100644
> --- a/libmpathcmd/mpath_cmd.c
> +++ b/libmpathcmd/mpath_cmd.c
> @@ -102,7 +102,10 @@ int mpath_connect__(int nonblocking)
>  	size_t len;
>  	struct sockaddr_un addr;
>  	int flags = 0;
> +	const char *names[2] = {PATHNAME_SOCKET, ABSTRACT_SOCKET};
> +	int name_idx = 0;
>  
> +retry:
>  	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
>  	if (fd == -1)
>  		return -1;
> @@ -113,13 +116,17 @@ int mpath_connect__(int nonblocking)
>  			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
>  	}
>  
> -	len = mpath_fill_sockaddr__(&addr, ABSTRACT_SOCKET);
> +	len = mpath_fill_sockaddr__(&addr, names[name_idx]);
>  	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
>  		int err = errno;
>  
>  		close(fd);
> -		errno = err;
> -		return -1;
> +		if (err == ECONNREFUSED && ++name_idx == 1)

Most of the connect() return codes are things that could presumably be
fixed by trying a different address (not the errors related to a problem
with sockfd, but we just created the socket, so those seem pretty
impossible). Is there a reason why we don't just retry on any error?

-Ben

> +			goto retry;
> +		else {
> +			errno = err;
> +			return -1;
> +		}
>  	}
>  
>  	if (nonblocking && flags != -1)
> -- 
> 2.48.1

Re: [PATCH v3 08/10] libmpathcmd: try both abstract and pathname sockets

[Martin Wilck]

On Mon, 2025-02-17 at 17:57 -0500, Benjamin Marzinski wrote:
> On Fri, Feb 14, 2025 at 11:10:09PM +0100, Martin Wilck wrote:
> > When connecting to the multipathd socket, try the pathname socket
> > first, then the abstract socket. Fail only if both connection
> > attempts
> > fail.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  libmpathcmd/mpath_cmd.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
> > index c7cf954..ba5bb31 100644
> > --- a/libmpathcmd/mpath_cmd.c
> > +++ b/libmpathcmd/mpath_cmd.c
> > @@ -102,7 +102,10 @@ int mpath_connect__(int nonblocking)
> >  	size_t len;
> >  	struct sockaddr_un addr;
> >  	int flags = 0;
> > +	const char *names[2] = {PATHNAME_SOCKET, ABSTRACT_SOCKET};
> > +	int name_idx = 0;
> >  
> > +retry:
> >  	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> >  	if (fd == -1)
> >  		return -1;
> > @@ -113,13 +116,17 @@ int mpath_connect__(int nonblocking)
> >  			(void)fcntl(fd, F_SETFL,
> > flags|O_NONBLOCK);
> >  	}
> >  
> > -	len = mpath_fill_sockaddr__(&addr, ABSTRACT_SOCKET);
> > +	len = mpath_fill_sockaddr__(&addr, names[name_idx]);
> >  	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
> >  		int err = errno;
> >  
> >  		close(fd);
> > -		errno = err;
> > -		return -1;
> > +		if (err == ECONNREFUSED && ++name_idx == 1)
> 
> Most of the connect() return codes are things that could presumably
> be
> fixed by trying a different address (not the errors related to a
> problem
> with sockfd, but we just created the socket, so those seem pretty
> impossible). Is there a reason why we don't just retry on any error?

I've done some testing and when I was using a wrong address name, I 
always got ECONNREFUSED. So I thought it might be cleaner to not
blindly assume retrying might help for other error codes. But I can see
your point, it's just a single retry and can't really hurt to just try
the other socket name.

I'll remove that condition.

Regards
Martin

[PATCH v3 09/10] libmpathcmd: honor MULTIPATH_SOCKET_NAME environment variable

[Martin Wilck]

In systemd installments, users can already override the socket names
that systemd listens on. With this patch, clients using libmpathcmd
can be customized to use a non-standard socket by setting an environment
variable.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathcmd/mpath_cmd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index ba5bb31..405267f 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -20,6 +20,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -104,6 +105,7 @@ int mpath_connect__(int nonblocking)
 	int flags = 0;
 	const char *names[2] = {PATHNAME_SOCKET, ABSTRACT_SOCKET};
 	int name_idx = 0;
+	const char *env_name = getenv("MULTIPATH_SOCKET_NAME"), *name;
 
 retry:
 	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
@@ -116,12 +118,13 @@ retry:
 			(void)fcntl(fd, F_SETFL, flags|O_NONBLOCK);
 	}
 
-	len = mpath_fill_sockaddr__(&addr, names[name_idx]);
+	name = env_name ? env_name : names[name_idx];
+	len = mpath_fill_sockaddr__(&addr, name);
 	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
 		int err = errno;
 
 		close(fd);
-		if (err == ECONNREFUSED && ++name_idx == 1)
+		if (name != env_name && err == ECONNREFUSED && ++name_idx == 1)
 			goto retry;
 		else {
 			errno = err;
-- 
2.48.1

[PATCH v3 10/10] multipathd: honor MULTIPATH_SOCKET_NAME environment variable

[Martin Wilck]

If multipathd is started via socket activation, it will obtain
sockets from systemd. The names of these sockets, and whether
the abstract and / or pathname socket is created, is configurable
in the systemd unit file.

Add support for passing a socket name via the environment, so that
it's possible to configure the socket name at runtime even without
socket activation. In this case, only this single socket will be created.
If creating the socket fails, multipathd startup will fail, too.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/multipathd/main.c b/multipathd/main.c
index b41c181..3468b21 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1895,11 +1895,16 @@ uxlsnrloop (void * ap)
 {
 	long ux_sock[2] = {-1, -1};
 	int num;
+	const char *env_name = getenv("MULTIPATH_SOCKET_NAME");
 
 	pthread_cleanup_push(rcu_unregister, NULL);
 	rcu_register_thread();
 
 	num = get_systemd_sockets(ux_sock);
+	if (num < 1 && env_name != NULL) {
+		ux_sock[0] = ux_socket_listen(env_name);
+		num = 1;
+	}
 	if (num < 1) {
 		ux_sock[0] = ux_socket_listen(ABSTRACT_SOCKET);
 		ux_sock[1] = ux_socket_listen(PATHNAME_SOCKET);
-- 
2.48.1

Re: [PATCH v3 00/10] multipath-tools: provide pathname and abstract sockets

[Benjamin Marzinski]

On Fri, Feb 14, 2025 at 11:10:01PM +0100, Martin Wilck wrote:
> This set is an attempt to fix https://github.com/opensvc/multipath-tools/issues/111

For all patches except for 08, which I have a question about:

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> 
> Changes v2 -> v3:
> 
> Many, as a result of further discussions on GitHub. multipathd will now open
> both the abstract socket and the pathname socket /run/multipathd.socket by
> default, and client programs will try both sockets before giving up.  The
> "use_regular_socket=1" build parameter is not necessary any more.
> 
> Note that we'd switched from the regular socket "/var/run/multipath.sock" to
> an abstract socket in bb89077 ("multipathd: switch to abstract sockets for CLI
> commands") in multipath-tools 0.5.0. The rationale back then was that creating
> a pathname socket might fail if the directory structure isn't set up yet. As
> we now create both, and fail only if neither could be created, and as there is
> the early /run directory on modern systems, this argument shouldn't apply to
> this patch set. The name of the socket has been changed in order to indicate
> that the new approach is different from the old one.
> 
> Changes v1 -> v2:
> 
> - Introduced the mpath_fill_sockaddr() helper to cleanup struct sockaddr_un handling,
> in response to comments from Ben Marzinski on GitHub.
> - Fixed double remove of multipathd.socket during "make clean" (Ben).
> 
> Martin Wilck (10):
>   multipath-tools: move DEFAULT_SOCKET definition into Makefile.inc
>   multipath-tools: add helper mpath_fill_sockaddr__()
>   libmpathutil: add support for Unix pathname sockets
>   libmpathutil: move systemd_listen_fds() support into multipathd
>   multipathd: make uxsock_listen() take a pointer to fd
>   multipathd: allow receiving two socket fds from systemd
>   multipathd: listen on pathname and abstract socket by default
>   libmpathcmd: try both abstract and pathname sockets
>   libmpathcmd: honor MULTIPATH_SOCKET_NAME environment variable
>   multipathd: honor MULTIPATH_SOCKET_NAME environment variable
> 
>  .gitignore                                    |  1 +
>  Makefile.inc                                  | 10 +++-
>  create-config.mk                              |  1 +
>  libmpathcmd/mpath_cmd.c                       | 25 ++++++----
>  libmpathcmd/mpath_cmd.h                       |  1 -
>  libmpathcmd/mpath_fill_sockaddr.c             | 32 +++++++++++++
>  libmpathutil/uxsock.c                         | 41 +++++++---------
>  libmultipath/defaults.h                       |  1 -
>  multipathd/Makefile                           |  4 +-
>  multipathd/main.c                             | 48 +++++++++++++++++--
>  ...multipathd.socket => multipathd.socket.in} |  3 +-
>  multipathd/uxlsnr.c                           | 34 +++++++++----
>  multipathd/uxlsnr.h                           |  3 +-
>  13 files changed, 149 insertions(+), 55 deletions(-)
>  create mode 100644 libmpathcmd/mpath_fill_sockaddr.c
>  rename multipathd/{multipathd.socket => multipathd.socket.in} (84%)
> 
> -- 
> 2.48.1