Merge pull request ClusterLabs#1355 from kgaillot/fixes11

Lots of small fixes

Author: Ken Gaillot

include/crm/common/internal.h

@@ -67,6 +67,7 @@ gboolean crm_ends_with_ext(const char *s, const char *match);
 char *add_list_element(char *list, const char *value);
 bool crm_compress_string(const char *data, int length, int max, char **result,
                          unsigned int *result_len);
+gint crm_alpha_sort(gconstpointer a, gconstpointer b);
 
 static inline int
 crm_strlen_zero(const char *s)

lib/cluster/membership.c

@@ -837,10 +837,19 @@ crm_update_peer(const char *source, unsigned int id, uint64_t born, uint64_t see
 void
 crm_update_peer_uname(crm_node_t *node, const char *uname)
 {
-    int i, len = strlen(uname);
+    CRM_CHECK(uname != NULL,
+              crm_err("Bug: can't update node name without name"); return);
+    CRM_CHECK(node != NULL,
+              crm_err("Bug: can't update node name to %s without node", uname);
+              return);
+
+    if (safe_str_eq(uname, node->uname)) {
+        crm_debug("Node uname '%s' did not change", uname);
+        return;
+    }
 
-    for (i = 0; i < len; i++) {
-        if (uname[i] >= 'A' && uname[i] <= 'Z') {
+    for (const char *c = uname; *c; ++c) {
+        if ((*c >= 'A') && (*c <= 'Z')) {
             crm_warn("Node names with capitals are discouraged, consider changing '%s'",
                      uname);
             break;
@@ -849,6 +858,8 @@ crm_update_peer_uname(crm_node_t *node, const char *uname)
 
     free(node->uname);
     node->uname = strdup(uname);
+    CRM_ASSERT(node->uname != NULL);
+
     if (crm_status_callback) {
         crm_status_callback(crm_status_uname, node, NULL);
     }

lib/common/strings.c

@@ -427,3 +427,27 @@ crm_compress_string(const char *data, int length, int max, char **result, unsign
     *result = compressed;
     return TRUE;
 }
+
+/*!
+ * \brief Compare two strings alphabetically (case-insensitive)
+ *
+ * \param[in] a  First string to compare
+ * \param[in] b  Second string to compare
+ *
+ * \return 0 if strings are equal, -1 if a < b, 1 if a > b
+ *
+ * \note Usable as a GCompareFunc with g_list_sort().
+ *       NULL is considered less than non-NULL.
+ */
+gint
+crm_alpha_sort(gconstpointer a, gconstpointer b)
+{
+    if (!a && !b) {
+        return 0;
+    } else if (!a) {
+        return -1;
+    } else if (!b) {
+        return 1;
+    }
+    return strcasecmp(a, b);
+}

lib/pengine/container.c

@@ -18,6 +18,8 @@
 
 #include <crm_internal.h>
 
+#include <ctype.h>
+
 #include <crm/pengine/rules.h>
 #include <crm/pengine/status.h>
 #include <crm/pengine/internal.h>
@@ -1123,7 +1125,15 @@ container_print_xml(resource_t * rsc, const char *pre_text, long options, void *
 
     status_print("%s<bundle ", pre_text);
     status_print("id=\"%s\" ", rsc->id);
-    status_print("type=\"%s\" ", container_type_as_string(container_data->type));
+
+    // Always lowercase the container technology type for use as XML value
+    status_print("type=\"");
+    for (const char *c = container_type_as_string(container_data->type);
+         *c; ++c) {
+        status_print("%c", tolower(*c));
+    }
+    status_print("\" ");
+
     status_print("image=\"%s\" ", container_data->image);
     status_print("unique=\"%s\" ", is_set(rsc->flags, pe_rsc_unique)? "true" : "false");
     status_print("managed=\"%s\" ", is_set(rsc->flags, pe_rsc_managed) ? "true" : "false");

lib/services/services.c

@@ -904,7 +904,7 @@ handle_blocked_ops(void)
     "<resource-agent name='%s' version='" PCMK_DEFAULT_AGENT_VERSION "'>\n" \
     "  <version>1.0</version>\n"                                        \
     "  <longdesc lang='en'>\n"                                          \
-    "    %s\n"                                                          \
+    "%s"                                                                \
     "  </longdesc>\n"                                                   \
     "  <shortdesc lang='en'>%s</shortdesc>\n"                           \
     "  <parameters>\n"                                                  \
@@ -929,6 +929,9 @@ handle_blocked_ops(void)
     "  </special>\n"                                                    \
     "</resource-agent>\n"
 
+/* See "Comment Conventions for Init Scripts" in the LSB core specification at:
+ * http://refspecs.linuxfoundation.org/lsb.shtml
+ */
 #define LSB_INITSCRIPT_INFOBEGIN_TAG "### BEGIN INIT INFO"
 #define LSB_INITSCRIPT_INFOEND_TAG "### END INIT INFO"
 #define PROVIDES    "# Provides:"
@@ -962,7 +965,7 @@ handle_blocked_ops(void)
 static inline gboolean
 lsb_meta_helper_get_value(const char *line, char **value, const char *prefix)
 {
-    if (!*value && !strncasecmp(line, prefix, strlen(prefix))) {
+    if (!*value && !strncmp(line, prefix, strlen(prefix))) {
         *value = (char *)xmlEncodeEntitiesReentrant(NULL, BAD_CAST line+strlen(prefix));
         return TRUE;
     }
@@ -975,8 +978,8 @@ static int
 lsb_get_metadata(const char *type, char **output)
 {
     char ra_pathname[PATH_MAX] = { 0, };
-    FILE *fp;
-    char buffer[1024];
+    FILE *fp = NULL;
+    char buffer[1024] = { 0, };
     char *provides = NULL;
     char *req_start = NULL;
     char *req_stop = NULL;
@@ -987,7 +990,8 @@ lsb_get_metadata(const char *type, char **output)
     char *s_dscrpt = NULL;
     char *xml_l_dscrpt = NULL;
     int offset = 0;
-    char description[DESC_MAX];
+    bool in_header = FALSE;
+    char description[DESC_MAX] = { 0, };
 
     if (type[0] == '/') {
         snprintf(ra_pathname, sizeof(ra_pathname), "%s", type);
@@ -1005,6 +1009,16 @@ lsb_get_metadata(const char *type, char **output)
     /* Enter into the LSB-compliant comment block */
     while (fgets(buffer, sizeof(buffer), fp)) {
 
+        // Ignore lines up to and including the block delimiter
+        if (!strncmp(buffer, LSB_INITSCRIPT_INFOBEGIN_TAG,
+                     strlen(LSB_INITSCRIPT_INFOBEGIN_TAG))) {
+            in_header = TRUE;
+            continue;
+        }
+        if (!in_header) {
+            continue;
+        }
+
         /* Assume each of the following eight arguments contain one line */
         if (lsb_meta_helper_get_value(buffer, &provides, PROVIDES)) {
             continue;
@@ -1032,37 +1046,48 @@ lsb_get_metadata(const char *type, char **output)
         }
 
         /* Long description may cross multiple lines */
-        if ((offset == 0)
-            && !strncasecmp(buffer, DESCRIPTION, strlen(DESCRIPTION))) {
-            /* Between # and keyword, more than one space, or a tab
-             * character, indicates the continuation line.
-             *
-             * Extracted from LSB init script standard
-             */
+        if ((offset == 0) // haven't already found long description
+            && !strncmp(buffer, DESCRIPTION, strlen(DESCRIPTION))) {
+            bool processed_line = TRUE;
+
+            // Get remainder of description line itself
+            offset += snprintf(description, DESC_MAX, "%s",
+                               buffer + strlen(DESCRIPTION));
+
+            // Read any continuation lines of the description
+            buffer[0] = '\0';
             while (fgets(buffer, sizeof(buffer), fp)) {
                 if (!strncmp(buffer, "#  ", 3) || !strncmp(buffer, "#\t", 2)) {
-                    buffer[0] = ' ';
+                    /* '#' followed by a tab or more than one space indicates a
+                     * continuation of the long description.
+                     */
                     offset += snprintf(description + offset, DESC_MAX - offset,
-                                       "%s", buffer);
+                                       "%s", buffer + 1);
                 } else {
-                    fputs(buffer, fp);
-                    break;      /* Long description ends */
+                    /* This line is not part of the long description,
+                     * so continue with normal processing.
+                     */
+                    processed_line = FALSE;
+                    break;
                 }
             }
-            continue;
-        }
 
-        if ((xml_l_dscrpt == NULL) && (offset > 0)) {
+            // Make long description safe to use in XML
             xml_l_dscrpt = (char *)xmlEncodeEntitiesReentrant(NULL, BAD_CAST(description));
+
+            if (processed_line) {
+                // We grabbed the line into the long description
+                continue;
+            }
         }
 
-        if (!strncasecmp(buffer, LSB_INITSCRIPT_INFOEND_TAG,
-                         strlen(LSB_INITSCRIPT_INFOEND_TAG))) {
-            /* Get to the out border of LSB comment block */
+        // Stop if we leave the header block
+        if (!strncmp(buffer, LSB_INITSCRIPT_INFOEND_TAG,
+                     strlen(LSB_INITSCRIPT_INFOEND_TAG))) {
             break;
         }
         if (buffer[0] != '#') {
-            break;              /* Out of comment block in the beginning */
+            break;
         }
     }
     fclose(fp);

lib/services/systemd.c

@@ -357,7 +357,7 @@ systemd_unit_by_name(const gchar * arg_name, svc_action_t *op)
 GList *
 systemd_unit_listall(void)
 {
-    int lpc = 0;
+    int nfiles = 0;
     GList *units = NULL;
     DBusMessageIter args;
     DBusMessageIter unit;
@@ -369,64 +369,95 @@ systemd_unit_listall(void)
     }
 
 /*
-        "  <method name=\"ListUnits\">\n"                               \
-        "   <arg name=\"units\" type=\"a(ssssssouso)\" direction=\"out\"/>\n" \
+        "  <method name=\"ListUnitFiles\">\n"                               \
+        "   <arg name=\"files\" type=\"a(ss)\" direction=\"out\"/>\n" \
         "  </method>\n"                                                 \
 */
 
-    reply = systemd_call_simple_method("ListUnits");
+    reply = systemd_call_simple_method("ListUnitFiles");
     if (reply == NULL) {
         return NULL;
     }
     if (!dbus_message_iter_init(reply, &args)) {
-        crm_err("Could not list systemd units: systemd reply has no arguments");
+        crm_err("Could not list systemd unit files: systemd reply has no arguments");
         dbus_message_unref(reply);
         return NULL;
     }
     if (!pcmk_dbus_type_check(reply, &args, DBUS_TYPE_ARRAY,
                               __FUNCTION__, __LINE__)) {
-        crm_err("Could not list systemd units: systemd reply has invalid arguments");
+        crm_err("Could not list systemd unit files: systemd reply has invalid arguments");
         dbus_message_unref(reply);
         return NULL;
     }
 
     dbus_message_iter_recurse(&args, &unit);
-    while (dbus_message_iter_get_arg_type (&unit) != DBUS_TYPE_INVALID) {
+    for (; dbus_message_iter_get_arg_type(&unit) != DBUS_TYPE_INVALID;
+        dbus_message_iter_next(&unit)) {
+
         DBusBasicValue value;
+        const char *match = NULL;
+        char *unit_name = NULL;
+        char *basename = NULL;
 
         if(!pcmk_dbus_type_check(reply, &unit, DBUS_TYPE_STRUCT, __FUNCTION__, __LINE__)) {
+            crm_debug("ListUnitFiles reply has unexpected type");
             continue;
         }
 
         dbus_message_iter_recurse(&unit, &elem);
         if(!pcmk_dbus_type_check(reply, &elem, DBUS_TYPE_STRING, __FUNCTION__, __LINE__)) {
+            crm_debug("ListUnitFiles reply does not contain a string");
             continue;
         }
 
         dbus_message_iter_get_basic(&elem, &value);
-        crm_trace("DBus ListUnits listed: %s", value.str);
-        if(value.str) {
-            const char *match = systemd_unit_extension(value.str);
-
-            if (match) {
-                char *unit_name;
-
-                if (!strcmp(match, ".service")) {
-                    /* service is the "default" unit type, so strip it */
-                    unit_name = strndup(value.str, match - value.str);
-                } else {
-                    unit_name = strdup(value.str);
-                }
-                lpc++;
-                units = g_list_append(units, unit_name);
-            }
+        if (value.str == NULL) {
+            crm_debug("ListUnitFiles reply did not provide a string");
+            continue;
+        }
+        crm_trace("DBus ListUnitFiles listed: %s", value.str);
+
+        match = systemd_unit_extension(value.str);
+        if (match == NULL) {
+            // Unit files always have an extension, so skip if not present
+            crm_debug("ListUnitFiles entry '%s' does not have an extension",
+                      value.str);
+            continue;
+        }
+
+        // ListUnitFiles returns full path names
+        basename = strrchr(value.str, '/');
+        if (basename) {
+            basename = basename + 1;
+        } else {
+            basename = value.str;
+        }
+
+        /* Unit files will include types (such as .target) that we can't manage,
+         * so filter the replies here.
+         */
+        if (!strcmp(match, ".service")) {
+            // Service is the "default" unit type, so strip it
+            unit_name = strndup(basename, match - basename);
+
+        } else if (!strcmp(match, ".mount")
+                   || !strcmp(match, ".socket")) {
+            unit_name = strdup(basename);
+        }
+        if (unit_name == NULL) {
+            crm_trace("ListUnitFiles entry '%s' is not manageable",
+                      value.str);
+            continue;
         }
-        dbus_message_iter_next (&unit);
+
+        nfiles++;
+        units = g_list_prepend(units, unit_name);
     }
 
     dbus_message_unref(reply);
 
-    crm_trace("Found %d systemd services", lpc);
+    crm_trace("Found %d manageable systemd unit files", nfiles);
+    units = g_list_sort(units, crm_alpha_sort);
     return units;
 }
 

mcp/pacemaker.sysconfig

@@ -64,7 +64,7 @@
 # start, unless they were previously put into standby mode. If this variable is
 # set to "standby" or "online", it will force this node to join in the
 # specified state when starting.
-# (experimental; currently ignored for Pacemaker Remote nodes)
+# (only supported for cluster nodes, not Pacemaker Remote nodes)
 # PCMK_node_start_state=default
 
 # If the cluster uses an older version of corosync (prior to 2.0), set this to

pengine/graph.c

@@ -442,7 +442,11 @@ update_colo_start_chain(action_t *action)
     if (is_not_set(action->flags, pe_action_runnable) && safe_str_eq(action->task, RSC_START)) {
         rsc = uber_parent(action->rsc);
         if (rsc->parent) {
-            // This is a bundle (uber_parent() stops _before_ the bundle)
+            /* For bundles, uber_parent() returns the clone/master, not the
+             * bundle, so the existence of rsc->parent implies this is a bundle.
+             * In this case, we need the bundle resource, so that we can check
+             * if all containers are stopped/stopping.
+             */
             rsc = rsc->parent;
         }
     }

pengine/regression.sh

@@ -866,6 +866,7 @@ do_test remote-recover-all        "Fencing when the connection has no home"
 do_test remote-recover-no-resources   "Fencing when the connection has no home and no active resources"
 do_test remote-recover-unknown        "Fencing when the connection has no home and the remote has no operation history"
 do_test remote-reconnect-delay        "Waiting for remote reconnect interval to expire"
+do_test remote-connection-unrecoverable  "Remote connection host must be fenced, with connection unrecoverable"
 
 echo ""
 do_test resource-discovery      "Exercises resource-discovery location constraint option."