From 3c1145cc6b520cca5180fc91c8345e666b09ebce Mon Sep 17 00:00:00 2001 From: Ken Gaillot Date: Tue, 14 Jan 2025 14:19:23 -0600 Subject: [PATCH 16/16] Refactor: controller: best practices for send_stonith_update() Add a doxygen block, rename function to update_node_state_after_fencing() and uuid argument to target_xml_id for readability, and improve log messages, comments, and formatting. --- daemons/controld/controld_fencing.c | 53 ++++++++++++----------------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/daemons/controld/controld_fencing.c b/daemons/controld/controld_fencing.c index e5f03ef51c..49d1142cb3 100644 --- a/daemons/controld/controld_fencing.c +++ b/daemons/controld/controld_fencing.c @@ -207,11 +207,19 @@ cib_fencing_updated(xmlNode *msg, int call_id, int rc, xmlNode *output, } } +/*! + * \internal + * \brief Update a fencing target's node state + * + * \param[in] target Node that was successfully fenced + * \param[in] target_xml_id CIB XML ID of target + */ static void -send_stonith_update(const char *target, const char *uuid) +update_node_state_after_fencing(const char *target, const char *target_xml_id) { int rc = pcmk_ok; pcmk__node_status_t *peer = NULL; + xmlNode *node_state = NULL; /* We (usually) rely on the membership layer to do node_update_cluster, * and the peer status callback to do node_update_peer, because the node @@ -219,18 +227,10 @@ send_stonith_update(const char *target, const char *uuid) */ int flags = node_update_join | node_update_expected; - /* zero out the node-status & remove all LRM status info */ - xmlNode *node_state = NULL; - - CRM_CHECK(target != NULL, return); - CRM_CHECK(uuid != NULL, return); - - /* Make sure the membership and join caches are accurate. - * Try getting any existing node cache entry also by node uuid in case it - * doesn't have an uname yet. - */ - peer = pcmk__get_node(0, target, uuid, pcmk__node_search_any); + CRM_CHECK((target != NULL) && (target_xml_id != NULL), return); + // Ensure target is cached + peer = pcmk__get_node(0, target, target_xml_id, pcmk__node_search_any); CRM_CHECK(peer != NULL, return); if (peer->state == NULL) { @@ -242,16 +242,15 @@ send_stonith_update(const char *target, const char *uuid) } if (peer->xml_id == NULL) { - crm_info("Recording XML ID '%s' for node '%s'", uuid, target); - peer->xml_id = pcmk__str_copy(uuid); + crm_info("Recording XML ID '%s' for node '%s'", target_xml_id, target); + peer->xml_id = pcmk__str_copy(target_xml_id); } crmd_peer_down(peer, TRUE); - /* Generate a node state update for the CIB */ node_state = create_node_state_update(peer, flags, NULL, __func__); + crm_xml_add(node_state, PCMK_XA_ID, target_xml_id); - /* we have to mark whether or not remote nodes have already been fenced */ if (pcmk_is_set(peer->flags, pcmk__node_status_remote)) { char *now_s = pcmk__ttoa(time(NULL)); @@ -259,25 +258,15 @@ send_stonith_update(const char *target, const char *uuid) free(now_s); } - /* Force our known ID */ - crm_xml_add(node_state, PCMK_XA_ID, uuid); - rc = controld_globals.cib_conn->cmds->modify(controld_globals.cib_conn, PCMK_XE_STATUS, node_state, cib_can_create); + pcmk__xml_free(node_state); - /* Delay processing the trigger until the update completes */ - crm_debug("Sending fencing update %d for %s", rc, target); + crm_debug("Updating node state for %s after fencing (call %d)", target, rc); fsa_register_cib_callback(rc, pcmk__str_copy(target), cib_fencing_updated); - // Make sure it sticks - /* controld_globals.cib_conn->cmds->bump_epoch(controld_globals.cib_conn, - * cib_none); - */ - controld_delete_node_state(peer->name, controld_section_all, cib_none); - pcmk__xml_free(node_state); - return; } /*! @@ -386,7 +375,7 @@ execute_stonith_cleanup(void) const char *uuid = pcmk__cluster_get_xml_id(target_node); crm_notice("Marking %s, target of a previous stonith action, as clean", target); - send_stonith_update(target, uuid); + update_node_state_after_fencing(target, uuid); free(target); } g_list_free(stonith_cleanup_list); @@ -601,7 +590,7 @@ handle_fence_notification(stonith_t *st, stonith_event_t *event) if (AM_I_DC) { /* The DC always sends updates */ - send_stonith_update(event->target, uuid); + update_node_state_after_fencing(event->target, uuid); /* @TODO Ideally, at this point, we'd check whether the fenced node * hosted any guest nodes, and call remote_node_down() for them. @@ -638,7 +627,7 @@ handle_fence_notification(stonith_t *st, stonith_event_t *event) * have them do so too after the election */ if (controld_is_local_node(event->executioner)) { - send_stonith_update(event->target, uuid); + update_node_state_after_fencing(event->target, uuid); } add_stonith_cleanup(event->target); } @@ -886,7 +875,7 @@ tengine_stonith_callback(stonith_t *stonith, stonith_callback_data_t *data) is_remote_node); } else if (!(pcmk_is_set(action->flags, pcmk__graph_action_sent_update))) { - send_stonith_update(target, uuid); + update_node_state_after_fencing(target, uuid); pcmk__set_graph_action_flags(action, pcmk__graph_action_sent_update); } -- 2.43.0