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.
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
Storage: Add Pure Storage storage driver #14599
Storage: Add Pure Storage storage driver #14599
Changes from 1 commit
44db9b7
7a10271
53cac8d
44d05ad
b6b8876
668ff1c
2313ef3
332f955
26320b3
4c9c611
bfb237c
bc70f8e
0460682
dc9992b
0950016
685bbd4
60c2ab0
9d37bd3
ff78ff4
c55b252
9f05ea1
b9c5eb7
2224238
b217d75
5931f83
cf938f3
f9d28da
1984b93
050390e
a4c9246
bba1c1b
a8ca309
d59b924
a41a5d4
2a93472
36c7cc9
4b43b63
2a0051a
d573f87
f30463e
6b1a04b
f7a0a9d
12508d4
e6aaa1b
a353baa
729aaac
9418590
b5159d3
c79502c
d0f8f5a
40b6b51
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@MusicDin question for you:
What happens if I've got a snapshot mounted (so a temporary volume has been created), and then lxd gets stopped and the host restarted uncleanly, and i go to mount the snapshot again, will this call fail because the temporary volume already exists?
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.
copyVolumeSnapshot
creates a new volume, or overwrites an existing one if it already exists.Mounting is handled by the NVMe/iSCSI if host is mapped with a certain volume.
When we attempt to create a mapping, the error is ignored if it already exists, so it cannot fail there.
The only potential issue I can see is that if LXD is abruptly shut down, it will not cleanup the temporary snapshot. For example, if the instance is then removed, this temporary snapshot could remain in Pure Storage.
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.
^ Until the pool itself is removed, then everything is wiped.
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.
OK cool,that was the bit I was mainly interested in.
Can we scan volumes on instance (or snapshot) delete to find any temporary volumes related and delete them too?
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.
I cannot foreseen anything that would block us from doing that.
What is your suggestion, at which phase should this happen? Should we run the cleanup on LXD boot / when storage is loaded?
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.
when deleting the instance snapshot, and when deleting the instance (which by extension is deleting the snapshot)
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.
Ahh yeah of course. So in practice this would mean when we are deleting a specific snapshot, we check if a volume exists with such uuid.
For example:
snap0-uuid
(obviously correctly resolving volume prefixes and suffixes in the process)snap0
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.
not a blocker to merge, but typo here