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

[PATCH v4 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 v3 -> v4:

- 08/10: retry independently of error condition (Ben Marzinski)
- 08/10: add a const qualifier
- 09/10: adapt to 08/10

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 v4 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..83cb6ad 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 *const 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 (++name_idx == 1)
+			goto retry;
+		else {
+			errno = err;
+			return -1;
+		}
 	}
 
 	if (nonblocking && flags != -1)
-- 
2.48.1

[PATCH v4 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>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.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 83cb6ad..5a23236 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 *const 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 (++name_idx == 1)
+		if (name != env_name && +name_idx == 1)
 			goto retry;
 		else {
 			errno = err;
-- 
2.48.1

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

[Benjamin Marzinski]

On Fri, Feb 21, 2025 at 09:41:07PM +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.

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

> 
> 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..83cb6ad 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 *const 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 (++name_idx == 1)
> +			goto retry;
> +		else {
> +			errno = err;
> +			return -1;
> +		}
>  	}
>  
>  	if (nonblocking && flags != -1)
> -- 
> 2.48.1

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

[Benjamin Marzinski]

On Fri, Feb 21, 2025 at 09:41:08PM +0100, Martin Wilck wrote:
> 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>
> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.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 83cb6ad..5a23236 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 *const 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 (++name_idx == 1)

You lost a "+" from ++name_idx

Otherwise, looks good. Assuming that gets updated when you push it to
your queue branch:

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

> +		if (name != env_name && +name_idx == 1)
>  			goto retry;
>  		else {
>  			errno = err;
> -- 
> 2.48.1

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

[Martin Wilck]

On Mon, 2025-02-24 at 12:35 -0500, Benjamin Marzinski wrote:
> On Fri, Feb 21, 2025 at 09:41:08PM +0100, Martin Wilck wrote:
> > 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>
> > Reviewed-by: Benjamin Marzinski <bmarzins@redhat.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 83cb6ad..5a23236 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 *const 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 (++name_idx == 1)
> 
> You lost a "+" from ++name_idx
> 

Ouch, thanks a bunch for spotting that.

Martin