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

VM: Avoid Invalid compat argument from virtiofsd #14744

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

hamistao
Copy link
Contributor

@hamistao hamistao commented Jan 6, 2025

When using a = in a device path, as in lxc storage volume attach default vol vm /mnt/path=, we get a stale operation for that VM, the disk device set up freezes midway and further operations on that VM become impossible and always halts. Upon looking the virtiofsd logs we can see something like: error: Invalid compat argument '-o source=/var/snap/lxd/common/lxd/devices/vm/disk.vol.path='. As a fix, this PR escapes '=' from the source path for VMs.

This updates `createDevice` with the idea of factoring out of it the logic of generating the devPath, since for containers '=' is allowed and for VMs it is not.

Signed-off-by: hamistao <[email protected]>
@hamistao hamistao marked this pull request as ready for review January 7, 2025 04:20
@hamistao hamistao requested a review from tomponline January 7, 2025 11:16
@@ -1637,13 +1637,10 @@ func (d *disk) mountPoolVolume() (func(), string, *storagePools.MountInfo, error
// createDevice creates a disk device mount on host.
// The srcPath argument is the source of the disk device on the host.
// Returns the created device path, and whether the path is a file or not.
func (d *disk) createDevice(srcPath string) (func(), string, bool, error) {
func (d *disk) createDevice(srcPath string, devPath string) (func(), bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

as an aside, the createDevice function returns a revert hook func() but to make this clearer please can you change the type of the return value to revert.Hook.

@hamistao hamistao marked this pull request as draft January 7, 2025 13:49
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

@hamistao thanks for this

As discussed lets explore probing the virtiofsd version at LXD start, and storing its location, as well as an indicator if it is the newer unbundled version or the older bundled version. Then we can switch to using the non-deprecated flags that dont exhibit this issue.

We have a struct called type OS struct that is used to store stuff like this, e.g. AppArmorFeatures, so we could probe and store it there.

Or we could keep it with the qemu features and add it here https://github.com/canonical/lxd/blob/main/lxd/instance/drivers/driver_qemu.go#L8637

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