Skip to content

Conversation

nrwahl2
Copy link
Contributor

@nrwahl2 nrwahl2 commented Apr 10, 2025

Follow-up to #3863

@nrwahl2 nrwahl2 marked this pull request as ready for review April 10, 2025 20:24
nrwahl2 added 27 commits April 10, 2025 13:27
...with stonith__param_is_supported().

The parameter flags are the only stonith device flags used outside of
the fencer. This will allow us to move the flag enum into the fencer.

Two lines in the fencer become longer than 80 characters because the
flag names are so long, but this is temporary.

Signed-off-by: Reid Wahl <[email protected]>
...in stonith__action_create(). make_args() uses an explicit
PCMK_STONITH_HOST_ARGUMENT if configured.

Signed-off-by: Reid Wahl <[email protected]>
To reduce duplication

Signed-off-by: Reid Wahl <[email protected]>
To replace fenced_device_t:automatic_unfencing

Signed-off-by: Reid Wahl <[email protected]>
To replace fenced_device_t:verified

Signed-off-by: Reid Wahl <[email protected]>
To replace fenced_device_t:api_registered

Signed-off-by: Reid Wahl <[email protected]>
To replace stonith_device_t:cib_registered

Signed-off-by: Reid Wahl <[email protected]>
To replace st_callback_notify_history

Signed-off-by: Reid Wahl <[email protected]>
To replace st_callback_notify_history_synced

Signed-off-by: Reid Wahl <[email protected]>
* Add Doxygen.
* Return an enum value rather than a uint64_t value.
* Rename to fenced_parse_notify_flag() in accordance with our current
  convention for flag parsing functions.
* Rename "name" argument to "type".
* Replace "else if" with "if" where possible.
* Match case-sensitively. STONITH_OP_DEVICE_ADD and
  STONITH_OP_DEVICE_DEL may be coming from an arbitrary request.
  However, if the user is making the request via the stonith API
  functions (as any sane user should be), then the case will always
  match exactly.

Signed-off-by: Reid Wahl <[email protected]>
These were never hooked up. No internal client tries to register for
these notifications. If an external client tried (via
stonith_api_operations_t:register_notification), it would be a no-op.
fenced_parse_notify_flag() (previously get_stonith_flag()) returns the 0
flag for these notification type strings. So the "client gets notified
for this type of event" bit could never get set.

We could fix it, since some amount of work was done to set this up.
However, if nothing internal needs it, I figure we might as well drop
it.

Signed-off-by: Reid Wahl <[email protected]>
The logic is now simple enough not to merit separate functions. All we
have to do is allow fenced_register_level() and
fenced_unregister_level() to accept a NULL result argument.
pcmk__set_result() and related functions accept a NULL argument.

Signed-off-by: Reid Wahl <[email protected]>
@nrwahl2 nrwahl2 requested a review from clumens April 10, 2025 23:32
nrwahl2 added 24 commits April 15, 2025 10:46
To replace stonith_action_str()

Signed-off-by: Reid Wahl <[email protected]>
Similar to the many "sbd ... uses this" comments, to ensure that we
don't delete anything that dlm depends on.

Eventually dlm might use libpacemaker functions or something more sane.
However, we'll have to keep these helpers public for a while to support
old dlm versions.

Signed-off-by: Reid Wahl <[email protected]>
These can't go away as long as stonith_api_operations_t is public, but
perhaps they can eventually.

Signed-off-by: Reid Wahl <[email protected]>
To replace stonith_text2namespace()

Signed-off-by: Reid Wahl <[email protected]>
To replace stonith_namespace2text()

Signed-off-by: Reid Wahl <[email protected]>
It was already ignored (by stonith_get_namespace()) unless set to
"internal". No external caller should be passing "internal" as the
namespace_s argument of the stonith_api_operations_t:validate() method.

Signed-off-by: Reid Wahl <[email protected]>
Provider is supported and allowed by the schema only for OCF-class
resources. It was already being ignored for stonith-class resources
unless provider was set to "internal", in which case behavior was
undefined.

Signed-off-by: Reid Wahl <[email protected]>
It was already ignored (by stonith_get_namespace()) unless set to
"internal". No external caller should be passing "internal" as the
namespace_s argument of the stonith_api_operations_t:validate() method.

Signed-off-by: Reid Wahl <[email protected]>
To replace stonith_get_namespace()

Signed-off-by: Reid Wahl <[email protected]>
These were used externally only with stonith_api_operations_t methods,
which are already deprecated.

Signed-off-by: Reid Wahl <[email protected]>
And struct stonith_key_value_s

Signed-off-by: Reid Wahl <[email protected]>
And struct stonith_history_s

Signed-off-by: Reid Wahl <[email protected]>
And struct stonith_callback_data_s

Signed-off-by: Reid Wahl <[email protected]>
The list_agents method ignores it.

Signed-off-by: Reid Wahl <[email protected]>
It's always ignored this argument. It likely doesn't make sense to
implement a timeout, since the work currently consists of calls to
libstonith (not libstonithd!) functions and to scandir(). Implementing a
timeout would mean spawning one or more child processes and getting the
results via a pipe, which doesn't seem worth the effort.

Signed-off-by: Reid Wahl <[email protected]>
This method has always ignored the argument since it was added in 2011
via 8d6be4b. It likely doesn't make sense to implement a timeout, since
the work currently consists of calls to libstonith (not libstonithd!)
functions and to scandir(). Implementing a timeout would mean spawning
one or more child processes and getting the results via a pipe, which
doesn't seem worth the effort.

Signed-off-by: Reid Wahl <[email protected]>
@nrwahl2
Copy link
Contributor Author

nrwahl2 commented Apr 15, 2025

Updated to address review

@clumens clumens merged commit 6085fd0 into ClusterLabs:main Apr 16, 2025
1 check passed
@nrwahl2 nrwahl2 deleted the nrwahl2-fencing3 branch April 16, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review: in progress PRs that are currently being reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants