CA-142451: Fail to report all LUNs advertised on targets sharing the sam... #207
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
|
Coverage decreased (-0.0%) when pulling ad213c690e33bf299fc7566b44266fb1a4346afc on germanop:netapp-77 into bce4501 on xapi-project:master. |
…same IQN If a multicontroller array advertises its targets using the same IQN but different IPs and the LUNs in those targets are distinct, we are able to show only LUNs coming from the first IP address. The fix reinstates a line that was removed in commit 391bdf055dff in our old sm.hg repository. The purporse of the line is to first check that we have a valid active iscsi connection using the original pair IP,IQN. Only if it fails (it should not in my opinion) we fall back to other ip addresses. Signed-off-by: Germano Percossi <[email protected]>
| util.SMlog("Path found: %s" % self.path) | ||
| break | ||
| self.address = self.tgtidx | ||
| if not os.path.exists(self.path): |
There was a problem hiding this comment.
Could we move this functionality to a separate method, so that we could cover it with unit tests? What I am thinking about:
- commit 1 to apply a refactor - a simple extract method
- commit 2 to add Unit Tests for the existing functionality
- commit 3 to cover the new functionality.
What do you think?
|
Test with some basic configurations and it doesn't seem to introduce any regressions. Good to merge! |
|
This change did not come with unit tests, and it was merged, why did it happen? |
siddharthv
pushed a commit
to siddharthv/sm
that referenced
this pull request
Aug 18, 2014
…e same IQN If a multicontroller array advertises its targets using the same IQN but different IPs and the LUNs in those targets are distinct, we are able to show only LUNs coming from the first IP address. The fix reinstates a line that was removed in commit 391bdf055dff in our old sm.hg repository. The purporse of the line is to first check that we have a valid active iscsi connection using the original pair IP,IQN. Only if it fails (it should not in my opinion) we fall back to other ip addresses. Signed-off-by: Germano Percossi <[email protected]> Reviewed-by: Chandrika Srinivasan <[email protected]> Imported-by: Siddharth Vinothkumar <[email protected]> GitHub: closes xapi-project#207 on xapi-project/sm (cherry picked from commit 66c89ea)
fquesnel
pushed a commit
to fquesnel/sm
that referenced
this pull request
Aug 21, 2014
…e same IQN If a multicontroller array advertises its targets using the same IQN but different IPs and the LUNs in those targets are distinct, we are able to show only LUNs coming from the first IP address. The fix reinstates a line that was removed in commit 391bdf055dff in our old sm.hg repository. The purporse of the line is to first check that we have a valid active iscsi connection using the original pair IP,IQN. Only if it fails (it should not in my opinion) we fall back to other ip addresses. Signed-off-by: Germano Percossi <[email protected]> Reviewed-by: Chandrika Srinivasan <[email protected]> GitHub: closes xapi-project#207 on xapi-project/sm
|
@matelakat , unit tests are not mandatory. Morevoer, when it comes to fixes, especially (as in this case) for critical things, the golden |
andrey-podko
pushed a commit
to andrey-podko/sm
that referenced
this pull request
Aug 16, 2022
…e same IQN If a multicontroller array advertises its targets using the same IQN but different IPs and the LUNs in those targets are distinct, we are able to show only LUNs coming from the first IP address. The fix reinstates a line that was removed in commit 391bdf055dff in our old sm.hg repository. The purporse of the line is to first check that we have a valid active iscsi connection using the original pair IP,IQN. Only if it fails (it should not in my opinion) we fall back to other ip addresses. Reviewed-by: Chandrika Srinivasan <[email protected]> GitHub: closes xapi-project#207 on xapi-project/sm
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.
...e IQN
If a multicontroller array advertises its targets using the same IQN
but different IPs and the LUNs in those targets are distinct,
we are able to show only LUNs coming from the first IP address.
The fix reinstates a line that was removed in commit 391bdf055dff
in our old sm.hg repository.
Signed-off-by: Germano Percossi [email protected]