Add experimental support for VirtIO-FS #3493

vladikr · 2020-05-31T04:15:53Z

What this PR does / why we need it:

This PR will add experimental support for virtio-fs.
Users will be able to provide a Filesystem based volume and passthrough filesystem to the guest.

spec:                                                                           
  domain:                                                                       
    devices:          
      filesystems:
      - name: disk1
        virtiofs: {}
...
...
  volumes:
  - name: disk1
    persistentVolumeClaim:
    claimName: test-nfs2v2nb75bds2ddvkslxt5gv922nvpdnlm7n6dhrdmflp597t7

depends on #3268

Special notes for your reviewer:

Release note:

virtIO-FS is being added as experimental, protected by a feature-gate that needs to be enabled in the kubevirt config by the administrator

vladikr · 2020-05-31T04:16:04Z

/hold

vladikr · 2020-05-31T04:16:30Z

/cc @fabiand

vladikr · 2020-06-03T12:07:24Z

/retest

fabiand · 2020-06-03T22:30:57Z

@jean-edouard mind giving this a review?

fabiand · 2020-06-03T22:31:24Z

@vladikr in the end it's probably worth to have a release note ,)

vladikr · 2020-06-04T21:18:11Z

tests/storage_test.go

+			})
+			It("should start a VMI with virtiofs", func() {
+				hugepageSize := "2Mi"
+				memory := "64Mi"


64 is to small, requires at least 512...

vladikr · 2020-06-04T21:20:28Z

In order to make the tests pass on kubevirtci, the kernel needs to be updated to 4.18.0-193 and the node should have at least 512Mi of 2m hugepages available

I wonder how to go around this. Should I follow kubevirt/kubevirtci#344 and increase the number of huge pages or should I create a new lane?
@rmohr @dhiller @danielBelenky

jean-edouard

This looks sane, example VM(s) would be nice to have too if possible.
Don't we need a libvirt bump? https://virtio-fs.gitlab.io/ seems to say it's available starting from v6.2.0.
What is the plan for SELinux, do we want to create a new type?
What's the plan for CI? Since the kernel in Centos 8 is too old, do we need a new lane?

jean-edouard · 2020-06-04T21:16:57Z

pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter_test.go

+			vmi.Spec.Domain.Resources.Requests = k8sv1.ResourceList{
+				k8sv1.ResourceMemory: resource.MustParse("64Mi"),
+			}
+			vmi.Spec.Domain.Memory = &v1.Memory{Guest: &guestMemory}


Duplicate line?

jean-edouard · 2020-06-04T21:42:36Z

tests/storage_test.go

+
+				By("Checking that virtio-fs is mounted")
+				_, err = expecter.ExpectBatch([]expect.Batcher{
+					&expect.BSnd{S: "ls -l %s/*disk* | wc -l\n"},


What does %s refer to here? Nit: maybe use ls -1?

@jean-edouard any chance you could see what did I do wrong here? This fails with the new expecter.

jean-edouard · 2020-06-04T21:44:10Z

pkg/virt-launcher/virtwrap/api/converter.go

@@ -1,6 +1,6 @@
 /*
 * This file is part of the KubeVirt project
- *
+*


vladikr · 2020-06-05T18:36:52Z

Don't we need a libvirt bump? https://virtio-fs.gitlab.io/ seems to say it's available starting from v6.2.0.

Yeah, libvirt is already updated to 6.0 (as part of #3268 ) - our kubevirt/libvirt already have that virtiofs support it in

vladikr · 2020-06-05T18:39:26Z

What is the plan for SELinux, do we want to create a new type?

Working on it :)
First I had to figure out CI.
Basically we will need to enable exactly what conainer_kvm_t allows for virtiofs: https://github.com/containers/container-selinux/blob/master/container.te#L1096

vladikr · 2020-06-05T18:41:47Z

What's the plan for CI? Since the kernel in Centos 8 is too old, do we need a new lane?

We just need to update the kernel in this image. virtiofs also needs more hugepages, I'm not sure what's the best route.

openshift-ci-robot · 2020-06-18T11:51:08Z

@vladikr: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-azure-nested-virt	`05f5ec2`	link	`/test e2e-azure-nested-virt`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fabiand · 2020-06-29T11:48:44Z

@rmohr @davidvossel could one of you please give this a look?
@dhiller @danielBelenky do you know something about the libvirt rebase and needed ci changes?

davidvossel · 2020-09-15T20:20:59Z

staging/src/kubevirt.io/client-go/api/v1/schema.go

@@ -372,6 +372,9 @@ type Devices struct {
 	//Whether to attach a GPU device to the vmi.
 	// +optional
 	GPUs []GPU `json:"gpus,omitempty"`
+	// Filesystems describes filesystem which is connected to the vmi.
+	// +optional


add // +listType=set or // +listType=atomic here

👍 thanks!

vladikr · 2020-09-16T02:38:10Z

/retest

vladikr · 2020-09-16T10:42:03Z

/retest

vladikr · 2020-09-17T13:57:12Z

/retest

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

…label Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

davidvossel

/lgtm
/approve

davidvossel · 2020-09-18T18:07:06Z

pkg/virt-launcher/virtwrap/api/converter.go

+
+			volume := volumes[fs.Name]
+			if volume == nil {
+				return fmt.Errorf("No matching volume with name %s found", fs.Name)


i think it's fine to keep this check here. It would be good to add this to the validation webhook as well though so we can catch this condition earlier.

i don't think the PR necesssarily needs to be blocked for that though. it can come in a follow up pr

kubevirt-bot · 2020-09-18T18:07:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidvossel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [davidvossel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

vladikr · 2020-09-18T21:20:02Z

/retest

kubevirt-commenter-bot · 2020-09-18T23:32:12Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-bot · 2020-09-19T01:21:37Z

@vladikr: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubevirt-e2e-k8s-1.15-ceph	`c19b324`	link	`/test pull-kubevirt-e2e-k8s-1.15-ceph`
pull-kubevirt-e2e-kind-1.17-sriov	`ec398f4`	link	`/test pull-kubevirt-e2e-kind-1.17-sriov`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

kubevirt-commenter-bot · 2020-09-19T03:36:10Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-commenter-bot · 2020-09-19T06:39:11Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XL labels May 31, 2020

kubevirt-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels May 31, 2020

kubevirt-bot requested review from fabiand, rnetser and stu-gott May 31, 2020 04:16

vladikr force-pushed the virtiofs_experimental branch 2 times, most recently from 57d7539 to 4b6741a Compare June 1, 2020 21:45

fabiand requested a review from jean-edouard June 3, 2020 22:30

vladikr commented Jun 4, 2020

View reviewed changes

jean-edouard reviewed Jun 4, 2020

View reviewed changes

vladikr mentioned this pull request Jun 16, 2020

allow setting hugepages via gocli kubevirt/kubevirtci#390

Closed

vladikr force-pushed the virtiofs_experimental branch 3 times, most recently from 7e83baf to 05f5ec2 Compare June 18, 2020 05:13

vladikr force-pushed the virtiofs_experimental branch from 05f5ec2 to c19b324 Compare June 18, 2020 18:19

davidvossel reviewed Sep 15, 2020

View reviewed changes

vladikr force-pushed the virtiofs_experimental branch from 81ca9d2 to 95be26c Compare September 15, 2020 21:28

vladikr added 12 commits September 18, 2020 09:52

Adding feature gate for experimental virtiofs support

200db90

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

add a memorybacking access schema element

73106e7

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

add a filesystem device schema element

5e2b4a7

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

allow CAP_SYS_ADMIN when the experimental virtiofs is required

49ed03a

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

vmis with virtiofs require memory backing shared access

34a6a33

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

handle filesystem virtiofs devices

835249b

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

enable virtiofsd debug logs be setting by setting virtiofsdDebugLogs …

8bfe7e8

…label Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

virtiofs requires virt-launcher selinux policy changes

af06bcd

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

update generated files

e0a07c7

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

functional test to verify that virtiofs is enabled

5156341

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

test that vitriofs file written in the guest is present in the pod

0095b78

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

switch virtiofs tests to use datavolume

e072af2

Signed-off-by: Vladik Romanovsky <vromanso@redhat.com>

vladikr force-pushed the virtiofs_experimental branch from 95be26c to e072af2 Compare September 18, 2020 14:40

davidvossel approved these changes Sep 18, 2020

View reviewed changes

kubevirt-bot assigned davidvossel Sep 18, 2020

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 18, 2020

kubevirt-bot merged commit 647515f into kubevirt:master Sep 19, 2020

salanki mentioned this pull request Sep 30, 2020

Virtio-FS failing on hosts with AppArmor #4290

Open

salanki mentioned this pull request Oct 28, 2020

Disk image files are being created for Virtio-FS mounted volumes #4440

Closed

Add experimental support for VirtIO-FS #3493

Add experimental support for VirtIO-FS #3493

vladikr commented May 31, 2020 •

edited

vladikr commented May 31, 2020

vladikr commented May 31, 2020

vladikr commented Jun 3, 2020

fabiand commented Jun 3, 2020

fabiand commented Jun 3, 2020

vladikr Jun 4, 2020

vladikr commented Jun 4, 2020 •

edited

jean-edouard left a comment

jean-edouard Jun 4, 2020

vladikr Sep 15, 2020

jean-edouard Jun 4, 2020

vladikr Jul 2, 2020

jean-edouard Jun 4, 2020

vladikr commented Jun 5, 2020

vladikr commented Jun 5, 2020

vladikr commented Jun 5, 2020

openshift-ci-robot commented Jun 18, 2020

fabiand commented Jun 29, 2020

davidvossel Sep 15, 2020

vladikr Sep 15, 2020

vladikr commented Sep 16, 2020

vladikr commented Sep 16, 2020

vladikr commented Sep 17, 2020

davidvossel left a comment

davidvossel Sep 18, 2020

kubevirt-bot commented Sep 18, 2020

vladikr commented Sep 18, 2020

kubevirt-commenter-bot commented Sep 18, 2020

kubevirt-bot commented Sep 19, 2020 •

edited

kubevirt-commenter-bot commented Sep 19, 2020

kubevirt-commenter-bot commented Sep 19, 2020

Add experimental support for VirtIO-FS #3493

Add experimental support for VirtIO-FS #3493

Conversation

vladikr commented May 31, 2020 • edited

vladikr commented May 31, 2020

vladikr commented May 31, 2020

vladikr commented Jun 3, 2020

fabiand commented Jun 3, 2020

fabiand commented Jun 3, 2020

Choose a reason for hiding this comment

vladikr commented Jun 4, 2020 • edited

jean-edouard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladikr commented Jun 5, 2020

vladikr commented Jun 5, 2020

vladikr commented Jun 5, 2020

openshift-ci-robot commented Jun 18, 2020

fabiand commented Jun 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladikr commented Sep 16, 2020

vladikr commented Sep 16, 2020

vladikr commented Sep 17, 2020

davidvossel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubevirt-bot commented Sep 18, 2020

vladikr commented Sep 18, 2020

kubevirt-commenter-bot commented Sep 18, 2020

kubevirt-bot commented Sep 19, 2020 • edited

kubevirt-commenter-bot commented Sep 19, 2020

kubevirt-commenter-bot commented Sep 19, 2020

vladikr commented May 31, 2020 •

edited

vladikr commented Jun 4, 2020 •

edited

kubevirt-bot commented Sep 19, 2020 •

edited