No longer supply properties on the D-Bus by means of external calls #1539
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
Member
Author
|
Please see #1211 for prior discussion. |
drckeefe
approved these changes
May 23, 2019
Member
Author
|
Ok, thanks. I'm going to examine the problem and see if the total staleness problem can be mitigated. Also, verify that signals are being set where appropriate. |
Set it on creation and set up. Use the cached value in ThinPool::total_physical_used. Change the return type of total_physical_used so that it returns a simple value, not a Result, since none of the functions that it calls can return an error. Change the return type of methods that invoke this method, including Pool::total_physical_used. Change D-Bus layer property method for pool's total physical size so that it can not return an error. Cache the value of ThinPool::thin_pool_status field in ThinPool::check(), which is the only place where the thin pool device's status is checked. Signed-off-by: mulhern <[email protected]>
Signed-off-by: mulhern <[email protected]>
Since it uses a cached value, it cannot fail. Signed-off-by: mulhern <[email protected]>
mulkieran
force-pushed
the
master-cache-thinpool-status-2
branch
from
7e47122 to
8c85de5
Compare
Member
Author
|
rebased. |
Member
Author
|
Closing as probably obsolete. |
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.
This PR's goal is to eliminate all possibility of failure in methods that are used to supply property values on the D-Bus. Currently, the consequence of a failure in obtaining one of these properties is a message with the form:
no total physical size computed for pool with uuid <some UUID>when most or all of the stratis CLI commands that require interaction with stratisd are run. None of these CLI commands can succeed when this happens. One of these methods supplies its value via a devicemapper ioctl, another by means of a stat call.
The approach taken here is to cache the values used at the time when these methods are necessarily called, which occurs in the corresponding
checkmethods for each module. The reason this seems like a good idea is that the view that the user gets of the stratisd system is consistent with the view that stratisd has of the system.However, there is currently a defect. The view that stratisd has based on the information it got in the
checkmethods is the last information that could be obtained. Perhaps subsequently, thecheckmethod was called, but the result returned wasFailorError. In that case, the information that is placed on the D-Bus and is cached in the objects, is not just slightly stale as is inevitable w/ a cached value, but potentially totally wrong.Resolves: #1148.