Some fencer device registration refactors #3863
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Remove some nesting. * Use const where possible. * Improve variable names and scope. * Use a macro for repeated string literal. * Use sizeof() - 1 instead of strlen() for string literal. * Remove unnecessary escaping of single quote within double quotes. * Copy only what is needed of the xpath string, if anything. * Log assertion on malformed patchset, since we created it in cib_perform_op(). * Add a comment to clarify why we're (apparently) doing all this in the first place. Keep the watchdog_device_update() comment as-is because I haven't looked into it. Signed-off-by: Reid Wahl <[email protected]>
An ID is required when schema validation is enabled anyway, and this allows us to be sure rsc_id will be non-NULL within the block. It also allows us to shorten some strstr() searches by starting later in the string, not that this will matter in practice. Signed-off-by: Reid Wahl <[email protected]>
The sole caller has already done the checking, we don't need event, and we don't need msg except as a way to get the patchset. Signed-off-by: Reid Wahl <[email protected]>
|
retest this please |
* Rename to fenced_device_register(). * Return standard Pacemaker return code. * Rename rv to rc. * Take const and bool arguments. * Improve if-test formatting. * Make watchdog handling block more readable, using a done label to free the device if needed and to avoid the do-while loop (which was there to accommodate a break statement). * Check pointers against NULL explicitly. * Limit variable scope. * Limit comment line lengths. * Improve messages. * Drop redundant lookup (device_has_duplicate() already did this lookup). Signed-off-by: Reid Wahl <[email protected]>
If the child of a primitive element representing a fencing device was deleted, and we didn't catch it earlier as instance_attributes or meta_attributes, then previously we would remove the fencing device. We should remove the device only if the primitive itself was deleted. By inspection, it looks like this would unregister a fencing device if one of its operations (op elements) were deleted. If we find that only a child of the primitive was deleted, fall through to the next if block -- that is, "continue" only if the primitive itself was deleted and we call stonith_device_remove(). Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
It's unused. Also drop name stonith_device_s for struct that gets typedef'd to stonith_device_t. Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-fencing2
branch
3 times, most recently
from
5ab542e to
d46e960
Compare
clumens
reviewed
Apr 9, 2025
No known fence agent besides fence_dummy advertises the nodeid parameter in its metadata, so is_nodeid_required() should never return true unless some custom fence agent advertises nodeid and expects Pacemaker to pass it automatically. I checked all the agents in both fence-agents and cluster-glue. fence_dummy advertises nodeid purely for cts-fencing's use. So this test in cts-fencing is for a scenario that is not encountered in practice, and it can be dropped. The nodeid parameter in fence_dummy can be dropped accordingly. If anyone complains about this, we can deprecate this functionality for a while and drop it at a major or minor release. However, it seems likely this has been unused for a long time. It was added by 08c78ad (and again by c2265a1, oddly) in 2012. The commit message gives no information about what motivated the change, nor does the linked pull request give any real insight. The PR mentions "stonith scsi" support, but the fence_scsi agent at the time did not support a nodeid parameter. Signed-off-by: Reid Wahl <[email protected]>
...for fence devices whose agent metadata advertises support for a nodeid parameter. No known fence agent advertises the nodeid parameter in its metadata, so is_nodeid_required() should never return true unless some custom fence agent advertises nodeid and expects Pacemaker to pass it automatically. If anyone complains about this, we can deprecate this functionality for a while and drop it at a major or minor release. However, it seems likely this has been unused for a long time. It was added by 08c78ad (and again by c2265a1, oddly) in 2012. The commit message gives no information about what motivated the change, nor does the linked pull request give any real insight. The PR mentions "stonith scsi" support, but the fence_scsi agent at the time did not support a nodeid parameter. Signed-off-by: Reid Wahl <[email protected]>
Everything passes 0 currently. Signed-off-by: Reid Wahl <[email protected]>
We've had a TODO to "hook up priority" for over 15 years: c6bac1b. At this point, it's safe to skip this and come back to it if needed. Fencing levels are always an option in the meantime. This seems like a nice-to-have at best, with fencing levels providing an explicit way to accomplish it. (I can't think of anything priority could do that fencing levels couldn't.) Signed-off-by: Reid Wahl <[email protected]>
Proper prefix for fencer daemon. "stonith_" belongs to libstonithd. Signed-off-by: Reid Wahl <[email protected]>
Seems to be unused since de0e565 in 2012. Also drop name of struct that we immediately typedef. Signed-off-by: Reid Wahl <[email protected]>
Unused Signed-off-by: Reid Wahl <[email protected]>
Unused Signed-off-by: Reid Wahl <[email protected]>
Unused Signed-off-by: Reid Wahl <[email protected]>
I was confused for a bit because it's not directly used for anything. Signed-off-by: Reid Wahl <[email protected]>
This is a step toward making device_list a file-scope variable. Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Signed-off-by: Reid Wahl <[email protected]>
Proper prefix for a non-static symbol, and more importantly says "table" now. "List" was misleading. Signed-off-by: Reid Wahl <[email protected]>
Proper prefix for a non-static symbol, and more importantly says "table" now. "List" was misleading. Signed-off-by: Reid Wahl <[email protected]>
nrwahl2
force-pushed
the
nrwahl2-fencing2
branch
from
d46e960 to
fd4669a
Compare
clumens
approved these changes
Apr 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A few things that came up while trying to figure out #3855.