From 95b4f87aae5fb2cf771cf9a8f8e5420b65fb213f Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 21 Sep 2021 10:47:51 -0500

Subject: [PATCH 01/12] Refactor: fencing: use pcmk__action_result_t in

 stonith_action_t

stonith_action_t previously had an rc member for a legacy return code, along

with output and error members for action stdout/stderr. When setting rc based

on the svc_action_t result, it used a mapping function svc_action_to_errno().

This replaces those with a pcmk__action_result_t member, which means we now

track the exit status and execution status as originally set by libcrmservice,

rather than the mapped rc. The library now calls the mapping function, now

returning standard codes and called result2rc(), when calling the client

callback.

The exit_reason member is unused as of this commit.

The behavior should be identical, with the small exception of

services_action_async() failure leaving the exit status as set by the services

library, which means callers will get the result2rc() mapping of the actual

result instead of the former -ECONNABORTED.

---

 lib/fencing/st_client.c | 118 +++++++++++++++++++++++-----------------

 1 file changed, 68 insertions(+), 50 deletions(-)

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

index 08adb51c6..6c607b010 100644

--- a/lib/fencing/st_client.c

+++ b/lib/fencing/st_client.c

@@ -29,6 +29,7 @@

 #include <crm/msg_xml.h>

 #include <crm/common/xml.h>

 #include <crm/common/xml_internal.h>

+#include <crm/services_internal.h>

 #include <crm/common/mainloop.h>

@@ -57,9 +58,7 @@ struct stonith_action_s {

     int max_retries;

     int pid;

-    int rc;

-    char *output;

-    char *error;

+    pcmk__action_result_t result;

 };

 typedef struct stonith_private_s {

@@ -120,6 +119,7 @@ static void stonith_connection_destroy(gpointer user_data);

 static void stonith_send_notification(gpointer data, gpointer user_data);

 static int internal_stonith_action_execute(stonith_action_t * action);

 static void log_action(stonith_action_t *action, pid_t pid);

+static int result2rc(const pcmk__action_result_t *result);

 /*!

  * \brief Get agent namespace by name

@@ -196,6 +196,23 @@ stonith_get_namespace(const char *agent, const char *namespace_s)

     return st_namespace_invalid;

 }

+/*!

+ * \internal

+ * \brief Set an action's result based on services library result

+ *

+ * \param[in] action      Fence action to set result for

+ * \param[in] svc_action  Service action to get result from

+ */

+static void

+set_result_from_svc_action(stonith_action_t *action, svc_action_t *svc_action)

+{

+    pcmk__set_result(&(action->result), svc_action->rc, svc_action->status,

+                     NULL);

+    pcmk__set_result_output(&(action->result),

+                            services__grab_stdout(svc_action),

+                            services__grab_stderr(svc_action));

+}

+

 gboolean

 stonith__watchdog_fencing_enabled_for_node_api(stonith_t *st, const char *node)

 {

@@ -259,19 +276,19 @@ stonith__watchdog_fencing_enabled_for_node(const char *node)

 static void

 log_action(stonith_action_t *action, pid_t pid)

 {

-    if (action->output) {

+    if (action->result.action_stdout != NULL) {

         /* Logging the whole string confuses syslog when the string is xml */

         char *prefix = crm_strdup_printf("%s[%d] stdout:", action->agent, pid);

-        crm_log_output(LOG_TRACE, prefix, action->output);

+        crm_log_output(LOG_TRACE, prefix, action->result.action_stdout);

         free(prefix);

     }

-    if (action->error) {

+    if (action->result.action_stderr != NULL) {

         /* Logging the whole string confuses syslog when the string is xml */

         char *prefix = crm_strdup_printf("%s[%d] stderr:", action->agent, pid);

-        crm_log_output(LOG_WARNING, prefix, action->error);

+        crm_log_output(LOG_WARNING, prefix, action->result.action_stderr);

         free(prefix);

     }

 }

@@ -645,8 +662,7 @@ stonith__destroy_action(stonith_action_t *action)

         if (action->svc_action) {

             services_action_free(action->svc_action);

         }

-        free(action->output);

-        free(action->error);

+        pcmk__reset_result(&(action->result));

         free(action);

     }

 }

@@ -678,15 +694,15 @@ stonith__action_result(stonith_action_t *action, int *rc, char **output,

     }

     if (action != NULL) {

         if (rc) {

-            *rc = action->rc;

+            *rc = pcmk_rc2legacy(result2rc(&(action->result)));

         }

-        if (output && action->output) {

-            *output = action->output;

-            action->output = NULL; // hand off memory management to caller

+        if ((output != NULL) && (action->result.action_stdout != NULL)) {

+            *output = action->result.action_stdout;

+            action->result.action_stdout = NULL; // hand off ownership to caller

         }

-        if (error_output && action->error) {

-            *error_output = action->error;

-            action->error = NULL; // hand off memory management to caller

+        if ((error_output != NULL) && (action->result.action_stderr != NULL)) {

+            *error_output = action->result.action_stderr;

+            action->result.action_stderr = NULL; // hand off ownership to caller

         }

     }

 }

@@ -715,6 +731,9 @@ stonith_action_create(const char *agent,

     action->timeout = action->remaining_timeout = timeout;

     action->max_retries = FAILURE_MAX_RETRIES;

+    pcmk__set_result(&(action->result), PCMK_OCF_UNKNOWN, PCMK_EXEC_UNKNOWN,

+                     NULL);

+

     if (device_args) {

         char buffer[512];

         const char *value = NULL;

@@ -739,7 +758,8 @@ update_remaining_timeout(stonith_action_t * action)

         crm_info("Attempted to execute agent %s (%s) the maximum number of times (%d) allowed",

                  action->agent, action->action, action->max_retries);

         action->remaining_timeout = 0;

-    } else if ((action->rc != -ETIME) && diff < (action->timeout * 0.7)) {

+    } else if ((action->result.execution_status != PCMK_EXEC_TIMEOUT)

+               && (diff < (action->timeout * 0.7))) {

         /* only set remaining timeout period if there is 30%

          * or greater of the original timeout period left */

         action->remaining_timeout = action->timeout - diff;

@@ -750,31 +770,31 @@ update_remaining_timeout(stonith_action_t * action)

 }

 static int

-svc_action_to_errno(svc_action_t *svc_action) {

-    int rv = pcmk_ok;

+result2rc(const pcmk__action_result_t *result) {

+    int rc = pcmk_rc_ok;

-    if (svc_action->status == PCMK_EXEC_TIMEOUT) {

-            rv = -ETIME;

+    if (result->execution_status == PCMK_EXEC_TIMEOUT) {

+            rc = ETIME;

-    } else if (svc_action->rc != PCMK_OCF_OK) {

+    } else if (result->exit_status != CRM_EX_OK) {

         /* Try to provide a useful error code based on the fence agent's

          * error output.

          */

-        if (svc_action->stderr_data == NULL) {

-            rv = -ENODATA;

+        if (result->action_stderr == NULL) {

+            rc = ENODATA;

-        } else if (strstr(svc_action->stderr_data, "imed out")) {

+        } else if (strstr(result->action_stderr, "imed out")) {

             /* Some agents have their own internal timeouts */

-            rv = -ETIME;

+            rc = ETIME;

-        } else if (strstr(svc_action->stderr_data, "Unrecognised action")) {

-            rv = -EOPNOTSUPP;

+        } else if (strstr(result->action_stderr, "Unrecognised action")) {

+            rc = EOPNOTSUPP;

         } else {

-            rv = -pcmk_err_generic;

+            rc = pcmk_rc_error;

         }

     }

-    return rv;

+    return rc;

 }

 static void

@@ -782,11 +802,7 @@ stonith_action_async_done(svc_action_t *svc_action)

 {

     stonith_action_t *action = (stonith_action_t *) svc_action->cb_data;

-    action->rc = svc_action_to_errno(svc_action);

-    action->output = svc_action->stdout_data;

-    svc_action->stdout_data = NULL;

-    action->error = svc_action->stderr_data;

-    svc_action->stderr_data = NULL;

+    set_result_from_svc_action(action, svc_action);

     svc_action->params = NULL;

@@ -795,7 +811,9 @@ stonith_action_async_done(svc_action_t *svc_action)

     log_action(action, action->pid);

-    if (action->rc != pcmk_ok && update_remaining_timeout(action)) {

+    if ((action->result.exit_status != CRM_EX_OK)

+        && update_remaining_timeout(action)) {

+

         int rc = internal_stonith_action_execute(action);

         if (rc == pcmk_ok) {

             return;

@@ -803,7 +821,8 @@ stonith_action_async_done(svc_action_t *svc_action)

     }

     if (action->done_cb) {

-        action->done_cb(action->pid, action->rc, action->output, action->userdata);

+        action->done_cb(action->pid, pcmk_rc2legacy(result2rc(&(action->result))),

+                        action->result.action_stdout, action->userdata);

     }

     action->svc_action = NULL; // don't remove our caller

@@ -835,9 +854,13 @@ internal_stonith_action_execute(stonith_action_t * action)

     static int stonith_sequence = 0;

     char *buffer = NULL;

-    if ((action == NULL) || (action->action == NULL) || (action->args == NULL)

+    CRM_CHECK(action != NULL, return -EINVAL);

+

+    if ((action->action == NULL) || (action->args == NULL)

         || (action->agent == NULL)) {

-        return -EPROTO;

+        pcmk__set_result(&(action->result), PCMK_OCF_UNKNOWN_ERROR,

+                         PCMK_EXEC_ERROR_FATAL, NULL);

+        return -EINVAL;

     }

     if (!action->tries) {

@@ -857,6 +880,7 @@ internal_stonith_action_execute(stonith_action_t * action)

     free(buffer);

     if (svc_action->rc != PCMK_OCF_UNKNOWN) {

+        set_result_from_svc_action(action, svc_action);

         services_action_free(svc_action);

         return -E2BIG;

     }

@@ -877,10 +901,7 @@ internal_stonith_action_execute(stonith_action_t * action)

     /* keep retries from executing out of control and free previous results */

     if (is_retry) {

-        free(action->output);

-        action->output = NULL;

-        free(action->error);

-        action->error = NULL;

+        pcmk__reset_result(&(action->result));

         sleep(1);

     }

@@ -889,22 +910,19 @@ internal_stonith_action_execute(stonith_action_t * action)

         if (services_action_async_fork_notify(svc_action,

                                               &stonith_action_async_done,

                                               &stonith_action_async_forked)) {

+            pcmk__set_result(&(action->result), PCMK_OCF_UNKNOWN,

+                             PCMK_EXEC_PENDING, NULL);

             return pcmk_ok;

         }

     } else if (services_action_sync(svc_action)) { // sync success

         rc = pcmk_ok;

-        action->rc = svc_action_to_errno(svc_action);

-        action->output = svc_action->stdout_data;

-        svc_action->stdout_data = NULL;

-        action->error = svc_action->stderr_data;

-        svc_action->stderr_data = NULL;

     } else { // sync failure

-        action->rc = -ECONNABORTED;

-        rc = action->rc;

+        rc = -ECONNABORTED;

     }

+    set_result_from_svc_action(action, svc_action);

     svc_action->params = NULL;

     services_action_free(svc_action);

     return rc;

-- 

2.27.0

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

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 21 Sep 2021 11:14:31 -0500

Subject: [PATCH 02/12] Low: fencing: improve return code given back to library

 callers

Expose result2rc() internally for future reuse, and expand it to handle more

cases. In theory, this can give us better log messages and status output for

failures.

---

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

 lib/fencing/st_client.c        | 63 +++++++++++++++++++++-------------

 2 files changed, 41 insertions(+), 23 deletions(-)

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

index fa9059e6f..0d23967bb 100644

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

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

@@ -60,6 +60,7 @@ stonith_action_t *stonith_action_create(const char *agent,

 void stonith__destroy_action(stonith_action_t *action);

 void stonith__action_result(stonith_action_t *action, int *rc, char **output,

                             char **error_output);

+int stonith__result2rc(const pcmk__action_result_t *result);

 int

 stonith_action_execute_async(stonith_action_t * action,

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

index 6c607b010..809be1640 100644

--- a/lib/fencing/st_client.c

+++ b/lib/fencing/st_client.c

@@ -119,7 +119,6 @@ static void stonith_connection_destroy(gpointer user_data);

 static void stonith_send_notification(gpointer data, gpointer user_data);

 static int internal_stonith_action_execute(stonith_action_t * action);

 static void log_action(stonith_action_t *action, pid_t pid);

-static int result2rc(const pcmk__action_result_t *result);

 /*!

  * \brief Get agent namespace by name

@@ -694,7 +693,7 @@ stonith__action_result(stonith_action_t *action, int *rc, char **output,

     }

     if (action != NULL) {

         if (rc) {

-            *rc = pcmk_rc2legacy(result2rc(&(action->result)));

+            *rc = pcmk_rc2legacy(stonith__result2rc(&(action->result)));

         }

         if ((output != NULL) && (action->result.action_stdout != NULL)) {

             *output = action->result.action_stdout;

@@ -769,32 +768,49 @@ update_remaining_timeout(stonith_action_t * action)

     return action->remaining_timeout ? TRUE : FALSE;

 }

-static int

-result2rc(const pcmk__action_result_t *result) {

-    int rc = pcmk_rc_ok;

+/*!

+ * \internal

+ * \brief Map a fencing action result to a standard return code

+ *

+ * \param[in] result  Fencing action result to map

+ *

+ * \return Standard Pacemaker return code that best corresponds to \p result

+ */

+int

+stonith__result2rc(const pcmk__action_result_t *result)

+{

+    switch (result->execution_status) {

+        case PCMK_EXEC_CANCELLED:       return ECANCELED;

+        case PCMK_EXEC_TIMEOUT:         return ETIME;

+        case PCMK_EXEC_NOT_INSTALLED:   return ENOENT;

+        case PCMK_EXEC_NOT_SUPPORTED:   return EOPNOTSUPP;

+        case PCMK_EXEC_NOT_CONNECTED:   return ENOTCONN;

+        case PCMK_EXEC_NO_FENCE_DEVICE: return ENODEV;

+        case PCMK_EXEC_NO_SECRETS:      return EACCES;

+        default:                        break;

+    }

-    if (result->execution_status == PCMK_EXEC_TIMEOUT) {

-            rc = ETIME;

+    if (result->exit_status == CRM_EX_OK) {

+        return pcmk_rc_ok;

+    }

-    } else if (result->exit_status != CRM_EX_OK) {

-        /* Try to provide a useful error code based on the fence agent's

-         * error output.

-         */

-        if (result->action_stderr == NULL) {

-            rc = ENODATA;

+    // Try to provide useful error code based on result's error output

-        } else if (strstr(result->action_stderr, "imed out")) {

-            /* Some agents have their own internal timeouts */

-            rc = ETIME;

+    if (result->action_stderr == NULL) {

+        return ENODATA;

-        } else if (strstr(result->action_stderr, "Unrecognised action")) {

-            rc = EOPNOTSUPP;

+    } else if (strcasestr(result->action_stderr, "timed out")

+               || strcasestr(result->action_stderr, "timeout")) {

+        return ETIME;

-        } else {

-            rc = pcmk_rc_error;

-        }

+    } else if (strcasestr(result->action_stderr, "unrecognised action")

+               || strcasestr(result->action_stderr, "unrecognized action")

+               || strcasestr(result->action_stderr, "unsupported action")) {

+        return EOPNOTSUPP;

     }

-    return rc;

+

+    // Oh well, we tried

+    return pcmk_rc_error;

 }

 static void

@@ -821,7 +837,8 @@ stonith_action_async_done(svc_action_t *svc_action)

     }

     if (action->done_cb) {

-        action->done_cb(action->pid, pcmk_rc2legacy(result2rc(&(action->result))),

+        action->done_cb(action->pid,

+                        pcmk_rc2legacy(stonith__result2rc(&(action->result))),

                         action->result.action_stdout, action->userdata);

     }

-- 

2.27.0

From 153c9b552a5bad9dd36e8635fa478ed9cad1f240 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 7 Oct 2021 11:35:44 -0500

Subject: [PATCH 03/12] Refactor: fencing: return full result from

 stonith__action_result()

Previously, stonith__action_result() grabbed an action's legacy rc, stdout, and

stderr separately. Now, directly return a pointer to the action's result

object, and map that to a legacy rc in the callers when needed.

---

 include/crm/fencing/internal.h |  3 +--

 lib/fencing/st_client.c        | 36 ++++---------------------

 lib/fencing/st_rhcs.c          | 48 ++++++++++++++++++++++++----------

 3 files changed, 40 insertions(+), 47 deletions(-)

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

index 0d23967bb..4e9f50fe8 100644

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

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

@@ -58,8 +58,7 @@ stonith_action_t *stonith_action_create(const char *agent,

                                         GHashTable * port_map,

                                         const char * host_arg);

 void stonith__destroy_action(stonith_action_t *action);

-void stonith__action_result(stonith_action_t *action, int *rc, char **output,

-                            char **error_output);

+pcmk__action_result_t *stonith__action_result(stonith_action_t *action);

 int stonith__result2rc(const pcmk__action_result_t *result);

 int

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

index 809be1640..b9df18465 100644

--- a/lib/fencing/st_client.c

+++ b/lib/fencing/st_client.c

@@ -670,40 +670,14 @@ stonith__destroy_action(stonith_action_t *action)

  * \internal

  * \brief Get the result of an executed stonith action

  *

- * \param[in,out] action        Executed action

- * \param[out]    rc            Where to store result code (or NULL)

- * \param[out]    output        Where to store standard output (or NULL)

- * \param[out]    error_output  Where to store standard error output (or NULL)

+ * \param[in] action  Executed action

  *

- * \note If output or error_output is not NULL, the caller is responsible for

- *       freeing the memory.

+ * \return Pointer to action's result (or NULL if \p action is NULL)

  */

-void

-stonith__action_result(stonith_action_t *action, int *rc, char **output,

-                       char **error_output)

+pcmk__action_result_t *

+stonith__action_result(stonith_action_t *action)

 {

-    if (rc) {

-        *rc = pcmk_ok;

-    }

-    if (output) {

-        *output = NULL;

-    }

-    if (error_output) {

-        *error_output = NULL;

-    }

-    if (action != NULL) {

-        if (rc) {

-            *rc = pcmk_rc2legacy(stonith__result2rc(&(action->result)));

-        }

-        if ((output != NULL) && (action->result.action_stdout != NULL)) {

-            *output = action->result.action_stdout;

-            action->result.action_stdout = NULL; // hand off ownership to caller

-        }

-        if ((error_output != NULL) && (action->result.action_stderr != NULL)) {

-            *error_output = action->result.action_stderr;

-            action->result.action_stderr = NULL; // hand off ownership to caller

-        }

-    }

+    return (action == NULL)? NULL : &(action->result);

 }

 #define FAILURE_MAX_RETRIES 2

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

index 89a2625bd..23e694975 100644

--- a/lib/fencing/st_rhcs.c

+++ b/lib/fencing/st_rhcs.c

@@ -1,5 +1,5 @@

 /*

- * Copyright 2004-2020 the Pacemaker project contributors

+ * Copyright 2004-2021 the Pacemaker project contributors

  *

  * The version control history for this file may have further details.

  *

@@ -123,10 +123,10 @@ stonith_rhcs_parameter_not_required(xmlNode *metadata, const char *parameter)

 static int

 stonith__rhcs_get_metadata(const char *agent, int timeout, xmlNode **metadata)

 {

-    char *buffer = NULL;

     xmlNode *xml = NULL;

     xmlNode *actions = NULL;

     xmlXPathObject *xpathObj = NULL;

+    pcmk__action_result_t *result = NULL;

     stonith_action_t *action = stonith_action_create(agent, "metadata", NULL, 0,

                                                      5, NULL, NULL, NULL);

     int rc = stonith__execute(action);

@@ -138,23 +138,31 @@ stonith__rhcs_get_metadata(const char *agent, int timeout, xmlNode **metadata)

         return rc;

     }

-    stonith__action_result(action, &rc, &buffer, NULL);

-    stonith__destroy_action(action);

-    if (rc < 0) {

-        crm_warn("Metadata action for %s failed: %s " CRM_XS "rc=%d",

-                 agent, pcmk_strerror(rc), rc);

-        free(buffer);

-        return rc;

+    result = stonith__action_result(action);

+

+    if (result->execution_status != PCMK_EXEC_DONE) {

+        crm_warn("Could not execute metadata action for %s: %s",

+                 agent, pcmk_exec_status_str(result->execution_status));

+        stonith__destroy_action(action);

+        return pcmk_rc2legacy(stonith__result2rc(result));

     }

-    if (buffer == NULL) {

+    if (result->exit_status != CRM_EX_OK) {

+        crm_warn("Metadata action for %s returned error code %d",

+                 agent, result->exit_status);

+        stonith__destroy_action(action);

+        return pcmk_rc2legacy(stonith__result2rc(result));

+    }

+

+    if (result->action_stdout == NULL) {

         crm_warn("Metadata action for %s returned no data", agent);

+        stonith__destroy_action(action);

         return -ENODATA;

     }

-    xml = string2xml(buffer);

-    free(buffer);

-    buffer = NULL;

+    xml = string2xml(result->action_stdout);

+    stonith__destroy_action(action);

+

     if (xml == NULL) {

         crm_warn("Metadata for %s is invalid", agent);

         return -pcmk_err_schema_validation;

@@ -289,7 +297,19 @@ stonith__rhcs_validate(stonith_t *st, int call_options, const char *target,

     rc = stonith__execute(action);

     if (rc == pcmk_ok) {

-        stonith__action_result(action, &rc, output, error_output);

+        pcmk__action_result_t *result = stonith__action_result(action);

+

+        rc = pcmk_rc2legacy(stonith__result2rc(result));

+

+        // Take ownership of output so stonith__destroy_action() doesn't free it

+        if (output != NULL) {

+            *output = result->action_stdout;

+            result->action_stdout = NULL;

+        }

+        if (error_output != NULL) {

+            *error_output = result->action_stderr;

+            result->action_stderr = NULL;

+        }

     }

     stonith__destroy_action(action);

     return rc;

-- 

2.27.0

From 7f7067014357cccb229a0bef091e234eb3765f7a Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Tue, 21 Sep 2021 13:05:54 -0500

Subject: [PATCH 04/12] Refactor: fencing: pass full result to async action

 callback

When executing an asynchronous fence agent command, the fencing library gets

the full result (exit status, execution status, and exit reason) from the

services library, then maps that to a legacy return code.

Now, pass the full result object to the fencing async callback, rather than

separate arguments for legacy code and stdout. The mapping to a legacy code now

happens in the fencer rather than the fencing library.

The goal of this and following commits is to push the full result object

further down the code path, so that ultimately the full result is always

available internally, and the legacy code mapping is only done for backward

compatibility when sending the result back to a client.

This commit focuses on the async callback (done_cb() in both the fencer's

async_command_t and the fencing library's stonith_action_t). Later commits will

follow the chain:

	st_child_done() and stonith_fence_get_devices_cb()

	-> stonith_send_async_reply()

	-> stonith_construct_async_reply() and log_async_result()

---

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

 include/crm/fencing/internal.h   |  3 +-

 lib/fencing/st_client.c          | 10 ++--

 3 files changed, 58 insertions(+), 33 deletions(-)

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

index b5ae28d90..d5d04ae69 100644

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

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

@@ -62,7 +62,8 @@ struct device_search_s {

 };

 static gboolean stonith_device_dispatch(gpointer user_data);

-static void st_child_done(int pid, int rc, const char *output, void *user_data);

+static void st_child_done(int pid, const pcmk__action_result_t *result,

+                          void *user_data);

 static void stonith_send_reply(xmlNode * reply, int call_options, const char *remote_peer,

                                const char *client_id);

@@ -99,7 +100,8 @@ typedef struct async_command_s {

     GList *device_next;

     void *internal_user_data;

-    void (*done_cb) (int pid, int rc, const char *output, void *user_data);

+    void (*done_cb) (int pid, const pcmk__action_result_t *result,

+                     void *user_data);

     guint timer_sigterm;

     guint timer_sigkill;

     /*! If the operation timed out, this is the last signal

@@ -377,13 +379,25 @@ get_agent_metadata_cb(gpointer data) {

  * \internal

  * \brief Call a command's action callback for an internal (not library) result

  *

- * \param[in] cmd     Command to report result for

- * \param[in] rc      Legacy return code to pass to callback

+ * \param[in] cmd               Command to report result for

+ * \param[in] execution_status  Execution status to use for result

+ * \param[in] exit_status       Exit status to use for result

+ * \param[in] exit_reason       Exit reason to use for result

  */

 static void

-report_internal_result(async_command_t *cmd, int rc)

+report_internal_result(async_command_t *cmd, int exit_status,

+                       int execution_status, const char *exit_reason)

 {

-    cmd->done_cb(0, rc, NULL, cmd);

+    pcmk__action_result_t result = {

+        // Ensure we don't pass garbage to free()

+        .exit_reason = NULL,

+        .action_stdout = NULL,

+        .action_stderr = NULL

+    };

+

+    pcmk__set_result(&result, exit_status, execution_status, exit_reason);

+    cmd->done_cb(0, &result, cmd);

+    pcmk__reset_result(&result);

 }

 static gboolean

@@ -446,7 +460,7 @@ stonith_device_execute(stonith_device_t * device)

             }

         } else {

             crm_info("Faking success for %s watchdog operation", cmd->action);

-            report_internal_result(cmd, pcmk_ok);

+            report_internal_result(cmd, CRM_EX_OK, PCMK_EXEC_DONE, NULL);

             goto done;

         }

     }

@@ -462,7 +476,8 @@ stonith_device_execute(stonith_device_t * device)

             crm_err("Considering %s unconfigured "

                     "because unable to load CIB secrets: %s",

                      device->id, pcmk_rc_str(exec_rc));

-            report_internal_result(cmd, -EACCES);

+            report_internal_result(cmd, CRM_EX_ERROR, PCMK_EXEC_NO_SECRETS,

+                                   NULL);

             goto done;

         }

     }

@@ -501,7 +516,7 @@ stonith_device_execute(stonith_device_t * device)

                                            cmd->done_cb, fork_cb);

     if (exec_rc < 0) {

         cmd->activating_on = NULL;

-        report_internal_result(cmd, exec_rc);

+        cmd->done_cb(0, stonith__action_result(action), cmd);

         stonith__destroy_action(action);

     }