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

Add virtiofs file sharing support to the libvirt machine driver #101

Merged
merged 8 commits into from
Aug 2, 2022

Conversation

cfergeau
Copy link

@cfergeau cfergeau commented Jul 28, 2022

This goes together with crc-org/crc#3269
This configures the VM for file sharing when libvirt/qemu/the host supports it.
I've only tested this on rhel8 for now.

We can let libvirt/QEMU figure out the best value to use.
Cache has always been set to 'default' anyway, and IO is set to
'threads', I don't think tweaking it makes a significant difference in
CRC's usecases (?)
This adds the required libvirt XML in order to share the host directories
listed in SharedDirs.
This will be made conditional on virtiofs support availability in the
next commits.
This adds a getBestGuestFromCaps helper which will be reused in the next
commits.
On older systems (RHEL7), virtiofs is not necessarily present.
This commit uses virsh domcapabilities to detect if virtiofs is
supported, and returns an error if not.
@@ -116,6 +119,34 @@ func domainXML(d *Driver, machineType string) (string, error) {
},
}
}
if virtiofsSupported(d.conn) == nil && len(d.SharedDirs) != 0 {
Copy link
Member

@anjannath anjannath Jul 28, 2022

Choose a reason for hiding this comment

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

it seems the test is failing because there's no real d.conn while running the test

github.com/code-ready/machine-driver-libvirt/pkg/libvirt.getBestGuestFromCaps(0x7f3a3c21c758?)
	/go/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/libvirt.go:268 +0x2b
github.com/code-ready/machine-driver-libvirt/pkg/libvirt.virtiofsSupported(0xc000040ef0?)
	/go/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/domain.go:162 +0x30

Cache: d.CacheMode,
IO: d.IOMode,
Name: "qemu",
Type: "qcow2",
Copy link
Member

Choose a reason for hiding this comment

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

will need to also update the expected xml in domain_test.go

since IO/Cache is not set in DomainDiskDriver anymore
those are removed from the expected domain xml
the unit test TestTemplating is failing because the virtiofsSupported
check depends on a having a connection to an actual libvirt daemon
which is not present while running the unit tests

as a workaround, since we are not testing the virtiofs functionality
in unit tests, we call virtiofsSupported only when shared directories
are requested

this fixes the following error in test:
```
--- FAIL: TestTemplating (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x542294]

goroutine 20 [running]:
testing.tRunner.func1.2({0x65f1a0, 0xa269d0})
	/usr/lib/golang/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/usr/lib/golang/src/testing/testing.go:1392 +0x39f
panic({0x65f1a0, 0xa269d0})
	/usr/lib/golang/src/runtime/panic.go:838 +0x207
github.com/libvirt/libvirt-go.(*Connect).GetCapabilities.func1(0x7f3a0c5f1fff?)
	/go/src/github.com/code-ready/machine-driver-libvirt/vendor/github.com/libvirt/libvirt-go/connect.go:493 +0x14
github.com/libvirt/libvirt-go.(*Connect).GetCapabilities(0x7f3a0a95bb00?)
	/go/src/github.com/code-ready/machine-driver-libvirt/vendor/github.com/libvirt/libvirt-go/connect.go:493 +0x1d
github.com/code-ready/machine-driver-libvirt/pkg/libvirt.getBestGuestFromCaps(0x7f3a3c21c758?)
	/go/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/libvirt.go:268 +0x2b
github.com/code-ready/machine-driver-libvirt/pkg/libvirt.virtiofsSupported(0xc000040ef0?)
	/go/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/domain.go:162 +0x30
github.com/code-ready/machine-driver-libvirt/pkg/libvirt.domainXML(0xc0000e3ef0, {0x6b1498, 0x3})
	/go/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/domain.go:122 +0x9dc
github.com/code-ready/machine-driver-libvirt/pkg/libvirt.TestTemplating(0xc000087ba0?)
	/go/src/github.com/code-ready/machine-driver-libvirt/pkg/libvirt/domain_test.go:12 +0x1b9
testing.tRunner(0xc000126000, 0x6c8730)
	/usr/lib/golang/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/usr/lib/golang/src/testing/testing.go:1486 +0x35f
FAIL	github.com/code-ready/machine-driver-libvirt/pkg/libvirt	0.012s
FAIL
```
@anjannath
Copy link
Member

@anjannath
Copy link
Member

pushed the commits from https://github.com/anjannath/crc-machine-driver-libvirt/commits/virtiofs to cfergeau:virtiofs PR branch

Copy link
Member

@anjannath anjannath left a comment

Choose a reason for hiding this comment

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

LGTM :)

@praveenkumar praveenkumar merged commit ce66284 into crc-org:master Aug 2, 2022
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.

3 participants