image-builder: fill out device namespace information into kata image by devimc · Pull Request #236 · kata-containers/osbuilder

devimc · 2019-03-08T17:32:49Z

The new NVDIMM driver implementation (kernel >= 4.16) needs to know the device
namespace information to map pages, this metadata is read from the nvdimm
namespace at 4k offset.

fixes #235

devimc · 2019-03-08T17:33:50Z

/test

.gitignore

jodh-intel · 2019-03-08T17:42:51Z

image-builder/nsdax.c

@@ -0,0 +1,171 @@
+/*
+ * Copyright(c) 2013-2019 Intel Corporation. All rights reserved.


2013? Wow ;)

yep, copy&paste from NVDIMM driver

jodh-intel · 2019-03-08T17:45:24Z

image-builder/image_builder.sh

+mv "${tmp_img}" "${IMAGE}"
+# Set metadata header
+gcc -O2 "${script_dir}/nsdax.c" -o "${script_dir}/nsdax"
+"${script_dir}/nsdax" "${IMAGE}" "${IMG_HEADER_SZ_B}" "${IMG_HEADER_SZ_B}"


Random thought: I wonder if there would be any value in adding info to osbuilder.yaml for kata-collect-data.sh to use?

jodh-intel · 2019-03-08T17:47:35Z

image-builder/nsdax.c

+ * Copyright(c) 2013-2019 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as


Just mentioning that the license for this file (GPL-2.0) is different to the license for the containing osbuilder repo project (Apache-2.0).

/cc @grahamwhaley.

Good pick up. IANAL, but I believe as long as you are not linking this to any of our (apache2) code (and I think you are not), then you are probably good.
This file local license will I think over-rule the top level license. I don't know if we have to say something at the top level about 'some files may be under a different license' now though.

Overall, if you can confirm you are not linking this against any apache2 code, then I'm not going to worry about it.
Some attribution somewhere about where the code came from might be good if we did not add one already..

I confirm, I'm not linking this against any apache2 code

It would be interesting to discuss if we wanted to switch our LICENSE files to use the Debian machine readable copyright file format since that handles this sort of scenario nicely:

https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/

devimc · 2019-03-08T18:03:51Z

@jodh-intel CI is not happy

ERROR: Required license identifier ('SPDX-License-Identifier: Apache-2.0') missing from following files:

and the workaround is ... ?

jodh-intel · 2019-03-08T18:10:12Z

@devimc - hmm, well you've hit a new scenario - 1st file in one of our repos that originated elsewhere. Aside from introducing that machine-readable copyright file (which I'm liking more and more now ;-) I just mentioned (which would allow the CI to intelligently filter out files like this), I'm not sure. We can't add a different license but how do we retain the check for the majority of other files?

For speed, we could either hack the CI script to special-case this file. But how about instead we do one of the following:

Put the file in a special" directory and make the CI check ignore that directory.

Something like no-check/nsdax.c.
Add a "magic tag" to the file and tweak the CI script to ignore such files for license checks.

Something like @KATA_IGNORE_LICENSE_CHECK@ ?

This would be easy to implement and minimally invasive I reckon?

jodh-intel · 2019-03-08T18:49:19Z

@devimc - thinking about this a bit more, how about the following:

We update https://github.com/kata-containers/tests/blob/master/.ci/static-checks.sh to look for a .ci/ignore file.
If present, each non-comment and non-blank line in the file is in the format:

check-name:filename:rationale

So for your new file, we could create a .ci/ignore in the osbuilder repo containing:

# some explanatory comments

license:image-builder/nsdax.c:imported file has its own license.

wdyt @chavafg, @egernst, @grahamwhaley, @sboeuf?

But for the really quick solution, I think you can use a hack: Move the file to the following location since "vendored" files are ignored! :-):

image-builder/vendor/nsdax.c

grahamwhaley · 2019-03-11T09:28:38Z

In this specific case, I quite favor a one off hack or force merge. If we do get more and more files coming into the repos then I think we can discuss better workarounds. Let's not add a slew of complication to fix a problem that doesn't yet exist...

jodh-intel · 2019-03-11T11:00:42Z

@devimc - can you move the file to the vendor/ directory then to get this resolved? I'd also recommend creating a tiny README.md in that directory explaining why that .c file is in that directory.

devimc · 2019-03-11T13:46:17Z

@jodh-intel

ERROR: PR changes vendor files, but does not update Gopkg.lock
The command ".ci/setup.sh" failed and exited with 1 during .

devimc · 2019-03-11T14:13:27Z

@jodh-intel please take a look kata-containers/tests#1280

jodh-intel · 2019-03-11T14:27:05Z

Our CI is waaaaay too clever! 😆

devimc · 2019-03-11T14:58:47Z

/test

devimc · 2019-03-11T15:44:52Z

Euleros mirrors are down

[Errno 14] HTTP Error 404 - Not Found
Trying other mirror.


 One of the configured repositories failed (EulerOS-base),
 and yum doesn't have enough cached data to continue. At this point the only
 safe thing yum can do is fail. There are a few ways to work "fix" this:

     1. Contact the upstream for the repository and get them to fix the problem.

     2. Reconfigure the baseurl/etc. for the repository, to point to a working
        upstream. This is most often useful if you are using a newer
        distribution release than is supported by the repository (and the
        packages for the previous distribution release still work).

     3. Disable the repository, so yum won't use it by default. Yum will then
        just ignore the repository until you permanently enable it again or use
        --enablerepo for temporary usage:

            yum-config-manager --disable EulerOS-base

     4. Configure the failing repository to be skipped, if it is unavailable.
        Note that yum will try to contact the repo. when it runs most commands,
        so will have to try and fail each time (and thus. yum will be be much
        slower). If it is a very temporary problem though, this is often a nice
        compromise:

            yum-config-manager --save --setopt=EulerOS-base.skip_if_unavailable=true

failure: repodata/362dfa7e4e1a25ea3d988a1af4848d61f67f10143dbdc5349d80c2c7890ecb44-primary.sqlite.bz2 from EulerOS-base: [Errno 256] No more mirrors to try.
http://developer.huawei.com/ict/site-euleros/euleros/repo/yum/2.2/os/x86_64/repodata/362dfa7e4e1a25ea3d988a1af4848d61f67f10143dbdc5349d80c2c7890ecb44-primary.sqlite.bz2: [Errno 14] HTTP Error 404 - Not Found

jodh-intel · 2019-03-11T16:10:11Z

@devimc, @chavafg, @marcov - as we continue to have problems with EulerOS due to mirror speed / availability (#46), and as it sounds like this cannot be resolved, I wonder if we should just disable it entirely for the CI. Something like:

diff --git a/tests/test_config.sh b/tests/test_config.sh
index 4e76274..7b4c8b4 100644
--- a/tests/test_config.sh
+++ b/tests/test_config.sh
@@ -6,9 +6,9 @@
 # List of distros not to test, when running all tests with test_images.sh
 typeset -a skipWhenTestingAll
 
-if [ -n "${TRAVIS:-}" ]; then
-       # (travis may timeout with euleros, see:
-       #  https://github.com/kata-containers/osbuilder/issues/46)"
+if [ -n "${CI:-}" ]; then
+       # CI tests may timeout with euleros, see:
+       #  https://github.com/kata-containers/osbuilder/issues/46"
        skipWhenTestingAll+=(euleros)
 fi

That way, it would still be used locally, but would not block our CI.

jodh-intel · 2019-03-11T16:10:17Z

/cc @jcvenegas.

chavafg · 2019-03-11T16:30:29Z

Yes, I agree with the change @jodh-intel

jcvenegas

Thanks for doing this, one comment related on the process about build it.

jcvenegas · 2019-03-11T16:51:54Z

image-builder/image_builder.sh

+# copy final image
+mv "${tmp_img}" "${IMAGE}"
+# Set metadata header
+gcc -O2 "${script_dir}/nsdax.gpl.c" -o "${script_dir}/nsdax"


Not necesary for this PR but I wonder if could create a separate makefile target for this, and would be really useful would be

make make install DESDIR

then check in this scritpt if this binary exist, it does run it.

I ask because fedora is packaging osbuilder scripts today. So I dont want to compile this at post install.

Thanks for add the issue.

marcov · 2019-03-11T17:10:32Z

@devimc, @chavafg, @marcov - as we continue to have problems with EulerOS due to mirror speed / availability (#46), and as it sounds like this cannot be resolved

agree

devimc · 2019-03-11T22:31:43Z

/test

jodh-intel · 2019-03-12T09:49:22Z

CI's are failing with:

+ docker run --rm -i --runtime kata-runtime busybox cat /proc/version
docker: Error response from daemon: OCI runtime create failed: Failed to check if grpc server is working: rpc error: code = Unavailable desc = transport is closing: unknown.

devimc · 2019-03-12T14:31:38Z

/test

devimc · 2019-03-12T15:27:12Z

CI is failing because the latest kernel is not installed, we need this change kata-containers/packaging#385 in this pr 😢

devimc · 2019-03-12T18:26:51Z

/test

devimc · 2019-03-12T21:06:54Z

depends on #243

dnf and rmp data are not needed in the final rootfs, removing them we save 2MB of disk Signed-off-by: Julio Montes <julio.montes@intel.com>

guest kernel needs 64 bytes of DRAM per 4K page of emulated PMEM, hence the image size should be as small as possible to reduce the container's memory footprint. The image size is recalculated automatically if it's too small to contain the rootfs. Signed-off-by: Julio Montes <julio.montes@intel.com>

$/${} is unnecessary on arithmetic variables. [SC2004] Signed-off-by: Julio Montes <julio.montes@intel.com>

Rootfs data must be sync'd after copying it into the image to avoid data corruption Signed-off-by: Julio Montes <julio.montes@intel.com>

gcc is required to build the binary in charge to fill out the device namespace information (matadata) into the kata containers image. Signed-off-by: Julio Montes <julio.montes@intel.com>

The new NVDIMM driver implementation (kernel >= 4.16) needs to know the device namespace information to map pages, this metadata is read from the nvdimm namespace at 4k offset. fixes kata-containers#235 Signed-off-by: Julio Montes <julio.montes@intel.com>

devimc · 2019-03-13T19:05:35Z

/test

jodh-intel

Thanks @devimc.

lgtm

osbuilder recently added the ability to create images with a DAX/NVDIMM header [1], however this change broke the data collection script. Update that script to handle images with and without this header. The data collection script will now assume a header is present. However, if it fails to find the required partition data, it will try again, this time assuming the image does not have a DAX/NVDIMM header. Fixes kata-containers#1404. [1] - kata-containers/osbuilder#236 Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

devimc force-pushed the topic/fixDAX branch from e07856c to 23a3440 Compare March 8, 2019 17:45

jodh-intel reviewed Mar 8, 2019

View reviewed changes

devimc force-pushed the topic/fixDAX branch 2 times, most recently from d4981a3 to 1737068 Compare March 8, 2019 17:59

jodh-intel mentioned this pull request Mar 11, 2019

Create a CI ignore facility kata-containers/tests#1278

Closed

devimc force-pushed the topic/fixDAX branch 2 times, most recently from 87e4b52 to 166d3d6 Compare March 11, 2019 13:40

devimc force-pushed the topic/fixDAX branch 3 times, most recently from 7ee7204 to d840a5a Compare March 11, 2019 14:40

jcvenegas mentioned this pull request Mar 11, 2019

tests: skip euleros #239

Merged

jcvenegas reviewed Mar 11, 2019

View reviewed changes

devimc force-pushed the topic/fixDAX branch from d840a5a to 0dcb8ae Compare March 11, 2019 18:02

jcvenegas approved these changes Mar 11, 2019

View reviewed changes

devimc force-pushed the topic/fixDAX branch from a31c642 to 0dcb8ae Compare March 11, 2019 21:20

devimc force-pushed the topic/fixDAX branch from 0dcb8ae to 5ff0936 Compare March 11, 2019 22:30

devimc force-pushed the topic/fixDAX branch from 2e7c822 to 5ff0936 Compare March 12, 2019 15:23

devimc mentioned this pull request Mar 12, 2019

tests: install kata components from master #242

Closed

devimc force-pushed the topic/fixDAX branch from 5ff0936 to 4520131 Compare March 12, 2019 18:26

devimc force-pushed the topic/fixDAX branch from 4520131 to f7f0c7c Compare March 12, 2019 21:06

Julio Montes added 6 commits March 13, 2019 13:05

rootfs-builder: delete dnf and rmp data

8065bb6

dnf and rmp data are not needed in the final rootfs, removing them we save 2MB of disk Signed-off-by: Julio Montes <julio.montes@intel.com>

image-builder: fix mem boundary recalculation

dc5bc07

$/${} is unnecessary on arithmetic variables. [SC2004] Signed-off-by: Julio Montes <julio.montes@intel.com>

image-builder: sync rootfs data after copying it into the image

7620066

Rootfs data must be sync'd after copying it into the image to avoid data corruption Signed-off-by: Julio Montes <julio.montes@intel.com>

image-builder: add gcc as dependecy to generate the image

cbe5642

gcc is required to build the binary in charge to fill out the device namespace information (matadata) into the kata containers image. Signed-off-by: Julio Montes <julio.montes@intel.com>

devimc force-pushed the topic/fixDAX branch from f7f0c7c to 726f798 Compare March 13, 2019 19:05

jodh-intel approved these changes Mar 14, 2019

View reviewed changes

jodh-intel merged commit ecd0724 into kata-containers:master Mar 14, 2019

jodh-intel mentioned this pull request Mar 15, 2019

dax fix makes osbuilder images unbootable #245

Closed

chavafg mentioned this pull request Mar 15, 2019

Firecracker not working kata-containers/runtime#1373

Closed

jodh-intel mentioned this pull request Mar 21, 2019

Fix data collection script for DAX header in images kata-containers/runtime#1404

Closed

jodh-intel mentioned this pull request Mar 21, 2019

Update collect script for dax nvdimm images kata-containers/runtime#1405

Merged

		@@ -0,0 +1,171 @@
		/*
		* Copyright(c) 2013-2019 Intel Corporation. All rights reserved.

Conversation

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

Uh oh!

Uh oh!

devimc commented Mar 8, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devimc commented Mar 8, 2019

Uh oh!

jodh-intel commented Mar 8, 2019

Uh oh!

jodh-intel commented Mar 8, 2019

Uh oh!

grahamwhaley commented Mar 11, 2019

Uh oh!

jodh-intel commented Mar 11, 2019

Uh oh!

devimc commented Mar 11, 2019

Uh oh!

devimc commented Mar 11, 2019

Uh oh!

jodh-intel commented Mar 11, 2019

Uh oh!

devimc commented Mar 11, 2019

Uh oh!

devimc commented Mar 11, 2019

Uh oh!

jodh-intel commented Mar 11, 2019

Uh oh!

jodh-intel commented Mar 11, 2019

Uh oh!

chavafg commented Mar 11, 2019

Uh oh!

jcvenegas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcov commented Mar 11, 2019

Uh oh!

devimc commented Mar 11, 2019

Uh oh!

jodh-intel commented Mar 12, 2019

Uh oh!

devimc commented Mar 12, 2019

Uh oh!

devimc commented Mar 12, 2019

Uh oh!

devimc commented Mar 12, 2019

Uh oh!

devimc commented Mar 12, 2019

Uh oh!

devimc commented Mar 13, 2019

Uh oh!

jodh-intel left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

devimc commented Mar 8, 2019 •

edited

Loading

devimc Mar 8, 2019 •

edited

Loading