Merge pull request #4469 from egernst/config-validation-refactor

Refactor how hypervisor config validation is handled

src/runtime/pkg/device/config/config.go

@@ -444,7 +444,7 @@ func getVhostUserDevName(dirname string, majorNum, minorNum uint32) (string, err
 
 // DeviceState is a structure which represents host devices
 // plugged to a hypervisor, one Device can be shared among containers in POD
-// Refs: virtcontainers/device/drivers/generic.go:GenericDevice
+// Refs: pkg/device/drivers/generic.go:GenericDevice
 type DeviceState struct {
 	// DriverOptions is specific options for each device driver
 	// for example, for BlockDevice, we can set DriverOptions["block-driver"]="virtio-blk"
@@ -459,7 +459,7 @@ type DeviceState struct {
 	ID string
 
 	// Type is used to specify driver type
-	// Refs: virtcontainers/device/config/config.go:DeviceType
+	// Refs: pkg/device/config/config.go:DeviceType
 	Type string
 
 	// Type of device: c, b, u or p

src/runtime/pkg/device/config/pmem.go

@@ -26,7 +26,7 @@ const (
 )
 
 var (
-	pmemLog = logrus.WithField("source", "virtcontainers/device/config")
+	pmemLog = logrus.WithField("source", "pkg/device/config")
 )
 
 // SetLogger sets up a logger for this pkg

src/runtime/pkg/katautils/config.go

@@ -220,7 +220,7 @@ func (h hypervisor) initrd() (string, error) {
 	p := h.Initrd
 
 	if p == "" {
-		return "", errors.New("initrd is not set")
+		return "", nil
 	}
 
 	return ResolvePath(p)
@@ -230,7 +230,7 @@ func (h hypervisor) image() (string, error) {
 	p := h.Image
 
 	if p == "" {
-		return "", errors.New("image is not set")
+		return "", nil
 	}
 
 	return ResolvePath(p)
@@ -474,24 +474,6 @@ func (h hypervisor) vhostUserStorePath() string {
 	return h.VhostUserStorePath
 }
 
-func (h hypervisor) getInitrdAndImage() (initrd string, image string, err error) {
-	initrd, errInitrd := h.initrd()
-
-	image, errImage := h.image()
-
-	if h.ConfidentialGuest && h.MachineType == vc.QemuCCWVirtio {
-		if image != "" || initrd != "" {
-			return "", "", errors.New("Neither the image nor initrd path may be set for Secure Execution")
-		}
-	} else if image != "" && initrd != "" {
-		return "", "", errors.New("having both an image and an initrd defined in the configuration file is not supported")
-	} else if errInitrd != nil && errImage != nil {
-		return "", "", fmt.Errorf("Either initrd or image must be set to a valid path (initrd: %v) (image: %v)", errInitrd, errImage)
-	}
-
-	return
-}
-
 func (h hypervisor) getDiskRateLimiterBwMaxRate() int64 {
 	return h.DiskRateLimiterBwMaxRate
 }
@@ -601,7 +583,12 @@ func newFirecrackerHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		return vc.HypervisorConfig{}, err
 	}
 
-	initrd, image, err := h.getInitrdAndImage()
+	initrd, err := h.initrd()
+	if err != nil {
+		return vc.HypervisorConfig{}, err
+	}
+
+	image, err := h.image()
 	if err != nil {
 		return vc.HypervisorConfig{}, err
 	}
@@ -663,7 +650,12 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		return vc.HypervisorConfig{}, err
 	}
 
-	initrd, image, err := h.getInitrdAndImage()
+	initrd, err := h.initrd()
+	if err != nil {
+		return vc.HypervisorConfig{}, err
+	}
+
+	image, err := h.image()
 	if err != nil {
 		return vc.HypervisorConfig{}, err
 	}
@@ -857,7 +849,12 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		return vc.HypervisorConfig{}, err
 	}
 
-	initrd, image, err := h.getInitrdAndImage()
+	initrd, err := h.initrd()
+	if err != nil {
+		return vc.HypervisorConfig{}, err
+	}
+
+	image, err := h.image()
 	if err != nil {
 		return vc.HypervisorConfig{}, err
 	}

src/runtime/pkg/katautils/config_test.go

@@ -1017,7 +1017,7 @@ func TestHypervisorDefaultsKernel(t *testing.T) {
 	assert.Equal(h.kernelParams(), kernelParams, "custom hypervisor kernel parameterms wrong")
 }
 
-// The default initrd path is not returned by h.initrd()
+// The default initrd path is not returned by h.initrd(), it isn't an error if path isn't provided
 func TestHypervisorDefaultsInitrd(t *testing.T) {
 	assert := assert.New(t)
 
@@ -1041,18 +1041,18 @@ func TestHypervisorDefaultsInitrd(t *testing.T) {
 	defaultInitrdPath = testInitrdPath
 	h := hypervisor{}
 	p, err := h.initrd()
-	assert.Error(err)
+	assert.NoError(err)
 	assert.Equal(p, "", "default Image path wrong")
 
 	// test path resolution
 	defaultInitrdPath = testInitrdLinkPath
 	h = hypervisor{}
 	p, err = h.initrd()
-	assert.Error(err)
+	assert.NoError(err)
 	assert.Equal(p, "")
 }
 
-// The default image path is not returned by h.image()
+// The default image path is not returned by h.image(), it isn't an error if path isn't provided
 func TestHypervisorDefaultsImage(t *testing.T) {
 	assert := assert.New(t)
 
@@ -1076,14 +1076,14 @@ func TestHypervisorDefaultsImage(t *testing.T) {
 	defaultImagePath = testImagePath
 	h := hypervisor{}
 	p, err := h.image()
-	assert.Error(err)
+	assert.NoError(err)
 	assert.Equal(p, "", "default Image path wrong")
 
 	// test path resolution
 	defaultImagePath = testImageLinkPath
 	h = hypervisor{}
 	p, err = h.image()
-	assert.Error(err)
+	assert.NoError(err)
 	assert.Equal(p, "")
 }
 

src/runtime/virtcontainers/acrn.go

@@ -348,10 +348,6 @@ func (a *Acrn) createDummyVirtioBlkDev(ctx context.Context, devices []Device) ([
 }
 
 func (a *Acrn) setConfig(config *HypervisorConfig) error {
-	if err := config.Valid(); err != nil {
-		return err
-	}
-
 	a.config = *config
 
 	return nil

src/runtime/virtcontainers/clh.go

@@ -268,11 +268,6 @@ var clhDebugKernelParams = []Param{
 //###########################################################
 
 func (clh *cloudHypervisor) setConfig(config *HypervisorConfig) error {
-	err := config.Valid()
-	if err != nil {
-		return err
-	}
-
 	clh.config = *config
 
 	return nil

src/runtime/virtcontainers/fc.go

@@ -189,11 +189,6 @@ func (fc *firecracker) truncateID(id string) string {
 }
 
 func (fc *firecracker) setConfig(config *HypervisorConfig) error {
-	err := config.Valid()
-	if err != nil {
-		return err
-	}
-
 	fc.config = *config
 
 	return nil

src/runtime/virtcontainers/hypervisor.go

@@ -568,61 +568,6 @@ func (conf *HypervisorConfig) CheckTemplateConfig() error {
 	return nil
 }
 
-func (conf *HypervisorConfig) Valid() error {
-	// Kata specific checks. Should be done outside the hypervisor
-	if conf.KernelPath == "" {
-		return fmt.Errorf("Missing kernel path")
-	}
-
-	if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio {
-		if conf.ImagePath != "" || conf.InitrdPath != "" {
-			fmt.Println("yes, failing")
-			return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution")
-		}
-	} else if conf.ImagePath == "" && conf.InitrdPath == "" {
-		return fmt.Errorf("Missing image and initrd path")
-	} else if conf.ImagePath != "" && conf.InitrdPath != "" {
-		return fmt.Errorf("Image and initrd path cannot be both set")
-	}
-
-	if err := conf.CheckTemplateConfig(); err != nil {
-		return err
-	}
-
-	if conf.NumVCPUs == 0 {
-		conf.NumVCPUs = defaultVCPUs
-	}
-
-	if conf.MemorySize == 0 {
-		conf.MemorySize = defaultMemSzMiB
-	}
-
-	if conf.DefaultBridges == 0 {
-		conf.DefaultBridges = defaultBridges
-	}
-
-	if conf.BlockDeviceDriver == "" {
-		conf.BlockDeviceDriver = defaultBlockDriver
-	} else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio {
-		conf.BlockDeviceDriver = config.VirtioBlockCCW
-	}
-
-	if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs {
-		conf.DefaultMaxVCPUs = defaultMaxVCPUs
-	}
-
-	if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs {
-		hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs)
-		conf.DefaultMaxVCPUs = conf.NumVCPUs
-	}
-
-	if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS {
-		conf.Msize9p = defaultMsize9p
-	}
-
-	return nil
-}
-
 // AddKernelParam allows the addition of new kernel parameters to an existing
 // hypervisor configuration.
 func (conf *HypervisorConfig) AddKernelParam(p Param) error {

src/runtime/virtcontainers/hypervisor_config_darwin.go

@@ -0,0 +1,33 @@
+// Copyright (c) 2022 Apple Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package virtcontainers
+
+import (
+	"fmt"
+)
+
+func validateHypervisorConfig(conf *HypervisorConfig) error {
+
+	if conf.KernelPath == "" {
+		return fmt.Errorf("Missing kernel path")
+	}
+
+	if conf.ImagePath == "" && conf.InitrdPath == "" {
+		return fmt.Errorf("Missing image and initrd path")
+	} else if conf.ImagePath != "" && conf.InitrdPath != "" {
+		return fmt.Errorf("Image and initrd path cannot be both set")
+	}
+
+	if conf.NumVCPUs == 0 {
+		conf.NumVCPUs = defaultVCPUs
+	}
+
+	if conf.MemorySize == 0 {
+		conf.MemorySize = defaultMemSzMiB
+	}
+
+	return nil
+}

src/runtime/virtcontainers/hypervisor_config_linux.go

@@ -0,0 +1,67 @@
+// Copyright (c) 2022 Apple Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package virtcontainers
+
+import (
+	"fmt"
+
+	"github.com/kata-containers/kata-containers/src/runtime/pkg/device/config"
+)
+
+func validateHypervisorConfig(conf *HypervisorConfig) error {
+
+	if conf.KernelPath == "" {
+		return fmt.Errorf("Missing kernel path")
+	}
+
+	if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio {
+		if conf.ImagePath != "" || conf.InitrdPath != "" {
+			fmt.Println("yes, failing")
+			return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution")
+		}
+	} else if conf.ImagePath == "" && conf.InitrdPath == "" {
+		return fmt.Errorf("Missing image and initrd path")
+	} else if conf.ImagePath != "" && conf.InitrdPath != "" {
+		return fmt.Errorf("Image and initrd path cannot be both set")
+	}
+
+	if err := conf.CheckTemplateConfig(); err != nil {
+		return err
+	}
+
+	if conf.NumVCPUs == 0 {
+		conf.NumVCPUs = defaultVCPUs
+	}
+
+	if conf.MemorySize == 0 {
+		conf.MemorySize = defaultMemSzMiB
+	}
+
+	if conf.DefaultBridges == 0 {
+		conf.DefaultBridges = defaultBridges
+	}
+
+	if conf.BlockDeviceDriver == "" {
+		conf.BlockDeviceDriver = defaultBlockDriver
+	} else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio {
+		conf.BlockDeviceDriver = config.VirtioBlockCCW
+	}
+
+	if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs {
+		conf.DefaultMaxVCPUs = defaultMaxVCPUs
+	}
+
+	if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs {
+		hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs)
+		conf.DefaultMaxVCPUs = conf.NumVCPUs
+	}
+
+	if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS {
+		conf.Msize9p = defaultMsize9p
+	}
+
+	return nil
+}

src/runtime/virtcontainers/hypervisor_config_linux_test.go

@@ -0,0 +1,114 @@
+// Copyright (c) 2022 Apple Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package virtcontainers
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestHypervisorConfigNoImagePath(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:      "",
+		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
+	}
+
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+}
+
+func TestHypervisorConfigNoHypervisorPath(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath: "",
+	}
+
+	testHypervisorConfigValid(t, hypervisorConfig, true)
+}
+
+func TestHypervisorConfigIsValid(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
+	}
+
+	testHypervisorConfigValid(t, hypervisorConfig, true)
+}
+
+func TestHypervisorConfigBothInitrdAndImage(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+		InitrdPath:     fmt.Sprintf("%s/%s", testDir, testInitrd),
+		HypervisorPath: "",
+	}
+
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+}
+
+func TestHypervisorConfigSecureExecution(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:            fmt.Sprintf("%s/%s", testDir, testKernel),
+		InitrdPath:            fmt.Sprintf("%s/%s", testDir, testInitrd),
+		ConfidentialGuest:     true,
+		HypervisorMachineType: QemuCCWVirtio,
+	}
+
+	// Secure Execution should only specify a kernel (encrypted image contains all components)
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+}
+
+func TestHypervisorConfigValidTemplateConfig(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:       fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:        fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath:   fmt.Sprintf("%s/%s", testDir, testHypervisor),
+		BootToBeTemplate: true,
+		BootFromTemplate: true,
+	}
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+
+	hypervisorConfig.BootToBeTemplate = false
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+	hypervisorConfig.MemoryPath = "foobar"
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+	hypervisorConfig.DevicesStatePath = "foobar"
+	testHypervisorConfigValid(t, hypervisorConfig, true)
+
+	hypervisorConfig.BootFromTemplate = false
+	hypervisorConfig.BootToBeTemplate = true
+	testHypervisorConfigValid(t, hypervisorConfig, true)
+	hypervisorConfig.MemoryPath = ""
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+}
+
+func TestHypervisorConfigDefaults(t *testing.T) {
+	assert := assert.New(t)
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath: "",
+	}
+	testHypervisorConfigValid(t, hypervisorConfig, true)
+
+	hypervisorConfigDefaultsExpected := &HypervisorConfig{
+		KernelPath:        fmt.Sprintf("%s/%s", testDir, testKernel),
+		ImagePath:         fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath:    "",
+		NumVCPUs:          defaultVCPUs,
+		MemorySize:        defaultMemSzMiB,
+		DefaultBridges:    defaultBridges,
+		BlockDeviceDriver: defaultBlockDriver,
+		DefaultMaxVCPUs:   defaultMaxVCPUs,
+		Msize9p:           defaultMsize9p,
+	}
+
+	assert.Exactly(hypervisorConfig, hypervisorConfigDefaultsExpected)
+}

src/runtime/virtcontainers/hypervisor_config_test.go

@@ -0,0 +1,30 @@
+// Copyright (c) 2022 Apple Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+
+package virtcontainers
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testHypervisorConfigValid(t *testing.T, hypervisorConfig *HypervisorConfig, success bool) {
+	err := validateHypervisorConfig(hypervisorConfig)
+	assert := assert.New(t)
+	assert.False(success && err != nil)
+	assert.False(!success && err == nil)
+}
+
+func TestHypervisorConfigNoKernelPath(t *testing.T) {
+	hypervisorConfig := &HypervisorConfig{
+		KernelPath:     "",
+		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
+	}
+
+	testHypervisorConfigValid(t, hypervisorConfig, false)
+}

src/runtime/virtcontainers/hypervisor_test.go

@@ -86,124 +86,6 @@ func TestNewHypervisorFromUnknownHypervisorType(t *testing.T) {
 	assert.Nil(hy)
 }
 
-func testHypervisorConfigValid(t *testing.T, hypervisorConfig *HypervisorConfig, success bool) {
-	err := hypervisorConfig.Valid()
-	assert := assert.New(t)
-	assert.False(success && err != nil)
-	assert.False(!success && err == nil)
-}
-
-func TestHypervisorConfigNoKernelPath(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:     "",
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
-	}
-
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-}
-
-func TestHypervisorConfigNoImagePath(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      "",
-		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
-	}
-
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-}
-
-func TestHypervisorConfigNoHypervisorPath(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath: "",
-	}
-
-	testHypervisorConfigValid(t, hypervisorConfig, true)
-}
-
-func TestHypervisorConfigIsValid(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
-	}
-
-	testHypervisorConfigValid(t, hypervisorConfig, true)
-}
-
-func TestHypervisorConfigBothInitrdAndImage(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		InitrdPath:     fmt.Sprintf("%s/%s", testDir, testInitrd),
-		HypervisorPath: "",
-	}
-
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-}
-
-func TestHypervisorConfigSecureExecution(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:            fmt.Sprintf("%s/%s", testDir, testKernel),
-		InitrdPath:            fmt.Sprintf("%s/%s", testDir, testInitrd),
-		ConfidentialGuest:     true,
-		HypervisorMachineType: QemuCCWVirtio,
-	}
-
-	// Secure Execution should only specify a kernel (encrypted image contains all components)
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-}
-
-func TestHypervisorConfigValidTemplateConfig(t *testing.T) {
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:       fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:        fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath:   fmt.Sprintf("%s/%s", testDir, testHypervisor),
-		BootToBeTemplate: true,
-		BootFromTemplate: true,
-	}
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-
-	hypervisorConfig.BootToBeTemplate = false
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-	hypervisorConfig.MemoryPath = "foobar"
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-	hypervisorConfig.DevicesStatePath = "foobar"
-	testHypervisorConfigValid(t, hypervisorConfig, true)
-
-	hypervisorConfig.BootFromTemplate = false
-	hypervisorConfig.BootToBeTemplate = true
-	testHypervisorConfigValid(t, hypervisorConfig, true)
-	hypervisorConfig.MemoryPath = ""
-	testHypervisorConfigValid(t, hypervisorConfig, false)
-}
-
-func TestHypervisorConfigDefaults(t *testing.T) {
-	assert := assert.New(t)
-	hypervisorConfig := &HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath: "",
-	}
-	testHypervisorConfigValid(t, hypervisorConfig, true)
-
-	hypervisorConfigDefaultsExpected := &HypervisorConfig{
-		KernelPath:        fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:         fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath:    "",
-		NumVCPUs:          defaultVCPUs,
-		MemorySize:        defaultMemSzMiB,
-		DefaultBridges:    defaultBridges,
-		BlockDeviceDriver: defaultBlockDriver,
-		DefaultMaxVCPUs:   defaultMaxVCPUs,
-		Msize9p:           defaultMsize9p,
-	}
-
-	assert.Exactly(hypervisorConfig, hypervisorConfigDefaultsExpected)
-}
-
 func TestAppendParams(t *testing.T) {
 	assert := assert.New(t)
 	paramList := []Param{

src/runtime/virtcontainers/mock_hypervisor.go

@@ -31,10 +31,6 @@ func (m *mockHypervisor) HypervisorConfig() HypervisorConfig {
 }
 
 func (m *mockHypervisor) setConfig(config *HypervisorConfig) error {
-	if err := config.Valid(); err != nil {
-		return err
-	}
-
 	return nil
 }
 

src/runtime/virtcontainers/mock_hypervisor_test.go

@@ -21,9 +21,9 @@ func TestMockHypervisorCreateVM(t *testing.T) {
 		config: &SandboxConfig{
 			ID: "mock_sandbox",
 			HypervisorConfig: HypervisorConfig{
-				KernelPath:     "",
-				ImagePath:      "",
-				HypervisorPath: "",
+				KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
+				ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
+				HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
 			},
 		},
 	}
@@ -33,16 +33,6 @@ func TestMockHypervisorCreateVM(t *testing.T) {
 
 	ctx := context.Background()
 
-	// wrong config
-	err = m.CreateVM(ctx, sandbox.config.ID, network, &sandbox.config.HypervisorConfig)
-	assert.Error(err)
-
-	sandbox.config.HypervisorConfig = HypervisorConfig{
-		KernelPath:     fmt.Sprintf("%s/%s", testDir, testKernel),
-		ImagePath:      fmt.Sprintf("%s/%s", testDir, testImage),
-		HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
-	}
-
 	err = m.CreateVM(ctx, sandbox.config.ID, network, &sandbox.config.HypervisorConfig)
 	assert.NoError(err)
 }

src/runtime/virtcontainers/qemu.go

@@ -461,11 +461,6 @@ func (q *qemu) setupFileBackedMem(knobs *govmmQemu.Knobs, memory *govmmQemu.Memo
 }
 
 func (q *qemu) setConfig(config *HypervisorConfig) error {
-	err := config.Valid()
-	if err != nil {
-		return err
-	}
-
 	q.config = *config
 
 	return nil
@@ -513,7 +508,6 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi
 	span, ctx := katatrace.Trace(ctx, q.Logger(), "CreateVM", qemuTracingTags, map[string]string{"VM_ID": q.id})
 	defer span.End()
 
-	// Breaks hypervisor abstraction Has Kata Specific logic: See within
 	if err := q.setup(ctx, id, hypervisorConfig); err != nil {
 		return err
 	}

src/runtime/virtcontainers/sandbox.go

@@ -593,6 +593,10 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
 		s.Logger().WithError(err).Debug("restore sandbox failed")
 	}
 
+	if err := validateHypervisorConfig(&sandboxConfig.HypervisorConfig); err != nil {
+		return nil, err
+	}
+
 	// store doesn't require hypervisor to be stored immediately
 	if err = s.hypervisor.CreateVM(ctx, s.id, s.network, &sandboxConfig.HypervisorConfig); err != nil {
 		return nil, err

src/runtime/virtcontainers/vm.go

@@ -42,9 +42,8 @@ type VMConfig struct {
 	HypervisorConfig HypervisorConfig
 }
 
-// Valid Check VMConfig validity.
 func (c *VMConfig) Valid() error {
-	return c.HypervisorConfig.Valid()
+	return validateHypervisorConfig(&c.HypervisorConfig)
 }
 
 // ToGrpc convert VMConfig struct to grpc format pb.GrpcVMConfig.