Fix: libpacemaker: set fail-count to INFINITY for fatal failures #3769
Conversation
| */ | ||
|
|
||
| #define PCMK__OPT_FAILED_STOP_OFFSET "failed-stop-offset" | ||
| #define PCMK__OPT_FAILED_START_OFFSET "failed-start-offset" |
There was a problem hiding this comment.
Thanks :)
These are purely internal, not set by the user, so an XA name in crm/common/xml_names_internal.h would be better
lib/pacemaker/pcmk_injections.c
Outdated
| @@ -93,7 +93,7 @@ inject_transient_attr(pcmk__output_t *out, xmlNode *cib_node, | |||
| void | |||
| pcmk__inject_failcount(pcmk__output_t *out, cib_t *cib_conn, xmlNode *cib_node, | |||
| const char *resource, const char *task, | |||
| guint interval_ms, int exit_status) | |||
| guint interval_ms, int exit_status, const char *offset) | |||
There was a problem hiding this comment.
Add to the doxygen comment above as well
I'm thinking a bool would be clearer since we don't actually use the value
There was a problem hiding this comment.
Add to the doxygen comment above as well
Ah, missed it.
I'm thinking a
boolwould be clearer since we don't actually use the value
I was thinking it was kind of straight-forward to pass the value of failed_start_offset or failed_stop_offset to be used rather than parsing them outside of pcmk__inject_failcount().
There was a problem hiding this comment.
I'd be fine with that, as long as the doxygen makes clear that the only thing that makes any difference is whether it's "INFINITY" or not
lib/pacemaker/pcmk_injections.c
Outdated
| value = pcmk__itoa(failcount + 1); | ||
|
|
||
| if (pcmk_str_is_infinity(offset)) { | ||
| value = strdup(offset); |
There was a problem hiding this comment.
We prefer pcmk__str_copy() now to assert on failure
There was a problem hiding this comment.
This is where the value of offset is technically used. But it's also fine to simply copy PCMK_VALUE_INFINITY based on the value of a bool fatal or bool infinity parameter.
| */ | ||
|
|
||
| #define PCMK__OPT_FAILED_STOP_OFFSET "failed-stop-offset" | ||
| #define PCMK__OPT_FAILED_START_OFFSET "failed-start-offset" |
lib/pacemaker/pcmk_injections.c
Outdated
| @@ -93,7 +93,7 @@ inject_transient_attr(pcmk__output_t *out, xmlNode *cib_node, | |||
| void | |||
| pcmk__inject_failcount(pcmk__output_t *out, cib_t *cib_conn, xmlNode *cib_node, | |||
| const char *resource, const char *task, | |||
| guint interval_ms, int exit_status) | |||
| guint interval_ms, int exit_status, const char *offset) | |||
There was a problem hiding this comment.
Add to the doxygen comment above as well
Ah, missed it.
I'm thinking a
boolwould be clearer since we don't actually use the value
I was thinking it was kind of straight-forward to pass the value of failed_start_offset or failed_stop_offset to be used rather than parsing them outside of pcmk__inject_failcount().
lib/pacemaker/pcmk_injections.c
Outdated
| value = pcmk__itoa(failcount + 1); | ||
|
|
||
| if (pcmk_str_is_infinity(offset)) { | ||
| value = strdup(offset); |
There was a problem hiding this comment.
This is where the value of offset is technically used. But it's also fine to simply copy PCMK_VALUE_INFINITY based on the value of a bool fatal or bool infinity parameter.
…_FAILED_STOP_OFFSET Besides, use PCMK_VALUE_INFINITY constant instead of "INFINITY".
... to be consistent with what controller does with update_failcount()
gao-yan
force-pushed
the
crm_simulate-fail-count-fatal-failures
branch
from
99a3ed6 to
103dcb4
Compare
... to be consistent with what controller does with update_failcount()