From fe78babd857f4c5c219c11ed2e30e401512feeda Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 7 Feb 2019 12:01:02 +0100 Subject: [PATCH 1/6] cli: Simplify the ARM check The getCPUDetails() routine can be simplified. Fixes: #1217 Signed-off-by: Samuel Ortiz --- cli/kata-check_arm64.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 } From 87698a2c624e0ff74d6d83220dd57d1988987b9f Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 7 Feb 2019 12:07:16 +0100 Subject: [PATCH 2/6] cli: Export the TestDataa structure This is no longer an internal structure. The architecture agnostic genericTestGetCPUDetails() routine relies on it, and ARM64 is not exporting it. Fixes: #1217 Signed-off-by: Samuel Ortiz --- cli/kata-check_arm64_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/kata-check_arm64_test.go b/cli/kata-check_arm64_test.go index 05c04043ae..9d554f08d1 100644 --- a/cli/kata-check_arm64_test.go +++ b/cli/kata-check_arm64_test.go @@ -126,16 +126,16 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -func TestGetCPUDetails(t *testing.T) { - type testData struct { - contents string - expectedVendor string - expectedModel string - expectedNormalizeVendor string - expectedNormalizeModel string - expectError bool - } +type TestDataa struct { + contents string + expectedVendor string + expectedModel string + expectedNormalizeVendor string + expectedNormalizeModel string + expectError bool +} +func TestGetCPUDetails(t *testing.T) { const validVendorName = "0x41" const validNormalizeVendorName = "ARM Limited" validVendor := fmt.Sprintf(`%s : %s`, archCPUVendorField, validVendorName) @@ -151,7 +151,7 @@ foo : bar %s `, validVendor, validModel) - data := []testData{ + data := []TestDataa{ {"", "", "", "", "", true}, {"invalid", "", "", "", "", true}, {archCPUVendorField, "", "", "", "", true}, From d9ad4658401320e186c5d1d329780cd5b83dcedb Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 7 Feb 2019 12:10:29 +0100 Subject: [PATCH 3/6] cli: Rename TestDataa to TestData Dataa (double a) is probably a typo. Signed-off-by: Samuel Ortiz --- cli/kata-check_amd64_test.go | 4 ++-- cli/kata-check_arm64_test.go | 4 ++-- cli/kata-check_ppc64le_test.go | 4 ++-- cli/kata-check_s390x_test.go | 4 ++-- cli/kata-check_test.go | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cli/kata-check_amd64_test.go b/cli/kata-check_amd64_test.go index 69e49083ab..d1786a3ec8 100644 --- a/cli/kata-check_amd64_test.go +++ b/cli/kata-check_amd64_test.go @@ -484,7 +484,7 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { +type TestData struct { contents string expectedVendor string expectedModel string @@ -505,7 +505,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []TestData{ {"", "", "", true}, {"invalid", "", "", true}, {archCPUVendorField, "", "", true}, diff --git a/cli/kata-check_arm64_test.go b/cli/kata-check_arm64_test.go index 9d554f08d1..1e8ec5fb8c 100644 --- a/cli/kata-check_arm64_test.go +++ b/cli/kata-check_arm64_test.go @@ -126,7 +126,7 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { +type TestData struct { contents string expectedVendor string expectedModel string @@ -151,7 +151,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []TestData{ {"", "", "", "", "", true}, {"invalid", "", "", "", "", true}, {archCPUVendorField, "", "", "", "", true}, diff --git a/cli/kata-check_ppc64le_test.go b/cli/kata-check_ppc64le_test.go index 6c1bd12996..6d748fee6d 100644 --- a/cli/kata-check_ppc64le_test.go +++ b/cli/kata-check_ppc64le_test.go @@ -208,7 +208,7 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { +type TestData struct { contents string expectedVendor string expectedModel string @@ -230,7 +230,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []TestData{ {"", "", "", true}, {"invalid", "", "", true}, {archCPUVendorField, "", "", true}, diff --git a/cli/kata-check_s390x_test.go b/cli/kata-check_s390x_test.go index 47b1b72291..3dc108e308 100644 --- a/cli/kata-check_s390x_test.go +++ b/cli/kata-check_s390x_test.go @@ -207,7 +207,7 @@ func TestKvmIsUsable(t *testing.T) { assert.Error(err) } -type TestDataa struct { +type TestData struct { contents string expectedVendor string expectedModel string @@ -235,7 +235,7 @@ foo : bar %s `, validVendor, validModel) - data := []TestDataa{ + data := []TestData{ {"", "", "", true}, {"invalid", "", "", true}, {archCPUVendorField, "", "", true}, diff --git a/cli/kata-check_test.go b/cli/kata-check_test.go index 441fb28f66..5442bfbe48 100644 --- a/cli/kata-check_test.go +++ b/cli/kata-check_test.go @@ -138,7 +138,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 []TestData) { tmpdir, err := ioutil.TempDir("", "") if err != nil { panic(err) From 941c8404f312601d64c4b6eaa6db9fb0d5bb3572 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 7 Feb 2019 12:17:14 +0100 Subject: [PATCH 4/6] virtcontainers: Fix ARM64 QEMU unit tests Fixes: #1217 Signed-off-by: Samuel Ortiz --- virtcontainers/qemu_arm64_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go index 30f3568010..38769c37b1 100644 --- a/virtcontainers/qemu_arm64_test.go +++ b/virtcontainers/qemu_arm64_test.go @@ -26,7 +26,7 @@ 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() @@ -40,18 +40,19 @@ func TestQemuArm64CPUModel(t *testing.T) { func TestQemuArm64MemoryTopology(t *testing.T) { assert := assert.New(t) - arm64 := newTestQemu(virt) + arm64 := newTestQemu(QemuVirt) memoryOffset := 1024 hostMem := uint64(1024) mem := uint64(120) + slots := uint8(10) expectedMemory := govmmQemu.Memory{ Size: fmt.Sprintf("%dM", mem), - Slots: defaultMemSlots, + Slots: slots, MaxMem: fmt.Sprintf("%dM", hostMem+uint64(memoryOffset)), } - m := arm64.memoryTopology(mem, hostMem) + m := arm64.memoryTopology(mem, hostMem, slots) assert.Equal(expectedMemory, m) } From 9a04ccb52411b910d10d2fbf8129045f829dbbd3 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 7 Feb 2019 17:24:21 +0100 Subject: [PATCH 5/6] cli: Improve the ARM64 checks And use the shared flags and definitions. Fixes: #1217 Signed-off-by: Samuel Ortiz --- cli/kata-check_arm64_test.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/cli/kata-check_arm64_test.go b/cli/kata-check_arm64_test.go index 1e8ec5fb8c..2c23406bf3 100644 --- a/cli/kata-check_arm64_test.go +++ b/cli/kata-check_arm64_test.go @@ -18,13 +18,27 @@ import ( "github.com/urfave/cli" ) -func setupCheckHostIsVMContainerCapable(assert *assert.Assertions, cpuInfoFile string, moduleData []testModuleData) { - //For now, Arm64 only deal with module check +func setupCheckHostIsVMContainerCapable(assert *assert.Assertions, cpuInfoFile string, cpuData []testCPUData, moduleData []testModuleData) { createModules(assert, cpuInfoFile, moduleData) - err := makeCPUInfoFile(cpuInfoFile, "", "") - assert.NoError(err) + for _, d := range cpuData { + err := makeCPUInfoFile(cpuInfoFile, d.vendorID, d.flags) + assert.NoError(err) + + details := vmContainerCapableDetails{ + cpuInfoFile: cpuInfoFile, + requiredCPUFlags: archRequiredCPUFlags, + requiredCPUAttribs: archRequiredCPUAttribs, + requiredKernelModules: archRequiredKernelModules, + } + err = hostIsVMContainerCapable(details) + if d.expectError { + assert.Error(err) + } else { + assert.NoError(err) + } + } } func TestCCCheckCLIFunction(t *testing.T) { @@ -55,6 +69,10 @@ func TestCCCheckCLIFunction(t *testing.T) { t.Fatal(err) } + cpuData := []testCPUData{ + {"", "", false}, + } + moduleData := []testModuleData{ {filepath.Join(sysModuleDir, "kvm"), true, ""}, {filepath.Join(sysModuleDir, "vhost"), true, ""}, @@ -74,7 +92,7 @@ func TestCCCheckCLIFunction(t *testing.T) { kataLog.Logger.Out = savedLogOutput }() - setupCheckHostIsVMContainerCapable(assert, cpuInfoFile, moduleData) + setupCheckHostIsVMContainerCapable(assert, cpuInfoFile, cpuData, moduleData) ctx := createCLIContext(nil) ctx.App.Name = "foo" From 47633248f4ee8174c08a7fe72e2daea3a9904516 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 7 Feb 2019 17:33:35 +0100 Subject: [PATCH 6/6] virtcontainers: Tag unused functions and routines for ARM64 ARM64 does not need all QEMU generic routines. Fixes: #1217 Signed-off-by: Samuel Ortiz --- cli/kata-check.go | 6 +++--- cli/kata-check_test.go | 1 + cli/kata-env_test.go | 1 + virtcontainers/qemu.go | 3 +++ virtcontainers/qemu_arm64.go | 2 +- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cli/kata-check.go b/cli/kata-check.go index d47328867c..a97469f21a 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_test.go b/cli/kata-check_test.go index 5442bfbe48..b8536bcb2e 100644 --- a/cli/kata-check_test.go +++ b/cli/kata-check_test.go @@ -138,6 +138,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 []TestData) { tmpdir, err := ioutil.TempDir("", "") if err != nil { diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index a3b6a5970a..4efdf51c65 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 487f91ba67..581ce2eb67 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1406,6 +1406,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 { @@ -1437,6 +1438,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 @@ -1469,6 +1471,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 diff --git a/virtcontainers/qemu_arm64.go b/virtcontainers/qemu_arm64.go index bc72b0fa79..cd383de33f 100644 --- a/virtcontainers/qemu_arm64.go +++ b/virtcontainers/qemu_arm64.go @@ -26,7 +26,7 @@ const defaultQemuMachineType = QemuVirt var defaultQemuMachineOptions = "usb=off,accel=kvm,gic-version=" + getGuestGICVersion() // Not used -const defaultPCBridgeBus = "" +const defaultPCBridgeBus = "" // nolint: unused var qemuPaths = map[string]string{ QemuVirt: defaultQemuPath,