Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/master' into issue-2392
Browse files Browse the repository at this point in the history
Signed-off-by: Jim Klimov <[email protected]>
  • Loading branch information
jimklimov committed Apr 13, 2024
2 parents eab95b8 + 7f36d49 commit aa25c9f
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 48 deletions.
12 changes: 12 additions & 0 deletions NEWS.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ https://github.com/networkupstools/nut/milestone/11
- (expected) Bug fixes for fallout possible due to "fightwarn" effort in 2.8.0
- drivers, upsd, upsmon: reduce "scary noise" about failure to `fopen()`
the PID file (which most of the time means that no previous instance of
the daemon was running to potentially conflict with), especially useless
since in recent NUT releases the verdicts from `sendsignal*()` methods
are analyzed and lead to layman worded situation reports in these programs.
[issue #1782, PR #2384]
- drivers started with the `-FF` command-line option (e.g. wrapped into the
systemd units to stay "foregrounded" *and* save a PID file anyway) should
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]
Expand Down
17 changes: 15 additions & 2 deletions clients/upsmon.c
Original file line number Diff line number Diff line change
Expand Up @@ -2894,6 +2894,8 @@ int main(int argc, char *argv[])
* for probing whether a competing older instance of this program
* is running (error if it is).
*/
/* Hush the fopen(pidfile) message but let "real errors" be seen */
nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_FOPEN_PIDFILE - 1;
#ifndef WIN32
/* If cmd == 0 we are starting and check if a previous instance
* is running by sending signal '0' (i.e. 'kill <pid> 0' equivalent)
Expand Down Expand Up @@ -2953,8 +2955,9 @@ int main(int argc, char *argv[])
*/
upslogx(LOG_WARNING, "Could not %s PID file "
"to see if previous upsmon instance is "
"already running!",
(cmdret == -3 ? "find" : "parse"));
"already running!%s",
(cmdret == -3 ? "find" : "parse"),
(checking_flag ? " This is okay during OS shutdown, which is when checking POWERDOWNFLAG makes most sense." : ""));
break;

case -1:
Expand Down Expand Up @@ -3007,11 +3010,21 @@ int main(int argc, char *argv[])
exit((cmdret == 0) ? EXIT_SUCCESS : EXIT_FAILURE);
}

/* Restore the signal errors verbosity */
nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_DEFAULT;

argc -= optind;
argv += optind;

open_syslog(prog);

if (checking_flag) {
/* Do not normally report the UPSes we would monitor, etc.
* from loadconfig() for just checking the killpower flag */
if (nut_debug_level == 0)
nut_debug_level = -2;
}

loadconfig();

/* CLI debug level can not be smaller than debug_min specified
Expand Down
46 changes: 38 additions & 8 deletions common/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,23 @@ pid_t get_max_pid_t(void)
#endif
}

/* Normally sendsignalfn(), sendsignalpid() and related methods call
* upslogx() to report issues such as failed fopen() of PID file,
* failed parse of its contents, inability to send a signal (absent
* process or some other issue like permissions).
* Some of these low-level reports look noisy and scary to users,
* others are a bit confusing ("PID file not found... is it bad or
* good, what do I do with that knowledge?") so several consuming
* programs actually parse the returned codes to formulate their
* own messages like "No earlier instance of this daemon was found
* running" and users benefit even less from low-level reports.
* 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_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_DEFAULT;

int nut_debug_level = 0;
int nut_log_level = 0;
static int upslog_flags = UPSLOG_STDERR;
Expand Down Expand Up @@ -459,17 +476,19 @@ int sendsignalpid(pid_t pid, int sig)
int ret;

if (pid < 2 || pid > get_max_pid_t()) {
upslogx(LOG_NOTICE,
"Ignoring invalid pid number %" PRIdMAX,
(intmax_t) pid);
if (nut_debug_level > 0 || nut_sendsignal_debug_level > 0)
upslogx(LOG_NOTICE,
"Ignoring invalid pid number %" PRIdMAX,
(intmax_t) pid);
return -1;
}

/* see if this is going to work first - does the process exist? */
ret = kill(pid, 0);

if (ret < 0) {
perror("kill");
if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1)
perror("kill");
return -1;
}

Expand All @@ -478,7 +497,8 @@ int sendsignalpid(pid_t pid, int sig)
ret = kill(pid, sig);

if (ret < 0) {
perror("kill");
if (nut_debug_level > 0 || nut_sendsignal_debug_level > 1)
perror("kill");
return -1;
}
}
Expand Down Expand Up @@ -513,7 +533,10 @@ pid_t parsepid(const char *buf)
if (_pid <= get_max_pid_t()) {
pid = (pid_t)_pid;
} else {
upslogx(LOG_NOTICE, "Received a pid number too big for a pid_t: %" PRIdMAX, _pid);
if (nut_debug_level > 0 || nut_sendsignal_debug_level > 0)
upslogx(LOG_NOTICE,
"Received a pid number too big for a pid_t: %"
PRIdMAX, _pid);
}

return pid;
Expand All @@ -533,12 +556,19 @@ int sendsignalfn(const char *pidfn, int sig)

pidf = fopen(pidfn, "r");
if (!pidf) {
upslog_with_errno(LOG_NOTICE, "fopen %s", pidfn);
/* This one happens quite often when a daemon starts
* for the first time and no opponent PID file exists,
* so the cut-off verbosity is higher.
*/
if (nut_debug_level > 0 ||
nut_sendsignal_debug_level >= NUT_SENDSIGNAL_DEBUG_LEVEL_FOPEN_PIDFILE)
upslog_with_errno(LOG_NOTICE, "fopen %s", pidfn);
return -3;
}

if (fgets(buf, sizeof(buf), pidf) == NULL) {
upslogx(LOG_NOTICE, "Failed to read pid from %s", pidfn);
if (nut_debug_level > 0 || nut_sendsignal_debug_level > 2)
upslogx(LOG_NOTICE, "Failed to read pid from %s", pidfn);
fclose(pidf);
return -2;
}
Expand Down
2 changes: 2 additions & 0 deletions docs/nut.dict
Original file line number Diff line number Diff line change
Expand Up @@ -2092,6 +2092,7 @@ fmt
fno
fontconfig
footnoteref
fopen
forceshutdown
forcessl
formatconfig
Expand Down Expand Up @@ -2990,6 +2991,7 @@ selftest
sendback
sendline
sendmail
sendsignal
sequentialized
ser
seria
Expand Down
60 changes: 36 additions & 24 deletions drivers/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
1999 Russell Kroll <[email protected]>
2005 - 2017 Arnaud Quette <[email protected]>
2017 Eaton (author: Emilien Kia <[email protected]>)
2017 - 2022 Jim Klimov <[email protected]>
2017 - 2024 Jim Klimov <[email protected]>
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
Expand Down Expand Up @@ -2194,12 +2194,16 @@ int main(int argc, char **argv)
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, and to stop a competing older instance.
* Or to send it a signal deliberately.
* instance once backgrounded (or staying foregrounded with `-FF`),
* and to stop a competing older instance. Or to send it a signal
* deliberately.
*/
if (cmd || ((foreground == 0) && (!do_forceshutdown))) {
if (cmd || ((foreground == 0 || foreground == 2) && (!do_forceshutdown))) {
char pidfnbuf[SMALLBUF];

snprintf(pidfnbuf, sizeof(pidfnbuf), "%s/%s-%s.pid", altpidpath(), progname, upsname);
Expand Down Expand Up @@ -2237,21 +2241,21 @@ int main(int argc, char **argv)
/* if cmd was nontrivial - speak up below, else be quiet */
upsdebugx(1, "Just failed to send signal, no daemon was running");
break;
}
} /* switch (cmdret) */

/* We were signalling a daemon, successfully or not - exit now...
* Modulo the possibility of a "reload-or-something" where we
* effectively terminate the old driver and start a new one due
* to configuration changes that were not reloadable. Such mode
* is not implemented currently.
*/
if (cmdret != 0) {
/* sendsignal*() above might have logged more details
* for troubleshooting, e.g. about lack of PID file
/* We were signalling a daemon, successfully or not - exit now...
* Modulo the possibility of a "reload-or-something" where we
* effectively terminate the old driver and start a new one due
* to configuration changes that were not reloadable. Such mode
* is not implemented currently.
*/
upslogx(LOG_NOTICE, "Failed to signal the currently running daemon (if any)");
if (cmdret != 0) {
/* sendsignal*() above might have logged more details
* for troubleshooting, e.g. about lack of PID file
*/
upslogx(LOG_NOTICE, "Failed to signal the currently running daemon (if any)");
# ifdef HAVE_SYSTEMD
switch (cmd) {
switch (cmd) {
case SIGCMD_RELOAD:
upslogx(LOG_NOTICE, "Try something like "
"'systemctl reload nut-driver@%s.service'%s",
Expand All @@ -2276,7 +2280,7 @@ int main(int argc, char **argv)
upsname,
(oldpid < 0 ? " or add '-P $PID' argument" : ""));
break;
}
} /* switch (cmd) */
/* ... or edit nut-server.service locally to start `upsd -FF`
* and so save the PID file for ability to manage the daemon
* beside the service framework, possibly confusing things...
Expand All @@ -2286,30 +2290,34 @@ int main(int argc, char **argv)
upslogx(LOG_NOTICE, "Try to add '-P $PID' argument");
}
# endif /* HAVE_SYSTEMD */
}
} /* if (cmdret != 0) */

exit((cmdret == 0) ? EXIT_SUCCESS : EXIT_FAILURE);
}
} /* if (cmd) */

/* Try to prevent that driver is started multiple times. If a PID file */
/* already exists, send a TERM signal to the process and try if it goes */
/* away. If not, retry a couple of times. */
for (i = 0; i < 3; i++) {
struct stat st;
int sigret;

if (stat(pidfnbuf, &st) != 0) {
/* PID file not found */
if ((sigret = stat(pidfnbuf, &st)) != 0) {
upsdebugx(1, "PID file %s not found; stat() returned %d (errno=%d): %s",
pidfnbuf, sigret, errno, strerror(errno));
break;
}

upslogx(LOG_WARNING, "Duplicate driver instance detected (PID file %s exists)! Terminating other driver!", pidfnbuf);

if (sendsignalfn(pidfnbuf, SIGTERM) != 0) {
/* Can't send signal to PID, assume invalid file */
if ((sigret = sendsignalfn(pidfnbuf, SIGTERM) != 0)) {
upsdebugx(1, "Can't send signal to PID, assume invalid PID file %s; "
"sendsignalfn() returned %d (errno=%d): %s",
pidfnbuf, sigret, errno, strerror(errno));
break;
}

/* Allow driver some time to quit */
upsdebugx(1, "Signal sent without errors, allow the other driver instance some time to quit");
sleep(5);

if (exit_flag)
Expand Down Expand Up @@ -2387,6 +2395,9 @@ int main(int argc, char **argv)
}
#endif /* WIN32 */

/* Restore the signal errors verbosity */
nut_sendsignal_debug_level = NUT_SENDSIGNAL_DEBUG_LEVEL_DEFAULT;

/* clear out callback handler data */
memset(&upsh, '\0', sizeof(upsh));

Expand Down Expand Up @@ -2613,6 +2624,7 @@ int main(int argc, char **argv)
snprintf(pidfnbuf, sizeof(pidfnbuf), "%s/%s-%s.pid", altpidpath(), progname, upsname);
pidfn = xstrdup(pidfnbuf);
}
/* Probably was saved above already, but better safe than sorry */
upslogx(LOG_WARNING, "Running as foreground process, but saving a PID file anyway");
writepid(pidfn);
break;
Expand Down
Loading

0 comments on commit aa25c9f

Please sign in to comment.