From 6c529bb624ad548f66ce6ef1fa80b77c688918f4 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 22 Nov 2019 16:39:54 -0600
Subject: [PATCH 1/4] Refactor: controller: rename struct recurring_op_s to
active_op_t
... because it holds both recurring and pending non-recurring actions,
and the name was confusing
---
daemons/controld/controld_execd.c | 18 +++++++++---------
daemons/controld/controld_execd_state.c | 4 ++--
daemons/controld/controld_lrm.h | 8 ++++----
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c
index 9e8dd36..48f35dd 100644
--- a/daemons/controld/controld_execd.c
+++ b/daemons/controld/controld_execd.c
@@ -403,7 +403,7 @@ lrm_state_verify_stopped(lrm_state_t * lrm_state, enum crmd_fsa_state cur_state,
GHashTableIter gIter;
const char *key = NULL;
rsc_history_t *entry = NULL;
- struct recurring_op_s *pending = NULL;
+ active_op_t *pending = NULL;
crm_debug("Checking for active resources before exit");
@@ -909,7 +909,7 @@ static gboolean
lrm_remove_deleted_op(gpointer key, gpointer value, gpointer user_data)
{
const char *rsc = user_data;
- struct recurring_op_s *pending = value;
+ active_op_t *pending = value;
if (crm_str_eq(rsc, pending->rsc_id, TRUE)) {
crm_info("Removing op %s:%d for deleted resource %s",
@@ -1137,7 +1137,7 @@ cancel_op(lrm_state_t * lrm_state, const char *rsc_id, const char *key, int op,
{
int rc = pcmk_ok;
char *local_key = NULL;
- struct recurring_op_s *pending = NULL;
+ active_op_t *pending = NULL;
CRM_CHECK(op != 0, return FALSE);
CRM_CHECK(rsc_id != NULL, return FALSE);
@@ -1203,7 +1203,7 @@ cancel_action_by_key(gpointer key, gpointer value, gpointer user_data)
{
gboolean remove = FALSE;
struct cancel_data *data = user_data;
- struct recurring_op_s *op = (struct recurring_op_s *)value;
+ active_op_t *op = value;
if (crm_str_eq(op->op_key, data->key, TRUE)) {
data->done = TRUE;
@@ -2107,7 +2107,7 @@ stop_recurring_action_by_rsc(gpointer key, gpointer value, gpointer user_data)
{
gboolean remove = FALSE;
struct stop_recurring_action_s *event = user_data;
- struct recurring_op_s *op = (struct recurring_op_s *)value;
+ active_op_t *op = value;
if ((op->interval_ms != 0)
&& crm_str_eq(op->rsc_id, event->rsc->id, TRUE)) {
@@ -2124,7 +2124,7 @@ stop_recurring_actions(gpointer key, gpointer value, gpointer user_data)
{
gboolean remove = FALSE;
lrm_state_t *lrm_state = user_data;
- struct recurring_op_s *op = (struct recurring_op_s *)value;
+ active_op_t *op = value;
if (op->interval_ms != 0) {
crm_info("Cancelling op %d for %s (%s)", op->call_id, op->rsc_id,
@@ -2297,9 +2297,9 @@ do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operat
* for them to complete during shutdown
*/
char *call_id_s = make_stop_id(rsc->id, call_id);
- struct recurring_op_s *pending = NULL;
+ active_op_t *pending = NULL;
- pending = calloc(1, sizeof(struct recurring_op_s));
+ pending = calloc(1, sizeof(active_op_t));
crm_trace("Recording pending op: %d - %s %s", call_id, op_id, call_id_s);
pending->call_id = call_id;
@@ -2517,7 +2517,7 @@ did_lrm_rsc_op_fail(lrm_state_t *lrm_state, const char * rsc_id,
void
process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,
- struct recurring_op_s *pending, xmlNode *action_xml)
+ active_op_t *pending, xmlNode *action_xml)
{
char *op_id = NULL;
char *op_key = NULL;
diff --git a/daemons/controld/controld_execd_state.c b/daemons/controld/controld_execd_state.c
index 0e21d18..473da97 100644
--- a/daemons/controld/controld_execd_state.c
+++ b/daemons/controld/controld_execd_state.c
@@ -44,7 +44,7 @@ free_deletion_op(gpointer value)
static void
free_recurring_op(gpointer value)
{
- struct recurring_op_s *op = (struct recurring_op_s *)value;
+ active_op_t *op = value;
free(op->user_data);
free(op->rsc_id);
@@ -61,7 +61,7 @@ fail_pending_op(gpointer key, gpointer value, gpointer user_data)
{
lrmd_event_data_t event = { 0, };
lrm_state_t *lrm_state = user_data;
- struct recurring_op_s *op = (struct recurring_op_s *)value;
+ active_op_t *op = value;
crm_trace("Pre-emptively failing " CRM_OP_FMT " on %s (call=%s, %s)",
op->rsc_id, op->op_type, op->interval_ms,
diff --git a/daemons/controld/controld_lrm.h b/daemons/controld/controld_lrm.h
index 598682b..27df5d7 100644
--- a/daemons/controld/controld_lrm.h
+++ b/daemons/controld/controld_lrm.h
@@ -33,8 +33,8 @@ typedef struct resource_history_s {
void history_free(gpointer data);
-/* TODO - Replace this with lrmd_event_data_t */
-struct recurring_op_s {
+// In-flight action (recurring or pending)
+typedef struct active_op_s {
guint interval_ms;
int call_id;
gboolean remove;
@@ -45,7 +45,7 @@ struct recurring_op_s {
char *op_key;
char *user_data;
GHashTable *params;
-};
+} active_op_t;
typedef struct lrm_state_s {
const char *node_name;
@@ -164,4 +164,4 @@ void remote_ra_process_maintenance_nodes(xmlNode *xml);
gboolean remote_ra_controlling_guest(lrm_state_t * lrm_state);
void process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,
- struct recurring_op_s *pending, xmlNode *action_xml);
+ active_op_t *pending, xmlNode *action_xml);
--
1.8.3.1
From 93a59f1df8fe11d365032d75f10cb4189ad2f1f8 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 22 Nov 2019 16:45:31 -0600
Subject: [PATCH 2/4] Refactor: controller: convert active_op_t booleans to
bitmask
---
daemons/controld/controld_execd.c | 11 +++++------
daemons/controld/controld_lrm.h | 8 ++++++--
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c
index 48f35dd..2c9d9c0 100644
--- a/daemons/controld/controld_execd.c
+++ b/daemons/controld/controld_execd.c
@@ -1148,18 +1148,17 @@ cancel_op(lrm_state_t * lrm_state, const char *rsc_id, const char *key, int op,
pending = g_hash_table_lookup(lrm_state->pending_ops, key);
if (pending) {
- if (remove && pending->remove == FALSE) {
- pending->remove = TRUE;
+ if (remove && is_not_set(pending->flags, active_op_remove)) {
+ set_bit(pending->flags, active_op_remove);
crm_debug("Scheduling %s for removal", key);
}
- if (pending->cancelled) {
+ if (is_set(pending->flags, active_op_cancelled)) {
crm_debug("Operation %s already cancelled", key);
free(local_key);
return FALSE;
}
-
- pending->cancelled = TRUE;
+ set_bit(pending->flags, active_op_cancelled);
} else {
crm_info("No pending op found for %s", key);
@@ -2652,7 +2651,7 @@ process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,
crm_err("Recurring operation %s was cancelled without transition information",
op_key);
- } else if (pending->remove) {
+ } else if (is_set(pending->flags, active_op_remove)) {
/* This recurring operation was cancelled (by us) and pending, and we
* have been waiting for it to finish.
*/
diff --git a/daemons/controld/controld_lrm.h b/daemons/controld/controld_lrm.h
index 27df5d7..3ab7048 100644
--- a/daemons/controld/controld_lrm.h
+++ b/daemons/controld/controld_lrm.h
@@ -33,12 +33,16 @@ typedef struct resource_history_s {
void history_free(gpointer data);
+enum active_op_e {
+ active_op_remove = (1 << 0),
+ active_op_cancelled = (1 << 1),
+};
+
// In-flight action (recurring or pending)
typedef struct active_op_s {
guint interval_ms;
int call_id;
- gboolean remove;
- gboolean cancelled;
+ uint32_t flags; // bitmask of active_op_e
time_t start_time;
char *rsc_id;
char *op_type;
--
1.8.3.1
From 4d087d021d325e26b41a9b36b5b190dc7b25334c Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 22 Nov 2019 16:58:25 -0600
Subject: [PATCH 3/4] Refactor: controller: remove unused argument
---
daemons/controld/controld_execd.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c
index 2c9d9c0..46c1958 100644
--- a/daemons/controld/controld_execd.c
+++ b/daemons/controld/controld_execd.c
@@ -43,8 +43,8 @@ static int delete_rsc_status(lrm_state_t * lrm_state, const char *rsc_id, int ca
static lrmd_event_data_t *construct_op(lrm_state_t * lrm_state, xmlNode * rsc_op,
const char *rsc_id, const char *operation);
-static void do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operation,
- xmlNode * msg, xmlNode * request);
+static void do_lrm_rsc_op(lrm_state_t *lrm_state, lrmd_rsc_info_t *rsc,
+ const char *operation, xmlNode *msg);
void send_direct_ack(const char *to_host, const char *to_sys,
lrmd_rsc_info_t * rsc, lrmd_event_data_t * op, const char *rsc_id);
@@ -1858,7 +1858,7 @@ do_lrm_invoke(long long action,
crm_rsc_delete, user_name);
} else {
- do_lrm_rsc_op(lrm_state, rsc, operation, input->xml, input->msg);
+ do_lrm_rsc_op(lrm_state, rsc, operation, input->xml);
}
lrmd_free_rsc_info(rsc);
@@ -2170,8 +2170,8 @@ record_pending_op(const char *node_name, lrmd_rsc_info_t *rsc, lrmd_event_data_t
}
static void
-do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operation, xmlNode * msg,
- xmlNode * request)
+do_lrm_rsc_op(lrm_state_t *lrm_state, lrmd_rsc_info_t *rsc,
+ const char *operation, xmlNode *msg)
{
int call_id = 0;
char *op_id = NULL;
--
1.8.3.1
From 356b417274918b7da6cdd9c72c036c923160b318 Mon Sep 17 00:00:00 2001
From: Ken Gaillot <kgaillot@redhat.com>
Date: Fri, 6 Dec 2019 12:15:05 -0600
Subject: [PATCH 4/4] Refactor: scheduler: combine two "if" statements
... for readability, and ease of adding another block later
---
lib/pacemaker/pcmk_sched_graph.c | 120 +++++++++++++++++++--------------------
1 file changed, 60 insertions(+), 60 deletions(-)
diff --git a/lib/pacemaker/pcmk_sched_graph.c b/lib/pacemaker/pcmk_sched_graph.c
index e5a8a01..a6967fe 100644
--- a/lib/pacemaker/pcmk_sched_graph.c
+++ b/lib/pacemaker/pcmk_sched_graph.c
@@ -1088,71 +1088,71 @@ action2xml(action_t * action, gboolean as_input, pe_working_set_t *data_set)
return action_xml;
}
- /* List affected resource */
- if (action->rsc) {
- if (is_set(action->flags, pe_action_pseudo) == FALSE) {
- int lpc = 0;
-
- xmlNode *rsc_xml = create_xml_node(action_xml, crm_element_name(action->rsc->xml));
-
- const char *attr_list[] = {
- XML_AGENT_ATTR_CLASS,
- XML_AGENT_ATTR_PROVIDER,
- XML_ATTR_TYPE
- };
-
- if (is_set(action->rsc->flags, pe_rsc_orphan) && action->rsc->clone_name) {
- /* Do not use the 'instance free' name here as that
- * might interfere with the instance we plan to keep.
- * Ie. if there are more than two named /anonymous/
- * instances on a given node, we need to make sure the
- * command goes to the right one.
- *
- * Keep this block, even when everyone is using
- * 'instance free' anonymous clone names - it means
- * we'll do the right thing if anyone toggles the
- * unique flag to 'off'
- */
- crm_debug("Using orphan clone name %s instead of %s", action->rsc->id,
- action->rsc->clone_name);
- crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->clone_name);
- crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);
+ if (action->rsc && is_not_set(action->flags, pe_action_pseudo)) {
+ int lpc = 0;
+ xmlNode *rsc_xml = NULL;
+ const char *attr_list[] = {
+ XML_AGENT_ATTR_CLASS,
+ XML_AGENT_ATTR_PROVIDER,
+ XML_ATTR_TYPE
+ };
+
+ // List affected resource
+
+ rsc_xml = create_xml_node(action_xml,
+ crm_element_name(action->rsc->xml));
+ if (is_set(action->rsc->flags, pe_rsc_orphan)
+ && action->rsc->clone_name) {
+ /* Do not use the 'instance free' name here as that
+ * might interfere with the instance we plan to keep.
+ * Ie. if there are more than two named /anonymous/
+ * instances on a given node, we need to make sure the
+ * command goes to the right one.
+ *
+ * Keep this block, even when everyone is using
+ * 'instance free' anonymous clone names - it means
+ * we'll do the right thing if anyone toggles the
+ * unique flag to 'off'
+ */
+ crm_debug("Using orphan clone name %s instead of %s", action->rsc->id,
+ action->rsc->clone_name);
+ crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->clone_name);
+ crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);
- } else if (is_not_set(action->rsc->flags, pe_rsc_unique)) {
- const char *xml_id = ID(action->rsc->xml);
-
- crm_debug("Using anonymous clone name %s for %s (aka. %s)", xml_id, action->rsc->id,
- action->rsc->clone_name);
-
- /* ID is what we'd like client to use
- * ID_LONG is what they might know it as instead
- *
- * ID_LONG is only strictly needed /here/ during the
- * transition period until all nodes in the cluster
- * are running the new software /and/ have rebooted
- * once (meaning that they've only ever spoken to a DC
- * supporting this feature).
- *
- * If anyone toggles the unique flag to 'on', the
- * 'instance free' name will correspond to an orphan
- * and fall into the clause above instead
- */
- crm_xml_add(rsc_xml, XML_ATTR_ID, xml_id);
- if (action->rsc->clone_name && safe_str_neq(xml_id, action->rsc->clone_name)) {
- crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->clone_name);
- } else {
- crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);
- }
+ } else if (is_not_set(action->rsc->flags, pe_rsc_unique)) {
+ const char *xml_id = ID(action->rsc->xml);
+
+ crm_debug("Using anonymous clone name %s for %s (aka. %s)", xml_id, action->rsc->id,
+ action->rsc->clone_name);
+ /* ID is what we'd like client to use
+ * ID_LONG is what they might know it as instead
+ *
+ * ID_LONG is only strictly needed /here/ during the
+ * transition period until all nodes in the cluster
+ * are running the new software /and/ have rebooted
+ * once (meaning that they've only ever spoken to a DC
+ * supporting this feature).
+ *
+ * If anyone toggles the unique flag to 'on', the
+ * 'instance free' name will correspond to an orphan
+ * and fall into the clause above instead
+ */
+ crm_xml_add(rsc_xml, XML_ATTR_ID, xml_id);
+ if (action->rsc->clone_name && safe_str_neq(xml_id, action->rsc->clone_name)) {
+ crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->clone_name);
} else {
- CRM_ASSERT(action->rsc->clone_name == NULL);
- crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->id);
+ crm_xml_add(rsc_xml, XML_ATTR_ID_LONG, action->rsc->id);
}
- for (lpc = 0; lpc < DIMOF(attr_list); lpc++) {
- crm_xml_add(rsc_xml, attr_list[lpc],
- g_hash_table_lookup(action->rsc->meta, attr_list[lpc]));
- }
+ } else {
+ CRM_ASSERT(action->rsc->clone_name == NULL);
+ crm_xml_add(rsc_xml, XML_ATTR_ID, action->rsc->id);
+ }
+
+ for (lpc = 0; lpc < DIMOF(attr_list); lpc++) {
+ crm_xml_add(rsc_xml, attr_list[lpc],
+ g_hash_table_lookup(action->rsc->meta, attr_list[lpc]));
}
}
--
1.8.3.1