sbus: move to single sbus server #6971
Closed
Conversation
This file contains 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
Closed
pbrezina
force-pushed
the
sbus
branch
5 times, most recently
from
b71b567 to
ddfa69a
Compare
|
This is ready for review now. |
aplopez
approved these changes
Oct 18, 2023
|
Mistypes in commit messages: availble, individuall, they were changed (chained). |
|
In the commit |
Even in the unlikely case when multiple of these methods where called in parallel, we must treat them as separate calls and handle them individually to avoid situations like: - SetInconsistent - SetActive - SetInconsistent Here, if chained, it translates to SetInconsistent -> SetActive, which is wrong. Therefore we can not chain these signals.
This allows us to connect to the D-Bus server sooner and remove code duplication in responders and backend. Now we can connect to the bus when we are ready and register the process in the monitor when the process is fully initialized.
We used asynchronous connection because we were also creating a dbus server. Now we do not create the server anymore so we can switch back to a blocking call and simplify the code. Blocking call is alright at this moment, since we can not serve any requests anyway and the backend is not yet fully initialized. Whole initialization was postponed until this call is finished in non-blocking mode as well.
Switching to single dbus server allows us to remove this code since all clients can now be contacted via their own unique dbus name. This code is no longer used since the switch.
So we are able to match the logged messages with particular sender.
Asynchronous connection to D-Bus is only needed when creating a server and that is done by `sbus_server_create_and_connect_send`. Non-blocking connection does not make sense on other places in SSSD because we use D-Bus for IPC therefore if D-Bus connection is not functional, our process can not function as well and therefore the blocking connection actually makes more sense.
|
Everything should be fixed and addressed now. |
|
setactive/inconsistent is also build only for files provider. sbus codegen does not support conditional code though and its probably not worth spending time on it. |
Imo, |
These signals are only used by files provider, they are not completely reliable because they are prone to race conditions and should not really be used elsewhere.
Done. |
|
@aplopez, please take a look at the latest version. |
alexey-tikhonov
approved these changes
Oct 30, 2023
|
Thank you, ACK. |
aplopez
reviewed
Oct 31, 2023
|
ACK. Thanks. |
|
Pushed PR: #6971
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a continuation of #6257.
I rebased it and added descriptions to commit messages.
I need to verify functionality since the pull request was opened
for a long time and fix the issue that is currently just mitigated
(probably incorrectly). Therefore this is a draft so far.