[pve-devel] [PATCH ha-manager v3 0/6] watchdog-mux: sync log to disk before and after expiring

[pve-devel] [PATCH ha-manager v3 0/6] watchdog-mux: sync log to disk before and after expiring

[Maximiliano Sandoval]

Without a clear-cut message in the log, it is very hard to provide a definitive
answer to whether a host fenced or not. In some cases the journal on the disk
can be missing up to 2 minutes since its last logged entry and the time where
another node detects the corosync link is down, with such a gap, the fenced node
would not even record that it lost conenction and it is not possible to
fully-determine if the node was fenced or not.

This series:
 - adds a second warning 10 seconds before the watchdog expires
 - syncs the journal to disk after the warning was issued
 - syncs the journal to disk after the watchdog expires
 - allows for watchdog-mux to exit(EXIT_SUCCESS) before the fence (new in v3)

Differences from v2:
 - Instead of explicitly adding a call to sync the journal after we disable
   updates, we help the process breaking out of the loop, allowing it to reach
   the code that would call the sync and then exit()

Differences from v1:
 - Define the warning cuttoff based on the 60 second timeout
 - Change log messages and constant names
 - When not immediately fencing, run journal sync in double fork

Maximiliano Sandoval (6):
  watchdog-mux: Use #define for 60s timeout
  watchdog-mux: split if block in two if blocks
  watchdog-mux: warn when about to expire
  watchdog-mux: sync journal right after fence warning
  watchdog-mux: break out of loop when updates are disabled
  watchdog-mux: Remove wrapping if guard

 src/watchdog-mux.c | 61 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 12 deletions(-)

-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager v3 1/6] watchdog-mux: Use #define for 60s timeout

[Maximiliano Sandoval]

This change allows to have a second constant defined in terms of this
one.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/watchdog-mux.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/watchdog-mux.c b/src/watchdog-mux.c
index e324c20..d38116b 100644
--- a/src/watchdog-mux.c
+++ b/src/watchdog-mux.c
@@ -29,9 +29,10 @@
 
 #define JOURNALCTL_BIN "/bin/journalctl"
 
+#define CLIENT_WATCHDOG_TIMEOUT 60
+
 int watchdog_fd = -1;
 int watchdog_timeout = 10;
-int client_watchdog_timeout = 60;
 int update_watchdog = 1;
 
 typedef struct {
@@ -234,7 +235,7 @@ int main(void) {
                 time_t ctime = time(NULL);
                 for (i = 0; i < MAX_CLIENTS; i++) {
                     if (client_list[i].fd != 0 && client_list[i].time != 0 &&
-                        ((ctime - client_list[i].time) > client_watchdog_timeout)) {
+                        ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT)) {
                         update_watchdog = 0;
                         fprintf(stderr, "client watchdog expired - disable watchdog updates\n");
                     }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager v3 2/6] watchdog-mux: split if block in two if blocks

[Maximiliano Sandoval]

The sole purpose of this commit is to make the following commit's diff
easier to read.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/watchdog-mux.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/watchdog-mux.c b/src/watchdog-mux.c
index d38116b..2b8cebf 100644
--- a/src/watchdog-mux.c
+++ b/src/watchdog-mux.c
@@ -234,10 +234,11 @@ int main(void) {
                 int i;
                 time_t ctime = time(NULL);
                 for (i = 0; i < MAX_CLIENTS; i++) {
-                    if (client_list[i].fd != 0 && client_list[i].time != 0 &&
-                        ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT)) {
-                        update_watchdog = 0;
-                        fprintf(stderr, "client watchdog expired - disable watchdog updates\n");
+                    if (client_list[i].fd != 0 && client_list[i].time != 0) {
+                        if ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT) {
+                            update_watchdog = 0;
+                            fprintf(stderr, "client watchdog expired - disable watchdog updates\n");
+                        }
                     }
                 }
             }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager v3 3/6] watchdog-mux: warn when about to expire

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/watchdog-mux.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/watchdog-mux.c b/src/watchdog-mux.c
index 2b8cebf..0518e86 100644
--- a/src/watchdog-mux.c
+++ b/src/watchdog-mux.c
@@ -30,15 +30,23 @@
 #define JOURNALCTL_BIN "/bin/journalctl"
 
 #define CLIENT_WATCHDOG_TIMEOUT 60
+#define CLIENT_WATCHDOG_TIMEOUT_WARNING (CLIENT_WATCHDOG_TIMEOUT - 10)
 
 int watchdog_fd = -1;
 int watchdog_timeout = 10;
 int update_watchdog = 1;
 
+enum warning_state_t {
+    NONE,
+    WARNING_ISSUED,
+    FENCE_AVERTED,
+};
+
 typedef struct {
     int fd;
     time_t time;
     int magic_close;
+    enum warning_state_t warning_state;
 } wd_client_t;
 
 #define MAX_CLIENTS 100
@@ -53,6 +61,7 @@ static wd_client_t *alloc_client(int fd, time_t time) {
             client_list[i].fd = fd;
             client_list[i].time = time;
             client_list[i].magic_close = 0;
+            client_list[i].warning_state = NONE;
             return &client_list[i];
         }
     }
@@ -235,6 +244,18 @@ int main(void) {
                 time_t ctime = time(NULL);
                 for (i = 0; i < MAX_CLIENTS; i++) {
                     if (client_list[i].fd != 0 && client_list[i].time != 0) {
+                        if (client_list[i].warning_state == WARNING_ISSUED &&
+                            (ctime - client_list[i].time) <= CLIENT_WATCHDOG_TIMEOUT_WARNING) {
+                            client_list[i].warning_state = FENCE_AVERTED;
+                            fprintf(stderr, "client watchdog was updated before expiring\n");
+                        }
+
+                        if (client_list[i].warning_state != WARNING_ISSUED &&
+                            (ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT_WARNING) {
+                            client_list[i].warning_state = WARNING_ISSUED;
+                            fprintf(stderr, "client watchdog is about to expire\n");
+                        }
+
                         if ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT) {
                             update_watchdog = 0;
                             fprintf(stderr, "client watchdog expired - disable watchdog updates\n");
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager v3 4/6] watchdog-mux: sync journal right after fence warning

[Maximiliano Sandoval]

Since this journal entry can be logged multiple times in the lifespan on
the process, we double fork to prevent accumulating zombie processes.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/watchdog-mux.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/watchdog-mux.c b/src/watchdog-mux.c
index 0518e86..11571ec 100644
--- a/src/watchdog-mux.c
+++ b/src/watchdog-mux.c
@@ -12,6 +12,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/un.h>
+#include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -116,6 +117,22 @@ static void sync_journal_unsafe(void) {
     }
 }
 
+// Like sync_journal_unsafe but we double fork so we don't leave trailing zombie
+// processes.
+static void sync_journal_in_fork(void) {
+    pid_t child = fork();
+    if (child == 0) {
+        child = fork();
+        if (child == 0) {
+            execl(JOURNALCTL_BIN, JOURNALCTL_BIN, "--sync", NULL);
+            exit(-1);
+        }
+        exit(0);
+    } else if (child > 0) {
+        wait(NULL);
+    }
+}
+
 int main(void) {
     struct sockaddr_un my_addr, peer_addr;
     socklen_t peer_addr_size;
@@ -254,6 +271,7 @@ int main(void) {
                             (ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT_WARNING) {
                             client_list[i].warning_state = WARNING_ISSUED;
                             fprintf(stderr, "client watchdog is about to expire\n");
+                            sync_journal_in_fork ();
                         }
 
                         if ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager v3 5/6] watchdog-mux: break out of loop when updates are disabled

[Maximiliano Sandoval]

Without this check, if nfds is zero, the `continue` statement right
before of the `break` will prevent from breaking out of the loop and
existing the process.

If a node does not have corosync quorum, then neither the lrm or crm
will update watchdog-mux and this the epoll_wait will timeout, hence
nfds is zero in this case. The only case where one could break free from
the loop without this commit is if the watchdog updates are stopped, but
soon after we regain quorum.

In the next commit, the if guards for update_watchdog in the inner guard
will be removed, this is done to simplify the review process.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/watchdog-mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/watchdog-mux.c b/src/watchdog-mux.c
index 11571ec..4e2bdc6 100644
--- a/src/watchdog-mux.c
+++ b/src/watchdog-mux.c
@@ -253,7 +253,7 @@ int main(void) {
             goto err;
         }
 
-        if (nfds == 0) { // timeout
+        if (nfds == 0 && update_watchdog) { // timeout
 
             // check for timeouts
             if (update_watchdog) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH ha-manager v3 6/6] watchdog-mux: Remove wrapping if guard

[Maximiliano Sandoval]

We check for the same condition in the wrapping if-block added in the
previous commit making these redundant.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/watchdog-mux.c | 44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/src/watchdog-mux.c b/src/watchdog-mux.c
index 4e2bdc6..889cea2 100644
--- a/src/watchdog-mux.c
+++ b/src/watchdog-mux.c
@@ -256,36 +256,32 @@ int main(void) {
         if (nfds == 0 && update_watchdog) { // timeout
 
             // check for timeouts
-            if (update_watchdog) {
-                int i;
-                time_t ctime = time(NULL);
-                for (i = 0; i < MAX_CLIENTS; i++) {
-                    if (client_list[i].fd != 0 && client_list[i].time != 0) {
-                        if (client_list[i].warning_state == WARNING_ISSUED &&
-                            (ctime - client_list[i].time) <= CLIENT_WATCHDOG_TIMEOUT_WARNING) {
-                            client_list[i].warning_state = FENCE_AVERTED;
-                            fprintf(stderr, "client watchdog was updated before expiring\n");
-                        }
+            int i;
+            time_t ctime = time(NULL);
+            for (i = 0; i < MAX_CLIENTS; i++) {
+                if (client_list[i].fd != 0 && client_list[i].time != 0) {
+                    if (client_list[i].warning_state == WARNING_ISSUED &&
+                        (ctime - client_list[i].time) <= CLIENT_WATCHDOG_TIMEOUT_WARNING) {
+                        client_list[i].warning_state = FENCE_AVERTED;
+                        fprintf(stderr, "client watchdog was updated before expiring\n");
+                    }
 
-                        if (client_list[i].warning_state != WARNING_ISSUED &&
-                            (ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT_WARNING) {
-                            client_list[i].warning_state = WARNING_ISSUED;
-                            fprintf(stderr, "client watchdog is about to expire\n");
-                            sync_journal_in_fork ();
-                        }
+                    if (client_list[i].warning_state != WARNING_ISSUED &&
+                        (ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT_WARNING) {
+                        client_list[i].warning_state = WARNING_ISSUED;
+                        fprintf(stderr, "client watchdog is about to expire\n");
+                        sync_journal_in_fork ();
+                    }
 
-                        if ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT) {
-                            update_watchdog = 0;
-                            fprintf(stderr, "client watchdog expired - disable watchdog updates\n");
-                        }
+                    if ((ctime - client_list[i].time) > CLIENT_WATCHDOG_TIMEOUT) {
+                        update_watchdog = 0;
+                        fprintf(stderr, "client watchdog expired - disable watchdog updates\n");
                     }
                 }
             }
 
-            if (update_watchdog) {
-                if (ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0) == -1) {
-                    perror("watchdog update failed");
-                }
+            if (ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0) == -1) {
+                perror("watchdog update failed");
             }
 
             continue;
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pbs-devel] applied: [pve-devel] [PATCH ha-manager v3 0/6] watchdog-mux: sync log to disk before and after expiring

[Thomas Lamprecht]

On Fri, 04 Jul 2025 15:38:56 +0200, Maximiliano Sandoval wrote:
> Without a clear-cut message in the log, it is very hard to provide a definitive
> answer to whether a host fenced or not. In some cases the journal on the disk
> can be missing up to 2 minutes since its last logged entry and the time where
> another node detects the corosync link is down, with such a gap, the fenced node
> would not even record that it lost conenction and it is not possible to
> fully-determine if the node was fenced or not.
> 
> [...]

Applied, thanks!

[1/6] watchdog-mux: Use #define for 60s timeout
[2/6] watchdog-mux: split if block in two if blocks
[3/6] watchdog-mux: warn when about to expire
[4/6] watchdog-mux: sync journal right after fence warning
[5/6] watchdog-mux: break out of loop when updates are disabled
[6/6] watchdog-mux: Remove wrapping if guard


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pve-devel] applied: [PATCH ha-manager v3 0/6] watchdog-mux: sync log to disk before and after expiring

[Thomas Lamprecht]

On Fri, 04 Jul 2025 15:38:56 +0200, Maximiliano Sandoval wrote:
> Without a clear-cut message in the log, it is very hard to provide a definitive
> answer to whether a host fenced or not. In some cases the journal on the disk
> can be missing up to 2 minutes since its last logged entry and the time where
> another node detects the corosync link is down, with such a gap, the fenced node
> would not even record that it lost conenction and it is not possible to
> fully-determine if the node was fenced or not.
> 
> [...]

Applied, thanks!

[1/6] watchdog-mux: Use #define for 60s timeout
[2/6] watchdog-mux: split if block in two if blocks
[3/6] watchdog-mux: warn when about to expire
[4/6] watchdog-mux: sync journal right after fence warning
[5/6] watchdog-mux: break out of loop when updates are disabled
[6/6] watchdog-mux: Remove wrapping if guard


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel