-
Notifications
You must be signed in to change notification settings - Fork 982
Storage: Specify storage volume type in disk device source
#14491
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
Merged
tomponline
merged 18 commits into
canonical:main
from
MggMuggins:jira-2160/specify-storage-volume-type
Nov 29, 2024
Merged
Storage: Specify storage volume type in disk device source
#14491
tomponline
merged 18 commits into
canonical:main
from
MggMuggins:jira-2160/specify-storage-volume-type
Nov 29, 2024
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
3eabf13 to
be19fcc
Compare
tomponline
reviewed
Nov 22, 2024
tomponline
reviewed
Nov 22, 2024
tomponline
reviewed
Nov 22, 2024
tomponline
reviewed
Nov 22, 2024
tomponline
reviewed
Nov 22, 2024
tomponline
reviewed
Nov 22, 2024
be19fcc to
3d0131e
Compare
d546641 to
2e71130
Compare
3 tasks
2e71130 to
7b9f6d0
Compare
This function: - Is not clearly named (it returns a path) - Is used in exactly one place - Does 1 uneeded DB query Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
This saves us 1 db query. Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
virtual-machine/container volumes in the default project do not include the project name. Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Allows device `source` property as `custom/name` Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
Signed-off-by: Wesley Hershberger <[email protected]>
This section will be expanded for virtual-machine volumes later Signed-off-by: Wesley Hershberger <[email protected]>
7b9f6d0 to
af7c79d
Compare
tomponline
approved these changes
Nov 29, 2024
tomponline
added a commit
that referenced
this pull request
Dec 18, 2024
tomponline
added a commit
that referenced
this pull request
Jan 11, 2025
Includes commits from #14491 This PR enables attaching a virtual machine's root storage volume to another virtual machine via a disk device: ``` # vm2 yaml ... devices: v1-root: type: disk pool: default source: virtual-machine/vm1 ``` This has some constraints because simultaneous access to storage volumes with content-type `block` is unsafe: - `vm1`'s root volume can be attached to exactly one other instance if `vm1` has `security.protection.start: true` - `vm1`'s root volume can be attached to any number of other instances if the storage volume `virtual-machine/vm1` has `security.shared: true` `security.protection.start` is recommended for interactive use; e.g. a user temporarily needs to access a bricked machine's root volume to fix it or recover data. `security.shared` can be used if more than one running instance must have access to the block volume. ## TODO - [x] Docs - [x] [lxd-ci](https://github.com/canonical/lxd-ci/) tests to validate attachments to running VMs. [Done](canonical/lxd-ci#366) - [x] Bootorder bug (see comment) - current workaround is to hotplug disk devices to avoid UUID/LABEL collisions at boot time.
tomponline
added a commit
that referenced
this pull request
Feb 4, 2025
I'm torn between the `volumeType/volumeName` and `sourcetype` syntaxes for specifying other types of storage volumes in disk devices. A few factors here, some I knew when I wrote my spec, but a few I didn't: - By using `volumeType/volumeName`, we introduced novel syntax that the server doesn't use anywhere else, and doesn't fit nicely with the principle of "one value per map key" (see #14491). - This syntax is already overloaded in the client to mean both `volumeType/volumeName` and `poolName/volumeName` (see [`lxc storage volume copy`](https://canonical-microcloud.readthedocs-hosted.com/en/latest/lxd/reference/manpages/lxc/storage/volume/copy/#lxc-storage-volume-copy-md)) - The `storage.backups_volume` and `storage.image_volume` [server configuration keys](https://canonical-microcloud.readthedocs-hosted.com/en/latest/lxd/server/#miscellaneous-options) also use `poolName/volumeName` The current syntax for specifying volumes as disk devices is: ``` devices: cosmic-dust-analyzer-data: pool: default source: custom/cosmic-dust-analyzer-data type: disk ``` This PR uses this style instead: ``` devices: cosmic-dust-analyzer-data: pool: default source: cosmic-dust-analyzer-data sourcetype: custom type: disk ``` `sourcetype` is `custom` by default so it can be omitted for custom volumes.
tomponline
added a commit
that referenced
this pull request
Feb 4, 2025
This is largely a revert of #14491. The functionality provided by the reverted commits is implemented in the last commit here. Rather than using a common `MountVolume` which changes its behavior based on volume type, the disk device driver should choose the function to call based on the volume type provided by the user in the `source-type` field.
7 tasks
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.
This change lays the groundwork for attaching VM root disk devices to instances by making two changes:
Refactor
Renames
(Un)MountCustomVolume->(Un)MountVolumeto allow for generic activation ofvirtual-machineandcontainervolumes.Specify volume type in disk device
Allows the following syntax for specifying
customstorage volumes in disk devices:(I was incorrect in my assessment that we already support this syntax; the code was there but wasn't being used consistently)