From fa92bb6ea7eec559b3a7a055eb4a6eb1445ffe31 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 27 Jul 2023 09:37:34 -0400 Subject: [PATCH] Wait for a reply from various controller commands - Resolves: rhbz2225631 - Related: rhbz2189300 --- 006-controller-reply.patch | 109 +++++++++++++++++++++++++++++++++++++ pacemaker.spec | 8 ++- 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 006-controller-reply.patch diff --git a/006-controller-reply.patch b/006-controller-reply.patch new file mode 100644 index 0000000..efd4f9c --- /dev/null +++ b/006-controller-reply.patch @@ -0,0 +1,109 @@ +From 3e31da0016795397bfeacb2f3d76ecfe35cc1f67 Mon Sep 17 00:00:00 2001 +From: Ken Gaillot +Date: Mon, 17 Jul 2023 14:52:42 -0500 +Subject: [PATCH] Fix: libcrmcommon: wait for reply from appropriate controller + commands + +ipc_controld.c:reply_expected() wrongly omitted PCMK__CONTROLD_CMD_NODES (which +hasn't been a problem because crm_node uses a mainloop instead of sync dispatch +for that) and CRM_OP_RM_NODE_CACHE (which can be sent via +ipc_client.c:pcmk_ipc_purge_node()). + +Because CRM_OP_RM_NODE_CACHE gets only an ack and no further replies, we now +have to be careful not to return true from the controller's dispatch() +function, otherwise crm_node -R would wait forever for more data. That means +we have to check for whether any replies are expected, which means we have to +increment expected replies *before* sending a request (in case it's sync). + +Regression introduced in 2.0.5 by ae14fa4a + +Fixes T681 +--- + lib/common/ipc_controld.c | 49 ++++++++++++++------------------------- + 1 file changed, 17 insertions(+), 32 deletions(-) + +diff --git a/lib/common/ipc_controld.c b/lib/common/ipc_controld.c +index 3c3a98964..405fd0518 100644 +--- a/lib/common/ipc_controld.c ++++ b/lib/common/ipc_controld.c +@@ -177,18 +177,16 @@ set_nodes_data(pcmk_controld_api_reply_t *data, xmlNode *msg_data) + static bool + reply_expected(pcmk_ipc_api_t *api, xmlNode *request) + { +- const char *command = crm_element_value(request, F_CRM_TASK); +- +- if (command == NULL) { +- return false; +- } +- +- // We only need to handle commands that functions in this file can send +- return !strcmp(command, CRM_OP_REPROBE) +- || !strcmp(command, CRM_OP_NODE_INFO) +- || !strcmp(command, CRM_OP_PING) +- || !strcmp(command, CRM_OP_LRM_FAIL) +- || !strcmp(command, CRM_OP_LRM_DELETE); ++ // We only need to handle commands that API functions can send ++ return pcmk__str_any_of(crm_element_value(request, F_CRM_TASK), ++ PCMK__CONTROLD_CMD_NODES, ++ CRM_OP_LRM_DELETE, ++ CRM_OP_LRM_FAIL, ++ CRM_OP_NODE_INFO, ++ CRM_OP_PING, ++ CRM_OP_REPROBE, ++ CRM_OP_RM_NODE_CACHE, ++ NULL); + } + + static bool +@@ -202,22 +200,12 @@ dispatch(pcmk_ipc_api_t *api, xmlNode *reply) + pcmk_controld_reply_unknown, NULL, NULL, + }; + +- /* If we got an ACK, return true so the caller knows to expect more responses +- * from the IPC server. We do this before decrementing replies_expected because +- * ACKs are not going to be included in that value. +- * +- * Note that we cannot do the same kind of status checking here that we do in +- * ipc_pacemakerd.c. The ACK message we receive does not necessarily contain +- * a status attribute. That is, we may receive this: +- * +- * +- * +- * Instead of this: +- * +- * +- */ + if (pcmk__str_eq(crm_element_name(reply), "ack", pcmk__str_none)) { +- return true; // More replies needed ++ /* ACKs are trivial responses that do not count toward expected replies, ++ * and do not have all the fields that validation requires, so skip that ++ * processing. ++ */ ++ return private->replies_expected > 0; + } + + if (private->replies_expected > 0) { +@@ -344,18 +332,15 @@ static int + send_controller_request(pcmk_ipc_api_t *api, xmlNode *request, + bool reply_is_expected) + { +- int rc; +- + if (crm_element_value(request, XML_ATTR_REFERENCE) == NULL) { + return EINVAL; + } +- rc = pcmk__send_ipc_request(api, request); +- if ((rc == pcmk_rc_ok) && reply_is_expected) { ++ if (reply_is_expected) { + struct controld_api_private_s *private = api->api_data; + + private->replies_expected++; + } +- return rc; ++ return pcmk__send_ipc_request(api, request); + } + + static xmlNode * +-- +2.41.0 + diff --git a/pacemaker.spec b/pacemaker.spec index f49f82e..1398cfb 100644 --- a/pacemaker.spec +++ b/pacemaker.spec @@ -36,7 +36,7 @@ ## can be incremented to build packages reliably considered "newer" ## than previously built packages with the same pcmkversion) %global pcmkversion 2.1.6 -%global specversion 5 +%global specversion 6 ## Upstream commit (full commit ID, abbreviated commit ID, or tag) to build %global commit 6fdc9deea294bbad629b003c6ae036aaed8e3ee0 @@ -268,6 +268,7 @@ Patch002: 002-group-colocation-constraint.patch Patch003: 003-clone-shuffle.patch Patch004: 004-clone-rsc-display.patch Patch005: 005-attrd-dampen.patch +Patch006: 006-controller-reply.patch # downstream-only commits #Patch1xx: 1xx-xxxx.patch @@ -1006,6 +1007,11 @@ exit 0 %license %{nagios_name}-%{nagios_hash}/COPYING %changelog +* Thu Jul 27 2023 Chris Lumens - 2.1.6-6 +- Wait for a reply from various controller commands +- Resolves: rhbz2225631 +- Related: rhbz2189300 + * Tue Jul 25 2023 Chris Lumens - 2.1.6-5 - Apply dampening when creating attributes with attrd_updater -U - Resolves: rhbz2224046