More libcib and based cleanups: cib__{get,set}_calldata, cib__perform_op_{ro,rw}, etc.#4100
Open
nrwahl2 wants to merge 47 commits intoClusterLabs:mainfrom
Open
More libcib and based cleanups: cib__{get,set}_calldata, cib__perform_op_{ro,rw}, etc.#4100nrwahl2 wants to merge 47 commits intoClusterLabs:mainfrom
nrwahl2 wants to merge 47 commits intoClusterLabs:mainfrom
Conversation
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
...directly, instead of calling cib__process_apply_patch(). This will make an upcoming commit simpler. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The request argument provides the input value for functions that need it, via cib__get_calldata(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
We were passing the input argument just for logging on error. But we're already logging the request, and the request always contains the input (if non-NULL) as the child of a PCMK__XE_CIB_CALLDATA child. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
I'm ambivalent about this, since it adds lines of code, but request contains everything needed by a lot of these helper functions. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
We already warn for the same thing in based_process_request() if applicable. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This is already done a few lines prior. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
And drop a fairly useless trace message. cib_dryrun always has to be false there, by the way. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
So that we can use pcmk__xe_get_ll(). This also avoids theoretical overflow risk, if a value is sent as uint64_t and then scanned as long long upon receiving it. That overflow should never happen in practice, as it would require sending a massive number of pings before restarting based. Also improve documentation a bit and add a note of non-understanding. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It might be possible for a new DC to be elected after we send a ping request and before we process a modifying request (which would set ping_modified_since to true and cause the ping reply to be ignored). Add this check just in case. If we're no longer DC, then we don't want to sync our CIB to other nodes. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
These are called only by cib_perform_op()/cib__perform_query(), which assert that *answer is NULL. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's used for all non-modifying ops, not just queries. This also enables renaming cib_perform_op() to cib__perform_op_rw() next. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Avoid using the public API prefix, and make it clearer that this is not for all CIB ops, but rather the ones that may modify something. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This is part of request and will be freed when request is freed. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
We ensure that it's non-NULL in cib_init() before we start the mainloop. After that, no CIB operation should be able to set it to NULL. If it would do so, the operation should fail, and the current (non-NULL) CIB should be kept. (See also the assertions at the beginnings of cib__perform_op_ro() and cib__perform_op_rw() -- cib cannot be NULL if we're calling one of the cib__op_fn_t functions.) When processing requests within a transaction, we don't make a copy, so the_cib may temporarily become NULL. However, we don't return to the mainloop from processing a commit-transaction request until we're finished. At that point, either all requests were processed successfully (and the_cib != NULL) or something failed and we revert to the copy we made at the beginning of the commit-transaction. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This TODO will be addressed in the next commit. So the following note is pretty much irrelevant, but I'm leaving it here for completeness. based_process_shutdown() can currently change the state by calling based_terminate(), and it does not have this flag. That will be remedied in a later commit (though likely not in the same pull request as this commit). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The CIB changed if cib_diff is not NULL and the current request is not a dry-run. If the current request is part of a transaction, then we certainly don't want to sync the current CIB to other nodes, so one might think we'd want to set ping_modified_since to true. However, we process an entire transaction at once without returning to the main loop, so we won't enter process_ping_reply() until after the commit-transaction request anyway. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This technically changes public-facing behavior. I don't see any reason why anything external should be using our mainloop functions, however, so it's only a matter of time until they're deprecated from public API. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
This wasn't a memory leak, because we would eventually free the client table. Now every based remote client is guaranteed to have a remote->source. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Other daemons don't check the number of active IPC client connections via libqb. It seems reasonable to follow the other daemons' lead and assume that the return value of pcmk__ipc_client_count() is all we need to care about. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Somehow I got pretty confused last night about when or if the callback data was guaranteed to be destroyed. Note that the GSource itself is not guaranteed to be freed yet. There may be other references to it -- for example, if it has been dispatched but its callback hasn't run yet. This was the main reason for my confusion. However, since g_source_remove() marks the source as destroyed, nothing will happen if the source gets dispatched. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
To align better with attrd, fenced, and schedulerd. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
All IPC and remote clients have been disconnected and freed by the time we return from based_shutdown(). There's no need to wait for their destroy callbacks, or to have those callbacks call based_shutdown() again. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It no longer has a distinct role; this was no longer a logical split. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
The other call is already guarded. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Just an incremental change. This will simplify further in upcoming commits. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
If needs_reply or local_notify is true, then cib_discard_reply is not set. (If cib_discard_reply is set, then we set needs_reply and local_notify to false earlier in based_process_request().) If cib_discard_reply is not set, then cib_process_command() always creates a non-NULL reply (in the done section). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
It's redundant. If needs_reply is true, then cib_discard_reply is not set. (If cib_discard_reply is set, then we set needs_reply and local_notify to false earlier in based_process_request().) Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
process_transaction_requests() wasn't freeing output. Do that now. Also simplify the freeing of output. * process_request(): Make a copy of *output if it's not in the same doc as private->cib_xml. This lets us simplify file_perform_op_delegate(). * process_request(): No operation leaves result_cib set to the same value as *output unless it's also equal to private->cib_xml. (A query op can do this.) So drop a check in the done section. * file_perform_op_delegate(): Return only at the end of the function, within the done section. * file_perform_op_delegate(): Don't assign *output_data = NULL at the beginning. We'll do this in the done section if output is NULL. * file_perform_op_delegate(): Assume that output is not in the same document as private->cib_xml. We ensured this in process_request(). * file_perform_op_delegate(): Free output if and only if it has not been assigned to *output_data. And do the standard-to-legacy conversion at the end of file_perform_op_delegate(), in the done section. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
force-pushed
the
nrwahl2-based_first
branch
from
978a518 to
44ca8c4
Compare
Contributor
Author
|
Added a commit to address a Coverity false positive, and fixed an issue that Coverity found in commit "Drop remote clients during shutdown". |
Coverity is throwing a CHECKED_RETURN error here, because we check the return value of cib__get_operation() 4 out of 5 times. However, it was already checked in file_perform_op_delegate(). Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
nrwahl2
force-pushed
the
nrwahl2-based_first
branch
from
44ca8c4 to
1e7de54
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Next batch from #4011