watchdog: passing 0 to watchdog_setup now closes the watchdog

Passing 0 meant "disable the watchdog although still kept it opened". However
this case didn't seem to be useful especially since PID1 closes the device if
it is passed the nul timeout.

Hence let's change the meaning of watchdog_setup(0) to match PID1's behavior
which allows to simplify the code a bit.

Hence this patch also drops enable_watchdog().

diff --git a/src/core/main.c b/src/core/main.c
index ff7f189370ded18470c67adce52db1d409f3fcc7..8792ef88fc5c8f0e7bb0380ab2f5c2b6bc9f5638 100644 (file)
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1483,9 +1483,9 @@ static int become_shutdown(
         };
 
         _cleanup_strv_free_ char **env_block = NULL;
+        usec_t watchdog_timer = 0;
         size_t pos = 7;
         int r;
-        usec_t watchdog_timer = 0;
 
         assert(shutdown_verb);
         assert(!command_line[pos]);
@@ -1534,19 +1534,16 @@ static int become_shutdown(
         else if (streq(shutdown_verb, "kexec"))
                 watchdog_timer = arg_kexec_watchdog;
 
-        if (timestamp_is_set(watchdog_timer)) {
-                /* If we reboot or kexec let's set the shutdown watchdog and tell the shutdown binary to
-                 * repeatedly ping it */
-                r = watchdog_setup(watchdog_timer);
-                watchdog_close(r < 0);
+        /* If we reboot or kexec let's set the shutdown watchdog and tell the
+         * shutdown binary to repeatedly ping it */
+        r = watchdog_setup(watchdog_timer);
+        watchdog_close(r < 0);
 
-                /* Tell the binary how often to ping, ignore failure */
-                (void) strv_extendf(&env_block, "WATCHDOG_USEC="USEC_FMT, watchdog_timer);
+        /* Tell the binary how often to ping, ignore failure */
+        (void) strv_extendf(&env_block, "WATCHDOG_USEC="USEC_FMT, watchdog_timer);
 
-                if (arg_watchdog_device)
-                        (void) strv_extendf(&env_block, "WATCHDOG_DEVICE=%s", arg_watchdog_device);
-        } else
-                watchdog_close(true);
+        if (arg_watchdog_device)
+                (void) strv_extendf(&env_block, "WATCHDOG_DEVICE=%s", arg_watchdog_device);
 
         /* Avoid the creation of new processes forked by the kernel; at this
          * point, we will not listen to the signals anyway */

diff --git a/src/core/manager.c b/src/core/manager.c
index 2e7db509d1fc647f5b533dc576f4fa6757d71bd6..b231e36cbd130efe78d96b5996871a625bae8e6a 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3203,12 +3203,8 @@ void manager_set_watchdog(Manager *m, WatchdogType t, usec_t timeout) {
                 return;
 
         if (t == WATCHDOG_RUNTIME)
-                if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME])) {
-                        if (timestamp_is_set(timeout))
-                                (void) watchdog_setup(timeout);
-                        else
-                                watchdog_close(true);
-                }
+                if (!timestamp_is_set(m->watchdog_overridden[WATCHDOG_RUNTIME]))
+                        (void) watchdog_setup(timeout);
 
         m->watchdog[t] = timeout;
 }
@@ -3226,10 +3222,7 @@ int manager_override_watchdog(Manager *m, WatchdogType t, usec_t timeout) {
         if (t == WATCHDOG_RUNTIME) {
                 usec_t usec = timestamp_is_set(timeout) ? timeout : m->watchdog[t];
 
-                if (timestamp_is_set(usec))
-                        (void) watchdog_setup(usec);
-                else
-                        watchdog_close(true);
+                (void) watchdog_setup(usec);
         }
 
         m->watchdog_overridden[t] = timeout;

diff --git a/src/shared/watchdog.c b/src/shared/watchdog.c
index 1b5239888f1d011506f066c1f6917e78be8fd3e3..b6fddd193f5690085136d4092a21ed1076e4f81b 100644 (file)
--- a/src/shared/watchdog.c
+++ b/src/shared/watchdog.c
@@ -16,7 +16,7 @@
 
 static int watchdog_fd = -1;
 static char *watchdog_device;
-static usec_t watchdog_timeout; /* USEC_INFINITY → don't change timeout */
+static usec_t watchdog_timeout; /* 0 → close device and USEC_INFINITY → don't change timeout */
 static usec_t watchdog_last_ping = USEC_INFINITY;
 
 static int watchdog_set_enable(bool enable) {
@@ -87,12 +87,11 @@ static int watchdog_ping_now(void) {
 static int update_timeout(void) {
         int r;
 
+        assert(watchdog_timeout > 0);
+
         if (watchdog_fd < 0)
                 return 0;
 
-        if (watchdog_timeout == 0)
-                return watchdog_set_enable(false);
-
         if (watchdog_timeout != USEC_INFINITY) {
                 r = watchdog_set_timeout();
                 if (r < 0) {
@@ -158,8 +157,19 @@ int watchdog_set_device(const char *path) {
 
 int watchdog_setup(usec_t timeout) {
 
+        /* timeout=0 closes the device whereas passing timeout=USEC_INFINITY
+         * opens it (if needed) without configuring any particular timeout and
+         * thus reuses the programmed value (therefore it's a nop if the device
+         * is already opened).
+         */
+
+        if (timeout == 0) {
+                watchdog_close(true);
+                return 0;
+        }
+
         /* Let's shortcut duplicated requests */
-        if (watchdog_fd >= 0 && watchdog_timeout == timeout)
+        if (watchdog_fd >= 0 && (timeout == watchdog_timeout || timeout == USEC_INFINITY))
                 return 0;
 
         /* Initialize the watchdog timeout with the caller value. This value is
@@ -213,6 +223,11 @@ int watchdog_ping(void) {
 }
 
 void watchdog_close(bool disarm) {
+
+        /* Once closed, pinging the device becomes a NOP and we request a new
+         * call to watchdog_setup() to open the device again. */
+        watchdog_timeout = 0;
+
         if (watchdog_fd < 0)
                 return;
 
@@ -234,8 +249,4 @@ void watchdog_close(bool disarm) {
         }
 
         watchdog_fd = safe_close(watchdog_fd);
-
-        /* Once closed, pinging the device becomes a NOP and we request a new
-         * call to watchdog_setup() to open the device again. */
-        watchdog_timeout = 0;
 }