From 4c3e4049b08799094a64dac289a48deef4d3d916 Mon Sep 17 00:00:00 2001

From: Klaus Wenninger <klaus.wenninger@aon.at>

Date: Fri, 24 Jul 2020 14:31:01 +0200

Subject: [PATCH] Fix: sbd-cluster: match qdevice-sync_timeout against

 wd-timeout

---

 configure.ac      |  13 +++

 src/sbd-cluster.c | 252 +++++++++++++++++++++++++++++++++++++++++++++---------

 2 files changed, 223 insertions(+), 42 deletions(-)

diff --git a/configure.ac b/configure.ac

index 3391c5f..23547cf 100644

--- a/configure.ac

+++ b/configure.ac

@@ -109,6 +109,12 @@ AC_TEST_NO_QUORUM_POLICY(no_quorum_demote)

 dnl check for new pe-API

 AC_CHECK_FUNCS(pe_new_working_set)

+dnl check if votequorum comes with default for qdevice-sync_timeout

+AC_CHECK_DECLS([VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT],

+               HAVE_DECL_VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT=1,

+               HAVE_DECL_VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT=0,

+               [#include <corosync/votequorum.h>])

+

 if test "$missing" = "yes"; then

    AC_MSG_ERROR([Missing required libraries or functions.])

 fi

@@ -140,6 +146,13 @@ AM_CONDITIONAL(CHECK_TWO_NODE, test "$HAVE_cmap" = "1")

 AC_DEFINE_UNQUOTED(CHECK_VOTEQUORUM_HANDLE, $HAVE_votequorum, Turn on periodic checking of votequorum-handle)

 AM_CONDITIONAL(CHECK_VOTEQUORUM_HANDLE, test "$HAVE_votequorum" = "1")

+AC_DEFINE_UNQUOTED(CHECK_QDEVICE_SYNC_TIMEOUT,

+                   ($HAVE_DECL_VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT && $HAVE_cmap),

+                   Turn on checking if watchdog-timeout and qdevice-sync_timeout are matching)

+AM_CONDITIONAL(CHECK_QDEVICE_SYNC_TIMEOUT,

+               test "$HAVE_DECL_VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT" = "1" &&

+               test "$HAVE_cmap" = "1")

+

 CONFIGDIR=""

 AC_ARG_WITH(configdir,

     [  --with-configdir=DIR

diff --git a/src/sbd-cluster.c b/src/sbd-cluster.c

index 13fa580..b6c5512 100644

--- a/src/sbd-cluster.c

+++ b/src/sbd-cluster.c

@@ -33,7 +33,7 @@

 #include <crm/cluster.h>

 #include <crm/common/mainloop.h>

-#if CHECK_TWO_NODE

+#if CHECK_TWO_NODE || CHECK_QDEVICE_SYNC_TIMEOUT

 #include <glib-unix.h>

 #endif

@@ -86,11 +86,20 @@ sbd_plugin_membership_dispatch(cpg_handle_t handle,

 static votequorum_handle_t votequorum_handle = 0;

 #endif

+#if CHECK_TWO_NODE

 static bool two_node = false;

+#endif

 static bool ever_seen_both = false;

 static int cpg_membership_entries = -1;

-#if CHECK_TWO_NODE

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+#include <corosync/votequorum.h>

+static bool using_qdevice = false;

+static uint32_t qdevice_sync_timeout = /* in seconds */

+    VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT / 1000;

+#endif

+

+#if CHECK_TWO_NODE || CHECK_QDEVICE_SYNC_TIMEOUT

 #include <corosync/cmap.h>

 static cmap_handle_t cmap_handle = 0;

@@ -102,28 +111,59 @@ void

 sbd_cpg_membership_health_update()

 {

     if(cpg_membership_entries > 0) {

-        bool quorum_is_suspect =

+#if CHECK_TWO_NODE

+        bool quorum_is_suspect_two_node =

             (two_node && ever_seen_both && cpg_membership_entries == 1);

+#endif

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+        bool quorum_is_suspect_qdevice_timing =

+            using_qdevice && (qdevice_sync_timeout > timeout_watchdog);

+#endif

-        if (!quorum_is_suspect) {

+        do {

+#if CHECK_TWO_NODE

+            if (quorum_is_suspect_two_node) {

+               /* Alternative would be asking votequorum for number of votes.

+                * Using pacemaker's cpg as source for number of active nodes

+                * avoids binding to an additional library, is definitely

+                * less code to write and we wouldn't have to combine data

+                * from 3 sources (cmap, cpg & votequorum) in a potentially

+                * racy environment.

+                */

+                set_servant_health(pcmk_health_noquorum, LOG_WARNING,

+                    "Connected to %s but requires both nodes present",

+                    name_for_cluster_type(get_cluster_type())

+                    );

+                break;

+            }

+#endif

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+            if (quorum_is_suspect_qdevice_timing) {

+               /* We can't really trust quorum info as qdevice-sync_timeout

+                * makes reaction of quorum too sluggish for our

+                * watchdog-timeout.

+                */

+                set_servant_health(pcmk_health_noquorum, LOG_WARNING,

+                    "Connected to %s but quorum using qdevice is distrusted "

+                    "for SBD as qdevice-sync_timeout (%ds) > watchdog-timeout "

+                    "(%lus).",

+                    name_for_cluster_type(get_cluster_type()),

+                    qdevice_sync_timeout, timeout_watchdog

+                    );

+                break;

+            }

+#endif

             set_servant_health(pcmk_health_online, LOG_INFO,

-                           "Connected to %s (%u members)",

-                           name_for_cluster_type(get_cluster_type()),

-                           cpg_membership_entries

-                          );

-        } else {

-            /* Alternative would be asking votequorum for number of votes.

-             * Using pacemaker's cpg as source for number of active nodes

-             * avoids binding to an additional library, is definitely

-             * less code to write and we wouldn't have to combine data

-             * from 3 sources (cmap, cpq & votequorum) in a potentially

-             * racy environment.

-             */

-            set_servant_health(pcmk_health_noquorum, LOG_WARNING,

-                           "Connected to %s but requires both nodes present",

-                           name_for_cluster_type(get_cluster_type())

-                          );

-        }

+                "Connected to %s (%u members)%s",

+                name_for_cluster_type(get_cluster_type()),

+                cpg_membership_entries,

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+                using_qdevice?" using qdevice for quorum":""

+#else

+                ""

+#endif

+            );

+        } while (false);

         if (cpg_membership_entries > 1) {

             ever_seen_both = true;

@@ -146,7 +186,7 @@ sbd_cpg_membership_dispatch(cpg_handle_t handle,

     notify_parent();

 }

-#if CHECK_TWO_NODE

+#if CHECK_TWO_NODE || CHECK_QDEVICE_SYNC_TIMEOUT

 static void sbd_cmap_notify_fn(

     cmap_handle_t cmap_handle,

     cmap_track_handle_t cmap_track_handle,

@@ -156,21 +196,99 @@ static void sbd_cmap_notify_fn(

     struct cmap_notify_value old_val,

     void *user_data)

 {

-    if (new_val.type == CMAP_VALUETYPE_UINT8) {

-        switch (event) {

-            case CMAP_TRACK_ADD:

-            case CMAP_TRACK_MODIFY:

-                two_node = *((uint8_t *) new_val.data);

-                break;

-            case CMAP_TRACK_DELETE:

-                two_node = false;

-                break;

-            default:

-                return;

-        }

-        sbd_cpg_membership_health_update();

-        notify_parent();

+    switch (event) {

+        case CMAP_TRACK_ADD:

+        case CMAP_TRACK_MODIFY:

+            switch (new_val.type) {

+                case CMAP_VALUETYPE_UINT8:

+#if CHECK_TWO_NODE

+                    if (!strcmp(key_name, "quorum.two_node")) {

+                        two_node = *((uint8_t *) new_val.data);

+                    } else {

+                        return;

+                    }

+                    break;

+#else

+                    return;

+#endif

+                case CMAP_VALUETYPE_STRING:

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+                    if (!strcmp(key_name, "quorum.device.model")) {

+                        using_qdevice =

+                            ((new_val.data) && strlen((char *) new_val.data));

+                    } else {

+                        return;

+                    }

+                    break;

+#else

+                    return;

+#endif

+                case CMAP_VALUETYPE_UINT32:

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+                    if (!strcmp(key_name, "quorum.device.sync_timeout")) {

+                        if (new_val.data) {

+                            qdevice_sync_timeout =

+                                *((uint32_t *) new_val.data) / 1000;

+                        } else {

+                            qdevice_sync_timeout =

+                                VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT / 1000;

+                        }

+                    } else {

+                        return;

+                    }

+                    break;

+#else

+                    return;

+#endif

+                default:

+                    return;

+            }

+            break;

+        case CMAP_TRACK_DELETE:

+            switch (new_val.type) {

+                case CMAP_VALUETYPE_UINT8:

+#if CHECK_TWO_NODE

+                    if (!strcmp(key_name, "quorum.two_node")) {

+                        two_node = false;

+                    } else {

+                        return;

+                    }

+                    break;

+#else

+                    return;

+#endif

+                case CMAP_VALUETYPE_STRING:

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+                    if (!strcmp(key_name, "quorum.device.model")) {

+                        using_qdevice = false;

+                    } else {

+                        return;

+                    }

+                    break;

+#else

+                    return;

+#endif

+                case CMAP_VALUETYPE_UINT32:

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+                    if (!strcmp(key_name, "quorum.device.sync_timeout")) {

+                        qdevice_sync_timeout =

+                            VOTEQUORUM_QDEVICE_DEFAULT_SYNC_TIMEOUT / 1000;

+                    } else {

+                        return;

+                    }

+                    break;

+#else

+                    return;

+#endif

+                default:

+                    return;

+            }

+            break;

+        default:

+            return;

     }

+    sbd_cpg_membership_health_update();

+    notify_parent();

 }

 static gboolean

@@ -200,9 +318,14 @@ cmap_destroy(void)

 }

 static gboolean

-sbd_get_two_node(void)

+verify_against_cmap_config(void)

 {

+#if CHECK_TWO_NODE

     uint8_t two_node_u8 = 0;

+#endif

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+    char *qdevice_model = NULL;

+#endif

     int cmap_fd;

     if (!track_handle) {

@@ -211,12 +334,31 @@ sbd_get_two_node(void)

             goto out;

         }

+#if CHECK_TWO_NODE

         if (cmap_track_add(cmap_handle, "quorum.two_node",

                             CMAP_TRACK_DELETE|CMAP_TRACK_MODIFY|CMAP_TRACK_ADD,

                             sbd_cmap_notify_fn, NULL, &track_handle) != CS_OK) {

             cl_log(LOG_WARNING, "Failed adding CMAP tracker for 2Node-mode\n");

             goto out;

         }

+#endif

+

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+        if (cmap_track_add(cmap_handle, "quorum.device.model",

+                            CMAP_TRACK_DELETE|CMAP_TRACK_MODIFY|CMAP_TRACK_ADD,

+                            sbd_cmap_notify_fn, NULL, &track_handle) != CS_OK) {

+            cl_log(LOG_WARNING, "Failed adding CMAP tracker for qdevice-model\n");

+            goto out;

+        }

+

+        if (cmap_track_add(cmap_handle, "quorum.device.sync_timeout",

+                            CMAP_TRACK_DELETE|CMAP_TRACK_MODIFY|CMAP_TRACK_ADD,

+                            sbd_cmap_notify_fn, NULL, &track_handle) != CS_OK) {

+            cl_log(LOG_WARNING,

+                   "Failed adding CMAP tracker for qdevice-sync_timeout\n");

+            goto out;

+        }

+#endif

         /* add the tracker to mainloop */

         if (cmap_fd_get(cmap_handle, &cmap_fd) != CS_OK) {

@@ -232,13 +374,39 @@ sbd_get_two_node(void)

         g_source_attach(cmap_source, NULL);

     }

-    if (cmap_get_uint8(cmap_handle, "quorum.two_node", &two_node_u8) == CS_OK) {

+#if CHECK_TWO_NODE

+    if (cmap_get_uint8(cmap_handle, "quorum.two_node", &two_node_u8)

+            == CS_OK) {

         cl_log(two_node_u8? LOG_NOTICE : LOG_INFO,

                "Corosync is%s in 2Node-mode", two_node_u8?"":" not");

         two_node = two_node_u8;

     } else {

         cl_log(LOG_INFO, "quorum.two_node not present in cmap\n");

     }

+#endif

+

+#if CHECK_QDEVICE_SYNC_TIMEOUT

+    if (cmap_get_string(cmap_handle, "quorum.device.model",

+                        &qdevice_model) == CS_OK) {

+        using_qdevice = qdevice_model && strlen(qdevice_model);

+        cl_log(using_qdevice? LOG_NOTICE : LOG_INFO,

+               "Corosync is%s using qdevice", using_qdevice?"":" not");

+    } else {

+        cl_log(LOG_INFO, "quorum.device.model not present in cmap\n");

+    }

+

+    if (cmap_get_uint32(cmap_handle, "quorum.device.sync_timeout",

+                        &qdevice_sync_timeout) == CS_OK) {

+        qdevice_sync_timeout /= 1000;

+        cl_log(LOG_INFO,

+               "Corosync is using qdevice-sync_timeout=%ds",

+               qdevice_sync_timeout);

+    } else {

+        cl_log(LOG_INFO,

+               "quorum.device.sync_timeout not present in cmap\n");

+    }

+#endif

+

     return TRUE;

 out:

@@ -331,15 +499,15 @@ sbd_membership_connect(void)

         } else {

             cl_log(LOG_INFO, "Attempting connection to %s", name_for_cluster_type(stack));

-#if SUPPORT_COROSYNC && CHECK_TWO_NODE

-            if (sbd_get_two_node()) {

+#if SUPPORT_COROSYNC && (CHECK_TWO_NODE || CHECK_QDEVICE_SYNC_TIMEOUT)

+            if (verify_against_cmap_config()) {

 #endif

                 if(crm_cluster_connect(&cluster)) {

                     connected = true;

                 }

-#if SUPPORT_COROSYNC && CHECK_TWO_NODE

+#if SUPPORT_COROSYNC && (CHECK_TWO_NODE || CHECK_QDEVICE_SYNC_TIMEOUT)

             }

 #endif

         }

@@ -362,7 +530,7 @@ sbd_membership_destroy(gpointer user_data)

     cl_log(LOG_WARNING, "Lost connection to %s", name_for_cluster_type(get_cluster_type()));

     if (get_cluster_type() != pcmk_cluster_unknown) {

-#if SUPPORT_COROSYNC && CHECK_TWO_NODE

+#if SUPPORT_COROSYNC && (CHECK_TWO_NODE || CHECK_QDEVICE_SYNC_TIMEOUT)

         cmap_destroy();

 #endif

     }

-- 

1.8.3.1