diff --git a/src/runtime/pkg/device/config/config.go b/src/runtime/pkg/device/config/config.go index 748c5b4e51..61f9236b9c 100644 --- a/src/runtime/pkg/device/config/config.go +++ b/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 diff --git a/src/runtime/pkg/device/config/pmem.go b/src/runtime/pkg/device/config/pmem.go index 44fd321873..cdd4db9988 100644 --- a/src/runtime/pkg/device/config/pmem.go +++ b/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 diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index dbdfdac303..fd6df4c3ca 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/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 } diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 0bcac2137f..a33e37a1a4 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/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, "") } diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index f6da05ec58..1c3ebc1476 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/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 diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index 6984b73be5..85ec05f15a 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/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 diff --git a/src/runtime/virtcontainers/fc.go b/src/runtime/virtcontainers/fc.go index 25012d3f3d..434e320417 100644 --- a/src/runtime/virtcontainers/fc.go +++ b/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 diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index 119c4667c3..18ccd7ae26 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/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 { diff --git a/src/runtime/virtcontainers/hypervisor_config_darwin.go b/src/runtime/virtcontainers/hypervisor_config_darwin.go new file mode 100644 index 0000000000..e074d59ea6 --- /dev/null +++ b/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 +} diff --git a/src/runtime/virtcontainers/hypervisor_config_linux.go b/src/runtime/virtcontainers/hypervisor_config_linux.go new file mode 100644 index 0000000000..f1f20d3151 --- /dev/null +++ b/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 +} diff --git a/src/runtime/virtcontainers/hypervisor_config_linux_test.go b/src/runtime/virtcontainers/hypervisor_config_linux_test.go new file mode 100644 index 0000000000..609e52fd73 --- /dev/null +++ b/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) +} diff --git a/src/runtime/virtcontainers/hypervisor_config_test.go b/src/runtime/virtcontainers/hypervisor_config_test.go new file mode 100644 index 0000000000..49558f6a97 --- /dev/null +++ b/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) +} diff --git a/src/runtime/virtcontainers/hypervisor_test.go b/src/runtime/virtcontainers/hypervisor_test.go index ec475e86eb..31452200d3 100644 --- a/src/runtime/virtcontainers/hypervisor_test.go +++ b/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{ diff --git a/src/runtime/virtcontainers/mock_hypervisor.go b/src/runtime/virtcontainers/mock_hypervisor.go index 83e776d289..f4a0b934ec 100644 --- a/src/runtime/virtcontainers/mock_hypervisor.go +++ b/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 } diff --git a/src/runtime/virtcontainers/mock_hypervisor_test.go b/src/runtime/virtcontainers/mock_hypervisor_test.go index 5e89ae8a69..0159a993dd 100644 --- a/src/runtime/virtcontainers/mock_hypervisor_test.go +++ b/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) } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index c8e48953f6..7440c82587 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/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 } diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index a995f1f77b..d53de26566 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/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 diff --git a/src/runtime/virtcontainers/vm.go b/src/runtime/virtcontainers/vm.go index a9db8efc06..a96661d434 100644 --- a/src/runtime/virtcontainers/vm.go +++ b/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.