feat(linstorvolumemanager): cache controller uri in a file #83
+1,724
−872
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
Without this change we have an error in cleanup.py that interrupts the coalesce algorithm. Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
The current attach method of FileSR doesn't correctly override the method of the SR class. It actually adds a "bind" parameter, which is seen as an error by analyzers like mypy. The "bind" parameter was added by this commit: "CA-371791: Fix world readable permissions on EXTSR" Signed-off-by: Ronan Abhamon <[email protected]>
The current detach method of BaseISCSISR doesn't correctly override the method of the SR class. It actually adds a "delete" parameter, which is seen as an error by analyzers like mypy. The "delete" parameter was added by this commit: "iscsi: Delete LUN on detach of RawISCSI" Signed-off-by: Ronan Abhamon <[email protected]>
It triggers warns in analyzers like mypy. Signed-off-by: Ronan Abhamon <[email protected]>
`cloneOp` must be present. Parameters like `snapType` must must be written in the same way between the parent class and the child class. Otherwise a linter like mypy may return an error. Signed-off-by: Ronan Abhamon <[email protected]>
It triggers warns in analyzers like mypy. Signed-off-by: Ronan Abhamon <[email protected]>
Prevent mypy errors when a variable type is changed dynamically from list to string. Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Avoid mypy error: ``` error: Assignment to variable "e" outside except: block [misc] ``` Signed-off-by: Ronan Abhamon <[email protected]>
Without this change, mypy triggers an error `var-annoted`: `Need type annotation for "sr_dict"` Signed-off-by: Ronan Abhamon <[email protected]>
Prevent reuse of the `Dict` symbol from the `typing` module. Signed-off-by: Ronan Abhamon <[email protected]>
Log without this change on `chappasword` and `incoming_chappassword`: ``` error: Incompatible types in assignment (expression has type "bytes", variable has type "str") [assignment] ``` Signed-off-by: Ronan Abhamon <[email protected]>
Avoid: ``` error: Incompatible types in string interpolation (expression has type "object", placeholder has type "int | float | SupportsInt") [str-format] ``` Signed-off-by: Ronan Abhamon <[email protected]>
Before this change, IQNs were concatenated into a single string when `multiSession` was used. Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Ronan Abhamon <[email protected]>
Co-authored-by: Damien Thenot <[email protected]> Co-authored-by: Ronan Abhamon <[email protected]> Signed-off-by: Damien Thenot <[email protected]>
Also use return type annotations on these methods. Signed-off-by: Ronan Abhamon <[email protected]>
Due to mypy modifications, we can't build the sm RPM in Koji without a recent pylint version. So the precheck target is only executed in a github workflow now. Signed-off-by: Ronan Abhamon <[email protected]>
Avoid: ``` drivers/LVHDSR.py:195: error: Item "None" of "Any | None" has no attribute "get" [union-attr] drivers/LVHDSR.py:196: error: Value of type "Any | None" is not indexable [index] ``` Signed-off-by: Ronan Abhamon <[email protected]>
During `LinstorSR` init, only create the journaler to make `should_preempt` happy. The volume manager MUST always be created in a SR lock context. Otherwise, we can trigger major issues. For example, a volume can be deleted from the KV-store by `cleanup.py` during a snapshot rollback. Very rare situation but which allowed this problem to be discovered. Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Damien Thenot <[email protected]>
Until now the cleanup VHD resize commands were performed on the master. But it doesn't work every time when a VHD of a chain is opened for reading on another host. As a reminder, this portion of code is only executed rarely. A user must have resized a VHD that must later be coalesced. Signed-off-by: Ronan Abhamon <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Signed-off-by: Mathieu Labourier <[email protected]>
Millefeuille42
force-pushed
the
mlr-533-cache-controller-uri-in-a-file
branch
2 times, most recently
from
958caec to
42b42bc
Compare
Wescoeur
requested changes
Apr 14, 2025
Comment on lines
+238
to
+244
| address = uri.removeprefix("linstor://") | ||
| session = util.timeout_call(10, util.get_localAPI_session) | ||
| for host_ref, host_record in session.xenapi.host.get_all(): | ||
| if host_record.get('address', '') != address: | ||
| continue | ||
| return util.strtobool( | ||
| session.xenapi.host.call_plugin(host_ref, PLUGIN, PLUGIN_CMD, {}) |
There was a problem hiding this comment.
I'm not sure that's necessary here. We can instead do a refactoring of the codebase:
- We will assume that the cache is valid 99% of the time.
- We can directly attempt to create a LINSTOR instance from the URI without checking anything using a plugin. This is the initial idea.
- I think we can implement a new static function on
LinstorVolumeManagerthat is used to create an instance of the class using the cached value directly without any checks. If it fails, we rebuild the local cache and try again. This allows us to improve several smapi functions. - There are a few edge cases left with this idea:
- In some places we use the URI to create the journaler (linstor.KV) and to create a linstor object. We could try to use the cached URI without checks again and add a try/catch directly on these specific cases. In case of connection failure, we explicitly request the cache update.
- I see one last edge case concerning the creation of a linstor instance using
get_ips_from_xha_config_file, we could directly use the function to get the cached URI. In the worst can we can again fallback on the xha config file.
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.
No description provided.