Skip to content

Commit

Permalink
qemu: Remove useless table from qemuArchBase
Browse files Browse the repository at this point in the history
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 <david@gibson.dropbear.id.au>
  • Loading branch information
dgibson committed Jun 24, 2020
1 parent 97a0213 commit 5dffffd
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 121 deletions.
10 changes: 2 additions & 8 deletions src/runtime/virtcontainers/qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 8 additions & 15 deletions src/runtime/virtcontainers/qemu_amd64.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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,
Expand All @@ -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()
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
16 changes: 4 additions & 12 deletions src/runtime/virtcontainers/qemu_amd64_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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())
Expand Down Expand Up @@ -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) {
Expand All @@ -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")
}
35 changes: 12 additions & 23 deletions src/runtime/virtcontainers/qemu_arch_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -136,15 +136,14 @@ type qemuArch interface {
}

type qemuArchBase struct {
machineType string
qemuMachine govmmQemu.Machine
memoryOffset uint32
nestedRun bool
vhost bool
disableNvdimm bool
dax bool
networkIndex int
qemuPaths map[string]string
supportedQemuMachines []govmmQemu.Machine
kernelParamsNonDebug []Param
kernelParamsDebug []Param
kernelParams []Param
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
}
39 changes: 12 additions & 27 deletions src/runtime/virtcontainers/qemu_arch_base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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++ {
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 6 additions & 9 deletions src/runtime/virtcontainers/qemu_arm64.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
8 changes: 2 additions & 6 deletions src/runtime/virtcontainers/qemu_arm64_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
Loading

0 comments on commit 5dffffd

Please sign in to comment.