Merge pull request kata-containers#2373 from bergwolf/image

qemu: add disable_image_nvdimm option

cli/config/configuration-qemu-virtiofs.toml.in

@@ -205,6 +205,11 @@ enable_iothreads = @DEFENABLEIOTHREADS@
 # Default false
 #use_vsock = true
 
+# If false and nvdimm is supported, use nvdimm device to plug guest image.
+# Otherwise virtio-block device is used.
+# Default false
+#disable_image_nvdimm = true
+
 # VFIO devices are hotplugged on a bridge by default.
 # Enable hotplugging on root bus. This may be required for devices with
 # a large PCI bar, as this is a current limitation with hotplugging on

cli/config/configuration-qemu.toml.in

@@ -206,6 +206,11 @@ enable_iothreads = @DEFENABLEIOTHREADS@
 # Default false
 #use_vsock = true
 
+# If false and nvdimm is supported, use nvdimm device to plug guest image.
+# Otherwise virtio-block device is used.
+# Default is false
+#disable_image_nvdimm = true
+
 # VFIO devices are hotplugged on a bridge by default. 
 # Enable hotplugging on root bus. This may be required for devices with
 # a large PCI bar, as this is a current limitation with hotplugging on 

cli/kata-check.go

@@ -443,7 +443,7 @@ func genericCheckKVMExtensions(extensions map[string]kvmExtension) (map[string]i
 
 		// Generally return value(ret) 0 means no and 1 means yes,
 		// but some extensions may report additional information in the integer return value.
-		if errno != 0 || ret <= 0 {
+		if errno != 0 {
 			kataLog.WithFields(fields).Error("is not supported")
 			return results, errno
 		}

pkg/katautils/config-settings.go

@@ -46,6 +46,7 @@ const defaultHotplugVFIOOnRootBus bool = false
 const defaultEntropySource = "/dev/urandom"
 const defaultGuestHookPath string = ""
 const defaultVirtioFSCacheMode = "none"
+const defaultDisableImageNvdimm = false
 
 const defaultTemplatePath string = "/run/vc/vm/template"
 const defaultVMCacheEndpoint string = "/var/run/kata-containers/cache.sock"

pkg/katautils/config.go

@@ -120,6 +120,7 @@ type hypervisor struct {
 	DisableNestingChecks    bool     `toml:"disable_nesting_checks"`
 	EnableIOThreads         bool     `toml:"enable_iothreads"`
 	UseVSock                bool     `toml:"use_vsock"`
+	DisableImageNvdimm      bool     `toml:"disable_image_nvdimm"`
 	HotplugVFIOOnRootBus    bool     `toml:"hotplug_vfio_on_root_bus"`
 	DisableVhostNet         bool     `toml:"disable_vhost_net"`
 	GuestHookPath           string   `toml:"guest_hook_path"`
@@ -643,6 +644,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		EnableIOThreads:         h.EnableIOThreads,
 		Msize9p:                 h.msize9p(),
 		UseVSock:                useVSock,
+		DisableImageNvdimm:      h.DisableImageNvdimm,
 		HotplugVFIOOnRootBus:    h.HotplugVFIOOnRootBus,
 		DisableVhostNet:         h.DisableVhostNet,
 		GuestHookPath:           h.guestHookPath(),
@@ -1069,6 +1071,7 @@ func GetDefaultHypervisorConfig() vc.HypervisorConfig {
 		HotplugVFIOOnRootBus:    defaultHotplugVFIOOnRootBus,
 		GuestHookPath:           defaultGuestHookPath,
 		VirtioFSCache:           defaultVirtioFSCacheMode,
+		DisableImageNvdimm:      defaultDisableImageNvdimm,
 	}
 }
 

virtcontainers/hypervisor.go

@@ -90,6 +90,13 @@ var commonNvdimmKernelRootParams = []Param{ //nolint: unused, deadcode, varcheck
 	{"rootfstype", "ext4"},
 }
 
+// agnostic list of kernel root parameters for NVDIMM
+var commonNvdimmNoDAXKernelRootParams = []Param{ //nolint: unused, deadcode, varcheck
+	{"root", "/dev/pmem0p1"},
+	{"rootflags", "data=ordered,errors=remount-ro ro"},
+	{"rootfstype", "ext4"},
+}
+
 // agnostic list of kernel root parameters for virtio-blk
 var commonVirtioblkKernelRootParams = []Param{ //nolint: unused, deadcode, varcheck
 	{"root", "/dev/vda1"},
@@ -352,6 +359,9 @@ type HypervisorConfig struct {
 	// UseVSock use a vsock for agent communication
 	UseVSock bool
 
+	// DisableImageNvdimm is used to disable guest rootfs image nvdimm devices
+	DisableImageNvdimm bool
+
 	// HotplugVFIOOnRootBus is used to indicate if devices need to be hotplugged on the
 	// root bus instead of a bridge.
 	HotplugVFIOOnRootBus bool

virtcontainers/persist.go

@@ -241,6 +241,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) {
 		Mlock:                   sconfig.HypervisorConfig.Mlock,
 		DisableNestingChecks:    sconfig.HypervisorConfig.DisableNestingChecks,
 		UseVSock:                sconfig.HypervisorConfig.UseVSock,
+		DisableImageNvdimm:      sconfig.HypervisorConfig.DisableImageNvdimm,
 		HotplugVFIOOnRootBus:    sconfig.HypervisorConfig.HotplugVFIOOnRootBus,
 		BootToBeTemplate:        sconfig.HypervisorConfig.BootToBeTemplate,
 		BootFromTemplate:        sconfig.HypervisorConfig.BootFromTemplate,
@@ -532,6 +533,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) {
 		Mlock:                   hconf.Mlock,
 		DisableNestingChecks:    hconf.DisableNestingChecks,
 		UseVSock:                hconf.UseVSock,
+		DisableImageNvdimm:      hconf.DisableImageNvdimm,
 		HotplugVFIOOnRootBus:    hconf.HotplugVFIOOnRootBus,
 		BootToBeTemplate:        hconf.BootToBeTemplate,
 		BootFromTemplate:        hconf.BootFromTemplate,

virtcontainers/persist/api/config.go

@@ -142,6 +142,9 @@ type HypervisorConfig struct {
 	// UseVSock use a vsock for agent communication
 	UseVSock bool
 
+	// DisableImageNvdimm disables nvdimm for guest rootfs image
+	DisableImageNvdimm bool
+
 	// HotplugVFIOOnRootBus is used to indicate if devices need to be hotplugged on the
 	// root bus instead of a bridge.
 	HotplugVFIOOnRootBus bool

virtcontainers/pkg/annotations/annotations.go

@@ -90,6 +90,9 @@ const (
 	// UseVSock is a sandbox annotation to specify use of vsock for agent communication.
 	UseVSock = kataAnnotHypervisorPrefix + "use_vsock"
 
+	// DisableImageNvdimm is a sandbox annotation to specify use of nvdimm device for guest rootfs image.
+	DisableImageNvdimm = kataAnnotHypervisorPrefix + "disable_image_nvdimm"
+
 	// HotplugVFIOOnRootBus is a sandbox annotation used to indicate if devices need to be hotplugged on the
 	// root bus instead of a bridge.
 	HotplugVFIOOnRootBus = kataAnnotHypervisorPrefix + "hotplug_vfio_on_root_bus"

virtcontainers/pkg/oci/utils.go

@@ -429,6 +429,15 @@ func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig)
 		config.HypervisorConfig.UseVSock = useVsock
 	}
 
+	if value, ok := ocispec.Annotations[vcAnnotations.DisableImageNvdimm]; ok {
+		disableNvdimm, err := strconv.ParseBool(value)
+		if err != nil {
+			return fmt.Errorf("Error parsing annotation for use_nvdimm: Please specify boolean value 'true|false'")
+		}
+
+		config.HypervisorConfig.DisableImageNvdimm = disableNvdimm
+	}
+
 	if value, ok := ocispec.Annotations[vcAnnotations.HotplugVFIOOnRootBus]; ok {
 		hotplugVFIOOnRootBus, err := strconv.ParseBool(value)
 		if err != nil {

virtcontainers/pkg/oci/utils_test.go

@@ -745,6 +745,7 @@ func TestAddHypervisorAnnotations(t *testing.T) {
 	ocispec.Annotations[vcAnnotations.DisableVhostNet] = "true"
 	ocispec.Annotations[vcAnnotations.GuestHookPath] = "/usr/bin/"
 	ocispec.Annotations[vcAnnotations.UseVSock] = "true"
+	ocispec.Annotations[vcAnnotations.DisableImageNvdimm] = "true"
 	ocispec.Annotations[vcAnnotations.HotplugVFIOOnRootBus] = "true"
 	ocispec.Annotations[vcAnnotations.EntropySource] = "/dev/urandom"
 
@@ -773,6 +774,7 @@ func TestAddHypervisorAnnotations(t *testing.T) {
 	assert.Equal(config.HypervisorConfig.DisableVhostNet, true)
 	assert.Equal(config.HypervisorConfig.GuestHookPath, "/usr/bin/")
 	assert.Equal(config.HypervisorConfig.UseVSock, true)
+	assert.Equal(config.HypervisorConfig.DisableImageNvdimm, true)
 	assert.Equal(config.HypervisorConfig.HotplugVFIOOnRootBus, true)
 	assert.Equal(config.HypervisorConfig.EntropySource, "/dev/urandom")
 

virtcontainers/qemu.go

@@ -248,7 +248,7 @@ func (q *qemu) setup(id string, hypervisorConfig *HypervisorConfig, vcStore *sto
 	if err != nil {
 		return err
 	}
-	if initrdPath == "" && imagePath != "" {
+	if initrdPath == "" && imagePath != "" && !q.config.DisableImageNvdimm {
 		q.nvdimmCount = 1
 	} else {
 		q.nvdimmCount = 0

virtcontainers/qemu_amd64.go

@@ -6,8 +6,6 @@
 package virtcontainers
 
 import (
-	"os"
-	"strings"
 	"time"
 
 	"github.com/kata-containers/runtime/virtcontainers/types"
@@ -26,8 +24,6 @@ const defaultQemuPath = "/usr/bin/qemu-system-x86_64"
 
 const defaultQemuMachineType = QemuPC
 
-const qemuNvdimmOption = "nvdimm"
-
 const defaultQemuMachineOptions = "accel=kvm,kernel_irqchip"
 
 const qmpMigrationWaitTimeout = 5 * time.Second
@@ -38,8 +34,6 @@ var qemuPaths = map[string]string{
 	QemuQ35:    defaultQemuPath,
 }
 
-var kernelRootParams = commonNvdimmKernelRootParams
-
 var kernelParams = []Param{
 	{"tsc", "reliable"},
 	{"no_timer_check", ""},
@@ -102,19 +96,12 @@ func newQemuArch(config HypervisorConfig) qemuArch {
 			kernelParamsNonDebug:  kernelParamsNonDebug,
 			kernelParamsDebug:     kernelParamsDebug,
 			kernelParams:          kernelParams,
+			disableNvdimm:         config.DisableImageNvdimm,
+			dax:                   true,
 		},
 		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
@@ -159,29 +146,10 @@ func (q *qemuAmd64) memoryTopology(memoryMb, hostMemoryMb uint64, slots uint8) g
 }
 
 func (q *qemuAmd64) appendImage(devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) {
-	imageFile, err := os.Open(path)
-	if err != nil {
-		return nil, err
-	}
-	defer func() { _ = imageFile.Close() }()
-
-	imageStat, err := imageFile.Stat()
-	if err != nil {
-		return nil, err
-	}
-
-	object := govmmQemu.Object{
-		Driver:   govmmQemu.NVDIMM,
-		Type:     govmmQemu.MemoryBackendFile,
-		DeviceID: "nv0",
-		ID:       "mem0",
-		MemPath:  path,
-		Size:     (uint64)(imageStat.Size()),
+	if !q.disableNvdimm {
+		return q.appendNvdimmImage(devices, path)
 	}
-
-	devices = append(devices, object)
-
-	return devices, nil
+	return q.appendBlockImage(devices, path)
 }
 
 // appendBridges appends to devices the given bridges

virtcontainers/qemu_amd64_test.go

@@ -114,7 +114,6 @@ func TestQemuAmd64MemoryTopology(t *testing.T) {
 }
 
 func TestQemuAmd64AppendImage(t *testing.T) {
-	var devices []govmmQemu.Device
 	assert := assert.New(t)
 
 	f, err := ioutil.TempFile("", "img")
@@ -131,6 +130,7 @@ func TestQemuAmd64AppendImage(t *testing.T) {
 
 	cfg := qemuConfig(QemuPC)
 	cfg.ImagePath = f.Name()
+	cfg.DisableImageNvdimm = false
 	amd64 := newQemuArch(cfg)
 	for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
 		assert.Contains(m.Options, qemuNvdimmOption)
@@ -147,12 +147,33 @@ func TestQemuAmd64AppendImage(t *testing.T) {
 		},
 	}
 
-	devices, err = amd64.appendImage(devices, f.Name())
+	devices, err := amd64.appendImage(nil, f.Name())
 	assert.NoError(err)
 	assert.Equal(expectedOut, devices)
 
 	// restore default supportedQemuMachines options
 	assert.Equal(len(supportedQemuMachines), copy(supportedQemuMachines, machinesCopy))
+
+	cfg.DisableImageNvdimm = true
+	amd64 = newQemuArch(cfg)
+	for _, m := range amd64.(*qemuAmd64).supportedQemuMachines {
+		assert.NotContains(m.Options, qemuNvdimmOption)
+	}
+
+	found := false
+	devices, err = amd64.appendImage(nil, f.Name())
+	assert.NoError(err)
+	for _, d := range devices {
+		if b, ok := d.(govmmQemu.BlockDevice); ok {
+			assert.Equal(b.Driver, govmmQemu.VirtioBlock)
+			assert.True(b.ShareRW)
+			found = true
+		}
+	}
+	assert.True(found)
+
+	// restore default supportedQemuMachines options
+	assert.Equal(len(supportedQemuMachines), copy(supportedQemuMachines, machinesCopy))
 }
 
 func TestQemuAmd64AppendBridges(t *testing.T) {