From 6277418f1a10be30969df5986f627f152b273c1a Mon Sep 17 00:00:00 2001 From: Wei Li Date: Wed, 23 Jan 2019 15:22:30 +0800 Subject: [PATCH 01/16] factory: do not destroy vm factory when checking status Fixes: #1163 Signed-off-by: Wei Li --- cli/factory.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/factory.go b/cli/factory.go index 3ed658aac0..cab0680106 100644 --- a/cli/factory.go +++ b/cli/factory.go @@ -133,11 +133,10 @@ var statusFactoryCommand = cli.Command{ }, } kataLog.WithField("factory", factoryConfig).Info("load vm factory") - f, err := vf.NewFactory(ctx, factoryConfig, true) + _, err := vf.NewFactory(ctx, factoryConfig, true) if err != nil { fmt.Fprintln(defaultOutputFile, "vm factory is off") } else { - f.CloseFactory(ctx) fmt.Fprintln(defaultOutputFile, "vm factory is on") } } else { From 41b90931df1dd9db210189ce70daa947e89ee88c Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Wed, 23 Jan 2019 00:27:55 -0600 Subject: [PATCH 02/16] makefile: honor DESDIR on install - Do symlink to a relative path to hypervisor config. - Create symlink on DESTDIR Fixes: #1161 Signed-off-by: Jose Carlos Venegas Munoz --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 1c11f748d6..2a084df1e8 100644 --- a/Makefile +++ b/Makefile @@ -248,11 +248,11 @@ ifeq (,$(findstring $(DEFAULT_HYPERVISOR),$(KNOWN_HYPERVISORS))) endif ifeq ($(DEFAULT_HYPERVISOR),$(HYPERVISOR_QEMU)) - DEFAULT_HYPERVISOR_CONFIG_PATH = $(CONFIG_PATH_QEMU) + DEFAULT_HYPERVISOR_CONFIG = $(CONFIG_FILE_QEMU) endif ifeq ($(DEFAULT_HYPERVISOR),$(HYPERVISOR_FC)) - DEFAULT_HYPERVISOR_CONFIG_PATH = $(CONFIG_PATH_FC) + DEFAULT_HYPERVISOR_CONFIG = $(CONFIG_FILE_FC) endif CONFDIR := $(DEFAULTSDIR)/$(PROJECT_DIR) @@ -546,7 +546,7 @@ install-bin-libexec: $(BINLIBEXECLIST) install-configs: $(CONFIGS) $(QUIET_INST)$(foreach f,$(CONFIGS),$(call INSTALL_CONFIG,$f,$(dir $(CONFIG_PATH)))) - $(QUIET_INST)ln -sf $(DEFAULT_HYPERVISOR_CONFIG_PATH) $(CONFIG_PATH) + $(QUIET_INST)ln -sf $(DEFAULT_HYPERVISOR_CONFIG) $(DESTDIR)/$(CONFIG_PATH) install-scripts: $(SCRIPTS) $(QUIET_INST)$(foreach f,$(SCRIPTS),$(call INSTALL_EXEC,$f,$(SCRIPTS_DIR))) From 6c9325df50499064669e026442053847d752ca6d Mon Sep 17 00:00:00 2001 From: Julio Montes Date: Mon, 28 Jan 2019 08:29:29 -0600 Subject: [PATCH 03/16] cli: set config options before showing config paths Config paths are set correctly but they must be set before handleShowConfig fixes #1185 Signed-off-by: Julio Montes --- cli/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/main.go b/cli/main.go index a01a02dc59..a021a0a775 100644 --- a/cli/main.go +++ b/cli/main.go @@ -250,6 +250,8 @@ func beforeSubcommands(c *cli.Context) error { var runtimeConfig oci.RuntimeConfig var err error + katautils.SetConfigOptions(name, defaultRuntimeConfiguration, defaultSysConfRuntimeConfiguration) + handleShowConfig(c) if userWantsUsage(c) || (c.NArg() == 1 && (c.Args()[0] == checkCmd)) { @@ -302,8 +304,6 @@ func beforeSubcommands(c *cli.Context) error { ignoreLogging = true } - katautils.SetConfigOptions(name, defaultRuntimeConfiguration, defaultSysConfRuntimeConfiguration) - configFile, runtimeConfig, err = katautils.LoadConfiguration(c.GlobalString(configFilePathOption), ignoreLogging, false) if err != nil { fatal(err) From 70c27acbcc063b34d1ea41192d735f4a4c9f43b1 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 30 Jan 2019 11:23:25 +0000 Subject: [PATCH 04/16] ci: Add a CODEOWNERS file for github ack checks Add a CODEOWNERS file so we can get github to automatically request reviews. In this instance, specifically the docs team for markdown documents. Fixes: #1192 Signed-off-by: Graham Whaley --- CODEOWNERS | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 CODEOWNERS diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000000..e48c094af2 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,13 @@ +# Copyright 2019 Intel Corporation. +# +# SPDX-License-Identifier: Apache-2.0 +# +# Define any code owners for this repository. +# The code owners lists are used to help automatically enforce +# reviews and acks of the right groups on the right PRs. + +# Order in this file is important. Only the last match will be +# used. See https://help.github.com/articles/about-code-owners/ + +*.md @kata-containers/documentation + From 09eb0122cd4a06aea71c15b0048dacb45ed68112 Mon Sep 17 00:00:00 2001 From: William Douglas Date: Thu, 24 Jan 2019 09:20:21 -0800 Subject: [PATCH 05/16] Makefile: Set arch regardless of GOPATH state Architecture-dependent settings were not being populated when GOPATH was set. This change ensures they are always set. Fixes #1169 Signed-off-by: William Douglas --- Makefile | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 2a084df1e8..6d5bd6c1c3 100644 --- a/Makefile +++ b/Makefile @@ -23,22 +23,20 @@ ifeq ($(SKIP_GO_VERSION_CHECK),) include golang.mk endif -ifneq ($(GOPATH),) - GOARCH=$(shell go env GOARCH) - ifeq ($(ARCH),) - ARCH = $(GOARCH) - endif - - ARCH_DIR = arch - ARCH_FILE_SUFFIX = -options.mk - ARCH_FILE = $(ARCH_DIR)/$(ARCH)$(ARCH_FILE_SUFFIX) - ARCH_FILES = $(wildcard arch/*$(ARCH_FILE_SUFFIX)) - ALL_ARCHES = $(patsubst $(ARCH_DIR)/%$(ARCH_FILE_SUFFIX),%,$(ARCH_FILES)) - - # Load architecture-dependent settings - include $(ARCH_FILE) +GOARCH=$(shell go env GOARCH) +ifeq ($(ARCH),) + ARCH = $(GOARCH) endif +ARCH_DIR = arch +ARCH_FILE_SUFFIX = -options.mk +ARCH_FILE = $(ARCH_DIR)/$(ARCH)$(ARCH_FILE_SUFFIX) +ARCH_FILES = $(wildcard arch/*$(ARCH_FILE_SUFFIX)) +ALL_ARCHES = $(patsubst $(ARCH_DIR)/%$(ARCH_FILE_SUFFIX),%,$(ARCH_FILES)) + +# Load architecture-dependent settings +include $(ARCH_FILE) + PROJECT_TYPE = kata PROJECT_NAME = Kata Containers PROJECT_TAG = kata-containers From 1cff8cbb0f41c17a11583d779aa657d5876b3677 Mon Sep 17 00:00:00 2001 From: Graham Whaley Date: Wed, 6 Feb 2019 15:50:31 +0000 Subject: [PATCH 06/16] pullapprove: remove it We are moving off pullapprove. Remove its config file. Fixes: #1215 Signed-off-by: Graham Whaley --- .pullapprove.yml | 43 ------------------------------------------- 1 file changed, 43 deletions(-) delete mode 100644 .pullapprove.yml diff --git a/.pullapprove.yml b/.pullapprove.yml deleted file mode 100644 index a1c1e959cb..0000000000 --- a/.pullapprove.yml +++ /dev/null @@ -1,43 +0,0 @@ -version: 2 - -requirements: - signed_off_by: - required: true - -# Disallow approval of PRs still under development -always_pending: - title_regex: '(WIP|RFC)' - labels: - - do-not-merge - - wip - - rfc - explanation: 'Work in progress - do not merge' - -group_defaults: - approve_by_comment: - enabled: true - approve_regex: '^(LGTM|lgtm|Approved|\+1|:\+1:)' - reject_regex: '^(Rejected|-1|:-1:)' - reset_on_push: - enabled: false - reset_on_reopened: - enabled: false - author_approval: - ignored: true - -groups: - approvers: - required: 2 - teams: - - runtime - - documentation: - required: 1 - teams: - - documentation - conditions: - files: - include: - - "*.md" - exclude: - - "vendor/*" From 847d940ba85fcbe56567189f938fb60fd23e764d Mon Sep 17 00:00:00 2001 From: Hui Zhu Date: Mon, 11 Feb 2019 16:00:50 +0800 Subject: [PATCH 07/16] Makefile: Set ARCH in GOPATH not set mode In GOPATH not set mode got: make: go: Command not found Makefile:38: arch/-options.mk: No such file or directory make: go: Command not found Makefile:237: *** "ERROR: No hypervisors known for architecture (looked for: firecracker qemu)". Stop. The root cause is GOPATH not set mode is not set ARCH. Set it to fix the issue. Fixes: #1224 Signed-off-by: Hui Zhu --- Makefile | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6d5bd6c1c3..51e5fd0f97 100644 --- a/Makefile +++ b/Makefile @@ -23,9 +23,17 @@ ifeq ($(SKIP_GO_VERSION_CHECK),) include golang.mk endif -GOARCH=$(shell go env GOARCH) -ifeq ($(ARCH),) - ARCH = $(GOARCH) +#Get ARCH. +ifneq ($(GOPATH),) + GOARCH=$(shell go env GOARCH) + ifeq ($(ARCH),) + ARCH = $(GOARCH) + endif +else + ARCH = $(shell uname -m) + ifeq ($(ARCH),x86_64) + ARCH = amd64 + endif endif ARCH_DIR = arch From c4eb1801e956ce501831faf8ca76a16ebc2007dc Mon Sep 17 00:00:00 2001 From: Jose Carlos Venegas Munoz Date: Thu, 7 Feb 2019 16:00:12 +0000 Subject: [PATCH 08/16] delete: force: Do not fail on non exiting container When a container does not exist, runc does not fail. Lets mimic this behavior, sometimes kuberentes will try to force delete containers that could not be created and gets confused if delete --force fails. Fixes: #1219 Signed-off-by: Jose Carlos Venegas Munoz --- cli/delete.go | 4 ++++ cli/delete_test.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/cli/delete.go b/cli/delete.go index cb74539027..2db13821e8 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -71,6 +71,10 @@ func delete(ctx context.Context, containerID string, force bool) error { // Checks the MUST and MUST NOT from OCI runtime specification status, sandboxID, err := getExistingContainerInfo(ctx, containerID) if err != nil { + if force { + kataLog.Warnf("Failed to get container, force will not fail: %s", err) + return nil + } return err } diff --git a/cli/delete_test.go b/cli/delete_test.go index e3cde76f71..51fc553bb9 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -65,6 +65,10 @@ func TestDeleteInvalidContainer(t *testing.T) { err = delete(context.Background(), testContainerID, false) assert.Error(err) assert.False(vcmock.IsMockError(err)) + + // Force to delete missing container + err = delete(context.Background(), "non-existing-test", true) + assert.NoError(err) } func TestDeleteMissingContainerTypeAnnotation(t *testing.T) { From 3d7a9b0141f0e3381585a8f5f43a40cf97117642 Mon Sep 17 00:00:00 2001 From: Nitesh Konkar Date: Wed, 20 Feb 2019 16:00:26 +0530 Subject: [PATCH 09/16] qemu: Cleanup Vm paths irrespective of Sandbox stop pass/fail Sometimes qemu/qmp commands error out and VM files get left behind on the host filesystem. Clen them up irrespective of `stopSandbox` succeeds or fails. Fixes: #1246 Signed-off-by: Nitesh Konkar niteshkonkar@in.ibm.com --- virtcontainers/qemu.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index e4d4ff78a8..38836e55ac 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -641,6 +641,7 @@ func (q *qemu) stopSandbox() error { span, _ := q.trace("stopSandbox") defer span.Finish() + defer q.cleanupVM() q.Logger().Info("Stopping Sandbox") err := q.qmpSetup() @@ -654,6 +655,11 @@ func (q *qemu) stopSandbox() error { return err } + return nil +} + +func (q *qemu) cleanupVM() error { + // cleanup vm path dir := filepath.Join(RunVMStoragePath, q.id) From efe96e852e678ea0589ec18e7592f482f112d9a6 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 20 Feb 2019 14:55:12 +0000 Subject: [PATCH 10/16] devices: fix attach count for vhost-user-blk Commit affd6e3216cbcfe797c617f61138bda1a5e928b2 ("devices: add reference count for devices.") introduced an attach count for devices. The vhost-user-blk device increments the counter instead of decrementing it when detaching. Fixes: #1259 Signed-off-by: Stefan Hajnoczi --- virtcontainers/device/drivers/vhost_user_blk.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtcontainers/device/drivers/vhost_user_blk.go b/virtcontainers/device/drivers/vhost_user_blk.go index cfc53d4145..a613abda82 100644 --- a/virtcontainers/device/drivers/vhost_user_blk.go +++ b/virtcontainers/device/drivers/vhost_user_blk.go @@ -56,7 +56,7 @@ func (device *VhostUserBlkDevice) Attach(devReceiver api.DeviceReceiver) (err er // Detach is standard interface of api.Device, it's used to remove device from some // DeviceReceiver func (device *VhostUserBlkDevice) Detach(devReceiver api.DeviceReceiver) error { - skip, err := device.bumpAttachCount(true) + skip, err := device.bumpAttachCount(false) if err != nil { return err } From e2a637321f78a672c2bb2bc78147686d56fe6ac7 Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 31 Jan 2019 11:17:31 +0800 Subject: [PATCH 11/16] unit-test: struct TestDataa should be included in arch-indenpedent .go file argument struct TestDataa in generic func genericTestGetCPUDetails is repeatedly defined in almost all arch-dependent .go file, cli/kata-check_amd64_test.go, cli/kata-check_ppc64le_test.go, etcm, except arm64. let's only declare it once in cli/kata-check_test.go. change its name to testCPUDetail for better understanding. Fixes: #1200 Signed-off-by: Penny Zheng --- cli/kata-check_amd64_test.go | 9 +-------- cli/kata-check_ppc64le_test.go | 9 +-------- cli/kata-check_s390x_test.go | 16 +--------------- cli/kata-check_test.go | 9 ++++++++- 4 files changed, 11 insertions(+), 32 deletions(-) diff --git a/cli/kata-check_amd64_test.go b/cli/kata-check_amd64_test.go index 69e49083ab..8cb06167fb 100644 --- a/cli/kata-check_amd64_test.go +++ b/cli/kata-check_amd64_test.go @@ -484,13 +484,6 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { - contents string - expectedVendor string - expectedModel string - expectError bool -} - func TestGetCPUDetails(t *testing.T) { const validVendorName = "a vendor" validVendor := fmt.Sprintf(`%s : %s`, archCPUVendorField, validVendorName) @@ -505,7 +498,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []testCPUDetail{ {"", "", "", true}, {"invalid", "", "", true}, {archCPUVendorField, "", "", true}, diff --git a/cli/kata-check_ppc64le_test.go b/cli/kata-check_ppc64le_test.go index 6c1bd12996..e0267d768f 100644 --- a/cli/kata-check_ppc64le_test.go +++ b/cli/kata-check_ppc64le_test.go @@ -208,13 +208,6 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { - contents string - expectedVendor string - expectedModel string - expectError bool -} - func TestGetCPUDetails(t *testing.T) { const validVendorName = "" @@ -230,7 +223,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []testCPUDetail{ {"", "", "", true}, {"invalid", "", "", true}, {archCPUVendorField, "", "", true}, diff --git a/cli/kata-check_s390x_test.go b/cli/kata-check_s390x_test.go index 47b1b72291..d04b44a650 100644 --- a/cli/kata-check_s390x_test.go +++ b/cli/kata-check_s390x_test.go @@ -207,21 +207,7 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { - contents string - expectedVendor string - expectedModel string - expectError bool -} - func TestGetCPUDetails(t *testing.T) { - type testData struct { - contents string - expectedVendor string - expectedModel string - expectError bool - } - const validVendorName = "a vendor" validVendor := fmt.Sprintf(`%s : %s`, archCPUVendorField, validVendorName) @@ -235,7 +221,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []testCPUDetail{ {"", "", "", true}, {"invalid", "", "", true}, {archCPUVendorField, "", "", true}, diff --git a/cli/kata-check_test.go b/cli/kata-check_test.go index 441fb28f66..7c2cde4d59 100644 --- a/cli/kata-check_test.go +++ b/cli/kata-check_test.go @@ -33,6 +33,13 @@ type testCPUData struct { expectError bool } +type testCPUDetail struct { + contents string + expectedVendor string + expectedModel string + expectError bool +} + func createFile(file, contents string) error { return ioutil.WriteFile(file, []byte(contents), testFileMode) } @@ -138,7 +145,7 @@ func makeCPUInfoFile(path, vendorID, flags string) error { return ioutil.WriteFile(path, contents.Bytes(), testFileMode) } -func genericTestGetCPUDetails(t *testing.T, validVendor string, validModel string, validContents string, data []TestDataa) { +func genericTestGetCPUDetails(t *testing.T, validVendor string, validModel string, validContents string, data []testCPUDetail) { tmpdir, err := ioutil.TempDir("", "") if err != nil { panic(err) From b074797cbc597d24bc374da2441f2edf36f2f4fe Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 31 Jan 2019 14:50:32 +0800 Subject: [PATCH 12/16] unit-test: refine qemu_arm64_test.go refine a set of test functions under qemu_arm64_test.go. e.g. test func for memoryTopology shouldn't be the same one on amd64, since for now, we don't support nvdimm on arm64. Fixes: #1200 Signed-off-by: Penny Zheng --- virtcontainers/qemu_arm64_test.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go index 30f3568010..ceb0699ad7 100644 --- a/virtcontainers/qemu_arm64_test.go +++ b/virtcontainers/qemu_arm64_test.go @@ -26,32 +26,27 @@ func newTestQemu(machineType string) qemuArch { func TestQemuArm64CPUModel(t *testing.T) { assert := assert.New(t) - arm64 := newTestQemu(virt) + arm64 := newTestQemu(QemuVirt) expectedOut := defaultCPUModel model := arm64.cpuModel() assert.Equal(expectedOut, model) - - arm64.enableNestingChecks() - expectedOut = defaultCPUModel + ",pmu=off" - model = arm64.cpuModel() - assert.Equal(expectedOut, model) } func TestQemuArm64MemoryTopology(t *testing.T) { assert := assert.New(t) - arm64 := newTestQemu(virt) - memoryOffset := 1024 + arm64 := newTestQemu(QemuVirt) - hostMem := uint64(1024) - mem := uint64(120) + hostMem := uint64(4096) + mem := uint64(1024) + slots := uint8(3) expectedMemory := govmmQemu.Memory{ Size: fmt.Sprintf("%dM", mem), - Slots: defaultMemSlots, - MaxMem: fmt.Sprintf("%dM", hostMem+uint64(memoryOffset)), + Slots: slots, + MaxMem: fmt.Sprintf("%dM", hostMem), } - m := arm64.memoryTopology(mem, hostMem) + m := arm64.memoryTopology(mem, hostMem, slots) assert.Equal(expectedMemory, m) } From ce2925255665ecef865e1bbb6c3e06e007239a98 Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 31 Jan 2019 16:05:50 +0800 Subject: [PATCH 13/16] unit-test: test func for RunningOnVMM should be arch-dependent original tests for func RunningOnVMM are sort of amd64-specific, since all other archs don't support nested VMM for now. Fixes: #1200 Signed-off-by: Penny Zheng --- virtcontainers/hypervisor_amd64_test.go | 84 +++++++++++++++++++ virtcontainers/hypervisor_arm64_test.go | 30 +++++++ virtcontainers/hypervisor_test.go | 104 +++++++----------------- 3 files changed, 144 insertions(+), 74 deletions(-) create mode 100644 virtcontainers/hypervisor_amd64_test.go create mode 100644 virtcontainers/hypervisor_arm64_test.go diff --git a/virtcontainers/hypervisor_amd64_test.go b/virtcontainers/hypervisor_amd64_test.go new file mode 100644 index 0000000000..36c509c805 --- /dev/null +++ b/virtcontainers/hypervisor_amd64_test.go @@ -0,0 +1,84 @@ +// Copyright (c) 2019 ARM Limited +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "io/ioutil" + "os" + "testing" +) + +var dataFlagsFieldWithoutHypervisor = []byte(` +fpu_exception : yes +cpuid level : 20 +wp : yes +flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology eagerfpu pni pclmulqdq vmx ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch tpr_shadow vnmi ept vpid fsgsbase bmi1 hle avx2 smep bmi2 erms rtm rdseed adx smap xsaveopt +bugs : +bogomips : 4589.35 +`) + +var dataFlagsFieldWithHypervisor = []byte(` +fpu_exception : yes +cpuid level : 20 +wp : yes +flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology eagerfpu pni pclmulqdq vmx ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch tpr_shadow vnmi ept vpid fsgsbase bmi1 hle avx2 smep bmi2 erms rtm rdseed adx smap xsaveopt +bugs : +bogomips : 4589.35 +`) + +var dataWithoutFlagsField = []byte(` +fpu_exception : yes +cpuid level : 20 +wp : yes +bugs : +bogomips : 4589.35 +`) + +func TestRunningOnVMM(t *testing.T) { + var data []testNestedVMMData + + //file cpuinfo doesn't contain 'hypervisor' flag + dataNestedVMMFalseSuccessful := testNestedVMMData{ + content: dataFlagsFieldWithoutHypervisor, + expectedErr: false, + expected: false, + } + data = append(data, dataNestedVMMFalseSuccessful) + + //file cpuinfo contains 'hypervisor' flag + dataNestedVMMTrueSuccessful := testNestedVMMData{ + content: dataFlagsFieldWithHypervisor, + expectedErr: false, + expected: true, + } + data = append(data, dataNestedVMMTrueSuccessful) + + //file cpuinfo doesn't contain field flags + dataNestedVMMWithoutFlagsField := testNestedVMMData{ + content: dataWithoutFlagsField, + expectedErr: true, + expected: false, + } + data = append(data, dataNestedVMMWithoutFlagsField) + + genericTestRunningOnVMM(t, data) +} + +func TestRunningOnVMMNotExistingCPUInfoPathFailure(t *testing.T) { + f, err := ioutil.TempFile("", "cpuinfo") + if err != nil { + t.Fatal(err) + } + + filePath := f.Name() + + f.Close() + os.Remove(filePath) + + if _, err := RunningOnVMM(filePath); err == nil { + t.Fatalf("Should fail because %q file path does not exist", filePath) + } +} diff --git a/virtcontainers/hypervisor_arm64_test.go b/virtcontainers/hypervisor_arm64_test.go new file mode 100644 index 0000000000..954cf163ba --- /dev/null +++ b/virtcontainers/hypervisor_arm64_test.go @@ -0,0 +1,30 @@ +// Copyright (c) 2019 ARM Limited +// +// SPDX-License-Identifier: Apache-2.0 +// + +package virtcontainers + +import ( + "io/ioutil" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRunningOnVMM(t *testing.T) { + assert := assert.New(t) + expectedOutput := false + + f, err := ioutil.TempFile("", "cpuinfo") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + defer f.Close() + + running, err := RunningOnVMM(f.Name()) + assert.NoError(err) + assert.Equal(expectedOutput, running) +} diff --git a/virtcontainers/hypervisor_test.go b/virtcontainers/hypervisor_test.go index 0a1b2514f4..1a012b9bab 100644 --- a/virtcontainers/hypervisor_test.go +++ b/virtcontainers/hypervisor_test.go @@ -437,84 +437,40 @@ func TestGetHostMemorySizeKb(t *testing.T) { } } -var dataFlagsFieldWithoutHypervisor = []byte(` -fpu_exception : yes -cpuid level : 20 -wp : yes -flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology eagerfpu pni pclmulqdq vmx ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch tpr_shadow vnmi ept vpid fsgsbase bmi1 hle avx2 smep bmi2 erms rtm rdseed adx smap xsaveopt -bugs : -bogomips : 4589.35 -`) - -var dataFlagsFieldWithHypervisor = []byte(` -fpu_exception : yes -cpuid level : 20 -wp : yes -flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl xtopology eagerfpu pni pclmulqdq vmx ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch tpr_shadow vnmi ept vpid fsgsbase bmi1 hle avx2 smep bmi2 erms rtm rdseed adx smap xsaveopt -bugs : -bogomips : 4589.35 -`) - -var dataWithoutFlagsField = []byte(` -fpu_exception : yes -cpuid level : 20 -wp : yes -bugs : -bogomips : 4589.35 -`) - -func testRunningOnVMMSuccessful(t *testing.T, cpuInfoContent []byte, expectedErr bool, expected bool) { - f, err := ioutil.TempFile("", "cpuinfo") - if err != nil { - t.Fatal(err) - } - defer os.Remove(f.Name()) - defer f.Close() - - n, err := f.Write(cpuInfoContent) - if err != nil { - t.Fatal(err) - } - if n != len(cpuInfoContent) { - t.Fatalf("Only %d bytes written out of %d expected", n, len(cpuInfoContent)) - } - - running, err := RunningOnVMM(f.Name()) - if !expectedErr && err != nil { - t.Fatalf("This test should succeed: %v", err) - } else if expectedErr && err == nil { - t.Fatalf("This test should fail") - } - - if running != expected { - t.Fatalf("Expecting running on VMM = %t, Got %t", expected, running) - } +// nolint: unused +type testNestedVMMData struct { + content []byte + expectedErr bool + expected bool } -func TestRunningOnVMMFalseSuccessful(t *testing.T) { - testRunningOnVMMSuccessful(t, dataFlagsFieldWithoutHypervisor, false, false) -} - -func TestRunningOnVMMTrueSuccessful(t *testing.T) { - testRunningOnVMMSuccessful(t, dataFlagsFieldWithHypervisor, false, true) -} - -func TestRunningOnVMMNoFlagsFieldFailure(t *testing.T) { - testRunningOnVMMSuccessful(t, dataWithoutFlagsField, true, false) -} - -func TestRunningOnVMMNotExistingCPUInfoPathFailure(t *testing.T) { - f, err := ioutil.TempFile("", "cpuinfo") - if err != nil { - t.Fatal(err) - } +// nolint: unused +func genericTestRunningOnVMM(t *testing.T, data []testNestedVMMData) { + for _, d := range data { + f, err := ioutil.TempFile("", "cpuinfo") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + defer f.Close() - filePath := f.Name() + n, err := f.Write(d.content) + if err != nil { + t.Fatal(err) + } + if n != len(d.content) { + t.Fatalf("Only %d bytes written out of %d expected", n, len(d.content)) + } - f.Close() - os.Remove(filePath) + running, err := RunningOnVMM(f.Name()) + if !d.expectedErr && err != nil { + t.Fatalf("This test should succeed: %v", err) + } else if d.expectedErr && err == nil { + t.Fatalf("This test should fail") + } - if _, err := RunningOnVMM(filePath); err == nil { - t.Fatalf("Should fail because %q file path does not exist", filePath) + if running != d.expected { + t.Fatalf("Expecting running on VMM = %t, Got %t", d.expected, running) + } } } From e1c544533d8bad45d5d2dd8f09d5ee54e4338894 Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Wed, 13 Feb 2019 12:37:07 +0800 Subject: [PATCH 14/16] runtime: add appendBridges for arm64 since generic func genericAppendBridges and genericBridges is also applied for machine type QemuVirt, we use it as implementation for appendBridges and bridges on aarch64. since const defaultPCBridgeBus is used in generic func genericAppendBridges for pc machine, we should define it once in generic file, instead of redefining it in different arch-specific files. Fixes: #1200 Signed-off-by: Penny Zheng --- virtcontainers/qemu_amd64.go | 2 -- virtcontainers/qemu_arch_base.go | 13 +++++++------ virtcontainers/qemu_arm64.go | 13 ++++++++++--- virtcontainers/qemu_arm64_test.go | 27 +++++++++++++++++++++++++++ virtcontainers/qemu_ppc64le.go | 2 -- virtcontainers/qemu_s390x.go | 2 -- 6 files changed, 44 insertions(+), 15 deletions(-) diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go index 65d9838e44..7a34270876 100644 --- a/virtcontainers/qemu_amd64.go +++ b/virtcontainers/qemu_amd64.go @@ -24,8 +24,6 @@ const defaultQemuMachineType = QemuPC const defaultQemuMachineOptions = "accel=kvm,kernel_irqchip,nvdimm" -const defaultPCBridgeBus = "pci.0" - var qemuPaths = map[string]string{ QemuPCLite: "/usr/bin/qemu-lite-system-x86_64", QemuPC: defaultQemuPath, diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 5e68bfd292..7b6bdc05bc 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -116,12 +116,13 @@ type qemuArchBase struct { } const ( - defaultCores uint32 = 1 - defaultThreads uint32 = 1 - defaultCPUModel = "host" - defaultBridgeBus = "pcie.0" - maxDevIDSize = 31 - defaultMsize9p = 8192 + defaultCores uint32 = 1 + defaultThreads uint32 = 1 + defaultCPUModel = "host" + defaultBridgeBus = "pcie.0" + defaultPCBridgeBus = "pci.0" + maxDevIDSize = 31 + defaultMsize9p = 8192 ) // This is the PCI start address assigned to the first bridge that diff --git a/virtcontainers/qemu_arm64.go b/virtcontainers/qemu_arm64.go index bc72b0fa79..49c0923090 100644 --- a/virtcontainers/qemu_arm64.go +++ b/virtcontainers/qemu_arm64.go @@ -11,6 +11,7 @@ import ( "strings" govmmQemu "github.com/intel/govmm/qemu" + "github.com/kata-containers/runtime/virtcontainers/types" "github.com/sirupsen/logrus" ) @@ -25,9 +26,6 @@ const defaultQemuMachineType = QemuVirt var defaultQemuMachineOptions = "usb=off,accel=kvm,gic-version=" + getGuestGICVersion() -// Not used -const defaultPCBridgeBus = "" - var qemuPaths = map[string]string{ QemuVirt: defaultQemuPath, } @@ -153,3 +151,12 @@ func newQemuArch(config HypervisorConfig) qemuArch { return q } + +func (q *qemuArm64) bridges(number uint32) []types.PCIBridge { + return genericBridges(number, q.machineType) +} + +// appendBridges appends to devices the given bridges +func (q *qemuArm64) appendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge) []govmmQemu.Device { + return genericAppendBridges(devices, bridges, q.machineType) +} diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go index ceb0699ad7..a9add75a63 100644 --- a/virtcontainers/qemu_arm64_test.go +++ b/virtcontainers/qemu_arm64_test.go @@ -98,3 +98,30 @@ func TestMaxQemuVCPUs(t *testing.T) { assert.Equal(d.expectedResult, vCPUs) } } + +func TestQemuArm64AppendBridges(t *testing.T) { + var devices []govmmQemu.Device + assert := assert.New(t) + + arm64 := newTestQemu(QemuVirt) + + bridges := arm64.bridges(1) + assert.Len(bridges, 1) + + devices = []govmmQemu.Device{} + devices = arm64.appendBridges(devices, bridges) + assert.Len(devices, 1) + + expectedOut := []govmmQemu.Device{ + govmmQemu.BridgeDevice{ + Type: govmmQemu.PCIEBridge, + Bus: defaultBridgeBus, + ID: bridges[0].ID, + Chassis: 1, + SHPC: true, + Addr: "2", + }, + } + + assert.Equal(expectedOut, devices) +} diff --git a/virtcontainers/qemu_ppc64le.go b/virtcontainers/qemu_ppc64le.go index d986e87963..cc77f683b2 100644 --- a/virtcontainers/qemu_ppc64le.go +++ b/virtcontainers/qemu_ppc64le.go @@ -27,8 +27,6 @@ const defaultQemuMachineType = QemuPseries const defaultQemuMachineOptions = "accel=kvm,usb=off" -const defaultPCBridgeBus = "pci.0" - const defaultMemMaxPPC64le = 32256 // Restrict MemMax to 32Gb on PPC64le var qemuPaths = map[string]string{ diff --git a/virtcontainers/qemu_s390x.go b/virtcontainers/qemu_s390x.go index 1ac865cbfa..e3dc14b573 100644 --- a/virtcontainers/qemu_s390x.go +++ b/virtcontainers/qemu_s390x.go @@ -23,8 +23,6 @@ const defaultQemuMachineType = QemuCCWVirtio const defaultQemuMachineOptions = "accel=kvm" -const defaultPCBridgeBus = "pci.0" - const VirtioSerialCCW = "virtio-serial-ccw" var qemuPaths = map[string]string{ From e770e6a4e7e9b313631ded5876998c64cb260ee3 Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Wed, 13 Feb 2019 13:20:01 +0800 Subject: [PATCH 15/16] unit-test: add nolint comment to avoid unused warning since all generic* could bring unused linter warnings, which lead to CI crash, we add nolint comment to avoid them. Fixes: #1200 Signed-off-by: Samuel Ortiz Signed-off-by: Penny Zheng --- cli/kata-check.go | 6 +++--- cli/kata-check_arm64.go | 10 +++++----- cli/kata-check_test.go | 3 +++ cli/kata-env_test.go | 1 + virtcontainers/qemu.go | 3 +++ 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/cli/kata-check.go b/cli/kata-check.go index d47328867c..22c615fcf7 100644 --- a/cli/kata-check.go +++ b/cli/kata-check.go @@ -52,9 +52,9 @@ const ( kernelPropertyCorrect = "Kernel property value correct" // these refer to fields in the procCPUINFO file - genericCPUFlagsTag = "flags" - genericCPUVendorField = "vendor_id" - genericCPUModelField = "model name" + genericCPUFlagsTag = "flags" // nolint: varcheck, unused + genericCPUVendorField = "vendor_id" // nolint: varcheck, unused + genericCPUModelField = "model name" // nolint: varcheck, unused ) // variables rather than consts to allow tests to modify them diff --git a/cli/kata-check_arm64.go b/cli/kata-check_arm64.go index 615e5aeaf4..0b116ad65b 100644 --- a/cli/kata-check_arm64.go +++ b/cli/kata-check_arm64.go @@ -121,12 +121,12 @@ func normalizeArmModel(model string) string { return model } -func getCPUDetails() (vendor, model string, err error) { - if vendor, model, err := genericGetCPUDetails(); err == nil { +func getCPUDetails() (string, string, error) { + vendor, model, err := genericGetCPUDetails() + if err == nil { vendor = normalizeArmVendor(vendor) model = normalizeArmModel(model) - return vendor, model, err - } else { - return vendor, model, err } + + return vendor, model, err } diff --git a/cli/kata-check_test.go b/cli/kata-check_test.go index 7c2cde4d59..6fcf3aa230 100644 --- a/cli/kata-check_test.go +++ b/cli/kata-check_test.go @@ -27,12 +27,14 @@ type testModuleData struct { contents string } +// nolint: structcheck, unused type testCPUData struct { vendorID string flags string expectError bool } +// nolint: structcheck, unused type testCPUDetail struct { contents string expectedVendor string @@ -145,6 +147,7 @@ func makeCPUInfoFile(path, vendorID, flags string) error { return ioutil.WriteFile(path, contents.Bytes(), testFileMode) } +// nolint: unused func genericTestGetCPUDetails(t *testing.T, validVendor string, validModel string, validContents string, data []testCPUDetail) { tmpdir, err := ioutil.TempDir("", "") if err != nil { diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 966b818bc6..7cb8071b6c 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -244,6 +244,7 @@ func getExpectedAgentDetails(config oci.RuntimeConfig) (AgentInfo, error) { }, nil } +// nolint: unused func genericGetExpectedHostDetails(tmpdir string, expectedVendor string, expectedModel string) (HostInfo, error) { type filesToCreate struct { file string diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 38836e55ac..8b06e89963 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1400,6 +1400,7 @@ func (q *qemu) resizeMemory(reqMemMB uint32, memoryBlockSizeMB uint32) (uint32, } // genericAppendBridges appends to devices the given bridges +// nolint: unused func genericAppendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge, machineType string) []govmmQemu.Device { bus := defaultPCBridgeBus switch machineType { @@ -1431,6 +1432,7 @@ func genericAppendBridges(devices []govmmQemu.Device, bridges []types.PCIBridge, return devices } +// nolint: unused func genericBridges(number uint32, machineType string) []types.PCIBridge { var bridges []types.PCIBridge var bt types.PCIType @@ -1463,6 +1465,7 @@ func genericBridges(number uint32, machineType string) []types.PCIBridge { return bridges } +// nolint: unused func genericMemoryTopology(memoryMb, hostMemoryMb uint64, slots uint8, memoryOffset uint32) govmmQemu.Memory { // image NVDIMM device needs memory space 1024MB // See https://github.com/clearcontainers/runtime/issues/380 From b7588dc67c13bafbc8fce3afb483380cc6f95bfe Mon Sep 17 00:00:00 2001 From: Penny Zheng Date: Thu, 14 Feb 2019 11:09:54 +0800 Subject: [PATCH 16/16] unit-test: refine func TestGetCPUDetails refine struct testData in func TestGetCPUDetails to remove redundant /unused struct field expectedVendor and expectedModel Fixes: #1200 Signed-off-by: Penny Zheng --- cli/kata-check_arm64_test.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/cli/kata-check_arm64_test.go b/cli/kata-check_arm64_test.go index 05c04043ae..433c0996fd 100644 --- a/cli/kata-check_arm64_test.go +++ b/cli/kata-check_arm64_test.go @@ -129,19 +129,17 @@ func TestKvmIsUsable(t *testing.T) { func TestGetCPUDetails(t *testing.T) { type testData struct { contents string - expectedVendor string - expectedModel string expectedNormalizeVendor string expectedNormalizeModel string expectError bool } - const validVendorName = "0x41" - const validNormalizeVendorName = "ARM Limited" + validVendorName := "0x41" + validNormalizeVendorName := "ARM Limited" validVendor := fmt.Sprintf(`%s : %s`, archCPUVendorField, validVendorName) - const validModelName = "8" - const validNormalizeModelName = "v8" + validModelName := "8" + validNormalizeModelName := "v8" validModel := fmt.Sprintf(`%s : %s`, archCPUModelField, validModelName) validContents := fmt.Sprintf(` @@ -152,12 +150,12 @@ foo : bar `, validVendor, validModel) data := []testData{ - {"", "", "", "", "", true}, - {"invalid", "", "", "", "", true}, - {archCPUVendorField, "", "", "", "", true}, - {validVendor, "", "", "", "", true}, - {validModel, "", "", "", "", true}, - {validContents, validVendorName, validModelName, validNormalizeVendorName, validNormalizeModelName, false}, + {"", "", "", true}, + {"invalid", "", "", true}, + {archCPUVendorField, "", "", true}, + {validVendor, "", "", true}, + {validModel, "", "", true}, + {validContents, validNormalizeVendorName, validNormalizeModelName, false}, } tmpdir, err := ioutil.TempDir("", "")