| Message ID | 20250214221011.136762-9-mwilck@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | multipath-tools: provide pathname and abstract sockets | expand |
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
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
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)
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(-)