arm64: support NVDIMM by Pennyzct · Pull Request #1323 · kata-containers/runtime

Pennyzct · 2019-03-05T08:02:29Z

As Arm64 is using the block device as the rootfs in guest. If we run two or more kata-container instances
on one host, we will get following error:

root@entos-thunderx2-desktop:~# docker run -dt --runtime kata-runtime ubuntu
7baf2c0f26100b0e642ad4122ce9ea1cb556fb3ed6cfcb454cb0586cbbe6194d
root@entos-thunderx2-desktop:~# docker run -dt --runtime kata-runtime ubuntu
2065fcf560c136091b5e69b6a12c95de94308309bb7fd5309df852503752203a
docker: Error response from daemon: OCI runtime create failed: qemu-system-aarch64: -device virtio-blk,drive=image-9f100592ac95eec6,scsi=off,config-wce=off: Failed to get "write" lock
Is another process using the image?: unknown.

Thanks to the new expanded IPA_SIZE feature in kernel 4.20 and Eric Auger's patch set ARM virt: Initial RAM expansion and PCDIMM/NVDIMM support in qemu(which are still under upstream review), we could fully support nvdimm on arm64.

Pennyzct · 2019-03-05T08:32:17Z

I'm not triggering ARM CI here for now, since this pr needs to wait packaging/#377 landed firstly.

jodh-intel · 2019-03-05T09:38:26Z

virtcontainers/qemu_arm64.go

+	if err != nil {
+		return nil, err
+	}
+	defer func() { _ = imageFile.Close() }()


Nit: You could make this simply:

defer imageFile.Close()

rignt, right, right. ;) update asap.

grahamwhaley

Nice work @Pennyzct Nice to see the test case added ;-)
lgtm

sboeuf

This looks good!
Please address comment from @jodh-intel and I have one comment regarding the commit qemu-arm64: disable dax on arm64. You should remove it, since you could remove the dax part from the initial commit.

Pennyzct · 2019-03-06T08:03:59Z

@jodh-intel @sboeuf already updated. ;) ptal

sboeuf

LGTM, thanks @Pennyzct

sboeuf · 2019-03-06T14:36:07Z

/test

Original guest image was reprensented as block device in qemu-aarch64, and it will bring up write lock error when running multiple containers. Thanks to the new expanded IPA_SIZE feature in kernel 4.20 and Eric Auger's related patch set in qemu(which are still under upstream review), we could fully support nvdimm on arm64. Depends-on: github.com/kata-containers/packaging#377 Fixes: kata-containers#843 Signed-off-by: Penny Zheng <penny.zheng@arm.com>

Pennyzct · 2019-03-07T10:10:20Z

/test

Since we overrided the func appendImage for aarch64, we should also provide related unit test. Fixes: kata-containers#843 Signed-off-by: Penny Zheng <penny.zheng@arm.com>

Pennyzct · 2019-03-07T10:40:21Z

/test

Pennyzct · 2019-03-08T02:38:42Z

Hi~ all. @grahamwhaley @jodh-intel @sboeuf a few sad news and one great news.
Great news is that ARM CI got green~~~
But I got dozens of build timeout in multiple x86_64 CIs, and it seems like a pattern to me. you may want to have a look. ;)

•Build timed out (after 5 minutes). Marking the build as aborted.
Build was aborted
Performing Post build task...
Match found for :.* : True
Logical operation result is TRUE
Running script  : #!/bin/bash

grahamwhaley · 2019-03-08T09:57:54Z

Hi @Pennyzct \o/ for the ARM CI :-)
The timeouts - yes, I think we have had a bunch of those overnight - I think it may be related to an image file/clearlinux update that happened - I think @chavafg and @jcvenegas are looking into it. Thanks for reporting!

grahamwhaley · 2019-03-08T14:44:14Z

/test
This looks to be very ARM specific, and the ARM CI is happy. We know the x86 ones had some wobbles overnight - I'm probably happy to merge this as is. @sboeuf wdyt?
I'll kick the CIs off again anyway, as the 'wobbles' should be fixed now - but we could force merge without waiting if we wanted...

sboeuf · 2019-03-08T15:58:40Z

@grahamwhaley I agree this is ARM specific, but the code being modified is in virtcontainers. There could be some unexpected relation with the common code that could cause some failures. I would feel more confident to merge this if we had at least our main jenkins-ci-ubuntu-18-04 job passing.

jodh-intel · 2019-03-11T11:35:35Z

Fedora CI failed with:

not ok 32 ctr with non-root user has no effective capabilities

Fedora vsocks CI failed with:

docker: Error response from daemon: OCI runtime create failed: Failed to check if grpc server is working: context deadline exceeded: unknown.
And we have hit the max 20 containers
Checking 20 containers have all relevant components
Wrong number of shims running (20 != 19) - stopping
Wrong number of netmons running (20 != 19) - stopping
Wrong number of 'runtime list' containers running (20 != 19) - stopping
Wrong number of pods in /var/lib/vc/sbs (20 != 19) - stopping)
Showing system state:

Let's re-spin the wheels and see if the errors are consistent...

amshinde · 2019-03-11T18:45:02Z

/test

amshinde

lgtm

Pennyzct · 2019-03-12T07:16:09Z

/test

jodh-intel · 2019-03-12T10:10:57Z

2 failing CI's are known issues, so merging...

Pennyzct · 2019-03-12T11:00:11Z

Hi~@jodh-intel this pr is tested on ARM CI with the dependency of packaging#377, for now, packaging#377 hasn't been merged yet, I'm afraid ARM ci could break down.
But I see that packaging#377 has got all green, maybe could we merge it?? ;)

jodh-intel · 2019-03-12T11:03:06Z

@Pennyzct - done ;)

Pennyzct changed the title ~~arm64: support~~ arm64: support NVDIMM Mar 5, 2019

jodh-intel approved these changes Mar 5, 2019

View reviewed changes

grahamwhaley approved these changes Mar 5, 2019

View reviewed changes

sboeuf reviewed Mar 5, 2019

View reviewed changes

Pennyzct mentioned this pull request Mar 6, 2019

nvdimm: support nvdimm on arm64 kernel kata-containers/packaging#377

Merged

Pennyzct force-pushed the nvdimm branch from 5d1206d to 353ea6f Compare March 6, 2019 07:49

sboeuf approved these changes Mar 6, 2019

View reviewed changes

Pennyzct force-pushed the nvdimm branch from 353ea6f to 5b1bf8b Compare March 7, 2019 10:09

qemu-arm64: add unit test for func appendImage on aarch64

d8bcddb

Since we overrided the func appendImage for aarch64, we should also provide related unit test. Fixes: kata-containers#843 Signed-off-by: Penny Zheng <penny.zheng@arm.com>

Pennyzct force-pushed the nvdimm branch from 5b1bf8b to d8bcddb Compare March 7, 2019 10:39

grahamwhaley mentioned this pull request Mar 8, 2019

kernel: pmem device should map pages kata-containers/packaging#385

Merged

amshinde approved these changes Mar 11, 2019

View reviewed changes

jodh-intel merged commit ffbae64 into kata-containers:master Mar 12, 2019

Conversation

Pennyzct commented Mar 5, 2019

Uh oh!

Pennyzct commented Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jodh-intel Mar 5, 2019

Choose a reason for hiding this comment

Uh oh!

Pennyzct Mar 5, 2019

Choose a reason for hiding this comment

Uh oh!

grahamwhaley left a comment

Choose a reason for hiding this comment

Uh oh!

sboeuf left a comment

Choose a reason for hiding this comment

Uh oh!

Pennyzct commented Mar 6, 2019

Uh oh!

sboeuf left a comment

Choose a reason for hiding this comment

Uh oh!

sboeuf commented Mar 6, 2019

Uh oh!

Pennyzct commented Mar 7, 2019

Uh oh!

Pennyzct commented Mar 7, 2019

Uh oh!

Pennyzct commented Mar 8, 2019

Uh oh!

grahamwhaley commented Mar 8, 2019

Uh oh!

grahamwhaley commented Mar 8, 2019

Uh oh!

sboeuf commented Mar 8, 2019

Uh oh!

jodh-intel commented Mar 11, 2019

Uh oh!

amshinde commented Mar 11, 2019

Uh oh!

amshinde left a comment

Choose a reason for hiding this comment

Uh oh!

Pennyzct commented Mar 12, 2019

Uh oh!

jodh-intel commented Mar 12, 2019

Uh oh!

Pennyzct commented Mar 12, 2019

Uh oh!

jodh-intel commented Mar 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Pennyzct commented Mar 5, 2019 •

edited

Loading