From 1e805c742a2787969cbe162e1ecf918c70b2a1d3 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 12 Apr 2024 13:12:18 +0200 Subject: [PATCH 1/9] drivers/upsdrvctl.c: bump (C) years and primary authors Signed-off-by: Jim Klimov --- drivers/upsdrvctl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c index f01c6b495e..aa90c04a63 100644 --- a/drivers/upsdrvctl.c +++ b/drivers/upsdrvctl.c @@ -1,6 +1,9 @@ /* upsdrvctl.c - UPS driver controller - Copyright (C) 2001 Russell Kroll + Copyright (C) + 2001 Russell Kroll + 2005 - 2017 Arnaud Quette + 2017 - 2024 Jim Klimov This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by From ffca781e0b04b880f9e3f2fe73d48f9381f4ac6a Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 12 Apr 2024 13:16:43 +0200 Subject: [PATCH 2/9] drivers/upsdrvctl.c: help(): prepend sections with a blank line Signed-off-by: Jim Klimov --- drivers/upsdrvctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c index aa90c04a63..4675a8ad8e 100644 --- a/drivers/upsdrvctl.c +++ b/drivers/upsdrvctl.c @@ -938,7 +938,7 @@ static void help(const char *arg_progname) printf(" -FF driver stays foregrounded and still saves the PID file\n"); printf(" -B driver(s) stay backgrounded even if debugging is bumped\n"); - printf("Signalling a running driver:\n"); + printf("\nSignalling a running driver:\n"); printf(" -c send via signal to running driver(s)\n"); printf(" supported commands:\n"); #ifndef WIN32 @@ -966,7 +966,7 @@ static void help(const char *arg_progname) printf(" systemd or SMF frameworks would start another copy)\n"); #endif /* WIN32 */ - printf("Driver life cycle options:\n"); + printf("\nDriver life cycle options:\n"); printf(" start start all UPS drivers in ups.conf\n"); printf(" start only start driver for UPS \n"); printf(" stop stop all UPS drivers in ups.conf\n"); From b380ddf2e73073aea83f8684471ae991585e491f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 12 Apr 2024 13:37:23 +0200 Subject: [PATCH 3/9] drivers/main.{c,h}, drivers/upsdrvctl.c, data/cmdvartab, man pages: Implement "INSTCMD driver.exit" [#2392] Signed-off-by: Jim Klimov --- NEWS.adoc | 4 +++ data/cmdvartab | 1 + docs/man/nutupsdrv.txt | 7 +++++ docs/man/upsdrvctl.txt | 3 ++ drivers/main.c | 70 ++++++++++++++++++++++++++++++++++++------ drivers/main.h | 2 ++ drivers/upsdrvctl.c | 58 ++++++++++++++++++++++++++-------- 7 files changed, 122 insertions(+), 23 deletions(-) diff --git a/NEWS.adoc b/NEWS.adoc index d9bb7e2404..9dc0a6e846 100644 --- a/NEWS.adoc +++ b/NEWS.adoc @@ -40,6 +40,10 @@ https://github.com/networkupstools/nut/milestone/11 - (expected) Bug fixes for fallout possible due to "fightwarn" effort in 2.8.0 + - 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, diff --git a/data/cmdvartab b/data/cmdvartab index 9a1d9ab3a9..38d2b356dd 100644 --- a/data/cmdvartab +++ b/data/cmdvartab @@ -194,6 +194,7 @@ VARDESC driver.version.usb "USB library version" # VARDESC driver.parameter.[[:alpha:]]+ "Driver parameter: " # VARDESC driver.flag.[[:alpha:]]+ "Driver flag: " +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)" diff --git a/docs/man/nutupsdrv.txt b/docs/man/nutupsdrv.txt index e1cc17722a..6d6fe6564a 100644 --- a/docs/man/nutupsdrv.txt +++ b/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 diff --git a/docs/man/upsdrvctl.txt b/docs/man/upsdrvctl.txt index 446ed2d411..58e48f0957 100644 --- a/docs/man/upsdrvctl.txt +++ b/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. diff --git a/drivers/main.c b/drivers/main.c index 72190b61f2..6bdc6f1ba9 100644 --- a/drivers/main.c +++ b/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 - 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,28 +2037,49 @@ 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"; + + upsdebugx(1, "Signalling UPS [%s]: driver.%s", + upsname, NUT_STRARG(cmdname)); + 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); */ diff --git a/drivers/main.h b/drivers/main.h index 1658ae3491..05e03d7926 100644 --- a/drivers/main.h +++ b/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 */ diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c index 4675a8ad8e..5c95e3a3c0 100644 --- a/drivers/upsdrvctl.c +++ b/drivers/upsdrvctl.c @@ -196,32 +196,48 @@ static void signal_driver_cmd(const ups_t *ups, int ret; #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 { /* not a signal, use socket protocol */ - char buf[LARGEBUF]; + char buf[LARGEBUF], cmdbuf[LARGEBUF]; struct timeval tv; + char *cmdname = NULL; - upsdebugx(1, "Signalling UPS [%s]: %s", - ups->upsname, "driver.reload-or-error"); +#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"; + + upsdebugx(1, "Signalling UPS [%s]: driver.%s", + ups->upsname, NUT_STRARG(cmdname)); - if (testmode) + if (testmode || !cmdname) return; /* 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); ret = upsdrvquery_oneshot(ups->driver, ups->upsname, - "INSTCMD driver.reload-or-error\n", - buf, sizeof(buf), &tv); + cmdbuf, buf, sizeof(buf), &tv); if (ret < 0) { goto socket_error; } else { - upslogx(LOG_INFO, "Request to reload-or-error returned code %d", ret); + upslogx(LOG_INFO, "Request for driver to %s returned code %d", + cmdname, ret); if (ret != STAT_INSTCMD_HANDLED) exec_error++; /* TODO: Propagate "ret" to caller, eventually CLI exit-code? */ @@ -495,7 +511,7 @@ static void reset_signal_flag(void) } #ifndef WIN32 -/* TODO: Equivalent for WIN32 - see SIGCMD_RELOAD in upd and upsmon */ +/* TODO: Equivalent for WIN32 - see SIGCMD_RELOAD in upsd and upsmon */ static void set_reload_flag(const #ifndef WIN32 int @@ -519,6 +535,10 @@ static void set_reload_flag(const break; # endif + case SIGCMD_EXIT: /* Not even a signal, but a socket protocol action */ + reload_flag = 15; + break; + case SIGCMD_RELOAD: /* SIGHUP */ case SIGCMD_RELOAD_OR_ERROR: /* Not even a signal, but a socket protocol action */ default: @@ -965,6 +985,9 @@ static void help(const char *arg_progname) printf(" driver instance if needed, so an external caller like the\n"); printf(" systemd or SMF frameworks would start another copy)\n"); #endif /* WIN32 */ + 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"); printf("\nDriver life cycle options:\n"); printf(" start start all UPS drivers in ups.conf\n"); @@ -1192,6 +1215,10 @@ int main(int argc, char **argv) if (!strncmp(optarg, "reload-or-error", strlen(optarg))) { signal_flag = SIGCMD_RELOAD_OR_ERROR; } + else + if (!strncmp(optarg, "exit", strlen(optarg))) { + signal_flag = SIGCMD_EXIT; + } #ifndef WIN32 /* FIXME: port event loop from upsd/upsmon to allow messaging fellow drivers in WIN32 builds */ else @@ -1222,9 +1249,14 @@ int main(int argc, char **argv) pt_cmd = optarg; #ifndef WIN32 - upsdebugx(1, "Will send signal %d (%s) for command '%s' " - "to already-running driver (if any) and exit", - signal_flag, strsignal(signal_flag), optarg); + if (signal_flag > 0) + upsdebugx(1, "Will send signal %d (%s) for command '%s' " + "to already-running driver (if any) and exit", + signal_flag, strsignal(signal_flag), optarg); + else + upsdebugx(1, "Will send request for command '%s' (internal code %d) " + "to already-running driver (if any) and exit", + optarg, signal_flag); #else upsdebugx(1, "Will send request '%s' for command '%s' " "to already-running driver (if any) and exit", From a22436996c4602dd436a4d2bf034832cff8d990f Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 13 Apr 2024 15:37:17 +0200 Subject: [PATCH 4/9] drivers/upsdrvquery.{c,h}: extend with nut_upsdrvquery_debug_level verbosity toggle [#1782, #2392, #2384] Signed-off-by: Jim Klimov --- drivers/upsdrvquery.c | 67 +++++++++++++++++++++++++++++++------------ drivers/upsdrvquery.h | 9 +++++- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/drivers/upsdrvquery.c b/drivers/upsdrvquery.c index 0936631ab5..cacb98766a 100644 --- a/drivers/upsdrvquery.c +++ b/drivers/upsdrvquery.c @@ -2,7 +2,7 @@ tracked until a response arrives, returning that line and closing a connection - Copyright (C) 2023 Jim Klimov + Copyright (C) 2023-2024 Jim Klimov This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -39,6 +39,19 @@ #include "upsdrvquery.h" #include "nut_stdint.h" +/* Normally the upsdrvquery*() methods call upslogx() to report issues + * such as failed fopen() of Unix socket file, or a dialog timeout or + * different error. + * In a few cases we call these methods opportunistically, and so if + * they fail - we do not care enough to raise a lot of "scary noise"; + * the caller can take care of logging as/if needed. + * This variable and its values are a bit of internal detail between + * certain NUT programs to hush the low-level reports when they are + * not being otherwise debugged (e.g. nut_debug_level < 1). + * Default value allows all those messages to appear. + */ +int nut_upsdrvquery_debug_level = NUT_UPSDRVQUERY_DEBUG_LEVEL_DEFAULT; + udq_pipe_conn_t *upsdrvquery_connect(const char *sockfn) { udq_pipe_conn_t *conn = (udq_pipe_conn_t*)xcalloc(1, sizeof(udq_pipe_conn_t)); @@ -54,13 +67,15 @@ udq_pipe_conn_t *upsdrvquery_connect(const char *sockfn) { conn->sockfd = socket(AF_UNIX, SOCK_STREAM, 0); if (conn->sockfd < 0) { - upslog_with_errno(LOG_ERR, "open socket"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "open socket"); free(conn); return NULL; } if (connect(conn->sockfd, (struct sockaddr *) &sa, sizeof(sa)) < 0) { - upslog_with_errno(LOG_ERR, "connect to driver socket at %s", sockfn); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "connect to driver socket at %s", sockfn); close(conn->sockfd); free(conn); return NULL; @@ -68,14 +83,16 @@ udq_pipe_conn_t *upsdrvquery_connect(const char *sockfn) { ret = fcntl(conn->sockfd, F_GETFL, 0); if (ret < 0) { - upslog_with_errno(LOG_ERR, "fcntl get on driver socket %s failed", sockfn); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "fcntl get on driver socket %s failed", sockfn); close(conn->sockfd); free(conn); return NULL; } if (fcntl(conn->sockfd, F_SETFL, ret | O_NDELAY) < 0) { - upslog_with_errno(LOG_ERR, "fcntl set O_NDELAY on driver socket %s failed", sockfn); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "fcntl set O_NDELAY on driver socket %s failed", sockfn); close(conn->sockfd); free(conn); return NULL; @@ -84,7 +101,8 @@ udq_pipe_conn_t *upsdrvquery_connect(const char *sockfn) { BOOL result = WaitNamedPipe(sockfn, NMPWAIT_USE_DEFAULT_WAIT); if (result == FALSE) { - upslog_with_errno(LOG_ERR, "WaitNamedPipe : %d\n", GetLastError()); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "WaitNamedPipe : %d\n", GetLastError()); return NULL; } @@ -99,7 +117,8 @@ udq_pipe_conn_t *upsdrvquery_connect(const char *sockfn) { NULL); /* no template file */ if (conn->sockfd == INVALID_HANDLE_VALUE) { - upslog_with_errno(LOG_ERR, "CreateFile : %d\n", GetLastError()); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "CreateFile : %d\n", GetLastError()); free(conn); return NULL; } @@ -114,7 +133,8 @@ udq_pipe_conn_t *upsdrvquery_connect(const char *sockfn) { ); if (conn->overlapped.hEvent == NULL) { - upslogx(LOG_ERR, "Can't create event for reading event log"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslogx(LOG_ERR, "Can't create event for reading event log"); free(conn); return NULL; } @@ -145,7 +165,8 @@ udq_pipe_conn_t *upsdrvquery_connect_drvname_upsname(const char *drvname, const dflt_statepath(), drvname, upsname); check_unix_socket_filename(pidfn); if (stat(pidfn, &fs)) { - upslog_with_errno(LOG_ERR, "Can't open %s", pidfn); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "Can't open %s", pidfn); return NULL; } #else @@ -170,9 +191,10 @@ void upsdrvquery_close(udq_pipe_conn_t *conn) { if (VALID_FD(conn->sockfd)) { if (DisconnectNamedPipe(conn->sockfd) == 0) { - upslogx(LOG_ERR, - "DisconnectNamedPipe error : %d", - (int)GetLastError()); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslogx(LOG_ERR, + "DisconnectNamedPipe error : %d", + (int)GetLastError()); } CloseHandle(conn->sockfd); } @@ -196,7 +218,8 @@ ssize_t upsdrvquery_read_timeout(udq_pipe_conn_t *conn, struct timeval tv) { #endif if (!conn || INVALID_FD(conn->sockfd)) { - upslog_with_errno(LOG_ERR, "socket not initialized"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "socket not initialized"); return -1; } @@ -205,7 +228,8 @@ ssize_t upsdrvquery_read_timeout(udq_pipe_conn_t *conn, struct timeval tv) { FD_SET(conn->sockfd, &rfds); if (select(conn->sockfd + 1, &rfds, NULL, NULL, &tv) < 0) { - upslog_with_errno(LOG_ERR, "select with socket"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_DIALOG) + upslog_with_errno(LOG_ERR, "select with socket"); /* upsdrvquery_close(conn); */ return -1; } @@ -219,7 +243,8 @@ ssize_t upsdrvquery_read_timeout(udq_pipe_conn_t *conn, struct timeval tv) { ret = read(conn->sockfd, conn->buf, sizeof(conn->buf)); #else /* - upslog_with_errno(LOG_ERR, "Support for this platform is not currently implemented"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level > 0) + upslog_with_errno(LOG_ERR, "Support for this platform is not currently implemented"); return -1; */ @@ -333,7 +358,8 @@ ssize_t upsdrvquery_write(udq_pipe_conn_t *conn, const char *buf) { upsdebugx(5, "%s: write to driver socket: %s", __func__, buf); if (!conn || INVALID_FD(conn->sockfd)) { - upslog_with_errno(LOG_ERR, "socket not initialized"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT) + upslog_with_errno(LOG_ERR, "socket not initialized"); return -1; } @@ -341,7 +367,8 @@ ssize_t upsdrvquery_write(udq_pipe_conn_t *conn, const char *buf) { ret = write(conn->sockfd, buf, buflen); if (ret < 0 || ret != (int)buflen) { - upslog_with_errno(LOG_ERR, "Write to socket %d failed", conn->sockfd); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_DIALOG) + upslog_with_errno(LOG_ERR, "Write to socket %d failed", conn->sockfd); goto socket_error; } @@ -349,7 +376,8 @@ ssize_t upsdrvquery_write(udq_pipe_conn_t *conn, const char *buf) { #else result = WriteFile(conn->sockfd, buf, buflen, &bytesWritten, NULL); if (result == 0 || bytesWritten != (DWORD)buflen) { - upslog_with_errno(LOG_ERR, "Write to handle %p failed", conn->sockfd); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_DIALOG) + upslog_with_errno(LOG_ERR, "Write to handle %p failed", conn->sockfd); goto socket_error; } @@ -433,7 +461,8 @@ ssize_t upsdrvquery_prepare(udq_pipe_conn_t *conn, struct timeval tv) { if (upsdrvquery_read_timeout(conn, tv) < 1) goto socket_error; if (strcmp(conn->buf, "ON")) { - upslog_with_errno(LOG_ERR, "Driver does not have TRACKING support enabled"); + if (nut_debug_level > 0 || nut_upsdrvquery_debug_level >= NUT_UPSDRVQUERY_DEBUG_LEVEL_DIALOG) + upslog_with_errno(LOG_ERR, "Driver does not have TRACKING support enabled"); goto socket_error; } */ diff --git a/drivers/upsdrvquery.h b/drivers/upsdrvquery.h index 6a7b879d6b..003337dfb9 100644 --- a/drivers/upsdrvquery.h +++ b/drivers/upsdrvquery.h @@ -2,7 +2,7 @@ tracked until a response arrives, returning that line and closing a connection - Copyright (C) 2023 Jim Klimov + Copyright (C) 2023-2024 Jim Klimov This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -48,4 +48,11 @@ ssize_t upsdrvquery_request(udq_pipe_conn_t *conn, struct timeval tv, const char /* if buf != NULL, last reply is copied there */ ssize_t upsdrvquery_oneshot(const char *drvname, const char *upsname, const char *query, char *buf, const size_t bufsz, struct timeval *tv); +/* Internal toggle for some NUT programs that deal with Unix socket chatter. + * For a detailed rationale comment see upsdrvquery.c */ +extern int nut_upsdrvquery_debug_level; +#define NUT_UPSDRVQUERY_DEBUG_LEVEL_DEFAULT 6 +#define NUT_UPSDRVQUERY_DEBUG_LEVEL_CONNECT 5 +#define NUT_UPSDRVQUERY_DEBUG_LEVEL_DIALOG 4 + #endif /* NUT_UPSDRVQUERY_H_SEEN */ From 705cb2ae08fd1ab6208cbe62b4fd5f6d2bdc5afb Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 13 Apr 2024 17:16:08 +0200 Subject: [PATCH 5/9] drivers/main.c: when looping to SIGTERM a duplicate driver, check if we got a signal ourselves (e.g. user pressed Ctrl+C) Signed-off-by: Jim Klimov --- drivers/main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/main.c b/drivers/main.c index 6bdc6f1ba9..ed4bec3f3c 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2203,6 +2203,9 @@ int main(int argc, char **argv) /* Allow driver 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) { From df7aed64145a688f9ec84ff5db501f851d5be9b3 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 13 Apr 2024 15:38:44 +0200 Subject: [PATCH 6/9] drivers/main.c: use nut_upsdrvquery_debug_level verbosity toggle to hush scary noise from attempts to "driver.exit" a sibling [#1782, #2392, #2384] Signed-off-by: Jim Klimov --- drivers/main.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/drivers/main.c b/drivers/main.c index ed4bec3f3c..54ea9bac57 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2086,6 +2086,89 @@ int main(int argc, char **argv) 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... + */ + 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)! Asking other driver 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?"); + } + } + + /* Restore the signal 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, and to stop a competing older instance. From 74a33f0102b67219267c3331f305cf1b3883bd91 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 13 Apr 2024 18:09:29 +0200 Subject: [PATCH 7/9] drivers/main.c: silence compiler warnings about NUT_STRARG() for "INSTCMD driver.exit" in some builds [#2392] Signed-off-by: Jim Klimov --- drivers/main.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/main.c b/drivers/main.c index 54ea9bac57..cb4cfcf26c 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2061,8 +2061,26 @@ int main(int argc, char **argv) #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"); From eab95b8e6e4d16eff635ae03a930fffe616321bb Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 13 Apr 2024 18:33:10 +0200 Subject: [PATCH 8/9] drivers/main.c: change messages around "INSTCMD driver.exit" activity [#2392] Signed-off-by: Jim Klimov --- drivers/main.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/main.c b/drivers/main.c index cb4cfcf26c..875bb319a2 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2147,7 +2147,8 @@ int main(int argc, char **argv) * 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)! Asking other driver to self-terminate!", + upslogx(LOG_WARNING, "Duplicate driver instance detected (local %s exists)! " + "Asked the other driver nicely to self-terminate!", #ifndef WIN32 "Unix socket" #else @@ -2179,7 +2180,13 @@ int main(int argc, char **argv) } if (i < 1) { - upslogx(LOG_WARNING, "Duplicate driver instance did not respond to termination requests! Is it stuck or from an older NUT release?"); + 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 :) + */ } } From deba0a96aac138399191a6bcb6186fc4680698f6 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Sat, 13 Apr 2024 18:40:01 +0200 Subject: [PATCH 9/9] drivers/main.c: Hush the fopen(pidfile) messages *before* dealing with "INSTCMD driver.exit" so that we can make that verbose again if dialog started but failed to stop the other driver [#2384, #2392] Signed-off-by: Jim Klimov --- drivers/main.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/main.c b/drivers/main.c index c17fc691dc..0a6a2ad8f8 100644 --- a/drivers/main.c +++ b/drivers/main.c @@ -2113,6 +2113,10 @@ int main(int argc, char **argv) * 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]; @@ -2187,16 +2191,20 @@ int main(int argc, char **argv) * 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 signal errors verbosity */ + /* Restore the socket protocol errors verbosity */ nut_upsdrvquery_debug_level = NUT_UPSDRVQUERY_DEBUG_LEVEL_DEFAULT; } - /* Hush the fopen(pidfile) message but let "real errors" be seen */ - nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_FOPEN_PIDFILE - 1; - #ifndef WIN32 /* Setup PID file to receive signals to communicate with this driver * instance once backgrounded (or staying foregrounded with `-FF`),