Merge pull request #7716 from bergwolf/github/image-initrd-assets

runtime: fix image and initrd assets handling

src/runtime/virtcontainers/clh.go

@@ -538,14 +538,14 @@ func (clh *cloudHypervisor) CreateVM(ctx context.Context, id string, network Net
 	clh.vmconfig.Rng = chclient.NewRngConfig(clh.config.EntropySource)
 
 	// set the initial root/boot disk of hypervisor
-	imagePath, err := clh.config.ImageAssetPath()
+	assetPath, assetType, err := clh.config.ImageOrInitrdAssetPath()
 	if err != nil {
 		return err
 	}
 
-	if imagePath != "" {
+	if assetType == types.ImageAsset {
 		if clh.config.ConfidentialGuest {
-			disk := chclient.NewDiskConfig(imagePath)
+			disk := chclient.NewDiskConfig(assetPath)
 			disk.SetReadonly(true)
 
 			diskRateLimiterConfig := clh.getDiskRateLimiterConfig()
@@ -559,7 +559,7 @@ func (clh *cloudHypervisor) CreateVM(ctx context.Context, id string, network Net
 				clh.vmconfig.Disks = &[]chclient.DiskConfig{*disk}
 			}
 		} else {
-			pmem := chclient.NewPmemConfig(imagePath)
+			pmem := chclient.NewPmemConfig(assetPath)
 			*pmem.DiscardWrites = true
 
 			if clh.vmconfig.Pmem != nil {
@@ -569,12 +569,8 @@ func (clh *cloudHypervisor) CreateVM(ctx context.Context, id string, network Net
 			}
 		}
 	} else {
-		initrdPath, err := clh.config.InitrdAssetPath()
+		// assetType == types.InitrdAsset
-		if err != nil {
+		clh.vmconfig.Payload.SetInitramfs(assetPath)
-			return err
-		}
-
-		clh.vmconfig.Payload.SetInitramfs(initrdPath)
 	}
 
 	if clh.config.ConfidentialGuest {

src/runtime/virtcontainers/fc.go

@@ -721,19 +721,12 @@ func (fc *firecracker) fcInitConfiguration(ctx context.Context) error {
 		return err
 	}
 
-	image, err := fc.config.InitrdAssetPath()
+	assetPath, _, err := fc.config.ImageOrInitrdAssetPath()
 	if err != nil {
 		return err
 	}
 
-	if image == "" {
+	if err := fc.fcSetVMRootfs(ctx, assetPath); err != nil {
-		image, err = fc.config.ImageAssetPath()
-		if err != nil {
-			return err
-		}
-	}
-
-	if err := fc.fcSetVMRootfs(ctx, image); err != nil {
 		return err
 	}
 

src/runtime/virtcontainers/hypervisor.go

@@ -84,6 +84,7 @@ const (
 var (
 	hvLogger                   = logrus.WithField("source", "virtcontainers/hypervisor")
 	noGuestMemHotplugErr error = errors.New("guest memory hotplug not supported")
+	conflictingAssets    error = errors.New("cannot set both image and initrd at the same time")
 )
 
 // In some architectures the maximum number of vCPUs depends on the number of physical cores.
@@ -698,6 +699,46 @@ func (conf *HypervisorConfig) AddCustomAsset(a *types.Asset) error {
 	return nil
 }
 
+// ImageOrInitrdAssetPath returns an image or an initrd path, along with the corresponding asset type
+// Annotation path is preferred to config path.
+func (conf *HypervisorConfig) ImageOrInitrdAssetPath() (string, types.AssetType, error) {
+	var image, initrd string
+
+	checkAndReturn := func(image string, initrd string) (string, types.AssetType, error) {
+		if image != "" && initrd != "" {
+			return "", types.UnkownAsset, conflictingAssets
+		}
+
+		if image != "" {
+			return image, types.ImageAsset, nil
+		}
+
+		if initrd != "" {
+			return initrd, types.InitrdAsset, nil
+		}
+
+		return "", types.UnkownAsset, fmt.Errorf("one of image and initrd must be set")
+	}
+
+	if a, ok := conf.customAssets[types.ImageAsset]; ok {
+		image = a.Path()
+	}
+
+	if a, ok := conf.customAssets[types.InitrdAsset]; ok {
+		initrd = a.Path()
+	}
+
+	path, assetType, err := checkAndReturn(image, initrd)
+	if assetType != types.UnkownAsset {
+		return path, assetType, nil
+	}
+	if err == conflictingAssets {
+		return "", types.UnkownAsset, errors.Wrapf(err, "conflicting annotations")
+	}
+
+	return checkAndReturn(conf.ImagePath, conf.InitrdPath)
+}
+
 func (conf *HypervisorConfig) assetPath(t types.AssetType) (string, error) {
 	// Custom assets take precedence over the configured ones
 	a, ok := conf.customAssets[t]

src/runtime/virtcontainers/qemu.go

@@ -347,22 +347,6 @@ func (q *qemu) getQemuMachine() (govmmQemu.Machine, error) {
 	return machine, nil
 }
 
-func (q *qemu) appendImage(ctx context.Context, devices []govmmQemu.Device) ([]govmmQemu.Device, error) {
-	imagePath, err := q.config.ImageAssetPath()
-	if err != nil {
-		return nil, err
-	}
-
-	if imagePath != "" {
-		devices, err = q.arch.appendImage(ctx, devices, imagePath)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return devices, nil
-}
-
 func (q *qemu) createQmpSocket() ([]govmmQemu.QMPSocket, error) {
 	monitorSockPath, err := q.qmpSocketPath(q.id)
 	if err != nil {
@@ -400,12 +384,16 @@ func (q *qemu) createQmpSocket() ([]govmmQemu.QMPSocket, error) {
 	return sockets, nil
 }
 
-func (q *qemu) buildDevices(ctx context.Context, initrdPath string) ([]govmmQemu.Device, *govmmQemu.IOThread, error) {
+func (q *qemu) buildDevices(ctx context.Context, kernelPath string) ([]govmmQemu.Device, *govmmQemu.IOThread, *govmmQemu.Kernel, error) {
 	var devices []govmmQemu.Device
 
+	kernel := &govmmQemu.Kernel{
+		Path: kernelPath,
+	}
+
 	_, console, err := q.GetVMConsole(ctx, q.id)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 
 	// Add bridges before any other devices. This way we make sure that
@@ -414,20 +402,28 @@ func (q *qemu) buildDevices(ctx context.Context, initrdPath string) ([]govmmQemu
 
 	devices, err = q.arch.appendConsole(ctx, devices, console)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 
-	if initrdPath == "" {
+	assetPath, assetType, err := q.config.ImageOrInitrdAssetPath()
-		devices, err = q.appendImage(ctx, devices)
 	if err != nil {
-			return nil, nil, err
+		return nil, nil, nil, err
 	}
+
+	if assetType == types.ImageAsset {
+		devices, err = q.arch.appendImage(ctx, devices, assetPath)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+	} else {
+		// InitrdAsset, need to set kernel initrd path
+		kernel.InitrdPath = assetPath
 	}
 
 	if q.config.IOMMU {
 		devices, err = q.arch.appendIOMMU(devices)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, nil, err
 		}
 	}
 
@@ -438,10 +434,13 @@ func (q *qemu) buildDevices(ctx context.Context, initrdPath string) ([]govmmQemu
 
 	var ioThread *govmmQemu.IOThread
 	if q.config.BlockDeviceDriver == config.VirtioSCSI {
-		return q.arch.appendSCSIController(ctx, devices, q.config.EnableIOThreads)
+		devices, ioThread, err = q.arch.appendSCSIController(ctx, devices, q.config.EnableIOThreads)
+		if err != nil {
+			return nil, nil, nil, err
+		}
 	}
 
-	return devices, ioThread, nil
+	return devices, ioThread, kernel, nil
 }
 
 func (q *qemu) setupTemplate(knobs *govmmQemu.Knobs, memory *govmmQemu.Memory) govmmQemu.Incoming {
@@ -562,16 +561,6 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi
 		IOMMUPlatform: q.config.IOMMUPlatform,
 	}
 
-	kernelPath, err := q.config.KernelAssetPath()
-	if err != nil {
-		return err
-	}
-
-	initrdPath, err := q.config.InitrdAssetPath()
-	if err != nil {
-		return err
-	}
-
 	incoming := q.setupTemplate(&knobs, &memory)
 
 	// With the current implementations, VM templating will not work with file
@@ -615,7 +604,12 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi
 		return err
 	}
 
-	devices, ioThread, err := q.buildDevices(ctx, initrdPath)
+	kernelPath, err := q.config.KernelAssetPath()
+	if err != nil {
+		return err
+	}
+
+	devices, ioThread, kernel, err := q.buildDevices(ctx, kernelPath)
 	if err != nil {
 		return err
 	}
@@ -643,13 +637,8 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi
 		return err
 	}
 
-	// Breaks hypervisor abstraction has Kata Specific logic
-	kernel := govmmQemu.Kernel{
-		Path:       kernelPath,
-		InitrdPath: initrdPath,
 	// some devices configuration may also change kernel params, make sure this is called afterwards
-		Params: q.kernelParameters(),
+	kernel.Params = q.kernelParameters()
-	}
 	q.checkBpfEnabled()
 
 	qemuConfig := govmmQemu.Config{
@@ -666,7 +655,7 @@ func (q *qemu) CreateVM(ctx context.Context, id string, network Network, hypervi
 		Devices:        devices,
 		CPUModel:       cpuModel,
 		SeccompSandbox: q.config.SeccompSandbox,
-		Kernel:         kernel,
+		Kernel:         *kernel,
 		RTC:            rtc,
 		QMPSockets:     qmpSockets,
 		Knobs:          knobs,

src/runtime/virtcontainers/types/asset.go

@@ -41,6 +41,8 @@ const (
 	FirmwareAsset AssetType = "firmware"
 
 	FirmwareVolumeAsset AssetType = "firmware_volume"
+
+	UnkownAsset AssetType = "unknown"
 )
 
 // AssetTypes returns a list of all known asset types.