-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port #51088 to master #56965
Merged
Merged
Port #51088 to master #56965
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
alexey-zhukovin
added
Needs-Testcase
PR needs test cases written, or the issue is about a bug/feature that needs test cases
master-port
labels
Apr 29, 2020
alexey-zhukovin
force-pushed
the
master-port-51088
branch
from
May 5, 2020 18:10
e121447
to
42a71ac
Compare
waynew
force-pushed
the
master-port-51088
branch
from
March 16, 2021 21:09
42a71ac
to
88f6643
Compare
waynew
removed
the
Needs-Testcase
PR needs test cases written, or the issue is about a bug/feature that needs test cases
label
Mar 16, 2021
re-run macosxmojave |
re-run freebsd122-py3 |
When we have an NVMe qualified name, then we should actually be setting the value on the host.
If we can't set the NVMe Qualified Name, then we need to halt creation and delete the host, since it was not correctly created.
If NVMe Qualified Name is not provided, we should not try to attach it to the host.
If we can provide a correct NQN then it should be set on the host, and result should be True.
When updating a host if adding a NVMe device fails then we should simply return False. No rollbacks required.
twangboy
force-pushed
the
master-port-51088
branch
from
October 24, 2022 16:33
b3313ad
to
b4bc22d
Compare
twangboy
previously approved these changes
Oct 24, 2022
twangboy
previously approved these changes
Oct 24, 2022
garethgreenaway
approved these changes
Dec 5, 2022
waynew
approved these changes
Dec 5, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure I wrote the tests for this, FWIW, but also LGTM 👍
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.
What does this PR do?
Port #51088 to master