From 5dffffd432f587bd9ac5e05bf573809ba5c5eb72 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 15 Jun 2020 20:29:33 +1000 Subject: [PATCH] qemu: Remove useless table from qemuArchBase The supportedQemuMachines array in qemuArchBase has a list of all the qemu machine types supported for the architecture, with the options for each. But, the machineType field already tells us which of the machine types we're actually using, and that's the only entry we actually care about. So, drop the table, and just have a single value with the machine type we're actually using. As a bonus that means the machine() method can no longer fail, so no longer needs an error return. Signed-off-by: David Gibson --- src/runtime/virtcontainers/qemu.go | 10 +---- src/runtime/virtcontainers/qemu_amd64.go | 23 ++++------- src/runtime/virtcontainers/qemu_amd64_test.go | 16 ++------ src/runtime/virtcontainers/qemu_arch_base.go | 35 ++++++----------- .../virtcontainers/qemu_arch_base_test.go | 39 ++++++------------- src/runtime/virtcontainers/qemu_arm64.go | 15 +++---- src/runtime/virtcontainers/qemu_arm64_test.go | 8 +--- src/runtime/virtcontainers/qemu_ppc64le.go | 17 ++++---- src/runtime/virtcontainers/qemu_s390x.go | 15 +++---- src/runtime/virtcontainers/qemu_test.go | 7 +++- 10 files changed, 64 insertions(+), 121 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 6e7922c545..31865b3b50 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -335,10 +335,7 @@ func (q *qemu) qmpSocketPath(id string) (string, error) { } func (q *qemu) getQemuMachine() (govmmQemu.Machine, error) { - machine, err := q.arch.machine() - if err != nil { - return govmmQemu.Machine{}, err - } + machine := q.arch.machine() accelerators := q.config.MachineAccelerators if accelerators != "" { @@ -1542,10 +1539,7 @@ func (q *qemu) hotplugAddCPUs(amount uint32) (uint32, error) { return 0, fmt.Errorf("failed to query hotpluggable CPUs: %v", err) } - machine, err := q.arch.machine() - if err != nil { - return 0, fmt.Errorf("failed to query machine type: %v", err) - } + machine := q.arch.machine() var hotpluggedVCPUs uint32 for _, hc := range hotpluggableVCPUs { diff --git a/src/runtime/virtcontainers/qemu_amd64.go b/src/runtime/virtcontainers/qemu_amd64.go index 28d46dd108..92c056de60 100644 --- a/src/runtime/virtcontainers/qemu_amd64.go +++ b/src/runtime/virtcontainers/qemu_amd64.go @@ -100,7 +100,6 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { factory = true } - var qemuMachines = supportedQemuMachines if config.IOMMU { var q35QemuIOMMUOptions = "accel=kvm,kernel_irqchip=split" @@ -109,22 +108,16 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { kernelParams = append(kernelParams, Param{"iommu", "pt"}) - for i, m := range qemuMachines { - if m.Type == QemuQ35 { - qemuMachines[i].Options = q35QemuIOMMUOptions - } + if mp.Type == QemuQ35 { + mp.Options = q35QemuIOMMUOptions } - } else { - kernelParams = append(kernelParams, - Param{"iommu", "off"}) } q := &qemuAmd64{ qemuArchBase: qemuArchBase{ - machineType: machineType, + qemuMachine: *mp, memoryOffset: config.MemOffset, qemuPaths: qemuPaths, - supportedQemuMachines: qemuMachines, kernelParamsNonDebug: kernelParamsNonDebug, kernelParamsDebug: kernelParamsDebug, kernelParams: kernelParams, @@ -142,9 +135,9 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { func (q *qemuAmd64) capabilities() types.Capabilities { var caps types.Capabilities - if q.machineType == QemuPC || - q.machineType == QemuQ35 || - q.machineType == QemuVirt { + if q.qemuMachine.Type == QemuPC || + q.qemuMachine.Type == QemuQ35 || + q.qemuMachine.Type == QemuVirt { caps.SetBlockDeviceHotplugSupport() } @@ -155,7 +148,7 @@ func (q *qemuAmd64) capabilities() types.Capabilities { } func (q *qemuAmd64) bridges(number uint32) { - q.Bridges = genericBridges(number, q.machineType) + q.Bridges = genericBridges(number, q.qemuMachine.Type) } func (q *qemuAmd64) cpuModel() string { @@ -187,5 +180,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) []govmmQemu.Device { - return genericAppendBridges(devices, q.Bridges, q.machineType) + return genericAppendBridges(devices, q.Bridges, q.qemuMachine.Type) } diff --git a/src/runtime/virtcontainers/qemu_amd64_test.go b/src/runtime/virtcontainers/qemu_amd64_test.go index d6e5b18159..b2d0b0cfc3 100644 --- a/src/runtime/virtcontainers/qemu_amd64_test.go +++ b/src/runtime/virtcontainers/qemu_amd64_test.go @@ -138,9 +138,7 @@ func TestQemuAmd64AppendImage(t *testing.T) { cfg.DisableImageNvdimm = false amd64, err := newQemuArch(cfg) assert.NoError(err) - for _, m := range amd64.(*qemuAmd64).supportedQemuMachines { - assert.Contains(m.Options, qemuNvdimmOption) - } + assert.Contains(amd64.machine().Options, qemuNvdimmOption) expectedOut := []govmmQemu.Device{ govmmQemu.Object{ @@ -163,9 +161,7 @@ func TestQemuAmd64AppendImage(t *testing.T) { cfg.DisableImageNvdimm = true amd64, err = newQemuArch(cfg) assert.NoError(err) - for _, m := range amd64.(*qemuAmd64).supportedQemuMachines { - assert.NotContains(m.Options, qemuNvdimmOption) - } + assert.NotContains(amd64.machine().Options, qemuNvdimmOption) found := false devices, err = amd64.appendImage(nil, f.Name()) @@ -243,9 +239,7 @@ func TestQemuAmd64WithInitrd(t *testing.T) { amd64, err := newQemuArch(cfg) assert.NoError(err) - for _, m := range amd64.(*qemuAmd64).supportedQemuMachines { - assert.NotContains(m.Options, qemuNvdimmOption) - } + assert.NotContains(amd64.machine().Options, qemuNvdimmOption) } func TestQemuAmd64Iommu(t *testing.T) { @@ -259,8 +253,6 @@ func TestQemuAmd64Iommu(t *testing.T) { p := qemu.kernelParameters(false) assert.Contains(p, Param{"intel_iommu", "on"}) - m, err := qemu.machine() - - assert.NoError(err) + m := qemu.machine() assert.Contains(m.Options, "kernel_irqchip=split") } diff --git a/src/runtime/virtcontainers/qemu_arch_base.go b/src/runtime/virtcontainers/qemu_arch_base.go index f790a36ee6..40fadc7c4c 100644 --- a/src/runtime/virtcontainers/qemu_arch_base.go +++ b/src/runtime/virtcontainers/qemu_arch_base.go @@ -38,7 +38,7 @@ type qemuArch interface { disableVhostNet() // machine returns the machine type - machine() (govmmQemu.Machine, error) + machine() govmmQemu.Machine // qemuPath returns the path to the QEMU binary qemuPath() (string, error) @@ -136,7 +136,7 @@ type qemuArch interface { } type qemuArchBase struct { - machineType string + qemuMachine govmmQemu.Machine memoryOffset uint32 nestedRun bool vhost bool @@ -144,7 +144,6 @@ type qemuArchBase struct { dax bool networkIndex int qemuPaths map[string]string - supportedQemuMachines []govmmQemu.Machine kernelParamsNonDebug []Param kernelParamsDebug []Param kernelParams []Param @@ -239,20 +238,14 @@ func (q *qemuArchBase) disableVhostNet() { q.vhost = false } -func (q *qemuArchBase) machine() (govmmQemu.Machine, error) { - for _, m := range q.supportedQemuMachines { - if m.Type == q.machineType { - return m, nil - } - } - - return govmmQemu.Machine{}, fmt.Errorf("unrecognised machine type: %v", q.machineType) +func (q *qemuArchBase) machine() govmmQemu.Machine { + return q.qemuMachine } func (q *qemuArchBase) qemuPath() (string, error) { - p, ok := q.qemuPaths[q.machineType] + p, ok := q.qemuPaths[q.qemuMachine.Type] if !ok { - return "", fmt.Errorf("Unknown machine type: %s", q.machineType) + return "", fmt.Errorf("Unknown machine type: %s", q.qemuMachine.Type) } return p, nil @@ -681,12 +674,9 @@ func (q *qemuArchBase) handleImagePath(config HypervisorConfig) { if config.ImagePath != "" { kernelRootParams := commonVirtioblkKernelRootParams if !q.disableNvdimm { - for i := range q.supportedQemuMachines { - q.supportedQemuMachines[i].Options = strings.Join([]string{ - q.supportedQemuMachines[i].Options, - qemuNvdimmOption, - }, ",") - } + q.qemuMachine.Options = strings.Join([]string{ + q.qemuMachine.Options, qemuNvdimmOption, + }, ",") if q.dax { kernelRootParams = commonNvdimmKernelRootParams } else { @@ -767,13 +757,12 @@ func (q *qemuArchBase) addBridge(b types.Bridge) { // appendPCIeRootPortDevice appends to devices the given pcie-root-port func (q *qemuArchBase) appendPCIeRootPortDevice(devices []govmmQemu.Device, number uint32) []govmmQemu.Device { - return genericAppendPCIeRootPort(devices, number, q.machineType) - + return genericAppendPCIeRootPort(devices, number, q.qemuMachine.Type) } // appendIOMMU appends a virtual IOMMU device func (q *qemuArchBase) appendIOMMU(devices []govmmQemu.Device) ([]govmmQemu.Device, error) { - switch q.machineType { + switch q.qemuMachine.Type { case QemuQ35: iommu := govmmQemu.IommuDev{ Intremap: true, @@ -784,6 +773,6 @@ func (q *qemuArchBase) appendIOMMU(devices []govmmQemu.Device) ([]govmmQemu.Devi devices = append(devices, iommu) return devices, nil default: - return devices, fmt.Errorf("Machine Type %s does not support vIOMMU", q.machineType) + return devices, fmt.Errorf("Machine Type %s does not support vIOMMU", q.qemuMachine.Type) } } diff --git a/src/runtime/virtcontainers/qemu_arch_base_test.go b/src/runtime/virtcontainers/qemu_arch_base_test.go index 8eacb085ac..417a32b2a4 100644 --- a/src/runtime/virtcontainers/qemu_arch_base_test.go +++ b/src/runtime/virtcontainers/qemu_arch_base_test.go @@ -23,12 +23,15 @@ import ( ) const ( - qemuArchBaseMachineType = "pc" qemuArchBaseQemuPath = "/usr/bin/qemu-system-x86_64" ) +var qemuArchBaseMachine = govmmQemu.Machine{ + Type: "pc", +} + var qemuArchBaseQemuPaths = map[string]string{ - qemuArchBaseMachineType: qemuArchBaseQemuPath, + qemuArchBaseMachine.Type: qemuArchBaseQemuPath, } var qemuArchBaseKernelParamsNonDebug = []Param{ @@ -47,18 +50,11 @@ var qemuArchBaseKernelParams = []Param{ {"rootfstype", "ext4"}, } -var qemuArchBaseSupportedQemuMachines = []govmmQemu.Machine{ - { - Type: qemuArchBaseMachineType, - }, -} - func newQemuArchBase() *qemuArchBase { return &qemuArchBase{ - machineType: qemuArchBaseMachineType, + qemuMachine: qemuArchBaseMachine, nestedRun: false, qemuPaths: qemuArchBaseQemuPaths, - supportedQemuMachines: qemuArchBaseSupportedQemuMachines, kernelParamsNonDebug: qemuArchBaseKernelParamsNonDebug, kernelParamsDebug: qemuArchBaseKernelParamsDebug, kernelParams: qemuArchBaseKernelParams, @@ -85,19 +81,8 @@ func TestQemuArchBaseMachine(t *testing.T) { assert := assert.New(t) qemuArchBase := newQemuArchBase() - m, err := qemuArchBase.machine() - assert.NoError(err) - assert.Equal(m.Type, qemuArchBaseMachineType) - - machines := []govmmQemu.Machine{ - { - Type: "bad", - }, - } - qemuArchBase.supportedQemuMachines = machines - m, err = qemuArchBase.machine() - assert.Error(err) - assert.Equal("", m.Type) + m := qemuArchBase.machine() + assert.Equal(m.Type, qemuArchBaseMachine.Type) } func TestQemuArchBaseQemuPath(t *testing.T) { @@ -166,7 +151,7 @@ func TestQemuAddDeviceToBridge(t *testing.T) { // addDeviceToBridge successfully q := newQemuArchBase() - q.machineType = QemuPC + q.qemuMachine.Type = QemuPC q.bridges(1) for i := uint32(1); i <= types.PCIBridgeMaxCapacity; i++ { @@ -181,7 +166,7 @@ func TestQemuAddDeviceToBridge(t *testing.T) { // addDeviceToBridge fails cause q.Bridges == 0 q = newQemuArchBase() - q.machineType = QemuPCLite + q.qemuMachine.Type = QemuPCLite q.bridges(0) _, _, err = q.addDeviceToBridge("qemu-bridge", types.PCI) if assert.Error(err) { @@ -581,11 +566,11 @@ func TestQemuArchBaseAppendIOMMU(t *testing.T) { }, } // Test IOMMU is not appended to PC machine type - qemuArchBase.machineType = QemuPC + qemuArchBase.qemuMachine.Type = QemuPC devices, err = qemuArchBase.appendIOMMU(devices) assert.Error(err) - qemuArchBase.machineType = QemuQ35 + qemuArchBase.qemuMachine.Type = QemuQ35 devices, err = qemuArchBase.appendIOMMU(devices) assert.NoError(err) assert.Equal(expectedOut, devices) diff --git a/src/runtime/virtcontainers/qemu_arm64.go b/src/runtime/virtcontainers/qemu_arm64.go index 6526c7398c..eaec0087cb 100644 --- a/src/runtime/virtcontainers/qemu_arm64.go +++ b/src/runtime/virtcontainers/qemu_arm64.go @@ -40,11 +40,9 @@ var kernelParams = []Param{ {"iommu.passthrough", "0"}, } -var supportedQemuMachines = []govmmQemu.Machine{ - { - Type: QemuVirt, - Options: defaultQemuMachineOptions, - }, +var supportedQemuMachine = govmmQemu.Machine { + Type: QemuVirt, + Options: defaultQemuMachineOptions, } // Logger returns a logrus logger appropriate for logging qemu-aarch64 messages @@ -137,10 +135,9 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { q := &qemuArm64{ qemuArchBase{ - machineType: machineType, + qemuMachine: supportedQemuMachine, memoryOffset: config.MemOffset, qemuPaths: qemuPaths, - supportedQemuMachines: supportedQemuMachines, kernelParamsNonDebug: kernelParamsNonDebug, kernelParamsDebug: kernelParamsDebug, kernelParams: kernelParams, @@ -154,12 +151,12 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { } func (q *qemuArm64) bridges(number uint32) { - q.Bridges = genericBridges(number, q.machineType) + q.Bridges = genericBridges(number, q.qemuMachine.Type) } // appendBridges appends to devices the given bridges func (q *qemuArm64) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { - return genericAppendBridges(devices, q.Bridges, q.machineType) + return genericAppendBridges(devices, q.Bridges, q.qemuMachine.Type) } func (q *qemuArm64) appendImage(devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) { diff --git a/src/runtime/virtcontainers/qemu_arm64_test.go b/src/runtime/virtcontainers/qemu_arm64_test.go index e5a36707e8..eeb6c2ed67 100644 --- a/src/runtime/virtcontainers/qemu_arm64_test.go +++ b/src/runtime/virtcontainers/qemu_arm64_test.go @@ -152,9 +152,7 @@ func TestQemuArm64AppendImage(t *testing.T) { cfg := qemuConfig(QemuVirt) cfg.ImagePath = f.Name() arm64 := newQemuArch(cfg) - for _, m := range arm64.(*qemuArm64).supportedQemuMachines { - assert.Contains(m.Options, qemuNvdimmOption) - } + assert.Contains(m.machine().Options, qemuNvdimmOption) expectedOut := []govmmQemu.Device{ govmmQemu.Object{ @@ -182,7 +180,5 @@ func TestQemuArm64WithInitrd(t *testing.T) { cfg.InitrdPath = "dummy-initrd" arm64 := newQemuArch(cfg) - for _, m := range arm64.(*qemuArm64).supportedQemuMachines { - assert.NotContains(m.Options, qemuNvdimmOption) - } + assert.NotContains(m.machine().Options, qemuNvdimmOption) } diff --git a/src/runtime/virtcontainers/qemu_ppc64le.go b/src/runtime/virtcontainers/qemu_ppc64le.go index 45cf0f7389..b6b22f0712 100644 --- a/src/runtime/virtcontainers/qemu_ppc64le.go +++ b/src/runtime/virtcontainers/qemu_ppc64le.go @@ -40,11 +40,9 @@ var kernelParams = []Param{ {"net.ifnames", "0"}, } -var supportedQemuMachines = []govmmQemu.Machine{ - { - Type: QemuPseries, - Options: defaultQemuMachineOptions, - }, +var supportedQemuMachine = govmmQemu.Machine { + Type: QemuPseries, + Options: defaultQemuMachineOptions, } // Logger returns a logrus logger appropriate for logging qemu messages @@ -69,10 +67,9 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { q := &qemuPPC64le{ qemuArchBase{ - machineType: machineType, + qemuMachine: supportedQemuMachine, memoryOffset: config.MemOffset, qemuPaths: qemuPaths, - supportedQemuMachines: supportedQemuMachines, kernelParamsNonDebug: kernelParamsNonDebug, kernelParamsDebug: kernelParamsDebug, kernelParams: kernelParams, @@ -90,7 +87,7 @@ func (q *qemuPPC64le) capabilities() types.Capabilities { var caps types.Capabilities // pseries machine type supports hotplugging drives - if q.machineType == QemuPseries { + if q.qemuMachine.Type == QemuPseries { caps.SetBlockDeviceHotplugSupport() } @@ -101,7 +98,7 @@ func (q *qemuPPC64le) capabilities() types.Capabilities { } func (q *qemuPPC64le) bridges(number uint32) { - q.Bridges = genericBridges(number, q.machineType) + q.Bridges = genericBridges(number, q.qemuMachine.Type) } func (q *qemuPPC64le) cpuModel() string { @@ -117,7 +114,7 @@ func (q *qemuPPC64le) memoryTopology(memoryMb, hostMemoryMb uint64, slots uint8) // appendBridges appends to devices the given bridges func (q *qemuPPC64le) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { - return genericAppendBridges(devices, q.Bridges, q.machineType) + return genericAppendBridges(devices, q.Bridges, q.qemuMachine.Type) } func (q *qemuPPC64le) appendIOMMU(devices []govmmQemu.Device) ([]govmmQemu.Device, error) { diff --git a/src/runtime/virtcontainers/qemu_s390x.go b/src/runtime/virtcontainers/qemu_s390x.go index d9e8c38145..4ad12a5e69 100644 --- a/src/runtime/virtcontainers/qemu_s390x.go +++ b/src/runtime/virtcontainers/qemu_s390x.go @@ -40,11 +40,9 @@ var kernelParams = []Param{ var ccwbridge = types.NewBridge(types.CCW, "", make(map[uint32]string, types.CCWBridgeMaxCapacity), 0) -var supportedQemuMachines = []govmmQemu.Machine{ - { - Type: QemuCCWVirtio, - Options: defaultQemuMachineOptions, - }, +var supportedQemuMachine = govmmQemu.Machine { + Type: QemuCCWVirtio, + Options: defaultQemuMachineOptions, } // MaxQemuVCPUs returns the maximum number of vCPUs supported @@ -67,10 +65,9 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { q := &qemuS390x{ qemuArchBase{ - machineType: machineType, + qemuMachine: supportedQemuMachine, memoryOffset: config.MemOffset, qemuPaths: qemuPaths, - supportedQemuMachines: supportedQemuMachines, kernelParamsNonDebug: kernelParamsNonDebug, kernelParamsDebug: kernelParamsDebug, kernelParams: kernelParams, @@ -89,7 +86,7 @@ func newQemuArch(config HypervisorConfig) (qemuArch, error) { } func (q *qemuS390x) bridges(number uint32) { - q.Bridges = genericBridges(number, q.machineType) + q.Bridges = genericBridges(number, q.qemuMachine.Type) } // appendConsole appends a console to devices. @@ -230,7 +227,7 @@ func (q *qemuS390x) append9PVolume(devices []govmmQemu.Device, volume types.Volu // appendBridges appends to devices the given bridges func (q *qemuS390x) appendBridges(devices []govmmQemu.Device) []govmmQemu.Device { - return genericAppendBridges(devices, q.Bridges, q.machineType) + return genericAppendBridges(devices, q.Bridges, q.qemuMachine.Type) } func (q *qemuS390x) appendSCSIController(devices []govmmQemu.Device, enableIOThreads bool) ([]govmmQemu.Device, *govmmQemu.IOThread, error) { diff --git a/src/runtime/virtcontainers/qemu_test.go b/src/runtime/virtcontainers/qemu_test.go index 08d6f859fc..f1fe9924e6 100644 --- a/src/runtime/virtcontainers/qemu_test.go +++ b/src/runtime/virtcontainers/qemu_test.go @@ -342,7 +342,10 @@ func TestQemuQemuPath(t *testing.T) { qemuConfig := newQemuConfig() qemuConfig.HypervisorPath = expectedPath qkvm := &qemuArchBase{ - machineType: "pc", + qemuMachine: govmmQemu.Machine{ + Type: "pc", + Options: "", + }, qemuPaths: map[string]string{ "pc": expectedPath, }, @@ -371,7 +374,7 @@ func TestQemuQemuPath(t *testing.T) { assert.Equal(path, expectedPath) // bad machine type, arch should fail - qkvm.machineType = "rgb" + qkvm.qemuMachine.Type = "rgb" q.arch = qkvm path, err = q.qemuPath() assert.Error(err)