diff --git a/cli/kata-check.go b/cli/kata-check.go index d47328867..22c615fcf 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_amd64_test.go b/cli/kata-check_amd64_test.go index 69e49083a..8cb06167f 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_arm64.go b/cli/kata-check_arm64.go index 615e5aeaf..0b116ad65 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_arm64_test.go b/cli/kata-check_arm64_test.go index 05c04043a..433c0996f 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("", "") diff --git a/cli/kata-check_ppc64le_test.go b/cli/kata-check_ppc64le_test.go index 6c1bd1299..e0267d768 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 47b1b7229..d04b44a65 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 441fb28f6..6fcf3aa23 100644 --- a/cli/kata-check_test.go +++ b/cli/kata-check_test.go @@ -27,12 +27,21 @@ 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 + expectedModel string + expectError bool +} + func createFile(file, contents string) error { return ioutil.WriteFile(file, []byte(contents), testFileMode) } @@ -138,7 +147,8 @@ 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) { +// nolint: unused +func genericTestGetCPUDetails(t *testing.T, validVendor string, validModel string, validContents string, data []testCPUDetail) { tmpdir, err := ioutil.TempDir("", "") if err != nil { panic(err) diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index a3b6a5970..4efdf51c6 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/hypervisor_amd64_test.go b/virtcontainers/hypervisor_amd64_test.go new file mode 100644 index 000000000..36c509c80 --- /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 000000000..954cf163b --- /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 0a1b2514f..1a012b9ba 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) -} +// 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() -func TestRunningOnVMMTrueSuccessful(t *testing.T) { - testRunningOnVMMSuccessful(t, dataFlagsFieldWithHypervisor, false, true) -} + 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)) + } -func TestRunningOnVMMNoFlagsFieldFailure(t *testing.T) { - testRunningOnVMMSuccessful(t, dataWithoutFlagsField, true, false) -} + 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") + } -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) + if running != d.expected { + t.Fatalf("Expecting running on VMM = %t, Got %t", d.expected, running) + } } } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 222010838..ba51584fd 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -1407,6 +1407,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 { @@ -1438,6 +1439,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 @@ -1470,6 +1472,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_amd64.go b/virtcontainers/qemu_amd64.go index 65d9838e4..7a3427087 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 5e68bfd29..7b6bdc05b 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 bc72b0fa7..49c092309 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 30f356801..a9add75a6 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) } @@ -103,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 d986e8796..cc77f683b 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 1ac865cbf..e3dc14b57 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{