WIP: Massive pacemaker-based refactors by nrwahl2 · Pull Request #4011 · ClusterLabs/pacemaker

nrwahl2 · 2025-12-24T05:59:45Z

No description provided.

nrwahl2 · 2026-01-08T20:11:06Z

+ *       reference to the client's \c GSource has been removed. There is likely
+ *       only one reference when we call this function, and thus the client is
+ *       likely to be freed before we return. The current GLib code looks as if
+ *       this is always the case. However, that is a GLib implementation detail.


g_main_context_dispatch() dispatches all pending sources in a batch before moving to the next main loop iteration. If we're running this function, then we're running it as a result of a particular source being dispatched. So I think there may be another reference to the source we're removing if it's also being dispatched as part of this batch.

So I'll weaken this assumption. But I still don't think we need to wait for remote clients to be destroyed by the callback before dropping clients. (See a couple of commits ahead of here.)

Replace it with a boolean "modifies_cib". Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

Using include-what-you-use. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

We return early if cib_discard_reply is set. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

Use cib_api_operations_t:query() instead. A CIB query should always be processed locally. Except for brief intervals when changes are being processed, the CIB contents should be the same on all nodes within a cluster partition. As of this commit, a query is in fact always processed locally. The host argument of query_from is ignored. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

parse_peer_options() is the only place where needs_reply can get set to true. If we're in stand-alone mode, we can't receive a message from a cluster peer. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

...meaning after the "if (!process)" guard. This is an incremental change to make other changes easier. This changes behavior in a small way -- if cib_status is not OK, we will return an error instead of calling process_ping_reply(), which would sync our CIB. However, returning an error seems more correct. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

This doesn't change behavior, but it facilitates an upcoming change. There are only two situations in which parse_peer_options() returns false. In both cases, we're supposed to ignore the request. An upcoming commit will make parse_peer_options() return void, and we'll make decisions about how to proceed based on the values of process, needs_reply, and local_notify. At that point, we don't want to check cib_status for the two cases that we're supposed to ignore. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

The meaning of the return code was never documented, and I've found it confusing. It meant "if false, don't do anything else -- don't check the CIB status, don't call a processor function, don't send a reply, and don't notify a local client." IMO it's simpler to let the existing logic in based_handle_request() handle these cases. We'll end up returning without doing anything if appropriate. The "return true" cases don't need any modification other than changing to "return". The "return false" cases need to negate the default values by setting process and needs_reply to false. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>

This is intended to be equivalent to the existing code. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>