commit 9c0c3d60b4df3076ff27dd176af9b8d131254337
Author: Ken Gaillot <kgaillot@redhat.com>
Date:   Thu Mar 2 13:52:47 2017 -0600

    Fix: libfencing,fencing: properly remap "action" in configuration
    
    "action" should never be specified in fence device configuration.
    
    Previously, this was handled by re-inserting the proper action when creating a
    fence command, if the proper action were on a "safe" list (list, monitor,
    status, or meta-data).
    
    However, that was insufficient, partly because "on" should have been on the
    list, but also because action="reboot" would interfere with reboots that were
    remapped to off or off+on.
    
    Now, stonithd intelligently maps any action parameter to pcmk_off_action and/or
    pcmk_reboot_action as appropriate when the device configuration is registered,
    and libfencing ignores any action parameter that makes it that far.

diff --git a/fencing/commands.c b/fencing/commands.c
index e7fce78b0..b4e6eb502 100644
--- a/fencing/commands.c
+++ b/fencing/commands.c
@@ -783,6 +783,70 @@ read_action_metadata(stonith_device_t *device)
     freeXpathObject(xpath);
 }
 
+/*!
+ * \internal
+ * \brief Set a pcmk_*_action parameter if not already set
+ *
+ * \param[in,out] params  Device parameters
+ * \param[in]     action  Name of action
+ * \param[in]     value   Value to use if action is not already set
+ */
+static void
+map_action(GHashTable *params, const char *action, const char *value)
+{
+    char *key = crm_strdup_printf("pcmk_%s_action", action);
+
+    if (g_hash_table_lookup(params, key)) {
+        crm_warn("Ignoring %s='%s', see %s instead",
+                 STONITH_ATTR_ACTION_OP, value, key);
+        free(key);
+    } else {
+        crm_warn("Mapping %s='%s' to %s='%s'",
+                 STONITH_ATTR_ACTION_OP, value, key, value);
+        g_hash_table_insert(params, key, strdup(value));
+    }
+}
+
+/*!
+ * \internal
+ * \brief Create device parameter table from XML
+ *
+ * \param[in]     name    Device name (used for logging only)
+ * \param[in,out] params  Device parameters
+ */
+static GHashTable *
+xml2device_params(const char *name, xmlNode *dev)
+{
+    GHashTable *params = xml2list(dev);
+    const char *value;
+
+    /* Action should never be specified in the device configuration,
+     * but we support it for users who are familiar with other software
+     * that worked that way.
+     */
+    value = g_hash_table_lookup(params, STONITH_ATTR_ACTION_OP);
+    if (value != NULL) {
+        crm_warn("%s has '%s' parameter, which should never be specified in configuration",
+                 name, STONITH_ATTR_ACTION_OP);
+
+        if (strcmp(value, "reboot") == 0) {
+            crm_warn("Ignoring %s='reboot' (see stonith-action cluster property instead)",
+                     STONITH_ATTR_ACTION_OP);
+
+        } else if (strcmp(value, "off") == 0) {
+            map_action(params, "reboot", value);
+
+        } else {
+            map_action(params, "off", value);
+            map_action(params, "reboot", value);
+        }
+
+        g_hash_table_remove(params, STONITH_ATTR_ACTION_OP);
+    }
+
+    return params;
+}
+
 static stonith_device_t *
 build_device_from_xml(xmlNode * msg)
 {
@@ -794,7 +858,7 @@ build_device_from_xml(xmlNode * msg)
     device->id = crm_element_value_copy(dev, XML_ATTR_ID);
     device->agent = crm_element_value_copy(dev, "agent");
     device->namespace = crm_element_value_copy(dev, "namespace");
-    device->params = xml2list(dev);
+    device->params = xml2device_params(device->id, dev);
 
     value = g_hash_table_lookup(device->params, STONITH_ATTR_HOSTLIST);
     if (value) {
diff --git a/lib/fencing/st_client.c b/lib/fencing/st_client.c
index c7f4079af..34bba8829 100644
--- a/lib/fencing/st_client.c
+++ b/lib/fencing/st_client.c
@@ -394,11 +394,10 @@ stonith_api_register_level(stonith_t * st, int options, const char *node, int le
 }
 
 static void
-append_arg(gpointer key, gpointer value, gpointer user_data)
+append_arg(const char *key, const char *value, char **args)
 {
     int len = 3;                /* =, \n, \0 */
     int last = 0;
-    char **args = user_data;
 
     CRM_CHECK(key != NULL, return);
     CRM_CHECK(value != NULL, return);
@@ -418,22 +417,20 @@ append_arg(gpointer key, gpointer value, gpointer user_data)
     }
 
     *args = realloc_safe(*args, last + len);
-    crm_trace("Appending: %s=%s", (char *)key, (char *)value);
-    sprintf((*args) + last, "%s=%s\n", (char *)key, (char *)value);
+    crm_trace("Appending: %s=%s", key, value);
+    sprintf((*args) + last, "%s=%s\n", key, value);
 }
 
 static void
-append_const_arg(const char *key, const char *value, char **arg_list)
+append_config_arg(gpointer key, gpointer value, gpointer user_data)
 {
-    CRM_LOG_ASSERT(key && value);
-    if(key && value) {
-        char *glib_sucks_key = strdup(key);
-        char *glib_sucks_value = strdup(value);
-
-        append_arg(glib_sucks_key, glib_sucks_value, arg_list);
-
-        free(glib_sucks_value);
-        free(glib_sucks_key);
+    /* stonithd will filter action out when it registers the device,
+     * but ignore it here just in case any other library callers
+     * fail to do so.
+     */
+    if (safe_str_neq(key, STONITH_ATTR_ACTION_OP)) {
+        append_arg(key, value, user_data);
+        return;
     }
 }
 
@@ -446,7 +443,7 @@ append_host_specific_args(const char *victim, const char *map, GHashTable * para
     if (map == NULL) {
         /* The best default there is for now... */
         crm_debug("Using default arg map: port=uname");
-        append_const_arg("port", victim, arg_list);
+        append_arg("port", victim, arg_list);
         return;
     }
 
@@ -489,7 +486,7 @@ append_host_specific_args(const char *victim, const char *map, GHashTable * para
 
             if (value) {
                 crm_debug("Setting '%s'='%s' (%s) for %s", name, value, param, victim);
-                append_const_arg(name, value, arg_list);
+                append_arg(name, value, arg_list);
 
             } else {
                 crm_err("No node attribute '%s' for '%s'", name, victim);
@@ -516,7 +513,6 @@ make_args(const char *agent, const char *action, const char *victim, uint32_t vi
     char buffer[512];
     char *arg_list = NULL;
     const char *value = NULL;
-    const char *_action = action;
 
     CRM_CHECK(action != NULL, return NULL);
 
@@ -542,7 +538,7 @@ make_args(const char *agent, const char *action, const char *victim, uint32_t vi
         action = value;
     }
 
-    append_const_arg(STONITH_ATTR_ACTION_OP, action, &arg_list);
+    append_arg(STONITH_ATTR_ACTION_OP, action, &arg_list);
     if (victim && device_args) {
         const char *alias = victim;
         const char *param = g_hash_table_lookup(device_args, STONITH_ATTR_HOSTARG);
@@ -554,13 +550,13 @@ make_args(const char *agent, const char *action, const char *victim, uint32_t vi
         /* Always supply the node's name too:
          *    https://fedorahosted.org/cluster/wiki/FenceAgentAPI
          */
-        append_const_arg("nodename", victim, &arg_list);
+        append_arg("nodename", victim, &arg_list);
         if (victim_nodeid) {
             char nodeid_str[33] = { 0, };
             if (snprintf(nodeid_str, 33, "%u", (unsigned int)victim_nodeid)) {
                 crm_info("For stonith action (%s) for victim %s, adding nodeid (%s) to parameters",
                          action, victim, nodeid_str);
-                append_const_arg("nodeid", nodeid_str, &arg_list);
+                append_arg("nodeid", nodeid_str, &arg_list);
             }
         }
 
@@ -592,24 +588,12 @@ make_args(const char *agent, const char *action, const char *victim, uint32_t vi
         if (value == NULL || safe_str_eq(value, "dynamic")) {
             crm_debug("Performing %s action for node '%s' as '%s=%s'", action, victim, param,
                       alias);
-            append_const_arg(param, alias, &arg_list);
+            append_arg(param, alias, &arg_list);
         }
     }
 
     if (device_args) {
-        g_hash_table_foreach(device_args, append_arg, &arg_list);
-    }
-
-    if(device_args && g_hash_table_lookup(device_args, STONITH_ATTR_ACTION_OP)) {
-        if(safe_str_eq(_action,"list")
-           || safe_str_eq(_action,"status")
-           || safe_str_eq(_action,"monitor")
-           || safe_str_eq(_action,"metadata")) {
-            /* Force use of the calculated command for support ops
-             * We don't want list or monitor ops initiating fencing, regardless of what the admin configured
-             */
-            append_const_arg(STONITH_ATTR_ACTION_OP, action, &arg_list);
-        }
+        g_hash_table_foreach(device_args, append_config_arg, &arg_list);
     }
 
     return arg_list;
