From 3d10dad9a555aae040d8473edfe31a4e4279c066 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 11 Nov 2021 12:34:03 -0600

Subject: [PATCH 01/19] Refactor: libcrmcommon: add internal API for checking

 for fencing action

The naming is a little awkward -- "fencing action" has multiple meanings

depending on the context. It can refer to fencer API requests, fence device

actions, fence agent actions, or just those actions that fence a node (off and

reboot).

This new function pcmk__is_fencing_action() uses the last meaning, so it does

*not* return true for unfencing ("on" actions).

---

 include/crm/common/internal.h |  1 +

 lib/common/operations.c       | 14 ++++++++++++++

 2 files changed, 15 insertions(+)

diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h

index a35c5769a..694fc6cd4 100644

--- a/include/crm/common/internal.h

+++ b/include/crm/common/internal.h

@@ -218,6 +218,7 @@ char *pcmk__notify_key(const char *rsc_id, const char *notify_type,

 char *pcmk__transition_key(int transition_id, int action_id, int target_rc,

                            const char *node);

 void pcmk__filter_op_for_digest(xmlNode *param_set);

+bool pcmk__is_fencing_action(const char *action);

 // bitwise arithmetic utilities

diff --git a/lib/common/operations.c b/lib/common/operations.c

index aa7106ce6..366c18970 100644

--- a/lib/common/operations.c

+++ b/lib/common/operations.c

@@ -523,3 +523,17 @@ crm_op_needs_metadata(const char *rsc_class, const char *op)

                             CRMD_ACTION_MIGRATE, CRMD_ACTION_MIGRATED,

                             CRMD_ACTION_NOTIFY, NULL);

 }

+

+/*!

+ * \internal

+ * \brief Check whether an action name is for a fencing action

+ *

+ * \param[in] action  Action name to check

+ *

+ * \return true if \p action is "off", "reboot", or "poweroff", otherwise false

+ */

+bool

+pcmk__is_fencing_action(const char *action)

+{

+    return pcmk__str_any_of(action, "off", "reboot", "poweroff", NULL);

+}

-- 

2.27.0

From 86ac00fb3e99d79ca2c442ae1670fe850146f734 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 11 Nov 2021 12:38:58 -0600

Subject: [PATCH 02/19] Low: fencer,scheduler: compare fence action names

 case-sensitively

Use the new convenience function pcmk__is_fencing_action() to check whether

an action name is a fencing action ("off", "reboot", or "poweroff"). This

changes the behavior from case-insensitive to case-sensitive, which is more

appropriate (the case-insensitivity was inherited from lazy use of the old

safe_str_eq() function which was always case-insensitive).

---

 daemons/fenced/fenced_commands.c    | 6 +++---

 daemons/fenced/fenced_remote.c      | 2 +-

 lib/pacemaker/pcmk_graph_producer.c | 2 +-

 lib/pengine/common.c                | 8 +-------

 4 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c

index 63bfad3a9..46c840f2a 100644

--- a/daemons/fenced/fenced_commands.c

+++ b/daemons/fenced/fenced_commands.c

@@ -128,7 +128,7 @@ get_action_delay_max(stonith_device_t * device, const char * action)

     const char *value = NULL;

     int delay_max = 0;

-    if (!pcmk__strcase_any_of(action, "off", "reboot", NULL)) {

+    if (!pcmk__is_fencing_action(action)) {

         return 0;

     }

@@ -146,7 +146,7 @@ get_action_delay_base(stonith_device_t *device, const char *action, const char *

     char *hash_value = NULL;

     int delay_base = 0;

-    if (!pcmk__strcase_any_of(action, "off", "reboot", NULL)) {

+    if (!pcmk__is_fencing_action(action)) {

         return 0;

     }

@@ -448,7 +448,7 @@ stonith_device_execute(stonith_device_t * device)

     if (pcmk__str_any_of(device->agent, STONITH_WATCHDOG_AGENT,

                          STONITH_WATCHDOG_AGENT_INTERNAL, NULL)) {

-        if (pcmk__strcase_any_of(cmd->action, "reboot", "off", NULL)) {

+        if (pcmk__is_fencing_action(cmd->action)) {

             if (node_does_watchdog_fencing(stonith_our_uname)) {

                 pcmk__panic(__func__);

                 goto done;

diff --git a/daemons/fenced/fenced_remote.c b/daemons/fenced/fenced_remote.c

index 963433bf3..358ea3aa7 100644

--- a/daemons/fenced/fenced_remote.c

+++ b/daemons/fenced/fenced_remote.c

@@ -1758,7 +1758,7 @@ all_topology_devices_found(remote_fencing_op_t * op)

     if (!tp) {

         return FALSE;

     }

-    if (pcmk__strcase_any_of(op->action, "off", "reboot", NULL)) {

+    if (pcmk__is_fencing_action(op->action)) {

         /* Don't count the devices on the target node if we are killing

          * the target node. */

         skip_target = TRUE;

diff --git a/lib/pacemaker/pcmk_graph_producer.c b/lib/pacemaker/pcmk_graph_producer.c

index ffcbd1274..5bec9d8ce 100644

--- a/lib/pacemaker/pcmk_graph_producer.c

+++ b/lib/pacemaker/pcmk_graph_producer.c

@@ -721,7 +721,7 @@ add_downed_nodes(xmlNode *xml, const pe_action_t *action,

         /* Fencing makes the action's node and any hosted guest nodes down */

         const char *fence = g_hash_table_lookup(action->meta, "stonith_action");

-        if (pcmk__strcase_any_of(fence, "off", "reboot", NULL)) {

+        if (pcmk__is_fencing_action(fence)) {

             xmlNode *downed = create_xml_node(xml, XML_GRAPH_TAG_DOWNED);

             add_node_to_xml_by_id(action->node->details->id, downed);

             pe_foreach_guest_node(data_set, action->node, add_node_to_xml, downed);

diff --git a/lib/pengine/common.c b/lib/pengine/common.c

index 236fc26b1..fe4223816 100644

--- a/lib/pengine/common.c

+++ b/lib/pengine/common.c

@@ -27,12 +27,6 @@ check_health(const char *value)

                            "migrate-on-red", NULL);

 }

-static bool

-check_stonith_action(const char *value)

-{

-    return pcmk__strcase_any_of(value, "reboot", "poweroff", "off", NULL);

-}

-

 static bool

 check_placement_strategy(const char *value)

 {

@@ -114,7 +108,7 @@ static pcmk__cluster_option_t pe_opts[] = {

     },

     {

         "stonith-action", NULL, "select", "reboot, off, poweroff",

-        "reboot", check_stonith_action,

+        "reboot", pcmk__is_fencing_action,

         "Action to send to fence device when a node needs to be fenced "

             "(\"poweroff\" is a deprecated alias for \"off\")",

         NULL

-- 

2.27.0

From c8f6e8a04c4fa4271db817af0a23aa941c9d7689 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 12 Nov 2021 17:42:21 -0600

Subject: [PATCH 03/19] Refactor: fencing: rename type for peer query replies

st_query_result_t contains the device information parsed from a peer's query

reply, but the name could easily be confused with the actual success/failure

result of the query action itself. Rename it to peer_device_info_t.

---

 daemons/fenced/fenced_remote.c | 103 +++++++++++++++++----------------

 1 file changed, 52 insertions(+), 51 deletions(-)

diff --git a/daemons/fenced/fenced_remote.c b/daemons/fenced/fenced_remote.c

index 358ea3aa7..9e2f62804 100644

--- a/daemons/fenced/fenced_remote.c

+++ b/daemons/fenced/fenced_remote.c

@@ -41,7 +41,7 @@

 /* When one fencer queries its peers for devices able to handle a fencing

  * request, each peer will reply with a list of such devices available to it.

- * Each reply will be parsed into a st_query_result_t, with each device's

+ * Each reply will be parsed into a peer_device_info_t, with each device's

  * information kept in a device_properties_t.

  */

@@ -72,18 +72,19 @@ typedef struct st_query_result_s {

     int ndevices;

     /* Devices available to this host that are capable of fencing the target */

     GHashTable *devices;

-} st_query_result_t;

+} peer_device_info_t;

 GHashTable *stonith_remote_op_list = NULL;

-void call_remote_stonith(remote_fencing_op_t * op, st_query_result_t * peer, int rc);

+void call_remote_stonith(remote_fencing_op_t *op, peer_device_info_t *peer,

+                         int rc);

 static void remote_op_done(remote_fencing_op_t * op, xmlNode * data, int rc, int dup);

 extern xmlNode *stonith_create_op(int call_id, const char *token, const char *op, xmlNode * data,

                                   int call_options);

 static void report_timeout_period(remote_fencing_op_t * op, int op_timeout);

 static int get_op_total_timeout(const remote_fencing_op_t *op,

-                                const st_query_result_t *chosen_peer);

+                                const peer_device_info_t *chosen_peer);

 static gint

 sort_strings(gconstpointer a, gconstpointer b)

@@ -95,7 +96,7 @@ static void

 free_remote_query(gpointer data)

 {

     if (data) {

-        st_query_result_t *query = data;

+        peer_device_info_t *query = data;

         crm_trace("Free'ing query result from %s", query->host);

         g_hash_table_destroy(query->devices);

@@ -150,8 +151,8 @@ count_peer_device(gpointer key, gpointer value, gpointer user_data)

  * \return Number of devices available to peer that were not already executed

  */

 static int

-count_peer_devices(const remote_fencing_op_t *op, const st_query_result_t *peer,

-                   gboolean verified_only)

+count_peer_devices(const remote_fencing_op_t *op,

+                   const peer_device_info_t *peer, gboolean verified_only)

 {

     struct peer_count_data data;

@@ -175,7 +176,7 @@ count_peer_devices(const remote_fencing_op_t *op, const st_query_result_t *peer,

  * \return Device properties if found, NULL otherwise

  */

 static device_properties_t *

-find_peer_device(const remote_fencing_op_t *op, const st_query_result_t *peer,

+find_peer_device(const remote_fencing_op_t *op, const peer_device_info_t *peer,

                  const char *device)

 {

     device_properties_t *props = g_hash_table_lookup(peer->devices, device);

@@ -196,7 +197,7 @@ find_peer_device(const remote_fencing_op_t *op, const st_query_result_t *peer,

  * \return TRUE if device was found and marked, FALSE otherwise

  */

 static gboolean

-grab_peer_device(const remote_fencing_op_t *op, st_query_result_t *peer,

+grab_peer_device(const remote_fencing_op_t *op, peer_device_info_t *peer,

                  const char *device, gboolean verified_devices_only)

 {

     device_properties_t *props = find_peer_device(op, peer, device);

@@ -1216,7 +1217,7 @@ enum find_best_peer_options {

     FIND_PEER_VERIFIED_ONLY = 0x0004,

 };

-static st_query_result_t *

+static peer_device_info_t *

 find_best_peer(const char *device, remote_fencing_op_t * op, enum find_best_peer_options options)

 {

     GList *iter = NULL;

@@ -1227,7 +1228,7 @@ find_best_peer(const char *device, remote_fencing_op_t * op, enum find_best_peer

     }

     for (iter = op->query_results; iter != NULL; iter = iter->next) {

-        st_query_result_t *peer = iter->data;

+        peer_device_info_t *peer = iter->data;

         crm_trace("Testing result from %s targeting %s with %d device%s: %d %x",

                   peer->host, op->target, peer->ndevices,

@@ -1257,11 +1258,11 @@ find_best_peer(const char *device, remote_fencing_op_t * op, enum find_best_peer

     return NULL;

 }

-static st_query_result_t *

+static peer_device_info_t *

 stonith_choose_peer(remote_fencing_op_t * op)

 {

     const char *device = NULL;

-    st_query_result_t *peer = NULL;

+    peer_device_info_t *peer = NULL;

     uint32_t active = fencing_active_peers();

     do {

@@ -1317,8 +1318,8 @@ stonith_choose_peer(remote_fencing_op_t * op)

 }

 static int

-get_device_timeout(const remote_fencing_op_t *op, const st_query_result_t *peer,

-                   const char *device)

+get_device_timeout(const remote_fencing_op_t *op,

+                   const peer_device_info_t *peer, const char *device)

 {

     device_properties_t *props;

@@ -1338,7 +1339,7 @@ get_device_timeout(const remote_fencing_op_t *op, const st_query_result_t *peer,

 struct timeout_data {

     const remote_fencing_op_t *op;

-    const st_query_result_t *peer;

+    const peer_device_info_t *peer;

     int total_timeout;

 };

@@ -1365,7 +1366,7 @@ add_device_timeout(gpointer key, gpointer value, gpointer user_data)

 }

 static int

-get_peer_timeout(const remote_fencing_op_t *op, const st_query_result_t *peer)

+get_peer_timeout(const remote_fencing_op_t *op, const peer_device_info_t *peer)

 {

     struct timeout_data timeout;

@@ -1380,7 +1381,7 @@ get_peer_timeout(const remote_fencing_op_t *op, const st_query_result_t *peer)

 static int

 get_op_total_timeout(const remote_fencing_op_t *op,

-                     const st_query_result_t *chosen_peer)

+                     const peer_device_info_t *chosen_peer)

 {

     int total_timeout = 0;

     stonith_topology_t *tp = find_topology_for_host(op->target);

@@ -1403,7 +1404,7 @@ get_op_total_timeout(const remote_fencing_op_t *op,

             }

             for (device_list = tp->levels[i]; device_list; device_list = device_list->next) {

                 for (iter = op->query_results; iter != NULL; iter = iter->next) {

-                    const st_query_result_t *peer = iter->data;

+                    const peer_device_info_t *peer = iter->data;

                     if (find_peer_device(op, peer, device_list->data)) {

                         total_timeout += get_device_timeout(op, peer,

@@ -1555,7 +1556,7 @@ check_watchdog_fencing_and_wait(remote_fencing_op_t * op)

 }

 void

-call_remote_stonith(remote_fencing_op_t * op, st_query_result_t * peer, int rc)

+call_remote_stonith(remote_fencing_op_t *op, peer_device_info_t *peer, int rc)

 {

     const char *device = NULL;

     int timeout = op->base_timeout;

@@ -1734,8 +1735,8 @@ call_remote_stonith(remote_fencing_op_t * op, st_query_result_t * peer, int rc)

 static gint

 sort_peers(gconstpointer a, gconstpointer b)

 {

-    const st_query_result_t *peer_a = a;

-    const st_query_result_t *peer_b = b;

+    const peer_device_info_t *peer_a = a;

+    const peer_device_info_t *peer_b = b;

     return (peer_b->ndevices - peer_a->ndevices);

 }

@@ -1768,7 +1769,7 @@ all_topology_devices_found(remote_fencing_op_t * op)

         for (device = tp->levels[i]; device; device = device->next) {

             match = NULL;

             for (iter = op->query_results; iter && !match; iter = iter->next) {

-                st_query_result_t *peer = iter->data;

+                peer_device_info_t *peer = iter->data;

                 if (skip_target && pcmk__str_eq(peer->host, op->target, pcmk__str_casei)) {

                     continue;

@@ -1850,31 +1851,31 @@ parse_action_specific(xmlNode *xml, const char *peer, const char *device,

  *

  * \param[in]     xml       XML node containing device properties

  * \param[in,out] op        Operation that query and reply relate to

- * \param[in,out] result    Peer's results

+ * \param[in,out] peer      Peer's device information

  * \param[in]     device    ID of device being parsed

  */

 static void

 add_device_properties(xmlNode *xml, remote_fencing_op_t *op,

-                      st_query_result_t *result, const char *device)

+                      peer_device_info_t *peer, const char *device)

 {

     xmlNode *child;

     int verified = 0;

     device_properties_t *props = calloc(1, sizeof(device_properties_t));

-    /* Add a new entry to this result's devices list */

+    /* Add a new entry to this peer's devices list */

     CRM_ASSERT(props != NULL);

-    g_hash_table_insert(result->devices, strdup(device), props);

+    g_hash_table_insert(peer->devices, strdup(device), props);

     /* Peers with verified (monitored) access will be preferred */

     crm_element_value_int(xml, F_STONITH_DEVICE_VERIFIED, &verified);

     if (verified) {

         crm_trace("Peer %s has confirmed a verified device %s",

-                  result->host, device);

+                  peer->host, device);

         props->verified = TRUE;

     }

     /* Parse action-specific device properties */

-    parse_action_specific(xml, result->host, device, op_requested_action(op),

+    parse_action_specific(xml, peer->host, device, op_requested_action(op),

                           op, st_phase_requested, props);

     for (child = pcmk__xml_first_child(xml); child != NULL;

          child = pcmk__xml_next(child)) {

@@ -1883,10 +1884,10 @@ add_device_properties(xmlNode *xml, remote_fencing_op_t *op,

          * winds up getting remapped.

          */

         if (pcmk__str_eq(ID(child), "off", pcmk__str_casei)) {

-            parse_action_specific(child, result->host, device, "off",

+            parse_action_specific(child, peer->host, device, "off",

                                   op, st_phase_off, props);

         } else if (pcmk__str_eq(ID(child), "on", pcmk__str_casei)) {

-            parse_action_specific(child, result->host, device, "on",

+            parse_action_specific(child, peer->host, device, "on",

                                   op, st_phase_on, props);

         }

     }

@@ -1903,17 +1904,17 @@ add_device_properties(xmlNode *xml, remote_fencing_op_t *op,

  *

  * \return Newly allocated result structure with parsed reply

  */

-static st_query_result_t *

+static peer_device_info_t *

 add_result(remote_fencing_op_t *op, const char *host, int ndevices, xmlNode *xml)

 {

-    st_query_result_t *result = calloc(1, sizeof(st_query_result_t));

+    peer_device_info_t *peer = calloc(1, sizeof(peer_device_info_t));

     xmlNode *child;

     // cppcheck seems not to understand the abort logic in CRM_CHECK

     // cppcheck-suppress memleak

-    CRM_CHECK(result != NULL, return NULL);

-    result->host = strdup(host);

-    result->devices = pcmk__strkey_table(free, free);

+    CRM_CHECK(peer != NULL, return NULL);

+    peer->host = strdup(host);

+    peer->devices = pcmk__strkey_table(free, free);

     /* Each child element describes one capable device available to the peer */

     for (child = pcmk__xml_first_child(xml); child != NULL;

@@ -1921,17 +1922,17 @@ add_result(remote_fencing_op_t *op, const char *host, int ndevices, xmlNode *xml

         const char *device = ID(child);

         if (device) {

-            add_device_properties(child, op, result, device);

+            add_device_properties(child, op, peer, device);

         }

     }

-    result->ndevices = g_hash_table_size(result->devices);

-    CRM_CHECK(ndevices == result->ndevices,

+    peer->ndevices = g_hash_table_size(peer->devices);

+    CRM_CHECK(ndevices == peer->ndevices,

               crm_err("Query claimed to have %d device%s but %d found",

-                      ndevices, pcmk__plural_s(ndevices), result->ndevices));

+                      ndevices, pcmk__plural_s(ndevices), peer->ndevices));

-    op->query_results = g_list_insert_sorted(op->query_results, result, sort_peers);

-    return result;

+    op->query_results = g_list_insert_sorted(op->query_results, peer, sort_peers);

+    return peer;

 }

 /*!

@@ -1957,7 +1958,7 @@ process_remote_stonith_query(xmlNode * msg)

     const char *id = NULL;

     const char *host = NULL;

     remote_fencing_op_t *op = NULL;

-    st_query_result_t *result = NULL;

+    peer_device_info_t *peer = NULL;

     uint32_t replies_expected;

     xmlNode *dev = get_xpath_object("//@" F_STONITH_REMOTE_OP_ID, msg, LOG_ERR);

@@ -1991,7 +1992,7 @@ process_remote_stonith_query(xmlNode * msg)

              op->replies, replies_expected, host,

              op->target, op->action, ndevices, pcmk__plural_s(ndevices), id);

     if (ndevices > 0) {

-        result = add_result(op, host, ndevices, dev);

+        peer = add_result(op, host, ndevices, dev);

     }

     if (pcmk_is_set(op->call_options, st_opt_topology)) {

@@ -2001,7 +2002,7 @@ process_remote_stonith_query(xmlNode * msg)

         if (op->state == st_query && all_topology_devices_found(op)) {

             /* All the query results are in for the topology, start the fencing ops. */

             crm_trace("All topology devices found");

-            call_remote_stonith(op, result, pcmk_ok);

+            call_remote_stonith(op, peer, pcmk_ok);

         } else if (have_all_replies) {

             crm_info("All topology query replies have arrived, continuing (%d expected/%d received) ",

@@ -2010,15 +2011,15 @@ process_remote_stonith_query(xmlNode * msg)

         }

     } else if (op->state == st_query) {

-        int nverified = count_peer_devices(op, result, TRUE);

+        int nverified = count_peer_devices(op, peer, TRUE);

         /* We have a result for a non-topology fencing op that looks promising,

          * go ahead and start fencing before query timeout */

-        if (result && (host_is_target == FALSE) && nverified) {

+        if ((peer != NULL) && !host_is_target && nverified) {

             /* we have a verified device living on a peer that is not the target */

             crm_trace("Found %d verified device%s",

                       nverified, pcmk__plural_s(nverified));

-            call_remote_stonith(op, result, pcmk_ok);

+            call_remote_stonith(op, peer, pcmk_ok);

         } else if (have_all_replies) {

             crm_info("All query replies have arrived, continuing (%d expected/%d received) ",

@@ -2029,10 +2030,10 @@ process_remote_stonith_query(xmlNode * msg)

             crm_trace("Waiting for more peer results before launching fencing operation");

         }

-    } else if (result && (op->state == st_done)) {

+    } else if ((peer != NULL) && (op->state == st_done)) {

         crm_info("Discarding query result from %s (%d device%s): "

-                 "Operation is %s", result->host,

-                 result->ndevices, pcmk__plural_s(result->ndevices),

+                 "Operation is %s", peer->host,

+                 peer->ndevices, pcmk__plural_s(peer->ndevices),

                  stonith_op_state_str(op->state));

     }

-- 

2.27.0

From 913e0620310089d2250e9ecde383df757f8e8063 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 11 Nov 2021 12:46:37 -0600

Subject: [PATCH 04/19] Low: fencer: improve broadcasting replies for fenced

 originators

If the target of a fencing action was also the originator, the executioner

broadcasts the result on their behalf.

Previously, it would check if the action was not in a list of actions that are

never broadcasted. However we really only want to broadcast off/reboot results

so just check for that instead.

This also rearranges reply creation slightly so we don't trace-log the reply

until it is fully created.

---

 daemons/fenced/fenced_commands.c | 19 +++++++++----------

 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c

index 46c840f2a..e4185f6e1 100644

--- a/daemons/fenced/fenced_commands.c

+++ b/daemons/fenced/fenced_commands.c

@@ -2385,32 +2385,31 @@ send_async_reply(async_command_t *cmd, const pcmk__action_result_t *result,

                  int pid, bool merged)

 {

     xmlNode *reply = NULL;

-    gboolean bcast = FALSE;

+    bool bcast = false;

     CRM_CHECK((cmd != NULL) && (result != NULL), return);

     reply = construct_async_reply(cmd, result);

-    // Only replies for certain actions are broadcast

-    if (pcmk__str_any_of(cmd->action, "metadata", "monitor", "list", "status",

-                         NULL)) {

-        crm_trace("Never broadcast '%s' replies", cmd->action);

+    // If target was also the originator, broadcast fencing results for it

+    if (!stand_alone && pcmk__is_fencing_action(cmd->action)

+        && pcmk__str_eq(cmd->origin, cmd->victim, pcmk__str_casei)) {

-    } else if (!stand_alone && pcmk__str_eq(cmd->origin, cmd->victim, pcmk__str_casei) && !pcmk__str_eq(cmd->action, "on", pcmk__str_casei)) {

-        crm_trace("Broadcast '%s' reply for %s", cmd->action, cmd->victim);

+        crm_trace("Broadcast '%s' result for %s (target was also originator)",

+                  cmd->action, cmd->victim);

         crm_xml_add(reply, F_SUBTYPE, "broadcast");

-        bcast = TRUE;

+        crm_xml_add(reply, F_STONITH_OPERATION, T_STONITH_NOTIFY);

+        bcast = true;

     }

     log_async_result(cmd, result, pid, NULL, merged);

-    crm_log_xml_trace(reply, "Reply");

     if (merged) {

         crm_xml_add(reply, F_STONITH_MERGED, "true");

     }

+    crm_log_xml_trace(reply, "Reply");

     if (bcast) {

-        crm_xml_add(reply, F_STONITH_OPERATION, T_STONITH_NOTIFY);

         send_cluster_message(NULL, crm_msg_stonith_ng, reply, FALSE);

     } else if (cmd->origin) {

-- 

2.27.0

From 8b8f94fd9ca5e61922cb81e32c8a3d0f1d75fb0b Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 11 Nov 2021 14:40:49 -0600

Subject: [PATCH 05/19] Refactor: fencer: avoid code duplication when sending

 async reply

... and clean up reply function

---

 daemons/fenced/fenced_commands.c | 33 ++++++++++++++++++--------------

 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c

index e4185f6e1..4ea0a337a 100644

--- a/daemons/fenced/fenced_commands.c

+++ b/daemons/fenced/fenced_commands.c

@@ -2411,15 +2411,8 @@ send_async_reply(async_command_t *cmd, const pcmk__action_result_t *result,

     if (bcast) {

         send_cluster_message(NULL, crm_msg_stonith_ng, reply, FALSE);

-

-    } else if (cmd->origin) {

-        crm_trace("Directed reply to %s", cmd->origin);

-        send_cluster_message(crm_get_peer(0, cmd->origin), crm_msg_stonith_ng, reply, FALSE);

-

     } else {

-        crm_trace("Directed local %ssync reply to %s",

-                  (cmd->options & st_opt_sync_call) ? "" : "a-", cmd->client_name);

-        do_local_reply(reply, cmd->client, cmd->options & st_opt_sync_call, FALSE);

+        stonith_send_reply(reply, cmd->options, cmd->origin, cmd->client);

     }

     if (stand_alone) {

@@ -2814,16 +2807,28 @@ check_alternate_host(const char *target)

     return alternate_host;

 }

+/*!

+ * \internal

+ * \brief Send a reply to a CPG peer or IPC client

+ *

+ * \param[in] reply         XML reply to send

+ * \param[in] call_options  Send synchronously if st_opt_sync_call is set here

+ * \param[in] remote_peer   If not NULL, name of peer node to send CPG reply

+ * \param[in] client_id     If not NULL, name of client to send IPC reply

+ */

 static void

-stonith_send_reply(xmlNode * reply, int call_options, const char *remote_peer,

+stonith_send_reply(xmlNode *reply, int call_options, const char *remote_peer,

                    const char *client_id)

 {

-    if (remote_peer) {

-        send_cluster_message(crm_get_peer(0, remote_peer), crm_msg_stonith_ng, reply, FALSE);

-    } else {

+    CRM_CHECK((reply != NULL) && ((remote_peer != NULL) || (client_id != NULL)),

+              return);

+

+    if (remote_peer == NULL) {

         do_local_reply(reply, client_id,

-                       pcmk_is_set(call_options, st_opt_sync_call),

-                       (remote_peer != NULL));

+                       pcmk_is_set(call_options, st_opt_sync_call), FALSE);

+    } else {

+        send_cluster_message(crm_get_peer(0, remote_peer), crm_msg_stonith_ng,

+                             reply, FALSE);

     }

 }

-- 

2.27.0

From 2cdbda58f0e9f38a0e302506107fd933cb415144 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 23 Nov 2021 17:24:09 -0600

Subject: [PATCH 06/19] Refactor: fencer: ensure all requests get clean-up

handle_request() has if-else blocks for each type of request. Previously, if a

request didn't need a reply, the function would do any clean-up needed and

return immediately. Now, we track whether a reply is needed, and all request

types flow to the end of the function for consistent clean-up.

This doesn't change any behavior at this point, but allows us to do more at the

end of request handling.

---

 daemons/fenced/fenced_commands.c | 46 ++++++++++++++++++--------------

 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/daemons/fenced/fenced_commands.c b/daemons/fenced/fenced_commands.c

index 4ea0a337a..19477b49b 100644

--- a/daemons/fenced/fenced_commands.c

+++ b/daemons/fenced/fenced_commands.c

@@ -2892,6 +2892,7 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

     xmlNode *data = NULL;

     xmlNode *reply = NULL;

+    bool need_reply = true;

     char *output = NULL;

     const char *op = crm_element_value(request, F_STONITH_OPERATION);

@@ -2921,10 +2922,12 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         pcmk__ipc_send_xml(client, id, reply, flags);

         client->request_id = 0;

         free_xml(reply);

-        return 0;

+        rc = pcmk_ok;

+        need_reply = false;

     } else if (pcmk__str_eq(op, STONITH_OP_EXEC, pcmk__str_none)) {

         rc = stonith_device_action(request, &output);

+        need_reply = (rc != -EINPROGRESS);

     } else if (pcmk__str_eq(op, STONITH_OP_TIMEOUT_UPDATE, pcmk__str_none)) {

         const char *call_id = crm_element_value(request, F_STONITH_CALLID);

@@ -2933,7 +2936,8 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         crm_element_value_int(request, F_STONITH_TIMEOUT, &op_timeout);

         do_stonith_async_timeout_update(client_id, call_id, op_timeout);

-        return 0;

+        rc = pcmk_ok;

+        need_reply = false;

     } else if (pcmk__str_eq(op, STONITH_OP_QUERY, pcmk__str_none)) {

         if (remote_peer) {

@@ -2944,7 +2948,8 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         remove_relay_op(request);

         stonith_query(request, remote_peer, client_id, call_options);

-        return 0;

+        rc = pcmk_ok;

+        need_reply = false;

     } else if (pcmk__str_eq(op, T_STONITH_NOTIFY, pcmk__str_none)) {

         const char *flag_name = NULL;