From fcd42a5926e9a63d425586552ecc7b543838d352 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 11 Nov 2021 16:57:03 -0600

Subject: [PATCH 01/23] Feature: fencer: pass full result in async command

 replies

The services library callbacks for async commands, which call

send_async_reply() -> construct_async_reply() to create the reply, now add

fields for exit status, operation status, and exit reason, in addition to the

existing action standard output and legacy return code.

Nothing uses the new fields yet.

---

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

 1 file changed, 4 insertions(+), 6 deletions(-)

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

index f34cb4f136..3497428c18 100644

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

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

@@ -2415,9 +2415,8 @@ send_async_reply(async_command_t *cmd, const pcmk__action_result_t *result,

     if (stand_alone) {

         /* Do notification with a clean data object */

         xmlNode *notify_data = create_xml_node(NULL, T_STONITH_NOTIFY_FENCE);

-        int rc = pcmk_rc2legacy(stonith__result2rc(result));

-        crm_xml_add_int(notify_data, F_STONITH_RC, rc);

+        stonith__xe_set_result(notify_data, result);

         crm_xml_add(notify_data, F_STONITH_TARGET, cmd->victim);

         crm_xml_add(notify_data, F_STONITH_OPERATION, cmd->op);

         crm_xml_add(notify_data, F_STONITH_DELEGATE, "localhost");

@@ -2425,7 +2424,7 @@ send_async_reply(async_command_t *cmd, const pcmk__action_result_t *result,

         crm_xml_add(notify_data, F_STONITH_REMOTE_OP_ID, cmd->remote_op_id);

         crm_xml_add(notify_data, F_STONITH_ORIGIN, cmd->client);

-        do_stonith_notify(T_STONITH_NOTIFY_FENCE, rc, notify_data);

+        do_stonith_notify(T_STONITH_NOTIFY_FENCE, pcmk_rc2legacy(stonith__result2rc(result)), notify_data);

         do_stonith_notify(T_STONITH_NOTIFY_HISTORY, pcmk_ok, NULL);

     }

 }

@@ -2728,9 +2727,8 @@ construct_async_reply(async_command_t *cmd, const pcmk__action_result_t *result)

     crm_xml_add(reply, F_STONITH_ORIGIN, cmd->origin);

     crm_xml_add_int(reply, F_STONITH_CALLID, cmd->id);

     crm_xml_add_int(reply, F_STONITH_CALLOPTS, cmd->options);

-    crm_xml_add_int(reply, F_STONITH_RC,

-                    pcmk_rc2legacy(stonith__result2rc(result)));

-    crm_xml_add(reply, F_STONITH_OUTPUT, result->action_stdout);

+

+    stonith__xe_set_result(reply, result);

     return reply;

 }

-- 

2.27.0

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

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 18 Nov 2021 12:41:29 -0600

Subject: [PATCH 02/23] Refactor: fencer: track full result for direct agent

 actions

This renames stonith_device_action() to execute_agent_action() for readability,

and has it set a full result rather than return a legacy return code.

As of this commit, handle_request() just maps the result back to a legacy code,

but it will make better use of it with planned changes.

---

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

 1 file changed, 56 insertions(+), 39 deletions(-)

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

index 3497428c18..2f59ef84b7 100644

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

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

@@ -1729,23 +1729,6 @@ stonith_level_remove(xmlNode *msg, char **desc)

     return pcmk_ok;

 }

-/*!

- * \internal

- * \brief Schedule an (asynchronous) action directly on a stonith device

- *

- * Handle a STONITH_OP_EXEC API message by scheduling a requested agent action

- * directly on a specified device. Only list, monitor, and status actions are

- * expected to use this call, though it should work with any agent command.

- *

- * \param[in]  msg     API message XML with desired action

- * \param[out] output  Unused

- *

- * \return -EINPROGRESS on success, -errno otherwise

- * \note If the action is monitor, the device must be registered via the API

- *       (CIB registration is not sufficient), because monitor should not be

- *       possible unless the device is "started" (API registered).

- */

-

 static char *

 list_to_string(GList *list, const char *delim, gboolean terminate_with_delim)

 {

@@ -1778,8 +1761,23 @@ list_to_string(GList *list, const char *delim, gboolean terminate_with_delim)

     return rv;

 }

-static int

-stonith_device_action(xmlNode * msg, char **output)

+/*!

+ * \internal

+ * \brief Execute a fence agent action directly (and asynchronously)

+ *

+ * Handle a STONITH_OP_EXEC API message by scheduling a requested agent action

+ * directly on a specified device. Only list, monitor, and status actions are

+ * expected to use this call, though it should work with any agent command.

+ *

+ * \param[in]  msg     Request XML specifying action

+ * \param[out] result  Where to store result of action

+ *

+ * \note If the action is monitor, the device must be registered via the API

+ *       (CIB registration is not sufficient), because monitor should not be

+ *       possible unless the device is "started" (API registered).

+ */

+static void

+execute_agent_action(xmlNode *msg, pcmk__action_result_t *result)

 {

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

     xmlNode *op = get_xpath_object("//@" F_STONITH_ACTION, msg, LOG_ERR);

@@ -1792,39 +1790,56 @@ stonith_device_action(xmlNode * msg, char **output)

         crm_info("Malformed API action request: device %s, action %s",

                  (id? id : "not specified"),

                  (action? action : "not specified"));

-        return -EPROTO;

+        fenced_set_protocol_error(result);

+        return;

     }

     if (pcmk__str_eq(id, STONITH_WATCHDOG_ID, pcmk__str_none)) {

+        // Watchdog agent actions are implemented internally

         if (stonith_watchdog_timeout_ms <= 0) {

-            return -ENODEV;

-        } else {

-            if (pcmk__str_eq(action, "list", pcmk__str_casei)) {

-                *output = list_to_string(stonith_watchdog_targets, "\n", TRUE);

-                return pcmk_ok;

-            } else if (pcmk__str_eq(action, "monitor", pcmk__str_casei)) {

-                return pcmk_ok;

-            }

+            pcmk__set_result(result, CRM_EX_ERROR, PCMK_EXEC_NO_FENCE_DEVICE,

+                             "Watchdog fence device not configured");

+            return;

+

+        } else if (pcmk__str_eq(action, "list", pcmk__str_casei)) {

+            pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);

+            pcmk__set_result_output(result,

+                                    list_to_string(stonith_watchdog_targets,

+                                                   "\n", TRUE),

+                                    NULL);

+            return;

+

+        } else if (pcmk__str_eq(action, "monitor", pcmk__str_casei)) {

+            pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);

+            return;

         }

     }

     device = g_hash_table_lookup(device_list, id);

-    if ((device == NULL)

-        || (!device->api_registered && !strcmp(action, "monitor"))) {

+    if (device == NULL) {

+        crm_info("Ignoring API '%s' action request because device %s not found",

+                 action, id);

+        pcmk__set_result(result, CRM_EX_ERROR, PCMK_EXEC_NO_FENCE_DEVICE,

+                         NULL);

+        return;

+    } else if (!device->api_registered && !strcmp(action, "monitor")) {

         // Monitors may run only on "started" (API-registered) devices

-        crm_info("Ignoring API '%s' action request because device %s not found",

+        crm_info("Ignoring API '%s' action request because device %s not active",

                  action, id);

-        return -ENODEV;

+        pcmk__set_result(result, CRM_EX_ERROR, PCMK_EXEC_NO_FENCE_DEVICE,

+                         "Fence device not active");

+        return;

     }

     cmd = create_async_command(msg);

     if (cmd == NULL) {

-        return -EPROTO;

+        fenced_set_protocol_error(result);

+        return;

     }

     schedule_stonith_command(cmd, device);

-    return -EINPROGRESS;

+    pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_PENDING, NULL);

 }

 static void

@@ -2911,8 +2926,8 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

     xmlNode *data = NULL;

     bool need_reply = true;

+    pcmk__action_result_t result = PCMK__UNKNOWN_RESULT;

-    char *output = NULL;

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

     const char *client_id = crm_element_value(request, F_STONITH_CLIENTID);

@@ -2935,8 +2950,9 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         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);

+        execute_agent_action(request, &result);

+        need_reply = (result.execution_status != PCMK_EXEC_PENDING);

+        rc = pcmk_rc2legacy(stonith__result2rc(&result));

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

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

@@ -3150,19 +3166,20 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

 done:

     // Reply if result is known

     if (need_reply) {

-        xmlNode *reply = stonith_construct_reply(request, output, data, rc);

+        xmlNode *reply = stonith_construct_reply(request, result.action_stdout, data, rc);

         stonith_send_reply(reply, call_options, remote_peer, client_id);

         free_xml(reply);

     }

-    free(output);

     free_xml(data);

     crm_debug("Processed %s request from %s %s: %s (rc=%d)",

               op, ((client == NULL)? "peer" : "client"),

               ((client == NULL)? remote_peer : pcmk__client_name(client)),

               ((rc > 0)? "" : pcmk_strerror(rc)), rc);

+

+    pcmk__reset_result(&result);

 }

 static void

-- 

2.27.0

From 9601b2aff1ea6a4eef0bb2701c22c1e971a657eb Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 18 Nov 2021 17:31:20 -0600

Subject: [PATCH 03/23] Refactor: fencer: track full result for local fencing

This renames stonith_fence() to fence_locally() for readability, and has it set

a full result rather than return a legacy return code.

As of this commit, handle_request() just maps the result back to a legacy code,

but it will make better use of it with planned changes.

---

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

 1 file changed, 25 insertions(+), 13 deletions(-)

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

index 2f59ef84b7..bfb0d71e5f 100644

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

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

@@ -2626,37 +2626,49 @@ stonith_fence_get_devices_cb(GList * devices, void *user_data)

     }

 }

-static int

-stonith_fence(xmlNode * msg)

+/*!

+ * \internal

+ * \brief Execute a fence action via the local node

+ *

+ * \param[in]  msg     Fencing request

+ * \param[out] result  Where to store result of fence action

+ */

+static void

+fence_locally(xmlNode *msg, pcmk__action_result_t *result)

 {

     const char *device_id = NULL;

     stonith_device_t *device = NULL;

     async_command_t *cmd = create_async_command(msg);

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

+    CRM_CHECK(result != NULL, return);

+

     if (cmd == NULL) {

-        return -EPROTO;

+        fenced_set_protocol_error(result);

+        return;

     }

     device_id = crm_element_value(dev, F_STONITH_DEVICE);

-    if (device_id) {

+    if (device_id != NULL) {

         device = g_hash_table_lookup(device_list, device_id);

         if (device == NULL) {

             crm_err("Requested device '%s' is not available", device_id);

-            return -ENODEV;

+            pcmk__set_result(result, CRM_EX_ERROR, PCMK_EXEC_NO_FENCE_DEVICE,

+                             "Requested fence device not found");

+            return;

         }

         schedule_stonith_command(cmd, device);

     } else {

         const char *host = crm_element_value(dev, F_STONITH_TARGET);

-        if (cmd->options & st_opt_cs_nodeid) {

-            int nodeid;

-            crm_node_t *node;

+        if (pcmk_is_set(cmd->options, st_opt_cs_nodeid)) {

+            int nodeid = 0;

+            crm_node_t *node = NULL;

             pcmk__scan_min_int(host, &nodeid, 0);

             node = pcmk__search_known_node_cache(nodeid, NULL, CRM_GET_PEER_ANY);

-            if (node) {

+            if (node != NULL) {

                 host = node->uname;

             }

         }

@@ -2666,7 +2678,7 @@ stonith_fence(xmlNode * msg)

                             TRUE, cmd, stonith_fence_get_devices_cb);

     }

-    return -EINPROGRESS;

+    pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_PENDING, NULL);

 }

 xmlNode *

@@ -3016,9 +3028,9 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         }

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

-

-        if (remote_peer || stand_alone) {

-            rc = stonith_fence(request);

+        if ((remote_peer != NULL) || stand_alone) {

+            fence_locally(request, &result);

+            rc = pcmk_rc2legacy(stonith__result2rc(&result));

         } else if (pcmk_is_set(call_options, st_opt_manual_ack)) {

             switch (fenced_handle_manual_confirmation(client, request)) {

-- 

2.27.0

From b7c7676cfd36fd72d3b29e86a23db97081e19b03 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Mon, 29 Nov 2021 17:06:52 -0600

Subject: [PATCH 04/23] Low: fencer: handle topology level registration errors

 better

Rename stonith_level_register() to fenced_register_level() for consistency, and

refactor it to return a full result rather than a legacy return code.

Return a protocol error for missing information in the request XML, and log

invalid level numbers at warning level. Use a new combination of

PCMK_EXEC_INVALID with CRM_EX_INVALID_PARAM for invalid levels, so it gets

mapped back to the legacy code -EINVAL (which was returned before).

---

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

 daemons/fenced/pacemaker-fenced.c |  9 +++---

 daemons/fenced/pacemaker-fenced.h |  3 +-

 lib/fencing/st_actions.c          |  1 +

 4 files changed, 44 insertions(+), 21 deletions(-)

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

index bfb0d71e5f..975f8633a4 100644

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

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

@@ -1583,20 +1583,19 @@ parse_device_list(const char *devices)

 /*!

  * \internal

- * \brief Register a STONITH level for a target

+ * \brief Register a fencing topology level for a target

  *

  * Given an XML request specifying the target name, level index, and device IDs

  * for the level, this will create an entry for the target in the global topology

  * table if one does not already exist, then append the specified device IDs to

  * the entry's device list for the specified level.

  *

- * \param[in]  msg   XML request for STONITH level registration

- * \param[out] desc  If not NULL, will be set to string representation ("TARGET[LEVEL]")

- *

- * \return pcmk_ok on success, -EINVAL if XML does not specify valid level index

+ * \param[in]  msg     XML request for STONITH level registration

+ * \param[out] desc    If not NULL, set to string representation "TARGET[LEVEL]"

+ * \param[out] result  Where to set result of registration

  */

-int

-stonith_level_register(xmlNode *msg, char **desc)

+void

+fenced_register_level(xmlNode *msg, char **desc, pcmk__action_result_t *result)

 {

     int id = 0;

     xmlNode *level;

@@ -1607,6 +1606,13 @@ stonith_level_register(xmlNode *msg, char **desc)

     stonith_key_value_t *dIter = NULL;

     stonith_key_value_t *devices = NULL;

+    CRM_CHECK(result != NULL, return);

+

+    if (msg == NULL) {

+        fenced_set_protocol_error(result);

+        return;

+    }

+

     /* Allow the XML here to point to the level tag directly, or wrapped in

      * another tag. If directly, don't search by xpath, because it might give

      * multiple hits (e.g. if the XML is the CIB).

@@ -1614,11 +1620,15 @@ stonith_level_register(xmlNode *msg, char **desc)

     if (pcmk__str_eq(TYPE(msg), XML_TAG_FENCING_LEVEL, pcmk__str_casei)) {

         level = msg;

     } else {

-        level = get_xpath_object("//" XML_TAG_FENCING_LEVEL, msg, LOG_ERR);

+        level = get_xpath_object("//" XML_TAG_FENCING_LEVEL, msg, LOG_WARNING);

+    }

+    if (level == NULL) {

+        fenced_set_protocol_error(result);

+        return;

     }

-    CRM_CHECK(level != NULL, return -EINVAL);

     mode = stonith_level_kind(level);

+

     target = stonith_level_key(level, mode);

     crm_element_value_int(level, XML_ATTR_STONITH_INDEX, &id;;

@@ -1626,18 +1636,26 @@ stonith_level_register(xmlNode *msg, char **desc)

         *desc = crm_strdup_printf("%s[%d]", target, id);

     }

-    /* Sanity-check arguments */

-    if (mode >= 3 || (id <= 0) || (id >= ST_LEVEL_MAX)) {

-        crm_trace("Could not add %s[%d] (%d) to the topology (%d active entries)", target, id, mode, g_hash_table_size(topology));

+    // Ensure level ID is in allowed range

+    if ((id <= 0) || (id >= ST_LEVEL_MAX)) {

+        crm_warn("Ignoring topology registration for %s with invalid level %d",

+                  target, id);

         free(target);

-        crm_log_xml_err(level, "Bad topology");

-        return -EINVAL;

+        crm_log_xml_warn(level, "Bad level");

+        pcmk__set_result(result, CRM_EX_INVALID_PARAM, PCMK_EXEC_INVALID,

+                         "Invalid topology level");

+        return;

     }

     /* Find or create topology table entry */

     tp = g_hash_table_lookup(topology, target);

     if (tp == NULL) {

         tp = calloc(1, sizeof(stonith_topology_t));

+        if (tp == NULL) {

+            pcmk__set_result(result, CRM_EX_ERROR, PCMK_EXEC_ERROR,

+                             strerror(ENOMEM));

+            return;

+        }

         tp->kind = mode;

         tp->target = target;

         tp->target_value = crm_element_value_copy(level, XML_ATTR_STONITH_TARGET_VALUE);

@@ -1671,7 +1689,8 @@ stonith_level_register(xmlNode *msg, char **desc)

         crm_info("Target %s has %d active fencing level%s",

                  tp->target, nlevels, pcmk__plural_s(nlevels));

     }

-    return pcmk_ok;

+

+    pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);

 }

 int

@@ -3142,7 +3161,8 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         char *device_id = NULL;

         if (is_privileged(client, op)) {

-            rc = stonith_level_register(request, &device_id);

+            fenced_register_level(request, &device_id, &result);

+            rc = pcmk_rc2legacy(stonith__result2rc(&result));

         } else {

             rc = -EACCES;

         }

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

index 0a8b3bf6f2..469304f67c 100644

--- a/daemons/fenced/pacemaker-fenced.c

+++ b/daemons/fenced/pacemaker-fenced.c

@@ -452,8 +452,8 @@ remove_cib_device(xmlXPathObjectPtr xpathObj)

 static void

 handle_topology_change(xmlNode *match, bool remove) 

 {

-    int rc;

     char *desc = NULL;

+    pcmk__action_result_t result = PCMK__UNKNOWN_RESULT;

     CRM_CHECK(match != NULL, return);

     crm_trace("Updating %s", ID(match));

@@ -467,9 +467,10 @@ handle_topology_change(xmlNode *match, bool remove)

         free(key);

     }

-    rc = stonith_level_register(match, &desc);

-    do_stonith_notify_level(STONITH_OP_LEVEL_ADD, rc, desc);

-

+    fenced_register_level(match, &desc, &result);

+    do_stonith_notify_level(STONITH_OP_LEVEL_ADD,

+                            pcmk_rc2legacy(stonith__result2rc(&result)), desc);

+    pcmk__reset_result(&result);

     free(desc);

 }

diff --git a/daemons/fenced/pacemaker-fenced.h b/daemons/fenced/pacemaker-fenced.h

index 5162ada75d..cf114fb979 100644

--- a/daemons/fenced/pacemaker-fenced.h

+++ b/daemons/fenced/pacemaker-fenced.h

@@ -218,7 +218,8 @@ void stonith_device_remove(const char *id, bool from_cib);

 char *stonith_level_key(xmlNode * msg, int mode);

 int stonith_level_kind(xmlNode * msg);

-int stonith_level_register(xmlNode * msg, char **desc);

+void fenced_register_level(xmlNode *msg, char **desc,

+                           pcmk__action_result_t *result);

 int stonith_level_remove(xmlNode * msg, char **desc);

diff --git a/lib/fencing/st_actions.c b/lib/fencing/st_actions.c

index 7eaa8b0f2b..37fa849847 100644

--- a/lib/fencing/st_actions.c

+++ b/lib/fencing/st_actions.c

@@ -325,6 +325,7 @@ stonith__result2rc(const pcmk__action_result_t *result)

          */

         case PCMK_EXEC_INVALID:

             switch (result->exit_status) {

+                case CRM_EX_INVALID_PARAM:      return EINVAL;

                 case CRM_EX_INSUFFICIENT_PRIV:  return EACCES;

                 case CRM_EX_PROTOCOL:           return EPROTO;

-- 

2.27.0

From 27cedca4070328ecac1761f81c2890059af19dcf Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Mon, 29 Nov 2021 17:29:38 -0600

Subject: [PATCH 05/23] Low: fencer: handle topology level unregistration

 errors better

Rename stonith_level_remove() to fenced_unregister_level() for consistency, and

refactor it to return a full result rather than a legacy return code.

Return a protocol error for missing information in the request XML, and log

invalid level numbers at warning level. Use PCMK_EXEC_INVALID with

CRM_EX_INVALID_PARAM for invalid levels, so it gets mapped back to the legacy

code -EINVAL (which reverses the recent change in ec60f014b, both for backward

compatibility and because it makes sense -- a missing parameter is a protocol

error, while an invalid parameter is an invalid parameter error).

---

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

 daemons/fenced/pacemaker-fenced.c |  9 +++---

 daemons/fenced/pacemaker-fenced.h |  4 +--

 3 files changed, 48 insertions(+), 17 deletions(-)

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

index 975f8633a4..ef41dc0e52 100644

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

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

@@ -1693,25 +1693,54 @@ fenced_register_level(xmlNode *msg, char **desc, pcmk__action_result_t *result)

     pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);

 }

-int

-stonith_level_remove(xmlNode *msg, char **desc)

+/*!

+ * \internal

+ * \brief Unregister a fencing topology level for a target

+ *

+ * Given an XML request specifying the target name and level index (or 0 for all

+ * levels), this will remove any corresponding entry for the target from the

+ * global topology table.

+ *

+ * \param[in]  msg     XML request for STONITH level registration

+ * \param[out] desc    If not NULL, set to string representation "TARGET[LEVEL]"

+ * \param[out] result  Where to set result of unregistration

+ */

+void

+fenced_unregister_level(xmlNode *msg, char **desc,

+                        pcmk__action_result_t *result)

 {

     int id = -1;

     stonith_topology_t *tp;

     char *target;

+    xmlNode *level = NULL;

+

+    CRM_CHECK(result != NULL, return);

-    /* Unlike additions, removal requests should always have one level tag */

-    xmlNode *level = get_xpath_object("//" XML_TAG_FENCING_LEVEL, msg, LOG_ERR);

+    if (msg == NULL) {

+        fenced_set_protocol_error(result);

+        return;

+    }

-    CRM_CHECK(level != NULL, return -EPROTO);

+    // Unlike additions, removal requests should always have one level tag

+    level = get_xpath_object("//" XML_TAG_FENCING_LEVEL, msg, LOG_WARNING);

+    if (level == NULL) {

+        fenced_set_protocol_error(result);

+        return;

+    }

     target = stonith_level_key(level, -1);

     crm_element_value_int(level, XML_ATTR_STONITH_INDEX, &id;;

-    CRM_CHECK((id >= 0) && (id < ST_LEVEL_MAX),

-              crm_log_xml_warn(msg, "invalid level");

-              free(target);

-              return -EPROTO);

+    // Ensure level ID is in allowed range

+    if ((id < 0) || (id >= ST_LEVEL_MAX)) {

+        crm_warn("Ignoring topology unregistration for %s with invalid level %d",

+                  target, id);

+        free(target);

+        crm_log_xml_warn(level, "Bad level");

+        pcmk__set_result(result, CRM_EX_INVALID_PARAM, PCMK_EXEC_INVALID,

+                         "Invalid topology level");

+        return;

+    }

     if (desc) {

         *desc = crm_strdup_printf("%s[%d]", target, id);

@@ -1745,7 +1774,7 @@ stonith_level_remove(xmlNode *msg, char **desc)

     }

     free(target);

-    return pcmk_ok;

+    pcmk__set_result(result, CRM_EX_OK, PCMK_EXEC_DONE, NULL);

 }

 static char *

@@ -3173,7 +3202,8 @@ handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

         char *device_id = NULL;

         if (is_privileged(client, op)) {

-            rc = stonith_level_remove(request, &device_id);

+            fenced_unregister_level(request, &device_id, &result);

+            rc = pcmk_rc2legacy(stonith__result2rc(&result));

         } else {

             rc = -EACCES;

         }

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

index 469304f67c..56acc93f31 100644

--- a/daemons/fenced/pacemaker-fenced.c

+++ b/daemons/fenced/pacemaker-fenced.c

@@ -409,17 +409,18 @@ do_stonith_notify_level(const char *op, int rc, const char *desc)

 static void

 topology_remove_helper(const char *node, int level)

 {

-    int rc;

     char *desc = NULL;

+    pcmk__action_result_t result = PCMK__UNKNOWN_RESULT;

     xmlNode *data = create_xml_node(NULL, XML_TAG_FENCING_LEVEL);

     crm_xml_add(data, F_STONITH_ORIGIN, __func__);

     crm_xml_add_int(data, XML_ATTR_STONITH_INDEX, level);

     crm_xml_add(data, XML_ATTR_STONITH_TARGET, node);

-    rc = stonith_level_remove(data, &desc);

-    do_stonith_notify_level(STONITH_OP_LEVEL_DEL, rc, desc);

-

+    fenced_unregister_level(data, &desc, &result);

+    do_stonith_notify_level(STONITH_OP_LEVEL_DEL,

+                            pcmk_rc2legacy(stonith__result2rc(&result)), desc);

+    pcmk__reset_result(&result);

     free_xml(data);

     free(desc);

 }

diff --git a/daemons/fenced/pacemaker-fenced.h b/daemons/fenced/pacemaker-fenced.h

index cf114fb979..0006e02e7d 100644

--- a/daemons/fenced/pacemaker-fenced.h

+++ b/daemons/fenced/pacemaker-fenced.h

@@ -220,8 +220,8 @@ char *stonith_level_key(xmlNode * msg, int mode);

 int stonith_level_kind(xmlNode * msg);

 void fenced_register_level(xmlNode *msg, char **desc,

                            pcmk__action_result_t *result);

-

-int stonith_level_remove(xmlNode * msg, char **desc);

+void fenced_unregister_level(xmlNode *msg, char **desc,

+                             pcmk__action_result_t *result);

 stonith_topology_t *find_topology_for_host(const char *host);

-- 

2.27.0

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

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 19 Nov 2021 12:22:33 -0600

Subject: [PATCH 06/23] Log: fencer: track and log full result when handling

 requests

handle_request() now tracks and logs a full result rather than just a

legacy return code.

---

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

 1 file changed, 53 insertions(+), 42 deletions(-)

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

index ef41dc0e52..996c18faaa 100644

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

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

@@ -2981,9 +2981,7 @@ static void

 handle_request(pcmk__client_t *client, uint32_t id, uint32_t flags,

                xmlNode *request, const char *remote_peer)

 {

-    int call_options = 0;

-    int rc = -EOPNOTSUPP;

-