QEMU: do not require nvdimm machine option with initrd

Do not add the "nvdimm" machine option to QEMU when the config specifies a initrd file. For arm64, this allows using a vanilla QEMU, where "virt" machine does not support the "nvdimm" option. Fixes: #2088 Signed-off-by: Marco Vedovati <[email protected]>
kata-containers · Sep 25, 2019 · 8b843c5 · 8b843c5
1 parent d3f480d
commit 8b843c5
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 10 deletions.
diff --git a/virtcontainers/qemu_amd64.go b/virtcontainers/qemu_amd64.go
@@ -7,6 +7,7 @@ package virtcontainers
 
 import (
 	"os"
+	"strings"
 	"time"
 
 	"github.com/kata-containers/runtime/virtcontainers/types"
@@ -25,7 +26,9 @@ const defaultQemuPath = "/usr/bin/qemu-system-x86_64"
 
 const defaultQemuMachineType = QemuPC
 
-const defaultQemuMachineOptions = "accel=kvm,kernel_irqchip,nvdimm"
+const qemuNvdimmOption = "nvdimm"
+
+const defaultQemuMachineOptions = "accel=kvm,kernel_irqchip"
 
 const qmpMigrationWaitTimeout = 5 * time.Second
 
@@ -103,6 +106,15 @@ func newQemuArch(config HypervisorConfig) qemuArch {
 		vmFactory: factory,
 	}
 
+	if config.ImagePath != "" {
+		for i := range q.supportedQemuMachines {
+			q.supportedQemuMachines[i].Options = strings.Join([]string{
+				q.supportedQemuMachines[i].Options,
+				qemuNvdimmOption,
+			}, ",")
+		}
+	}
+
 	q.handleImagePath(config)
 
 	return q

diff --git a/virtcontainers/qemu_amd64_test.go b/virtcontainers/qemu_amd64_test.go
@@ -16,10 +16,14 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func newTestQemu(machineType string) qemuArch {
-	config := HypervisorConfig{
+func qemuConfig(machineType string) HypervisorConfig {
+	return HypervisorConfig{
 		HypervisorMachineType: machineType,
 	}
+}
+
+func newTestQemu(machineType string) qemuArch {
+	config := qemuConfig(machineType)
 	return newQemuArch(config)
 }
 
@@ -112,7 +116,6 @@ func TestQemuAmd64MemoryTopology(t *testing.T) {
 func TestQemuAmd64AppendImage(t *testing.T) {
 	var devices []govmmQemu.Device
 	assert := assert.New(t)
-	amd64 := newTestQemu(QemuPC)
 
 	f, err := ioutil.TempFile("", "img")
 	assert.NoError(err)
@@ -122,6 +125,17 @@ func TestQemuAmd64AppendImage(t *testing.T) {
 	imageStat, err := f.Stat()
 	assert.NoError(err)
 
+	// save default supportedQemuMachines options
+	machinesCopy := make([]govmmQemu.Machine, len(supportedQemuMachines))
+	assert.Equal(len(supportedQemuMachines), copy(machinesCopy, supportedQemuMachines))
+
+	cfg := qemuConfig(QemuPC)
+	cfg.ImagePath = f.Name()
+	amd64 := newQemuArch(cfg)
+	for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
+		assert.Contains(m.Options, qemuNvdimmOption)
+	}
+
 	expectedOut := []govmmQemu.Device{
 		govmmQemu.Object{
 			Driver:   govmmQemu.NVDIMM,
@@ -135,8 +149,10 @@ func TestQemuAmd64AppendImage(t *testing.T) {
 
 	devices, err = amd64.appendImage(devices, f.Name())
 	assert.NoError(err)
-
 	assert.Equal(expectedOut, devices)
+
+	// restore default supportedQemuMachines options
+	assert.Equal(len(supportedQemuMachines), copy(supportedQemuMachines, machinesCopy))
 }
 
 func TestQemuAmd64AppendBridges(t *testing.T) {
@@ -190,3 +206,15 @@ func TestQemuAmd64AppendBridges(t *testing.T) {
 
 	assert.Equal(expectedOut, devices)
 }
+
+func TestQemuAmd64WithInitrd(t *testing.T) {
+	assert := assert.New(t)
+
+	cfg := qemuConfig(QemuPC)
+	cfg.InitrdPath = "dummy-initrd"
+	amd64 := newQemuArch(cfg)
+
+	for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
+		assert.NotContains(m.Options, qemuNvdimmOption)
+	}
+}
diff --git a/virtcontainers/qemu_arm64.go b/virtcontainers/qemu_arm64.go
@@ -26,9 +26,11 @@ const defaultQemuPath = "/usr/bin/qemu-system-aarch64"
 
 const defaultQemuMachineType = QemuVirt
 
+const qemuNvdimmOption = "nvdimm"
+
 const qmpMigrationWaitTimeout = 10 * time.Second
 
-var defaultQemuMachineOptions = "usb=off,accel=kvm,nvdimm,gic-version=" + getGuestGICVersion()
+var defaultQemuMachineOptions = "usb=off,accel=kvm,gic-version=" + getGuestGICVersion()
 
 var qemuPaths = map[string]string{
 	QemuVirt: defaultQemuPath,
@@ -153,6 +155,12 @@ func newQemuArch(config HypervisorConfig) qemuArch {
 	}
 
 	if config.ImagePath != "" {
+		for i := range q.supportedQemuMachines {
+			q.supportedQemuMachines[i].Options = strings.Join([]string{
+				q.supportedQemuMachines[i].Options,
+				qemuNvdimmOption,
+			}, ",")
+		}
 		q.kernelParams = append(q.kernelParams, kernelRootParams...)
 		q.kernelParamsNonDebug = append(q.kernelParamsNonDebug, kernelParamsSystemdNonDebug...)
 		q.kernelParamsDebug = append(q.kernelParamsDebug, kernelParamsSystemdDebug...)

diff --git a/virtcontainers/qemu_arm64_test.go b/virtcontainers/qemu_arm64_test.go
@@ -17,10 +17,14 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func newTestQemu(machineType string) qemuArch {
-	config := HypervisorConfig{
+func qemuConfig(machineType string) HypervisorConfig {
+	return HypervisorConfig{
 		HypervisorMachineType: machineType,
 	}
+}
+
+func newTestQemu(machineType string) qemuArch {
+	config := qemuConfig(machineType)
 	return newQemuArch(config)
 }
 
@@ -130,7 +134,6 @@ func TestQemuArm64AppendBridges(t *testing.T) {
 func TestQemuArm64AppendImage(t *testing.T) {
 	var devices []govmmQemu.Device
 	assert := assert.New(t)
-	arm64 := newTestQemu(QemuVirt)
 
 	f, err := ioutil.TempFile("", "img")
 	assert.NoError(err)
@@ -140,6 +143,17 @@ func TestQemuArm64AppendImage(t *testing.T) {
 	imageStat, err := f.Stat()
 	assert.NoError(err)
 
+	// save default supportedQemuMachines options
+	machinesCopy := make([]govmmQemu.Machine, len(supportedQemuMachines))
+	assert.Equal(len(supportedQemuMachines), copy(machinesCopy, supportedQemuMachines))
+
+	cfg := qemuConfig(QemuVirt)
+	cfg.ImagePath = f.Name()
+	arm64 := newQemuArch(cfg)
+	for _, m := range arm64.(*qemuArm64).supportedQemuMachines {
+		assert.Contains(m.Options, qemuNvdimmOption)
+	}
+
 	expectedOut := []govmmQemu.Device{
 		govmmQemu.Object{
 			Driver:   govmmQemu.NVDIMM,
@@ -153,6 +167,20 @@ func TestQemuArm64AppendImage(t *testing.T) {
 
 	devices, err = arm64.appendImage(devices, f.Name())
 	assert.NoError(err)
-
 	assert.Equal(expectedOut, devices)
+
+	// restore default supportedQemuMachines options
+	assert.Equal(len(supportedQemuMachines), copy(supportedQemuMachines, machinesCopy))
+}
+
+func TestQemuArm64WithInitrd(t *testing.T) {
+	assert := assert.New(t)
+
+	cfg := qemuConfig(QemuVirt)
+	cfg.InitrdPath = "dummy-initrd"
+	arm64 := newQemuArch(cfg)
+
+	for _, m := range arm64.(*qemuArm64).supportedQemuMachines {
+		assert.NotContains(m.Options, qemuNvdimmOption)
+	}
 }