Skip to content

Conversation

@MggMuggins
Copy link
Contributor

This change lays the groundwork for attaching VM root disk devices to instances by making two changes:

Refactor

Renames (Un)MountCustomVolume -> (Un)MountVolume to allow for generic activation of virtual-machine and container volumes.

Specify volume type in disk device

Allows the following syntax for specifying custom storage volumes in disk devices:

devices:
  cosmic-dust-analyzer-data:
	pool: default
	source: custom/cosmic-dust-analyzer-data
	type: disk

(I was incorrect in my assessment that we already support this syntax; the code was there but wasn't being used consistently)

@MggMuggins MggMuggins marked this pull request as ready for review November 19, 2024 15:52
@MggMuggins MggMuggins force-pushed the jira-2160/specify-storage-volume-type branch from 3eabf13 to be19fcc Compare November 20, 2024 06:32
@MggMuggins MggMuggins force-pushed the jira-2160/specify-storage-volume-type branch from be19fcc to 3d0131e Compare November 22, 2024 23:33
@MggMuggins MggMuggins force-pushed the jira-2160/specify-storage-volume-type branch 2 times, most recently from d546641 to 2e71130 Compare November 26, 2024 02:29
@MggMuggins MggMuggins force-pushed the jira-2160/specify-storage-volume-type branch from 2e71130 to 7b9f6d0 Compare November 26, 2024 23:10
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]>
This saves us 1 db query.

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]>
Allows device `source` property as `custom/name`

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]>
@MggMuggins MggMuggins force-pushed the jira-2160/specify-storage-volume-type branch from 7b9f6d0 to af7c79d Compare November 27, 2024 21:45
@tomponline tomponline merged commit 6c29a10 into canonical:main Nov 29, 2024
26 checks passed
@MggMuggins MggMuggins deleted the jira-2160/specify-storage-volume-type branch November 29, 2024 15:35
tomponline added a commit that referenced this pull request Dec 18, 2024
Follow-up to #14491; if a volume is specified as `source: custom/c1`,
then a rename of `c1`->`c2` should leave the device with `source:
custom/c2`.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants