Merge pull request networkupstools#2393 from jimklimov/issue-2392

Implement an `INSTCMD driver.exit` as alternative to PID files and signals

NEWS.adoc

@@ -52,6 +52,10 @@ https://github.com/networkupstools/nut/milestone/11
    now also handle an existing PID file to interact with the earlier instance
    of the driver program, if still running (e.g. started manually). [#2384]
 
+ - Extended instant commands for driver reloading with a `driver.exit`
+   command for a protocol equivalent of sending a `SIGTERM`, e.g. when
+   a newer instance of the driver program tries to start. [#1903, #2392]
+
  - riello_ser updates:
    * added `localcalculation` option to compute `battery.runtime` and
      `battery.charge` if the device provides bogus values [issue #2390,

data/cmdvartab

@@ -194,6 +194,7 @@ VARDESC driver.version.usb "USB library version"
 # VARDESC driver.parameter.[[:alpha:]]+ "Driver parameter: <name>"
 # VARDESC driver.flag.[[:alpha:]]+ "Driver flag: <name>"
 
+CMDDESC driver.exit "Tell the driver daemon to just exit its program (so the caller or service management framework can restart it with new options)"
 CMDDESC driver.killpower "Tell the driver daemon to initiate UPS shutdown; should be unlocked with driver.flag.allow_killpower option or variable setting"
 CMDDESC driver.reload "Reload running driver configuration from the file system (only works for changes in some options)"
 CMDDESC driver.reload-or-error "Reload running driver configuration from the file system (only works for changes in some options); return an error if something changed and could not be applied live (so the caller can restart it with new options)"

docs/man/nutupsdrv.txt

@@ -114,6 +114,13 @@ are:
 	           which can not be applied "on the fly" (may fail for
 	           critical changes like run-time user/group accounts)
 /////////
+	*exit*;;   tell the currently running driver instance to just exit
+	           (so an external caller like the new driver instance, or
+	           the systemd or SMF frameworks would start another copy)
+
+With recent NUT releases, such commands can be sent using the Unix socket
+for driver-server interaction. As a fallback, like older releases, signals
+can be sent to the old driver instance's PID (where possible).
 
 *-P* 'pid'::
 Send the command signal above using specified PID number, rather than

docs/man/upsdrvctl.txt

@@ -142,6 +142,9 @@ are:
                    which can not be applied "on the fly" (may fail for
                    critical changes like run-time user/group accounts)
 /////////
+        *exit*;;   tell the currently running driver instance to just exit
+                   (so an external caller like the new driver instance, or
+                   the systemd or SMF frameworks would start another copy)
 
 If the `upsdrvctl` was launched to remain in memory and manage NUT driver
 processes, it can receive supported signals and pass them to those drivers.

drivers/main.c

@@ -229,6 +229,9 @@ static void help_msg(void)
 	printf("                   - reload-or-exit: re-read configuration files (exit the old\n");
 	printf("                     driver instance if needed, so an external caller like the\n");
 	printf("                     systemd or SMF frameworks would start another copy)\n");
+	printf("                   - exit: tell the currently running driver instance to just exit\n");
+	printf("                     (so an external caller like the new driver instance, or the\n");
+	printf("                     systemd or SMF frameworks would start another copy)\n");
 	/* NOTE for FIXME above: PID-signalling is non-WIN32-only for us */
 	printf("  -P <pid>       - send the signal above to specified PID (bypassing PID file)\n");
 # endif	/* WIN32 */
@@ -748,6 +751,11 @@ int main_instcmd(const char *cmdname, const char *extra, conn_t *conn) {
 		}
 	}
 
+	if (!strcmp(cmdname, "driver.exit")) {
+		set_reload_flag(SIGCMD_EXIT);
+		return STAT_INSTCMD_HANDLED;
+	}
+
 #ifndef WIN32
 /* TODO: Equivalent for WIN32 - see SIGCMD_RELOAD in upd and upsmon */
 	if (!strcmp(cmdname, "driver.reload")) {
@@ -1551,6 +1559,15 @@ static void set_reload_flag(
 			break;
 #endif
 
+		case SIGCMD_EXIT:	/* Not even a signal, but a socket protocol action,
+			* and not a reload either - just applied here for consistency */
+/*
+			reload_flag = 15;
+			break;
+*/
+			set_exit_flag(-2);
+			return;
+
 		case SIGCMD_RELOAD:	/* SIGHUP */
 		case SIGCMD_RELOAD_OR_ERROR:	/* Not even a signal, but a socket protocol action */
 		default:
@@ -1564,10 +1581,13 @@ static void set_reload_flag(
 	if (sig && !strcmp(sig, SIGCMD_RELOAD_OR_ERROR)) {
 		/* reload what we can, log what needs a restart so skipped */
 		reload_flag = 1;
+	} else if (sig && !strcmp(sig, SIGCMD_EXIT)) {
+		set_exit_flag(-2);
+		return;
 	} else {
 		/* non-fatal reload as a fallback */
 		reload_flag = 1;
-        }
+	}
 
 	upsdebugx(1, "%s: raising reload flag due to command %s => reload_flag=%d",
 		__func__, sig, reload_flag);
@@ -1813,6 +1833,10 @@ int main(int argc, char **argv)
 				if (!strncmp(optarg, "reload-or-error", strlen(optarg))) {
 					cmd = SIGCMD_RELOAD_OR_ERROR;
 				}
+				else
+				if (!strncmp(optarg, "exit", strlen(optarg))) {
+					cmd = SIGCMD_EXIT;
+				}
 #ifndef WIN32
 				else
 				if (!strncmp(optarg, "reload", strlen(optarg))) {
@@ -1835,9 +1859,14 @@ int main(int argc, char **argv)
 						"Error: unknown argument to option -%c. Try -h for help.", i);
 				}
 #ifndef WIN32
-				upsdebugx(1, "Will send signal %d (%s) for command '%s' "
-					"to already-running driver %s-%s (if any) and exit",
-					cmd, strsignal(cmd), optarg, progname, upsname);
+				if (cmd > 0)
+					upsdebugx(1, "Will send signal %d (%s) for command '%s' "
+						"to already-running driver %s-%s (if any) and exit",
+						cmd, strsignal(cmd), optarg, progname, upsname);
+				else
+					upsdebugx(1, "Will send request for command '%s' (internal code %d) "
+						"to already-running driver %s-%s (if any) and exit",
+						optarg, cmd, progname, upsname);
 #else
 				upsdebugx(1, "Will send request '%s' for command '%s' "
 					"to already-running driver %s-%s (if any) and exit",
@@ -2008,37 +2037,174 @@ int main(int argc, char **argv)
 	/* Handle reload-or-error over socket protocol with
 	 * the running older driver instance */
 #ifndef WIN32
-	if (cmd == SIGCMD_RELOAD_OR_ERROR)
+	if (cmd == SIGCMD_RELOAD_OR_ERROR || cmd == SIGCMD_EXIT)
 #else
-	if (cmd && !strcmp(cmd, SIGCMD_RELOAD_OR_ERROR))
+	if (cmd && (!strcmp(cmd, SIGCMD_RELOAD_OR_ERROR) || !strcmp(cmd, SIGCMD_EXIT)))
 #endif  /* WIN32 */
 	{	/* Not a signal, but a socket protocol action */
 		ssize_t	cmdret = -1;
-		char	buf[LARGEBUF];
+		char	buf[LARGEBUF], cmdbuf[LARGEBUF];
 		struct timeval	tv;
+		char *cmdname = NULL;
+
+#ifndef WIN32
+		if (cmd == SIGCMD_RELOAD_OR_ERROR)
+#else
+		if (!strcmp(cmd, SIGCMD_RELOAD_OR_ERROR))
+#endif
+			cmdname = "reload-or-error";
+		else
+#ifndef WIN32
+		if (cmd == SIGCMD_EXIT)
+#else
+		if (!strcmp(cmd, SIGCMD_EXIT))
+#endif
+			cmdname = "exit";
+
+#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_OVERFLOW || defined HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_TRUNCATION)
+# pragma GCC diagnostic push
+# ifdef HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_OVERFLOW
+#  pragma GCC diagnostic ignored "-Wformat-overflow"
+# endif
+# ifdef HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_TRUNCATION
+#  pragma GCC diagnostic ignored "-Wformat-truncation"
+# endif
+#endif
+		/* Some compilers do detect a chance of cmdname=NULL with
+		 * NUT builds on systems where libc does not care and prints
+		 * the right thing anyway (so NUT_STRARG macro is trivial).
+		 * In this weird case gotta silence the static checks.
+		 */
+		upsdebugx(1, "Signalling UPS [%s]: driver.%s",
+			upsname, NUT_STRARG(cmdname));
+#if (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_PUSH_POP) && (defined HAVE_PRAGMA_GCC_DIAGNOSTIC_IGNORED_FORMAT_OVERFLOW || defined HAVE_PRAGMAS_FOR_GCC_DIAGNOSTIC_IGNORED_FORMAT_TRUNCATION)
+# pragma GCC diagnostic pop
+#endif
+
+		if (!cmdname)
+			fatalx(EXIT_FAILURE, "Command not recognized");
 
 		/* Post the query and wait for reply */
 		/* FIXME: coordinate with pollfreq? */
 		tv.tv_sec = 15;
 		tv.tv_usec = 0;
+		snprintf(cmdbuf, sizeof(cmdbuf), "INSTCMD driver.%s\n", cmdname);
 		cmdret = upsdrvquery_oneshot(progname, upsname,
-			"INSTCMD driver.reload-or-error\n",
-			buf, sizeof(buf), &tv);
+			cmdbuf, buf, sizeof(buf), &tv);
 
 		if (cmdret < 0) {
 			upslog_with_errno(LOG_ERR, "Socket dialog with the other driver instance");
 		} else {
 			/* TODO: handle buf reply contents */
-			upslogx(LOG_INFO, "Request to reload-or-error returned code %" PRIiSIZE, cmdret);
+			upslogx(LOG_INFO, "Request for driver to %s returned code %" PRIiSIZE,
+				cmdname, cmdret);
 		}
 
 		/* exit((cmdret == 0) ? EXIT_SUCCESS : EXIT_FAILURE); */
 		exit(((cmdret < 0) || (((uintmax_t)cmdret) > ((uintmax_t)INT_MAX))) ? 255 : (int)cmdret);
 	}
 
+	/* If we would be starting as a driver (not to command a sibling),
+	 * any earlier instances should be turned off - to release access
+	 * to hardware connections and to generally avoid any confusion.
+	 * Further below we would try to use a PID file (if at all used
+	 * and still present) to terminate an earlier instance, but first
+	 * we would try to use the Unix socket protocol to tell that
+	 * earlier instance to exit cleanly. After all, this socket file
+	 * should exist for the driver to talk to the NUT data server...
+	 */
+
 	/* Hush the fopen(pidfile) message but let "real errors" be seen */
 	nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_FOPEN_PIDFILE - 1;
 
+	if (!cmd && (!do_forceshutdown)) {
+		ssize_t	cmdret = -1;
+		char	buf[LARGEBUF];
+		struct timeval	tv;
+
+		upsdebugx(1, "Signalling UPS [%s]: driver.exit (quietly, no fuss if no driver is running or responding)", upsname);
+
+		/* Post the query and wait for reply */
+		/* FIXME: coordinate with pollfreq? */
+		tv.tv_sec = 15;
+		tv.tv_usec = 0;
+
+		/* Hush the messages about initial connection failure, but
+		 * let "real errors" from started communication be seen.
+		 * It is okay if no driver instance is running at this
+		 * point, but if it is running but not communicating -
+		 * that is another story.
+		 */
+		nut_upsdrvquery_debug_level = NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT - 1;
+		cmdret = upsdrvquery_oneshot(progname, upsname,
+			"INSTCMD driver.exit\n",
+			buf, sizeof(buf), &tv);
+
+		upsdebugx(1, "Request for other driver to exit returned code %" PRIiSIZE,
+			cmdret);
+		if (cmdret < 0) {
+			/* Failed to communicate, assume no other instance runs */
+			upsdebug_with_errno(1, "Socket dialog with the other driver instance "
+				"(may be absent) failed");
+		} else {
+			/* NOTE: Successful dialog does not mean the other
+			 * driver instance has stopped (just that it responded
+			 * "yes, sir!" - actual wind-down can take some time.
+			 */
+			upslogx(LOG_WARNING, "Duplicate driver instance detected (local %s exists)! "
+				"Asked the other driver nicely to self-terminate!",
+#ifndef WIN32
+				"Unix socket"
+#else
+				"pipe"
+#endif
+				);
+
+			for (i = 10; i > 0; i--) {
+				if (exit_flag)
+					fatalx(EXIT_FAILURE, "Got a break signal ourselves during attempt to terminate other driver");
+
+				/* Allow driver some time to quit, and
+				 * retry until it does not respond anymore */
+				sleep(5);
+
+				if (exit_flag)
+					fatalx(EXIT_FAILURE, "Got a break signal ourselves during attempt to terminate other driver");
+
+				tv.tv_sec = 3;
+				tv.tv_usec = 0;
+				cmdret = upsdrvquery_oneshot(progname, upsname,
+					"INSTCMD driver.exit\n",
+					buf, sizeof(buf), &tv);
+				upsdebugx(1, "Subsequent request for other driver to exit returned code %"
+					PRIiSIZE, cmdret);
+
+				if (cmdret < 0)
+					break;
+			}
+
+			if (i < 1) {
+				upslogx(LOG_WARNING, "Duplicate driver instance did not respond to termination requests! "
+					"Is it stuck or from an older NUT release? "
+					"Will retry via PID file and signals, if available.");
+				/* NOTE: We would try via PID in any case,
+				 * but as we report a fault here - let the
+				 * user know that not all is lost right now :)
+				 */
+
+				/* Restore the signal errors verbosity, so that
+				 * e.g. follow-up fopen() issues can be seen -
+				 * we did probably encounter a sibling driver
+				 * instance after all, so can talk about it.
+				 */
+				nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_DEFAULT;
+			}
+		}
+
+		/* Restore the socket protocol errors verbosity */
+		nut_upsdrvquery_debug_level = NUT_UPSDRVQUERY_DEBUG_LEVEL_DEFAULT;
+	}
+
 #ifndef WIN32
 	/* Setup PID file to receive signals to communicate with this driver
 	 * instance once backgrounded (or staying foregrounded with `-FF`),
@@ -2161,6 +2327,9 @@ int main(int argc, char **argv)
 
 			upsdebugx(1, "Signal sent without errors, allow the other driver instance some time to quit");
 			sleep(5);
+
+			if (exit_flag)
+				fatalx(EXIT_FAILURE, "Got a break signal during attempt to terminate other driver");
 		}
 
 		if (i > 0) {

drivers/main.h

@@ -131,6 +131,7 @@ void setup_signals(void);
 #ifndef WIN32
 # define SIGCMD_RELOAD                  SIGHUP
 /* not a signal, so negative; relies on socket protocol */
+# define SIGCMD_EXIT                    -SIGTERM
 # define SIGCMD_RELOAD_OR_ERROR         -SIGCMD_RELOAD
 # define SIGCMD_RELOAD_OR_EXIT          SIGUSR1
 /* // FIXME: Implement this self-recycling in drivers (keeping the PID):
@@ -153,6 +154,7 @@ void setup_signals(void);
 # endif
 #else
 /* FIXME: handle WIN32 builds for other signals too */
+# define SIGCMD_EXIT                    "driver.exit"
 # define SIGCMD_RELOAD_OR_ERROR         "driver.reload-or-error"
 #endif	/* WIN32 */