Skip to content
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

tests/storage-volumes-vm: Root volume disk device attachments #366

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MggMuggins
Copy link
Contributor

This should have a check for all corner cases around VM root volume attachments (coverage for canonical/lxd#14532):

  • security.protection.start allows one other VM to attach the machine's root disk, and can only be removed if the disk is not attached
  • security.shared allows unchecked attachments of root disks
  • VM attachments are correctly reported in used_by
  • hotplug of VM root attachments works (as this is the method reccomended by the docs to avoid UUID/LABEL conflicts)

I ran into an interesting behavior of udev in Noble VMs while testing that slowed this down a bit. In order for udev to create a symlink for SCSI_IDENT_SERIAL (/dev/disk/by-id/scsi-SQEMU_QEMU_HARDDISK_lxd_virtual--machine-vm5), I had to hotplug the device in an ubuntu:24.04 VM. Starting the VM from cold didn't create the link, and ubuntu-minimal:24.04 VMs never created it at all.

Without hotplug or in an ubuntu-minimal:24.04:

# udevadm info /dev/sdb
...
S: disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_virtual--machine
S: disk/by-path/pci-0000:02:00.0-scsi-0:0:1:1
S: disk/by-diskseq/10
E: DEVPATH=/devices/pci0000:00/0000:00:01.1/0000:02:00.0/virtio6/host0/target0:0:1/0:0:1:1/block/sdb
E: DEVNAME=/dev/sdb
...
E: ID_SERIAL=0QEMU_QEMU_HARDDISK_lxd_virtual--machine
E: ID_SERIAL_SHORT=lxd_virtual--machine
E: ID_SCSI_SERIAL=lxd_virtual--machine-v2
E: ID_BUS=scsi
...
E: DEVLINKS=/dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_virtual--machine /dev/disk/by-path/pci-0000:02:00.0-scsi-0:0:1:1 /dev/disk/by-diskseq/10
...

With hotplug:

# udevadm info /dev/sdb
...
S: disk/by-id/scsi-SQEMU_QEMU_HARDDISK_lxd_virtual--machine-vm5
S: disk/by-path/pci-0000:02:00.0-scsi-0:0:1:1
S: disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_virtual--machine
S: disk/by-diskseq/15
...
E: DEVPATH=/devices/pci0000:00/0000:00:01.1/0000:02:00.0/virtio6/host0/target0:0:1/0:0:1:1/block/sdb
E: DEVNAME=/dev/sdb
...
E: SCSI_IDENT_SERIAL=lxd_virtual--machine-vm5
E: SCSI_IDENT_LUN_VENDOR=lxd_virtual--machine
...
E: ID_SERIAL=0QEMU_QEMU_HARDDISK_lxd_virtual--machine
E: ID_SERIAL_SHORT=lxd_virtual--machine
E: ID_SCSI_SERIAL=lxd_virtual--machine-vm5
...
E: DEVLINKS=/dev/disk/by-id/scsi-SQEMU_QEMU_HARDDISK_lxd_virtual--machine-vm5 /dev/disk/by-path/pci-0000:02:00.0-scsi-0:0:1:1 /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_virtual--machin>
...

I worked around it by using a shorter LXD device name so that the symlink name wouldn't be truncated; not sure if this is worth chasing further.

Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I made some comments but they are mostly nit picks.

I haven't seen it run in GHA due to the extension not being in latest/edge just yet. I'm assuming it all works locally but wanted to check how much longer it made the test :)

tests/storage-volumes-vm Outdated Show resolved Hide resolved
tests/storage-volumes-vm Outdated Show resolved Hide resolved
tests/storage-volumes-vm Outdated Show resolved Hide resolved
tests/storage-volumes-vm Outdated Show resolved Hide resolved
tests/storage-volumes-vm Outdated Show resolved Hide resolved
tests/storage-volumes-vm Outdated Show resolved Hide resolved
! lxc storage volume attach "${poolName}" virtual-machine/v2 v3 || false

# Deleting the instance will fail while it's root volume is in use
! lxc delete v2 || false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now wondering if you can remove v2's root volume and permanently attach it to v1? I think that's something we want for scenario where we "rescue" files from an unbootable VM.

The test is fine but that question that popped :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this feature, the only meaningful distinction remaining between virtual-machine/container volumes and custom volumes is their lifecycle; that is, tied to instances vs independent. In the absence of that constraint (and any concerns around back-compat 😅 ), I'd advocate to remove the idea of storage volume types all together and just have "storage volumes".

If keeping the root's contents around without the instance is a capability that we want, it might be worth looking at whether creating a custom volume as a copy of an instance/snapshot is possible.

@tomponline
Copy link
Member

I ran into an interesting behavior of udev in Noble VMs while testing that slowed this down a bit. In order for udev to create a symlink for SCSI_IDENT_SERIAL (/dev/disk/by-id/scsi-SQEMU_QEMU_HARDDISK_lxd_virtual--machine-vm5), I had to hotplug the device in an ubuntu:24.04 VM. Starting the VM from cold didn't create the link, and ubuntu-minimal:24.04 VMs never created it at all.

@hamistao any experience with this?

@hamistao
Copy link
Contributor

hamistao commented Dec 6, 2024

Not particularly, but it is not the first time we have seen udev or other packages mangling with disk devices that LXD manages (canonical/lxd#13701 (comment)) and since it only happens on a specific distro version and not on minimal it is almost certainly a problem of the same nature and probably isn't worth digging too deep into.

On Without hotplug or in an ubuntu-minimal:24.04 we have S: disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_virtual--machine and E: DEVLINKS=/dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_virtual--machine ... so LXD seems to be doing its job just fine.

This should have a check for all corner cases around VM root volume
attachments:
- security.protection.start allows one other VM to attach the machine's
  root disk, and can only be removed if the disk is not attached
- security.shared allows unchecked attachments of root disks
- VM attachments are correctly reported in used_by
- hotplug of VM root attachments works (as this is the method reccomended
  by the docs to avoid UUID/LABEL conflicts)

Signed-off-by: Wesley Hershberger <[email protected]>
@MggMuggins MggMuggins force-pushed the attach-vm-root-volumes branch from 4ea7f64 to 2040505 Compare December 9, 2024 22:23
@MggMuggins
Copy link
Contributor Author

Thanks @tomponline @hamistao, I agree that this isn't worth chasing.

@simondeziel They do pass locally 😉 . It looks like it goes from 5 lxc starts to 6 lxc starts. So ~120% of the previous runtime for this test.

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.

4 participants