Storage D-Bus reorganization#412
Merged
joseivanlopez merged 14 commits intoagama-project:masterfrom Jan 26, 2023
Merged
Conversation
- It was added back by error when rebasing master changes
joseivanlopez
force-pushed
the
storage-dbus-reorganization
branch
from
34bfc17 to
86e2ed8
Compare
joseivanlopez
force-pushed
the
storage-dbus-reorganization
branch
from
005e143 to
0dfd914
Compare
imobachgs
reviewed
Jan 26, 2023
Contributor
imobachgs
left a comment
imobachgs
left a comment
There was a problem hiding this comment.
Just a minor comment (about the logger params). Otherwise, it looks good to me. Thanks a lot!
joseivanlopez
force-pushed
the
storage-dbus-reorganization
branch
from
e60a8b9 to
4af0d5d
Compare
imobachgs
approved these changes
Jan 26, 2023
Merged
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
3 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.
Problem
The storage D-Bus API is going to be extended in order to offer iSCSI iniciator support for discovering targets and logging. While researching about how to define the iSCSI API, we found some flaws in the current organization of the storage D-Bus API:
/DInstaller/Storage/Proposal1) has too many responsabilites: a) it provides neccesary info for calculating a proposal (e.g., available devices), b) calculates a new proposal, and c) provides info about the result./DInstaller/Storage1and/DInstaller/Storage/Proposal1.Those issues should be fixed before extending the API with iSCSI support.
Part of https://trello.com/c/iQ7BCexU/3261-5-d-installer-d-bus-interface-configure-the-iscsi-initiator-and-the-iscsi-targets.
Solution
The storage D-Bus API was re-organized to fix some of the dectected issues.
The API was structured in the following way:
And now the new API organization is:
The main changes are:
/DInstaller/Storage1implements a new iface.DInstaller.Storage1.Proposal.Calculator. That iface offers an API with all the methods/properties for calculating a proposal (available devices, volume templates, etc)./DInstaller/Storage1/Proposalwill be dynamic, and it will be present in the object tree only if a proposal was calculated (successful or not). For example, during the startup phase there is no proposal yet.Bonus: some refactoring was done for
questionsservice, removing unnecessary code.More improvements will come in the future to address these issues:
Testing