qemu: Detect and fail a bad machine type earlier

Currently, newQemuArch() doesn't return an error.  So, if passed an invalid
machine type, it will return a technically valid, but unusable qemuArch
object, which will probably fail with other errors shortly down the track.

Change this, to more cleanly fail the newQemuArch itself, letting us
detect a bad machine type earlier.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
David Gibson 2020-06-23 00:38:10 +10:00
parent d6e7a58ac9
commit 97a02131c6
9 changed files with 82 additions and 41 deletions

View File

@ -238,7 +238,10 @@ func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig) error {
q.id = id
q.config = *hypervisorConfig
q.arch = newQemuArch(q.config)
q.arch, err = newQemuArch(q.config)
if err != nil {
return err
}
initrdPath, err := q.config.InitrdAssetPath()
if err != nil {
@ -2172,7 +2175,10 @@ func (q *qemu) fromGrpc(ctx context.Context, hypervisorConfig *HypervisorConfig,
q.qmpMonitorCh.path = qp.QmpChannelpath
q.qemuConfig.Ctx = ctx
q.state = qp.State
q.arch = newQemuArch(q.config)
q.arch, err = newQemuArch(q.config)
if err != nil {
return err
}
q.ctx = ctx
q.nvdimmCount = qp.NvdimmCount

View File

@ -6,6 +6,7 @@
package virtcontainers
import (
"fmt"
"time"
"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types"
@ -77,12 +78,23 @@ func MaxQemuVCPUs() uint32 {
return uint32(240)
}
func newQemuArch(config HypervisorConfig) qemuArch {
func newQemuArch(config HypervisorConfig) (qemuArch, error) {
machineType := config.HypervisorMachineType
if machineType == "" {
machineType = defaultQemuMachineType
}
var mp *govmmQemu.Machine
for _, m := range supportedQemuMachines {
if m.Type == machineType {
mp = &m
break
}
}
if mp == nil {
return nil, fmt.Errorf("unrecognised machinetype: %v", machineType)
}
factory := false
if config.BootToBeTemplate || config.BootFromTemplate {
factory = true
@ -124,7 +136,7 @@ func newQemuArch(config HypervisorConfig) qemuArch {
q.handleImagePath(config)
return q
return q, nil
}
func (q *qemuAmd64) capabilities() types.Capabilities {

View File

@ -22,35 +22,36 @@ func qemuConfig(machineType string) HypervisorConfig {
}
}
func newTestQemu(machineType string) qemuArch {
func newTestQemu(assert *assert.Assertions, machineType string) qemuArch {
config := qemuConfig(machineType)
return newQemuArch(config)
arch, err := newQemuArch(config)
assert.NoError(err)
return arch
}
func TestQemuAmd64BadMachineType(t *testing.T) {
assert := assert.New(t)
amd64 := newTestQemu("no-such-machine-type")
amd64.bridges(5)
bridges := amd64.getBridges()
assert.Nil(bridges)
config := qemuConfig("no-such-machine-type")
_, err := newQemuArch(config)
assert.Error(err)
}
func TestQemuAmd64Capabilities(t *testing.T) {
assert := assert.New(t)
amd64 := newTestQemu(QemuPC)
amd64 := newTestQemu(assert, QemuPC)
caps := amd64.capabilities()
assert.True(caps.IsBlockDeviceHotplugSupported())
amd64 = newTestQemu(QemuQ35)
amd64 = newTestQemu(assert, QemuQ35)
caps = amd64.capabilities()
assert.True(caps.IsBlockDeviceHotplugSupported())
}
func TestQemuAmd64Bridges(t *testing.T) {
assert := assert.New(t)
amd64 := newTestQemu(QemuPC)
amd64 := newTestQemu(assert, QemuPC)
len := 5
amd64.bridges(uint32(len))
@ -64,7 +65,7 @@ func TestQemuAmd64Bridges(t *testing.T) {
assert.NotNil(b.Devices)
}
amd64 = newTestQemu(QemuQ35)
amd64 = newTestQemu(assert, QemuQ35)
amd64.bridges(uint32(len))
bridges = amd64.getBridges()
assert.Len(bridges, len)
@ -79,7 +80,7 @@ func TestQemuAmd64Bridges(t *testing.T) {
func TestQemuAmd64CPUModel(t *testing.T) {
assert := assert.New(t)
amd64 := newTestQemu(QemuPC)
amd64 := newTestQemu(assert, QemuPC)
expectedOut := defaultCPUModel
model := amd64.cpuModel()
@ -101,7 +102,7 @@ func TestQemuAmd64CPUModel(t *testing.T) {
func TestQemuAmd64MemoryTopology(t *testing.T) {
assert := assert.New(t)
amd64 := newTestQemu(QemuPC)
amd64 := newTestQemu(assert, QemuPC)
memoryOffset := 1024
hostMem := uint64(100)
@ -135,7 +136,8 @@ func TestQemuAmd64AppendImage(t *testing.T) {
cfg := qemuConfig(QemuPC)
cfg.ImagePath = f.Name()
cfg.DisableImageNvdimm = false
amd64 := newQemuArch(cfg)
amd64, err := newQemuArch(cfg)
assert.NoError(err)
for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
assert.Contains(m.Options, qemuNvdimmOption)
}
@ -159,7 +161,8 @@ func TestQemuAmd64AppendImage(t *testing.T) {
assert.Equal(len(supportedQemuMachines), copy(supportedQemuMachines, machinesCopy))
cfg.DisableImageNvdimm = true
amd64 = newQemuArch(cfg)
amd64, err = newQemuArch(cfg)
assert.NoError(err)
for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
assert.NotContains(m.Options, qemuNvdimmOption)
}
@ -185,7 +188,7 @@ func TestQemuAmd64AppendBridges(t *testing.T) {
assert := assert.New(t)
// check PC
amd64 := newTestQemu(QemuPC)
amd64 := newTestQemu(assert, QemuPC)
amd64.bridges(1)
bridges := amd64.getBridges()
@ -208,7 +211,7 @@ func TestQemuAmd64AppendBridges(t *testing.T) {
assert.Equal(expectedOut, devices)
// Check Q35
amd64 = newTestQemu(QemuQ35)
amd64 = newTestQemu(assert, QemuQ35)
amd64.bridges(1)
bridges = amd64.getBridges()
@ -237,7 +240,8 @@ func TestQemuAmd64WithInitrd(t *testing.T) {
cfg := qemuConfig(QemuPC)
cfg.InitrdPath = "dummy-initrd"
amd64 := newQemuArch(cfg)
amd64, err := newQemuArch(cfg)
assert.NoError(err)
for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
assert.NotContains(m.Options, qemuNvdimmOption)
@ -249,7 +253,8 @@ func TestQemuAmd64Iommu(t *testing.T) {
config := qemuConfig(QemuQ35)
config.IOMMU = true
qemu := newQemuArch(config)
qemu, err := newQemuArch(config)
assert.NoError(err)
p := qemu.kernelParameters(false)
assert.Contains(p, Param{"intel_iommu", "on"})

View File

@ -125,12 +125,16 @@ func MaxQemuVCPUs() uint32 {
return uint32(runtime.NumCPU())
}
func newQemuArch(config HypervisorConfig) qemuArch {
func newQemuArch(config HypervisorConfig) (qemuArch, error) {
machineType := config.HypervisorMachineType
if machineType == "" {
machineType = defaultQemuMachineType
}
if machineType != defaultQemuMachineType {
return nil, fmt.Errorf("unrecognised machinetype: %v", machineType)
}
q := &qemuArm64{
qemuArchBase{
machineType: machineType,
@ -146,7 +150,7 @@ func newQemuArch(config HypervisorConfig) qemuArch {
q.handleImagePath(config)
return q
return q, nil
}
func (q *qemuArm64) bridges(number uint32) {

View File

@ -23,14 +23,16 @@ func qemuConfig(machineType string) HypervisorConfig {
}
}
func newTestQemu(machineType string) qemuArch {
func newTestQemu(assert *assert.Assertions, machineType string) qemuArch {
config := qemuConfig(machineType)
return newQemuArch(config)
arch, err := newQemuArch(config)
assert.NoError(err)
return arch
}
func TestQemuArm64CPUModel(t *testing.T) {
assert := assert.New(t)
arm64 := newTestQemu(QemuVirt)
arm64 := newTestQemu(assert, QemuVirt)
expectedOut := defaultCPUModel
model := arm64.cpuModel()
@ -39,7 +41,7 @@ func TestQemuArm64CPUModel(t *testing.T) {
func TestQemuArm64MemoryTopology(t *testing.T) {
assert := assert.New(t)
arm64 := newTestQemu(QemuVirt)
arm64 := newTestQemu(assert, QemuVirt)
hostMem := uint64(4096)
mem := uint64(1024)
@ -107,7 +109,7 @@ func TestQemuArm64AppendBridges(t *testing.T) {
var devices []govmmQemu.Device
assert := assert.New(t)
arm64 := newTestQemu(QemuVirt)
arm64 := newTestQemu(assert, QemuVirt)
arm64.bridges(1)
bridges := arm64.getBridges()

View File

@ -57,12 +57,16 @@ func MaxQemuVCPUs() uint32 {
return uint32(128)
}
func newQemuArch(config HypervisorConfig) qemuArch {
func newQemuArch(config HypervisorConfig) (qemuArch, error) {
machineType := config.HypervisorMachineType
if machineType == "" {
machineType = defaultQemuMachineType
}
if machineType != defaultQemuMachineType {
return nil, fmt.Errorf("unrecognised machinetype: %v", machineType)
}
q := &qemuPPC64le{
qemuArchBase{
machineType: machineType,
@ -79,7 +83,7 @@ func newQemuArch(config HypervisorConfig) qemuArch {
q.memoryOffset = config.MemOffset
return q
return q, nil
}
func (q *qemuPPC64le) capabilities() types.Capabilities {

View File

@ -13,16 +13,18 @@ import (
"github.com/stretchr/testify/assert"
)
func newTestQemu(machineType string) qemuArch {
func newTestQemu(assert *assert.Assertions, machineType string) qemuArch {
config := HypervisorConfig{
HypervisorMachineType: machineType,
}
return newQemuArch(config)
arch, err := newQemuArch(config)
assert.NoError(err)
return arch
}
func TestQemuPPC64leCPUModel(t *testing.T) {
assert := assert.New(t)
ppc64le := newTestQemu(QemuPseries)
ppc64le := newTestQemu(assert, QemuPseries)
expectedOut := defaultCPUModel
model := ppc64le.cpuModel()
@ -31,7 +33,7 @@ func TestQemuPPC64leCPUModel(t *testing.T) {
func TestQemuPPC64leMemoryTopology(t *testing.T) {
assert := assert.New(t)
ppc64le := newTestQemu(QemuPseries)
ppc64le := newTestQemu(assert, QemuPseries)
memoryOffset := 1024
hostMem := uint64(1024)

View File

@ -55,12 +55,16 @@ func MaxQemuVCPUs() uint32 {
return uint32(248)
}
func newQemuArch(config HypervisorConfig) qemuArch {
func newQemuArch(config HypervisorConfig) (qemuArch, error) {
machineType := config.HypervisorMachineType
if machineType == "" {
machineType = defaultQemuMachineType
}
if machineType != defaultQemuMachineType {
return nil, fmt.Errorf("unrecognised machinetype: %v", machineType)
}
q := &qemuS390x{
qemuArchBase{
machineType: machineType,
@ -81,7 +85,7 @@ func newQemuArch(config HypervisorConfig) qemuArch {
q.kernelParamsDebug = append(q.kernelParamsDebug, kernelParamsSystemdDebug...)
}
return q
return q, nil
}
func (q *qemuS390x) bridges(number uint32) {

View File

@ -14,16 +14,18 @@ import (
"github.com/stretchr/testify/assert"
)
func newTestQemu(machineType string) qemuArch {
func newTestQemu(assert *assert.Assertions, machineType string) qemuArch {
config := HypervisorConfig{
HypervisorMachineType: machineType,
}
return newQemuArch(config)
arch, err := newQemuArch(config)
assert.NoError(err)
return arch
}
func TestQemuS390xCPUModel(t *testing.T) {
assert := assert.New(t)
s390x := newTestQemu(QemuCCWVirtio)
s390x := newTestQemu(assert, QemuCCWVirtio)
expectedOut := defaultCPUModel
model := s390x.cpuModel()
@ -37,7 +39,7 @@ func TestQemuS390xCPUModel(t *testing.T) {
func TestQemuS390xMemoryTopology(t *testing.T) {
assert := assert.New(t)
s390x := newTestQemu(QemuCCWVirtio)
s390x := newTestQemu(assert, QemuCCWVirtio)
hostMem := uint64(1024)
mem := uint64(120)