virtcontainers/cli: refactor code

Fixes #302

Signed-off-by: Nitesh Konkar niteshkonkar@in.ibm.com

cli/kata-check.go

@@ -7,6 +7,13 @@
 
 package main
 
+/*
+#include <linux/kvm.h>
+
+const int ioctl_KVM_CREATE_VM = KVM_CREATE_VM;
+*/
+import "C"
+
 import (
 	"fmt"
 	"os"
@@ -14,6 +21,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"syscall"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/sirupsen/logrus"
@@ -51,6 +59,11 @@ var (
 	modInfoCmd   = "modinfo"
 )
 
+// variables rather than consts to allow tests to modify them
+var (
+	kvmDevice = "/dev/kvm"
+)
+
 // getCPUInfo returns details of the first CPU read from the specified cpuinfo file
 func getCPUInfo(cpuInfoFile string) (string, error) {
 	text, err := getFileContents(cpuInfoFile)
@@ -222,9 +235,9 @@ func checkKernelModules(modules map[string]kernelModule, handler kernelParamHand
 	return count, nil
 }
 
-// hostIsVMContainerCapable checks to see if the host is theoretically capable
+// genericHostIsVMContainerCapable checks to see if the host is theoretically capable
 // of creating a VM container.
-func hostIsVMContainerCapable(details vmContainerCapableDetails) error {
+func genericHostIsVMContainerCapable(details vmContainerCapableDetails) error {
 	cpuinfo, err := getCPUInfo(details.cpuInfoFile)
 	if err != nil {
 		return err
@@ -293,3 +306,58 @@ var kataCheckCLICommand = cli.Command{
 		return nil
 	},
 }
+
+func genericArchKernelParamHandler(onVMM bool, fields logrus.Fields, msg string) bool {
+	param, ok := fields["parameter"].(string)
+	if !ok {
+		return false
+	}
+
+	// This option is not required when
+	// already running under a hypervisor.
+	if param == "unrestricted_guest" && onVMM {
+		kataLog.WithFields(fields).Warn(kernelPropertyCorrect)
+		return true
+	}
+
+	if param == "nested" {
+		kataLog.WithFields(fields).Warn(msg)
+		return true
+	}
+
+	// don't ignore the error
+	return false
+}
+
+// genericKvmIsUsable determines if it will be possible to create a full virtual machine
+// by creating a minimal VM and then deleting it.
+func genericKvmIsUsable() error {
+	flags := syscall.O_RDWR | syscall.O_CLOEXEC
+
+	f, err := syscall.Open(kvmDevice, flags, 0)
+	if err != nil {
+		return err
+	}
+	defer syscall.Close(f)
+
+	fieldLogger := kataLog.WithField("check-type", "full")
+
+	fieldLogger.WithField("device", kvmDevice).Info("device available")
+
+	vm, _, errno := syscall.Syscall(syscall.SYS_IOCTL,
+		uintptr(f),
+		uintptr(C.ioctl_KVM_CREATE_VM),
+		0)
+	if errno != 0 {
+		if errno == syscall.EBUSY {
+			fieldLogger.WithField("reason", "another hypervisor running").Error("cannot create VM")
+		}
+
+		return errno
+	}
+	defer syscall.Close(int(vm))
+
+	fieldLogger.WithField("feature", "create-vm").Info("feature available")
+
+	return nil
+}

cli/kata-check_amd64.go

@@ -5,24 +5,10 @@
 
 package main
 
-/*
-#include <linux/kvm.h>
-
-const int ioctl_KVM_CREATE_VM = KVM_CREATE_VM;
-*/
-import "C"
-
 import (
-	"syscall"
-
 	"github.com/sirupsen/logrus"
 )
 
-// variables rather than consts to allow tests to modify them
-var (
-	kvmDevice = "/dev/kvm"
-)
-
 // archRequiredCPUFlags maps a CPU flag value to search for and a
 // human-readable description of that value.
 var archRequiredCPUFlags = map[string]string{
@@ -66,58 +52,19 @@ var archRequiredKernelModules = map[string]kernelModule{
 // kvmIsUsable determines if it will be possible to create a full virtual machine
 // by creating a minimal VM and then deleting it.
 func kvmIsUsable() error {
-	flags := syscall.O_RDWR | syscall.O_CLOEXEC
+	return genericKvmIsUsable()
-
-	f, err := syscall.Open(kvmDevice, flags, 0)
-	if err != nil {
-		return err
-	}
-	defer syscall.Close(f)
-
-	fieldLogger := kataLog.WithField("check-type", "full")
-
-	fieldLogger.WithField("device", kvmDevice).Info("device available")
-
-	vm, _, errno := syscall.Syscall(syscall.SYS_IOCTL,
-		uintptr(f),
-		uintptr(C.ioctl_KVM_CREATE_VM),
-		0)
-	if errno != 0 {
-		if errno == syscall.EBUSY {
-			fieldLogger.WithField("reason", "another hypervisor running").Error("cannot create VM")
-		}
-
-		return errno
-	}
-	defer syscall.Close(int(vm))
-
-	fieldLogger.WithField("feature", "create-vm").Info("feature available")
-
-	return nil
 }
 
 func archHostCanCreateVMContainer() error {
 	return kvmIsUsable()
 }
 
+// hostIsVMContainerCapable checks to see if the host is theoretically capable
+// of creating a VM container.
+func hostIsVMContainerCapable(details vmContainerCapableDetails) error {
+	return genericHostIsVMContainerCapable(details)
+}
+
 func archKernelParamHandler(onVMM bool, fields logrus.Fields, msg string) bool {
-	param, ok := fields["parameter"].(string)
+	return genericArchKernelParamHandler(onVMM, fields, msg)
-	if !ok {
-		return false
-	}
-
-	// This option is not required when
-	// already running under a hypervisor.
-	if param == "unrestricted_guest" && onVMM {
-		kataLog.WithFields(fields).Warn(kernelPropertyCorrect)
-		return true
-	}
-
-	if param == "nested" {
-		kataLog.WithFields(fields).Warn(msg)
-		return true
-	}
-
-	// don't ignore the error
-	return false
 }

cli/kata-check_arm64.go

@@ -0,0 +1,22 @@
+// Copyright (c) 2018 Intel Corporation
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package main
+
+// kvmIsUsable determines if it will be possible to create a full virtual machine
+// by creating a minimal VM and then deleting it.
+func kvmIsUsable() error {
+	return genericKvmIsUsable()
+}
+
+func archHostCanCreateVMContainer() error {
+	return kvmIsUsable()
+}
+
+// hostIsVMContainerCapable checks to see if the host is theoretically capable
+// of creating a VM container.
+func hostIsVMContainerCapable(details vmContainerCapableDetails) error {
+	return genericHostIsVMContainerCapable(details)
+}

virtcontainers/qemu.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -926,3 +927,80 @@ func (q *qemu) addDevice(devInfo interface{}, devType deviceType) error {
 func (q *qemu) getSandboxConsole(sandboxID string) (string, error) {
 	return utils.BuildSocketPath(runStoragePath, sandboxID, defaultConsole)
 }
+
+// genericAppendBridges appends to devices the given bridges
+func genericAppendBridges(devices []govmmQemu.Device, bridges []Bridge, machineType string) []govmmQemu.Device {
+	bus := defaultPCBridgeBus
+	if machineType == QemuQ35 {
+		bus = defaultBridgeBus
+	}
+
+	for idx, b := range bridges {
+		t := govmmQemu.PCIBridge
+		if b.Type == pcieBridge {
+			t = govmmQemu.PCIEBridge
+		}
+
+		bridges[idx].Addr = bridgePCIStartAddr + idx
+
+		devices = append(devices,
+			govmmQemu.BridgeDevice{
+				Type: t,
+				Bus:  bus,
+				ID:   b.ID,
+				// Each bridge is required to be assigned a unique chassis id > 0
+				Chassis: (idx + 1),
+				SHPC:    true,
+				Addr:    strconv.FormatInt(int64(bridges[idx].Addr), 10),
+			},
+		)
+	}
+
+	return devices
+}
+
+func genericBridges(number uint32, machineType string) []Bridge {
+	var bridges []Bridge
+	var bt bridgeType
+
+	switch machineType {
+
+	case QemuQ35:
+		// currently only pci bridges are supported
+		// qemu-2.10 will introduce pcie bridges
+		fallthrough
+	case QemuPC:
+		bt = pciBridge
+	default:
+		return nil
+	}
+
+	for i := uint32(0); i < number; i++ {
+		bridges = append(bridges, Bridge{
+			Type:    bt,
+			ID:      fmt.Sprintf("%s-bridge-%d", bt, i),
+			Address: make(map[uint32]string),
+		})
+	}
+
+	return bridges
+}
+
+func genericMemoryTopology(memoryMb, hostMemoryMb uint64) govmmQemu.Memory {
+	// NVDIMM device needs memory space 1024MB
+	// See https://github.com/clearcontainers/runtime/issues/380
+	memoryOffset := 1024
+
+	// add 1G memory space for nvdimm device (vm guest image)
+	memMax := fmt.Sprintf("%dM", hostMemoryMb+uint64(memoryOffset))
+
+	mem := fmt.Sprintf("%dM", memoryMb)
+
+	memory := govmmQemu.Memory{
+		Size:   mem,
+		Slots:  defaultMemSlots,
+		MaxMem: memMax,
+	}
+
+	return memory
+}

virtcontainers/qemu_amd64.go

@@ -6,9 +6,7 @@
 package virtcontainers
 
 import (
-	"fmt"
 	"os"
-	"strconv"
 
 	govmmQemu "github.com/intel/govmm/qemu"
 )
@@ -93,12 +91,7 @@ func newQemuArch(config HypervisorConfig) qemuArch {
 		},
 	}
 
-	if config.ImagePath != "" {
+	q.handleImagePath(config)
-		q.kernelParams = append(q.kernelParams, kernelRootParams...)
-		q.kernelParamsNonDebug = append(q.kernelParamsNonDebug, kernelParamsSystemdNonDebug...)
-		q.kernelParamsDebug = append(q.kernelParamsDebug, kernelParamsSystemdDebug...)
-	}
-
 	return q
 }
 
@@ -114,29 +107,7 @@ func (q *qemuAmd64) capabilities() capabilities {
 }
 
 func (q *qemuAmd64) bridges(number uint32) []Bridge {
-	var bridges []Bridge
+	return genericBridges(number, q.machineType)
-	var bt bridgeType
-
-	switch q.machineType {
-	case QemuQ35:
-		// currently only pci bridges are supported
-		// qemu-2.10 will introduce pcie bridges
-		fallthrough
-	case QemuPC:
-		bt = pciBridge
-	default:
-		return nil
-	}
-
-	for i := uint32(0); i < number; i++ {
-		bridges = append(bridges, Bridge{
-			Type:    bt,
-			ID:      fmt.Sprintf("%s-bridge-%d", bt, i),
-			Address: make(map[uint32]string),
-		})
-	}
-
-	return bridges
 }
 
 func (q *qemuAmd64) cpuModel() string {
@@ -148,22 +119,7 @@ func (q *qemuAmd64) cpuModel() string {
 }
 
 func (q *qemuAmd64) memoryTopology(memoryMb, hostMemoryMb uint64) govmmQemu.Memory {
-	// NVDIMM device needs memory space 1024MB
+	return genericMemoryTopology(memoryMb, hostMemoryMb)
-	// See https://github.com/clearcontainers/runtime/issues/380
-	memoryOffset := 1024
-
-	// add 1G memory space for nvdimm device (vm guest image)
-	memMax := fmt.Sprintf("%dM", hostMemoryMb+uint64(memoryOffset))
-
-	mem := fmt.Sprintf("%dM", memoryMb)
-
-	memory := govmmQemu.Memory{
-		Size:   mem,
-		Slots:  defaultMemSlots,
-		MaxMem: memMax,
-	}
-
-	return memory
 }
 
 func (q *qemuAmd64) appendImage(devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) {
@@ -194,31 +150,5 @@ func (q *qemuAmd64) appendImage(devices []govmmQemu.Device, path string) ([]govm
 
 // appendBridges appends to devices the given bridges
 func (q *qemuAmd64) appendBridges(devices []govmmQemu.Device, bridges []Bridge) []govmmQemu.Device {
-	bus := defaultPCBridgeBus
+	return genericAppendBridges(devices, bridges, q.machineType)
-	if q.machineType == QemuQ35 {
-		bus = defaultBridgeBus
-	}
-
-	for idx, b := range bridges {
-		t := govmmQemu.PCIBridge
-		if b.Type == pcieBridge {
-			t = govmmQemu.PCIEBridge
-		}
-
-		bridges[idx].Addr = bridgePCIStartAddr + idx
-
-		devices = append(devices,
-			govmmQemu.BridgeDevice{
-				Type: t,
-				Bus:  bus,
-				ID:   b.ID,
-				// Each bridge is required to be assigned a unique chassis id > 0
-				Chassis: (idx + 1),
-				SHPC:    true,
-				Addr:    strconv.FormatInt(int64(bridges[idx].Addr), 10),
-			},
-		)
-	}
-
-	return devices
 }

virtcontainers/qemu_arch_base.go

@@ -82,6 +82,9 @@ type qemuArch interface {
 
 	// appendVFIODevice appends a VFIO device to devices
 	appendVFIODevice(devices []govmmQemu.Device, vfioDevice drivers.VFIODevice) []govmmQemu.Device
+
+	// handleImagePath handles the Hypervisor Config image path
+	handleImagePath(config HypervisorConfig)
 }
 
 type qemuArchBase struct {
@@ -495,3 +498,11 @@ func (q *qemuArchBase) appendVFIODevice(devices []govmmQemu.Device, vfioDevice d
 
 	return devices
 }
+
+func (q *qemuArchBase) handleImagePath(config HypervisorConfig) {
+	if config.ImagePath != "" {
+		q.kernelParams = append(q.kernelParams, kernelRootParams...)
+		q.kernelParamsNonDebug = append(q.kernelParamsNonDebug, kernelParamsSystemdNonDebug...)
+		q.kernelParamsDebug = append(q.kernelParamsDebug, kernelParamsSystemdDebug...)
+	}
+}