[PATCH 1/2] multipath: don't start multipathd unnecessarily

[PATCH 1/2] multipath: don't start multipathd unnecessarily

[Martin Wilck]

When multipathd is off, but systemd is monitoring the multipathd socket,
mpath_connect() has the side effect of starting multipathd. That's
superfluous if we aren't going to pass the command at hand to multipathd.
So, only try to open the socket if the command at hand is a candidate
for delegation.

Note: Despite this, some day in the future, we may decide to delegate
almost everything to multipathd.

Fixes: 506d253b "multipath: delegate dangerous commands to multipathd"
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/multipath/main.c b/multipath/main.c
index ed4f40ef..a0f3bde0 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -803,10 +803,6 @@ int delegate_to_multipathd(enum mpath_cmds cmd, const char *dev,
 	char command[1024], *p, *reply = NULL;
 	int n, r = 0;
 
-	fd = mpath_connect();
-	if (fd == -1)
-		return 0;
-
 	p = command;
 	*p = '\0';
 	n = sizeof(command);
@@ -819,7 +815,12 @@ int delegate_to_multipathd(enum mpath_cmds cmd, const char *dev,
 	if (strlen(command) == 0)
 		/* No command found, no need to delegate */
 		return 0;
-	else if (p >= command + sizeof(command)) {
+
+	fd = mpath_connect();
+	if (fd == -1)
+		return 0;
+
+	if (p >= command + sizeof(command)) {
 		condlog(0, "internal error - command buffer overflow");
 		r = -1;
 		goto out;
-- 
2.17.1

[PATCH 2/2] multipath: fix return code handling in delegate_to_multipathd

[Martin Wilck]

Error exit was handled in delegate_to_multipathd() and success
exit after it returned. That's inconsistent. Fix it. Introduce
symbolic return values for better readability.

Fixes: 506d253b "multipath: delegate dangerous commands to multipathd"
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/main.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/multipath/main.c b/multipath/main.c
index a0f3bde0..fc5bf169 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -796,12 +796,18 @@ get_dev_type(char *dev) {
  *
  * It is safer to use equivalent multipathd client commands instead.
  */
+enum {
+	DELEGATE_OK = 0,
+	DELEGATE_ERROR = -1,
+	NOT_DELEGATED = 1,
+};
+
 int delegate_to_multipathd(enum mpath_cmds cmd, const char *dev,
 			   enum devtypes dev_type, const struct config *conf)
 {
 	int fd;
 	char command[1024], *p, *reply = NULL;
-	int n, r = 0;
+	int n, r = DELEGATE_ERROR;
 
 	p = command;
 	*p = '\0';
@@ -814,22 +820,21 @@ int delegate_to_multipathd(enum mpath_cmds cmd, const char *dev,
 
 	if (strlen(command) == 0)
 		/* No command found, no need to delegate */
-		return 0;
+		return NOT_DELEGATED;
 
 	fd = mpath_connect();
 	if (fd == -1)
-		return 0;
+		return NOT_DELEGATED;
 
 	if (p >= command + sizeof(command)) {
 		condlog(0, "internal error - command buffer overflow");
-		r = -1;
 		goto out;
 	}
 
 	condlog(3, "delegating command to multipathd");
-	r = mpath_process_cmd(fd, command, &reply, conf->uxsock_timeout);
 
-	if (r == -1) {
+	if (mpath_process_cmd(fd, command, &reply, conf->uxsock_timeout)
+	    == -1) {
 		condlog(1, "error in multipath command %s: %s",
 			command, strerror(errno));
 		goto out;
@@ -837,13 +842,11 @@ int delegate_to_multipathd(enum mpath_cmds cmd, const char *dev,
 
 	if (reply != NULL && *reply != '\0' && strcmp(reply, "ok\n"))
 		printf("%s", reply);
-	r = 1;
+	r = DELEGATE_OK;
 
 out:
 	FREE(reply);
 	close(fd);
-	if (r < 0)
-		exit(1);
 	return r;
 }
 
@@ -1050,8 +1053,14 @@ main (int argc, char *argv[])
 		goto out;
 	}
 
-	if (delegate_to_multipathd(cmd, dev, dev_type, conf))
+	switch(delegate_to_multipathd(cmd, dev, dev_type, conf)) {
+	case DELEGATE_OK:
 		exit(0);
+	case DELEGATE_ERROR:
+		exit(1);
+	case NOT_DELEGATED:
+		break;
+	}
 
 	if (cmd == CMD_RESET_WWIDS) {
 		struct multipath * mpp;
-- 
2.17.1