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

Device: Support long device names for host path passthrough to VMs #13603

Merged
merged 17 commits into from
Jun 17, 2024

Conversation

tomponline
Copy link
Member

@tomponline tomponline commented Jun 13, 2024

  • Aligns naming differences between host drive passthrough at boot time and hotplug - this allows a virtiofs share added at boot time to be hot unplugged later.
  • Adds escaping and hashing to host drive device names to allow LXD disk device names to be long and contain / characters.
  • Updates devlxd events and lxd-agent event handler to support passing mount_tag info so that lxd-agent doesn't assume the mount tag is the device name.

Unfortunately due to the historic setup of these devices it has been impossible to maintain complete backward compatibility with the mount tag formats. For short and simple names they remain the same, but for longer ones or ones with / the mount tag has changed necessarily to support that functionality.

Closes #13596

@tomponline tomponline self-assigned this Jun 13, 2024
@tomponline tomponline requested a review from hamistao June 14, 2024 10:08
@tomponline tomponline changed the title Device: Fix disk device passthrough for VMs Device: Fix disk device filesystem passthrough for VMs Jun 14, 2024
@tomponline tomponline marked this pull request as ready for review June 14, 2024 12:22
MusicDin
MusicDin previously approved these changes Jun 14, 2024
Copy link
Member

@MusicDin MusicDin left a comment

Choose a reason for hiding this comment

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

Overall LGTM, thanks.

@tomponline
Copy link
Member Author

@hamistao @roosterfish ready for review, thanks

@tomponline tomponline changed the title Device: Fix disk device filesystem passthrough for VMs Device: Support long device names for host path passthrough to VMs Jun 14, 2024
@hamistao
Copy link
Contributor

hamistao commented Jun 14, 2024

@tomponline The way I see it it is impossible to escape from the problem regarding the chardev and fsdev sections being changed and requiring VMs that used the older versions to be restarted. Any attempt to keep them as they are would break either hotplugging or would require changing the device IDs and end up breaking previously existing devices anyway.

@github-actions github-actions bot added the Documentation Documentation needs updating label Jun 15, 2024
tomponline and others added 12 commits June 15, 2024 20:03
…ice of devlxd events

But only when performing device actions against instances which are running.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…vicesUpdate in Update

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…evicesUpdate in Update

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…ount tag for mounting inside guest

Then use the returned mount tag and add it to the mount options returned in RunConfig from deviceStart.

This will allow it to be passed to the guest as an event when hot plugging a disk filesystem share.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…eviceStart in devicesUpdate and pass to devlxd event

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…k devices

This ensures that if a disk device's mount tag is different from its name,
because its name is too long or needs escaping, that it can stil be mounted.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…ice name when booting and hotplugging

Fixes hot removal of boot time drive.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…o `hashIfLonger`

Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
(cherry picked from commit a0e0e52)
…DMaxLength`

Signed-off-by: hamistao <pedro.ribeiro@canonical.com>
(cherry picked from commit dd8386d)
… to support long device names

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…ost drive to that used when hotplugging

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…or drive share daemons

Allows use of supported "/" char in device names.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…ttachPath

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…lso do escaping

So device names can container supported "/" chars.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…eName

Where escaping was already taking place it has been removed.

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
… to reflect new consistent fsdev and chardev naming

Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
@github-actions github-actions bot removed the Documentation Documentation needs updating label Jun 15, 2024
@tomponline tomponline requested a review from hamistao June 16, 2024 15:18
Copy link
Member

@mihalicyn mihalicyn left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@roosterfish roosterfish left a comment

Choose a reason for hiding this comment

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

Looks good!

@tomponline tomponline merged commit e5d61c0 into canonical:main Jun 17, 2024
28 of 29 checks passed
@tomponline tomponline deleted the tp-vm-disk branch June 17, 2024 09:02
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.

5 participants