From a32b6e14ba51fefbda2d4a699cf1c48dd3a1bb5a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 7 Jan 2021 11:37:51 -0500 Subject: [PATCH 01/10] Fix: tools: Don't pass stonith history to print_simple_status. It's not being used. --- tools/crm_mon.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 4555516..729f6a1 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.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. * @@ -1451,14 +1451,13 @@ main(int argc, char **argv) * \brief Print one-line status suitable for use with monitoring software * * \param[in] data_set Working set of CIB state - * \param[in] history List of stonith actions * * \note This function's output (and the return code when the program exits) * should conform to https://www.monitoring-plugins.org/doc/guidelines.html */ static void print_simple_status(pcmk__output_t *out, pe_working_set_t * data_set, - stonith_history_t *history, unsigned int mon_ops) + unsigned int mon_ops) { GListPtr gIter = NULL; int nodes_online = 0; @@ -2012,7 +2011,7 @@ mon_refresh_display(gpointer user_data) break; case mon_output_monitor: - print_simple_status(out, mon_data_set, stonith_history, options.mon_ops); + print_simple_status(out, mon_data_set, options.mon_ops); if (pcmk_is_set(options.mon_ops, mon_op_has_warnings)) { clean_up(MON_STATUS_WARN); return FALSE; -- 1.8.3.1 From 8b9c47089c70295bc0529671ba5991c6d831e14b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 7 Jan 2021 13:17:04 -0500 Subject: [PATCH 02/10] Refactor: tools: Don't pass output_format to mon_refresh_display. output_format is a global variable. --- tools/crm_mon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 729f6a1..b801560 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -827,7 +827,7 @@ cib_connect(gboolean full) rc = cib->cmds->query(cib, NULL, ¤t_cib, cib_scope_local | cib_sync_call); if (rc == pcmk_ok) { - mon_refresh_display(&output_format); + mon_refresh_display(NULL); } if (rc == pcmk_ok && full) { -- 1.8.3.1 From a1b14ad96f12746167da8588dc086b20e6f6d1d6 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 7 Jan 2021 13:20:14 -0500 Subject: [PATCH 03/10] Refactor: tools: Remove unnecessary checks for cib != NULL. cib is guaranteed to not be NULL at these points, so there's no need to do an additional check. This code was leftover from a previous reorganization that changed when the cib variable gets initialized. --- tools/crm_mon.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index b801560..1eedd38 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -1346,7 +1346,7 @@ main(int argc, char **argv) /* Extra sanity checks when in CGI mode */ if (output_format == mon_output_cgi) { - if (cib && cib->variant == cib_file) { + if (cib->variant == cib_file) { g_set_error(&error, PCMK__EXITC_ERROR, CRM_EX_USAGE, "CGI mode used with CIB file"); return clean_up(CRM_EX_USAGE); } else if (options.external_agent != NULL) { @@ -1370,33 +1370,30 @@ main(int argc, char **argv) crm_info("Starting %s", crm_system_name); - if (cib) { - - do { - if (!pcmk_is_set(options.mon_ops, mon_op_one_shot)) { - print_as(output_format ,"Waiting until cluster is available on this node ...\n"); - } - rc = cib_connect(!pcmk_is_set(options.mon_ops, mon_op_one_shot)); + do { + if (!pcmk_is_set(options.mon_ops, mon_op_one_shot)) { + print_as(output_format ,"Waiting until cluster is available on this node ...\n"); + } + rc = cib_connect(!pcmk_is_set(options.mon_ops, mon_op_one_shot)); - if (pcmk_is_set(options.mon_ops, mon_op_one_shot)) { - break; + if (pcmk_is_set(options.mon_ops, mon_op_one_shot)) { + break; - } else if (rc != pcmk_ok) { - sleep(options.reconnect_msec / 1000); + } else if (rc != pcmk_ok) { + sleep(options.reconnect_msec / 1000); #if CURSES_ENABLED - if (output_format == mon_output_console) { - clear(); - refresh(); - } + if (output_format == mon_output_console) { + clear(); + refresh(); + } #endif - } else { - if (output_format == mon_output_html && out->dest != stdout) { - printf("Writing html to %s ...\n", args->output_dest); - } + } else { + if (output_format == mon_output_html && out->dest != stdout) { + printf("Writing html to %s ...\n", args->output_dest); } + } - } while (rc == -ENOTCONN); - } + } while (rc == -ENOTCONN); if (rc != pcmk_ok) { if (output_format == mon_output_monitor) { -- 1.8.3.1 From fe5284a12765e775905bdfe58711c5733a063132 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 7 Jan 2021 14:15:40 -0500 Subject: [PATCH 04/10] Fix: tools: mon_refresh_display should return an int. While GSourceFunc is defined as returning a boolean, our public API mainloop function expect the dispatch function to return an int. So change mon_refresh_display to do so. --- tools/crm_mon.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 1eedd38..8657a89 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -125,7 +125,7 @@ struct { static void clean_up_connections(void); static crm_exit_t clean_up(crm_exit_t exit_code); static void crm_diff_update(const char *event, xmlNode * msg); -static gboolean mon_refresh_display(gpointer user_data); +static int mon_refresh_display(gpointer user_data); static int cib_connect(gboolean full); static void mon_st_callback_event(stonith_t * st, stonith_event_t * e); static void mon_st_callback_display(stonith_t * st, stonith_event_t * e); @@ -1925,7 +1925,7 @@ crm_diff_update(const char *event, xmlNode * msg) kick_refresh(cib_updated); } -static gboolean +static int mon_refresh_display(gpointer user_data) { xmlNode *cib_copy = copy_xml(current_cib); @@ -1940,7 +1940,7 @@ mon_refresh_display(gpointer user_data) } out->err(out, "Upgrade failed: %s", pcmk_strerror(-pcmk_err_schema_validation)); clean_up(CRM_EX_CONFIG); - return FALSE; + return 0; } /* get the stonith-history if there is evidence we need it @@ -1966,7 +1966,7 @@ mon_refresh_display(gpointer user_data) } free_xml(cib_copy); out->err(out, "Reading stonith-history failed"); - return FALSE; + return 0; } if (mon_data_set == NULL) { @@ -1995,7 +1995,7 @@ mon_refresh_display(gpointer user_data) options.only_node, options.only_rsc) != 0) { g_set_error(&error, PCMK__EXITC_ERROR, CRM_EX_CANTCREAT, "Critical: Unable to output html file"); clean_up(CRM_EX_CANTCREAT); - return FALSE; + return 0; } break; @@ -2044,7 +2044,7 @@ mon_refresh_display(gpointer user_data) stonith_history_free(stonith_history); stonith_history = NULL; pe_reset_working_set(mon_data_set); - return TRUE; + return 1; } static void -- 1.8.3.1 From 7f88a5a428ed73fb5161096ece2517abe1119f06 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 7 Jan 2021 16:59:02 -0500 Subject: [PATCH 05/10] Refactor: tools: Change a conditional in cib_connect. This allows unindenting everything that occurs inside that conditional, which I think makes it a little bit easier to understand what is going on. --- tools/crm_mon.c | 86 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 8657a89..b8ba56b 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -804,55 +804,57 @@ cib_connect(gboolean full) } } - if (cib->state != cib_connected_query && cib->state != cib_connected_command) { - crm_trace("Connecting to the CIB"); - - /* Hack: the CIB signon will print the prompt for a password if needed, - * but to stderr. If we're in curses, show it on the screen instead. - * - * @TODO Add a password prompt (maybe including input) function to - * pcmk__output_t and use it in libcib. - */ - if ((output_format == mon_output_console) && need_pass && (cib->variant == cib_remote)) { - need_pass = FALSE; - print_as(output_format, "Password:"); - } + if (cib->state == cib_connected_query || cib->state == cib_connected_command) { + return rc; + } - rc = cib->cmds->signon(cib, crm_system_name, cib_query); - if (rc != pcmk_ok) { - out->err(out, "Could not connect to the CIB: %s", - pcmk_strerror(rc)); - return rc; - } + crm_trace("Connecting to the CIB"); - rc = cib->cmds->query(cib, NULL, ¤t_cib, cib_scope_local | cib_sync_call); - if (rc == pcmk_ok) { - mon_refresh_display(NULL); - } + /* Hack: the CIB signon will print the prompt for a password if needed, + * but to stderr. If we're in curses, show it on the screen instead. + * + * @TODO Add a password prompt (maybe including input) function to + * pcmk__output_t and use it in libcib. + */ + if ((output_format == mon_output_console) && need_pass && (cib->variant == cib_remote)) { + need_pass = FALSE; + print_as(output_format, "Password:"); + } - if (rc == pcmk_ok && full) { - if (rc == pcmk_ok) { - rc = cib->cmds->set_connection_dnotify(cib, mon_cib_connection_destroy_regular); - if (rc == -EPROTONOSUPPORT) { - print_as - (output_format, "Notification setup not supported, won't be able to reconnect after failure"); - if (output_format == mon_output_console) { - sleep(2); - } - rc = pcmk_ok; - } + rc = cib->cmds->signon(cib, crm_system_name, cib_query); + if (rc != pcmk_ok) { + out->err(out, "Could not connect to the CIB: %s", + pcmk_strerror(rc)); + return rc; + } - } + rc = cib->cmds->query(cib, NULL, ¤t_cib, cib_scope_local | cib_sync_call); + if (rc == pcmk_ok) { + mon_refresh_display(NULL); + } - if (rc == pcmk_ok) { - cib->cmds->del_notify_callback(cib, T_CIB_DIFF_NOTIFY, crm_diff_update); - rc = cib->cmds->add_notify_callback(cib, T_CIB_DIFF_NOTIFY, crm_diff_update); + if (rc == pcmk_ok && full) { + if (rc == pcmk_ok) { + rc = cib->cmds->set_connection_dnotify(cib, mon_cib_connection_destroy_regular); + if (rc == -EPROTONOSUPPORT) { + print_as + (output_format, "Notification setup not supported, won't be able to reconnect after failure"); + if (output_format == mon_output_console) { + sleep(2); + } + rc = pcmk_ok; } - if (rc != pcmk_ok) { - out->err(out, "Notification setup failed, could not monitor CIB actions"); - clean_up_connections(); - } + } + + if (rc == pcmk_ok) { + cib->cmds->del_notify_callback(cib, T_CIB_DIFF_NOTIFY, crm_diff_update); + rc = cib->cmds->add_notify_callback(cib, T_CIB_DIFF_NOTIFY, crm_diff_update); + } + + if (rc != pcmk_ok) { + out->err(out, "Notification setup failed, could not monitor CIB actions"); + clean_up_connections(); } } return rc; -- 1.8.3.1 From 178ba17e4ee62bef28f8e71cad2c002f823661b5 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 7 Jan 2021 17:37:36 -0500 Subject: [PATCH 06/10] Refactor: tools: Remove an unnecessary conditional in cib_connect. --- tools/crm_mon.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index b8ba56b..36249e8 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -834,17 +834,14 @@ cib_connect(gboolean full) } if (rc == pcmk_ok && full) { - if (rc == pcmk_ok) { - rc = cib->cmds->set_connection_dnotify(cib, mon_cib_connection_destroy_regular); - if (rc == -EPROTONOSUPPORT) { - print_as - (output_format, "Notification setup not supported, won't be able to reconnect after failure"); - if (output_format == mon_output_console) { - sleep(2); - } - rc = pcmk_ok; + rc = cib->cmds->set_connection_dnotify(cib, mon_cib_connection_destroy_regular); + if (rc == -EPROTONOSUPPORT) { + print_as + (output_format, "Notification setup not supported, won't be able to reconnect after failure"); + if (output_format == mon_output_console) { + sleep(2); } - + rc = pcmk_ok; } if (rc == pcmk_ok) { -- 1.8.3.1 From 33bac5886417afc5c7bbf56f4d31e0e36f8ae947 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 8 Jan 2021 10:00:50 -0500 Subject: [PATCH 07/10] Refactor: tools: Simplify another conditional in crm_mon. --- tools/crm_mon.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 36249e8..8b47bbc 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -1386,10 +1386,8 @@ main(int argc, char **argv) refresh(); } #endif - } else { - if (output_format == mon_output_html && out->dest != stdout) { - printf("Writing html to %s ...\n", args->output_dest); - } + } else if (output_format == mon_output_html && out->dest != stdout) { + printf("Writing html to %s ...\n", args->output_dest); } } while (rc == -ENOTCONN); -- 1.8.3.1 From 40bc8b3147e7ebef4318211fa69973a8b5d32e79 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Fri, 8 Jan 2021 12:12:37 -0500 Subject: [PATCH 08/10] Refactor: libcrmcommon,tools,daemons: Put common xpath code in one place. --- configure.ac | 1 + daemons/controld/controld_te_callbacks.c | 26 +++----------- include/crm/common/xml_internal.h | 14 +++++++- lib/common/tests/Makefile.am | 2 +- lib/common/tests/xpath/Makefile.am | 29 +++++++++++++++ lib/common/tests/xpath/pcmk__xpath_node_id_test.c | 43 +++++++++++++++++++++++ lib/common/xpath.c | 34 +++++++++++++++++- tools/crm_mon.c | 25 ++----------- 8 files changed, 127 insertions(+), 47 deletions(-) create mode 100644 lib/common/tests/xpath/Makefile.am create mode 100644 lib/common/tests/xpath/pcmk__xpath_node_id_test.c diff --git a/configure.ac b/configure.ac index 5959116..ce0f1fe 100644 --- a/configure.ac +++ b/configure.ac @@ -1920,6 +1920,7 @@ AC_CONFIG_FILES(Makefile \ lib/common/tests/operations/Makefile \ lib/common/tests/strings/Makefile \ lib/common/tests/utils/Makefile \ + lib/common/tests/xpath/Makefile \ lib/cluster/Makefile \ lib/cib/Makefile \ lib/gnu/Makefile \ diff --git a/daemons/controld/controld_te_callbacks.c b/daemons/controld/controld_te_callbacks.c index 66fc645..4e3e4e6 100644 --- a/daemons/controld/controld_te_callbacks.c +++ b/daemons/controld/controld_te_callbacks.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. * @@ -276,24 +276,6 @@ process_resource_updates(const char *node, xmlNode *xml, xmlNode *change, } } -#define NODE_PATT "/lrm[@id=" -static char *get_node_from_xpath(const char *xpath) -{ - char *nodeid = NULL; - char *tmp = strstr(xpath, NODE_PATT); - - if(tmp) { - tmp += strlen(NODE_PATT); - tmp += 1; - - nodeid = strdup(tmp); - tmp = strstr(nodeid, "\'"); - CRM_ASSERT(tmp); - tmp[0] = 0; - } - return nodeid; -} - static char *extract_node_uuid(const char *xpath) { char *mutable_path = strdup(xpath); @@ -522,19 +504,19 @@ te_update_diff_v2(xmlNode *diff) process_resource_updates(ID(match), match, change, op, xpath); } else if (strcmp(name, XML_LRM_TAG_RESOURCES) == 0) { - char *local_node = get_node_from_xpath(xpath); + char *local_node = pcmk__xpath_node_id(xpath, "lrm"); process_resource_updates(local_node, match, change, op, xpath); free(local_node); } else if (strcmp(name, XML_LRM_TAG_RESOURCE) == 0) { - char *local_node = get_node_from_xpath(xpath); + char *local_node = pcmk__xpath_node_id(xpath, "lrm"); process_lrm_resource_diff(match, local_node); free(local_node); } else if (strcmp(name, XML_LRM_TAG_RSC_OP) == 0) { - char *local_node = get_node_from_xpath(xpath); + char *local_node = pcmk__xpath_node_id(xpath, "lrm"); process_graph_event(match, local_node); free(local_node); diff --git a/include/crm/common/xml_internal.h b/include/crm/common/xml_internal.h index 1e80bc6..d8694ee 100644 --- a/include/crm/common/xml_internal.h +++ b/include/crm/common/xml_internal.h @@ -1,5 +1,5 @@ /* - * Copyright 2017-2020 the Pacemaker project contributors + * Copyright 2017-2021 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -273,4 +273,16 @@ pcmk__xe_first_attr(const xmlNode *xe) return (xe == NULL)? NULL : xe->properties; } +/*! + * \internal + * \brief Extract the ID attribute from an XML element + * + * \param[in] xpath String to search + * \param[in] node Node to get the ID for + * + * \return ID attribute of \p node in xpath string \p xpath + */ +char * +pcmk__xpath_node_id(const char *xpath, const char *node); + #endif // PCMK__XML_INTERNAL__H diff --git a/lib/common/tests/Makefile.am b/lib/common/tests/Makefile.am index 2c33cc5..4c6e8b4 100644 --- a/lib/common/tests/Makefile.am +++ b/lib/common/tests/Makefile.am @@ -1 +1 @@ -SUBDIRS = agents cmdline flags operations strings utils +SUBDIRS = agents cmdline flags operations strings utils xpath diff --git a/lib/common/tests/xpath/Makefile.am b/lib/common/tests/xpath/Makefile.am new file mode 100644 index 0000000..7a53683 --- /dev/null +++ b/lib/common/tests/xpath/Makefile.am @@ -0,0 +1,29 @@ +# +# Copyright 2021 the Pacemaker project contributors +# +# The version control history for this file may have further details. +# +# This source code is licensed under the GNU General Public License version 2 +# or later (GPLv2+) WITHOUT ANY WARRANTY. +# +AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include +LDADD = $(top_builddir)/lib/common/libcrmcommon.la + +include $(top_srcdir)/mk/glib-tap.mk + +# Add each test program here. Each test should be written as a little standalone +# program using the glib unit testing functions. See the documentation for more +# information. +# +# https://developer.gnome.org/glib/unstable/glib-Testing.html +# +# Add "_test" to the end of all test program names to simplify .gitignore. +test_programs = pcmk__xpath_node_id_test + +# If any extra data needs to be added to the source distribution, add it to the +# following list. +dist_test_data = + +# If any extra data needs to be used by tests but should not be added to the +# source distribution, add it to the following list. +test_data = diff --git a/lib/common/tests/xpath/pcmk__xpath_node_id_test.c b/lib/common/tests/xpath/pcmk__xpath_node_id_test.c new file mode 100644 index 0000000..f6b5c10 --- /dev/null +++ b/lib/common/tests/xpath/pcmk__xpath_node_id_test.c @@ -0,0 +1,43 @@ +/* + * Copyright 2021 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU Lesser General Public License + * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. + */ + +#include +#include + +static void +empty_input(void) { + g_assert_null(pcmk__xpath_node_id(NULL, "lrm")); + g_assert_null(pcmk__xpath_node_id("", "lrm")); + g_assert_null(pcmk__xpath_node_id("/blah/blah", NULL)); + g_assert_null(pcmk__xpath_node_id("/blah/blah", "")); + g_assert_null(pcmk__xpath_node_id(NULL, NULL)); +} + +static void +not_present(void) { + g_assert_null(pcmk__xpath_node_id("/some/xpath/string[@id='xyz']", "lrm")); + g_assert_null(pcmk__xpath_node_id("/some/xpath/containing[@id='lrm']", "lrm")); +} + +static void +present(void) { + g_assert_cmpint(strcmp(pcmk__xpath_node_id("/some/xpath/containing/lrm[@id='xyz']", "lrm"), "xyz"), ==, 0); + g_assert_cmpint(strcmp(pcmk__xpath_node_id("/some/other/lrm[@id='xyz']/xpath", "lrm"), "xyz"), ==, 0); +} + +int +main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + + g_test_add_func("/common/xpath/node_id/empty_input", empty_input); + g_test_add_func("/common/xpath/node_id/not_present", not_present); + g_test_add_func("/common/xpath/node_id/present", present); + return g_test_run(); +} diff --git a/lib/common/xpath.c b/lib/common/xpath.c index 6fa4941..7851a7c 100644 --- a/lib/common/xpath.c +++ b/lib/common/xpath.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. * @@ -11,6 +11,7 @@ #include #include #include +#include #include "crmcommon_private.h" /* @@ -297,3 +298,34 @@ xml_get_path(xmlNode *xml) } return NULL; } + +char * +pcmk__xpath_node_id(const char *xpath, const char *node) +{ + char *retval = NULL; + char *patt = NULL; + char *start = NULL; + char *end = NULL; + + if (node == NULL || xpath == NULL) { + return retval; + } + + patt = crm_strdup_printf("/%s[@id=", node); + start = strstr(xpath, patt); + + if (!start) { + free(patt); + return retval; + } + + start += strlen(patt); + start++; + + end = strstr(start, "\'"); + CRM_ASSERT(end); + retval = strndup(start, end-start); + + free(patt); + return retval; +} diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 8b47bbc..ff1b86b 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -1719,25 +1719,6 @@ mon_trigger_refresh(gpointer user_data) return FALSE; } -#define NODE_PATT "/lrm[@id=" -static char * -get_node_from_xpath(const char *xpath) -{ - char *nodeid = NULL; - char *tmp = strstr(xpath, NODE_PATT); - - if(tmp) { - tmp += strlen(NODE_PATT); - tmp += 1; - - nodeid = strdup(tmp); - tmp = strstr(nodeid, "\'"); - CRM_ASSERT(tmp); - tmp[0] = 0; - } - return nodeid; -} - static void crm_diff_update_v2(const char *event, xmlNode * msg) { @@ -1822,19 +1803,19 @@ crm_diff_update_v2(const char *event, xmlNode * msg) handle_rsc_op(match, node); } else if(strcmp(name, XML_LRM_TAG_RESOURCES) == 0) { - char *local_node = get_node_from_xpath(xpath); + char *local_node = pcmk__xpath_node_id(xpath, "lrm"); handle_rsc_op(match, local_node); free(local_node); } else if(strcmp(name, XML_LRM_TAG_RESOURCE) == 0) { - char *local_node = get_node_from_xpath(xpath); + char *local_node = pcmk__xpath_node_id(xpath, "lrm"); handle_rsc_op(match, local_node); free(local_node); } else if(strcmp(name, XML_LRM_TAG_RSC_OP) == 0) { - char *local_node = get_node_from_xpath(xpath); + char *local_node = pcmk__xpath_node_id(xpath, "lrm"); handle_rsc_op(match, local_node); free(local_node); -- 1.8.3.1 From b0126373d8b2a739ec5b985a7e1f530e850618d3 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 11 Jan 2021 10:20:11 -0500 Subject: [PATCH 09/10] Refactor: libpacemaker: Move reduce_stonith_history into the library. And also rename it to pcmk__reduce_fence_history. I don't see anywhere else that could use this function at the moment, but it seems too generic to keep in crm_mon. --- include/pcmki/pcmki_fence.h | 16 +++++++++++++- lib/pacemaker/pcmk_fence.c | 45 ++++++++++++++++++++++++++++++++++++++- tools/crm_mon.c | 52 +-------------------------------------------- 3 files changed, 60 insertions(+), 53 deletions(-) diff --git a/include/pcmki/pcmki_fence.h b/include/pcmki/pcmki_fence.h index 241d030..d4cef68 100644 --- a/include/pcmki/pcmki_fence.h +++ b/include/pcmki/pcmki_fence.h @@ -1,5 +1,5 @@ /* - * Copyright 2019-2020 the Pacemaker project contributors + * Copyright 2019-2021 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -219,4 +219,18 @@ int pcmk__fence_validate(pcmk__output_t *out, stonith_t *st, const char *agent, const char *id, stonith_key_value_t *params, unsigned int timeout); +/** + * \brief Reduce the STONITH history + * + * STONITH history is reduced as follows: + * - The last successful action of every action-type and target is kept + * - For failed actions, who failed is kept + * - All actions in progress are kept + * + * \param[in] history List of STONITH actions + * + * \return The reduced history + */ +stonith_history_t * +pcmk__reduce_fence_history(stonith_history_t *history); #endif diff --git a/lib/pacemaker/pcmk_fence.c b/lib/pacemaker/pcmk_fence.c index d591379..34540cc 100644 --- a/lib/pacemaker/pcmk_fence.c +++ b/lib/pacemaker/pcmk_fence.c @@ -1,5 +1,5 @@ /* - * Copyright 2009-2020 the Pacemaker project contributors + * Copyright 2009-2021 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -520,3 +520,46 @@ pcmk_fence_validate(xmlNodePtr *xml, stonith_t *st, const char *agent, return rc; } #endif + +stonith_history_t * +pcmk__reduce_fence_history(stonith_history_t *history) +{ + stonith_history_t *new, *hp, *np; + + if (!history) { + return history; + } + + new = history; + hp = new->next; + new->next = NULL; + + while (hp) { + stonith_history_t *hp_next = hp->next; + + hp->next = NULL; + + for (np = new; ; np = np->next) { + if ((hp->state == st_done) || (hp->state == st_failed)) { + /* action not in progress */ + if (pcmk__str_eq(hp->target, np->target, pcmk__str_casei) && + pcmk__str_eq(hp->action, np->action, pcmk__str_casei) && + (hp->state == np->state) && + ((hp->state == st_done) || + pcmk__str_eq(hp->delegate, np->delegate, pcmk__str_casei))) { + /* purge older hp */ + stonith_history_free(hp); + break; + } + } + + if (!np->next) { + np->next = hp; + break; + } + } + hp = hp_next; + } + + return new; +} diff --git a/tools/crm_mon.c b/tools/crm_mon.c index ff1b86b..2179f53 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -1520,56 +1520,6 @@ print_simple_status(pcmk__output_t *out, pe_working_set_t * data_set, /* coverity[leaked_storage] False positive */ } -/*! - * \internal - * \brief Reduce the stonith-history - * for successful actions we keep the last of every action-type & target - * for failed actions we record as well who had failed - * for actions in progress we keep full track - * - * \param[in] history List of stonith actions - * - */ -static stonith_history_t * -reduce_stonith_history(stonith_history_t *history) -{ - stonith_history_t *new = history, *hp, *np; - - if (new) { - hp = new->next; - new->next = NULL; - - while (hp) { - stonith_history_t *hp_next = hp->next; - - hp->next = NULL; - - for (np = new; ; np = np->next) { - if ((hp->state == st_done) || (hp->state == st_failed)) { - /* action not in progress */ - if (pcmk__str_eq(hp->target, np->target, pcmk__str_casei) && - pcmk__str_eq(hp->action, np->action, pcmk__str_casei) && - (hp->state == np->state) && - ((hp->state == st_done) || - pcmk__str_eq(hp->delegate, np->delegate, pcmk__str_casei))) { - /* purge older hp */ - stonith_history_free(hp); - break; - } - } - - if (!np->next) { - np->next = hp; - break; - } - } - hp = hp_next; - } - } - - return new; -} - static int send_custom_trap(const char *node, const char *rsc, const char *task, int target_rc, int rc, int status, const char *desc) @@ -1935,7 +1885,7 @@ mon_refresh_display(gpointer user_data) if (!pcmk_is_set(options.mon_ops, mon_op_fence_full_history) && (output_format != mon_output_xml)) { - stonith_history = reduce_stonith_history(stonith_history); + stonith_history = pcmk__reduce_fence_history(stonith_history); } break; /* all other cases are errors */ } -- 1.8.3.1 From af3f1368bc76eb498c2c96b3eda9324b579c9380 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Tue, 12 Jan 2021 15:46:55 -0500 Subject: [PATCH 10/10] Low: tools: Adjust fencing shown indicator in crm_mon. If any of the various fencing flags are set, but not all of them, no '*' will be shown next to the fencing line in the interactive change screen. This makes it seem like fencing should not be shown, and hitting 'm' should toggle the fencing display on. However, that's not the case and hitting 'm' will actually toggle fencing off. Hitting it again will toggle it on and the '*' will appear. This is confusing, so just display the '*' if any fencing flag is set. --- tools/crm_mon.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/crm_mon.c b/tools/crm_mon.c index 2179f53..8ec97bb 100644 --- a/tools/crm_mon.c +++ b/tools/crm_mon.c @@ -984,7 +984,10 @@ detect_user_input(GIOChannel *channel, GIOCondition condition, gpointer user_dat print_option_help(out, 'R', pcmk_is_set(options.mon_ops, mon_op_print_clone_detail)); print_option_help(out, 'b', pcmk_is_set(options.mon_ops, mon_op_print_brief)); print_option_help(out, 'j', pcmk_is_set(options.mon_ops, mon_op_print_pending)); - print_option_help(out, 'm', pcmk_is_set(show, mon_show_fencing_all)); + print_option_help(out, 'm', pcmk_any_flags_set(show, + mon_show_fence_failed + |mon_show_fence_pending + |mon_show_fence_worked)); out->info(out, "%s", "\nToggle fields via field letter, type any other key to return"); } -- 1.8.3.1