runtime: Refactor factory/hypervisor save/restore paths for clearer separation

wip commit message

Author: Camelron

src/runtime/virtcontainers/clh.go

@@ -596,8 +596,6 @@ func TestClhSaveVM(t *testing.T) {
 
 	clhConfig, err := newClhConfig()
 	assert.NoError(err)
-	// For testing, assume the memory path is located within the VM store path.
-	clhConfig.MemoryPath = filepath.Join(store.RunVMStoragePath(), "memory")
 	clhConfig.VMStorePath = store.RunVMStoragePath()
 	clhConfig.RunStorePath = store.RunStoragePath()
 
@@ -607,11 +605,12 @@ func TestClhSaveVM(t *testing.T) {
 		APIClient: mockClient,
 	}
 
-	err = clh.SaveVM()
+	snapshotDir := store.RunVMStoragePath()
+	err = clh.SaveVM(snapshotDir)
 	assert.NoError(err)
 
 	if assert.NotNil(mockClient.snapshotRequest) {
-		expectedDestinationURL := "file://" + filepath.Dir(clhConfig.MemoryPath)
+		expectedDestinationURL := "file://" + snapshotDir
 		assert.Equal(expectedDestinationURL, mockClient.snapshotRequest.GetDestinationUrl())
 	}
 }
@@ -670,7 +669,7 @@ func TestCloudHypervisorStartSandbox(t *testing.T) {
 	err = clh.PauseVM(context.Background())
 	assert.NoError(err)
 
-	err = clh.SaveVM()
+	err = clh.SaveVM(clhConfig.VMStorePath)
 	assert.NoError(err)
 
 	err = clh.ResumeVM(context.Background())

src/runtime/virtcontainers/clh_test.go

@@ -73,10 +73,7 @@ func NewFactory(ctx context.Context, config Config, fetchOnly bool) (vc.Factory,
 func resetHypervisorConfig(config *vc.VMConfig) {
 	config.HypervisorConfig.NumVCPUsF = 0
 	config.HypervisorConfig.MemorySize = 0
-	config.HypervisorConfig.BootToBeTemplate = false
-	config.HypervisorConfig.BootFromTemplate = false
-	config.HypervisorConfig.MemoryPath = ""
-	config.HypervisorConfig.DevicesStatePath = ""
+	config.HypervisorConfig.FileBackedMemory = nil
 	config.HypervisorConfig.SharedPath = ""
 	config.HypervisorConfig.VMStorePath = ""
 	config.HypervisorConfig.RunStorePath = ""

src/runtime/virtcontainers/factory/factory_linux.go

@@ -131,10 +131,13 @@ func (t *template) prepareTemplateFiles() error {
 func (t *template) createTemplateVM(ctx context.Context) error {
 	// create the template vm
 	config := t.config
-	config.HypervisorConfig.BootToBeTemplate = true
-	config.HypervisorConfig.BootFromTemplate = false
-	config.HypervisorConfig.MemoryPath = t.statePath + "/memory"
-	config.HypervisorConfig.DevicesStatePath = t.deviceStatePath()
+	// The template source VM is backed by a shared memory file so that clones
+	// can map the same file. The factory expresses this through the generic
+	// file-backed memory config rather than template-specific flags.
+	config.HypervisorConfig.FileBackedMemory = &vc.FileBackedMemoryConfig{
+		Path:   t.statePath + "/memory",
+		Shared: true,
+	}
 	config.HypervisorConfig.VMStorePath = t.statePath
 
 	vm, err := vc.NewVM(ctx, config)
@@ -160,24 +163,39 @@ func (t *template) createTemplateVM(ctx context.Context) error {
 		return err
 	}
 
-	if err = vm.Save(); err != nil {
+	if err = vm.Save(t.statePath); err != nil {
 		return err
 	}
 
+	// The template source VM runs with shared memory so that clones can map
+	// the same backing file, but the snapshot must record the memory as
+	// private so that clones restored from it get Copy-On-Write memory. The
+	// factory owns this policy decision (when to make a snapshot private),
+	// while the CLH snapshot-format details live in
+	// vc.PatchCLHSnapshotMemoryPrivate. Only Cloud Hypervisor records a
+	// config.json that needs patching; QEMU's device-state file does not.
+	if t.config.HypervisorType == vc.ClhHypervisor {
+		if err = vc.PatchCLHSnapshotMemoryPrivate(t.statePath); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
 func (t *template) createFromTemplateVM(ctx context.Context, c vc.VMConfig) (*vc.VM, error) {
 	config := t.config
-	config.HypervisorConfig.BootToBeTemplate = false
-	config.HypervisorConfig.BootFromTemplate = true
-	config.HypervisorConfig.MemoryPath = t.statePath + "/memory"
-	config.HypervisorConfig.DevicesStatePath = t.deviceStatePath()
+	// Clones restored from the template use private Copy-On-Write memory
+	// backed by the template's shared memory file.
+	config.HypervisorConfig.FileBackedMemory = &vc.FileBackedMemoryConfig{
+		Path:   t.statePath + "/memory",
+		Shared: false,
+	}
 	config.HypervisorConfig.SharedPath = c.HypervisorConfig.SharedPath
 	config.HypervisorConfig.VMStorePath = c.HypervisorConfig.VMStorePath
 	config.HypervisorConfig.RunStorePath = c.HypervisorConfig.RunStorePath
 
-	return vc.NewVM(ctx, config)
+	return vc.NewVMFromSnapshot(ctx, config, t.statePath)
 }
 
 func (t *template) checkTemplateVM() error {

src/runtime/virtcontainers/factory/template/template_linux.go

@@ -947,7 +947,13 @@ func (fc *firecracker) PauseVM(ctx context.Context) error {
 	return nil
 }
 
-func (fc *firecracker) SaveVM() error {
+func (fc *firecracker) SaveVM(snapshotDir string) error {
+	// Firecracker does not support snapshot/restore in this implementation.
+	return nil
+}
+
+func (fc *firecracker) RestoreVM(ctx context.Context, snapshotDir string) error {
+	// Firecracker does not support snapshot/restore in this implementation.
 	return nil
 }
 

src/runtime/virtcontainers/fc.go

@@ -124,7 +124,7 @@ func TestFCSaveVM(t *testing.T) {
 	assert := assert.New(t)
 
 	fc := firecracker{}
-	err := fc.SaveVM()
+	err := fc.SaveVM(t.TempDir())
 	assert.NoError(err)
 }
 

src/runtime/virtcontainers/fc_test.go

@@ -463,6 +463,22 @@ type Param struct {
 	Value string
 }
 
+// FileBackedMemoryConfig describes guest memory that is backed by a host file.
+// It is a generic primitive used by features such as VM templating, live
+// migration and checkpoint/restore. The hypervisor layer only needs to know
+// where the backing file lives and whether it should be mapped shared or
+// private; it does not need to know which higher-level feature requested it.
+type FileBackedMemoryConfig struct {
+	// Path is the host path of the memory backing file.
+	Path string
+
+	// Shared selects the mapping mode for the backing file: MAP_SHARED when
+	// true (e.g. a template source whose memory is shared with clones) and
+	// MAP_PRIVATE/Copy-On-Write when false (e.g. a clone restored from a
+	// template or a migration target that needs its own private memory).
+	Shared bool
+}
+
 // HypervisorConfig is the hypervisor configuration.
 // nolint: govet
 type HypervisorConfig struct {
@@ -514,13 +530,12 @@ type HypervisorConfig struct {
 	// emulated.
 	HypervisorMachineType string
 
-	// MemoryPath is the memory file path of VM memory. Used when either BootToBeTemplate or
-	// BootFromTemplate is true.
-	MemoryPath string
-
-	// DevicesStatePath is the VM device state file path. Used when either BootToBeTemplate or
-	// BootFromTemplate is true.
-	DevicesStatePath string
+	// FileBackedMemory describes file-backed guest memory. When non-nil the
+	// guest memory is backed by the file at Path and mapped either MAP_SHARED
+	// (Shared=true, e.g. a template source) or MAP_PRIVATE/Copy-On-Write
+	// (Shared=false, e.g. a clone restored from a template or a migration
+	// target). When nil the guest uses standard anonymous memory.
+	FileBackedMemory *FileBackedMemoryConfig
 
 	// EntropySource is the path to a host source of
 	// entropy (/dev/random, /dev/urandom or real hardware RNG device)
@@ -561,10 +576,20 @@ type HypervisorConfig struct {
 	// VMid is "" if the hypervisor is not created by the factory.
 	VMid string
 
-	// VMStorePath is the location on disk where VM information will persist
+	// VMStorePath is the root directory (typically /run/vc/vm, supplied by the
+	// persist driver's RunVMStoragePath()) under which each VM's VMM runtime
+	// artifacts live at <VMStorePath>/<id>: the hypervisor's control sockets
+	// (e.g. Cloud Hypervisor's API and hybrid-vsock sockets, QEMU's QMP and
+	// console sockets), QEMU's pid file, and the config.json/state.json files
+	// copied in when restoring a VM from a snapshot.
 	VMStorePath string
 
-	// VMStorePath is the location on disk where runtime information will persist
+	// RunStorePath is the root directory (typically /run/vc/sbs, supplied by
+	// the persist driver's RunStoragePath()) under which each sandbox's
+	// persisted state lives at <RunStorePath>/<id>: the sandbox and container
+	// state written by the persist driver. (A guest memory dump reads this
+	// state and copies it into the dump, but the dump itself — vmcore and
+	// hypervisor metadata — is written under GuestMemoryDumpPath, not here.)
 	RunStorePath string
 
 	// SELinux label for the VM
@@ -831,12 +856,6 @@ type HypervisorConfig struct {
 	// Enable SEV-SNP guests on AMD machines capable of both
 	SevSnpGuest bool
 
-	// BootToBeTemplate used to indicate if the VM is created to be a template VM
-	BootToBeTemplate bool
-
-	// BootFromTemplate used to indicate if the VM should be created from a template VM
-	BootFromTemplate bool
-
 	// DisableVhostNet is used to indicate if host supports vhost_net
 	DisableVhostNet bool
 
@@ -894,24 +913,6 @@ type VcpuThreadIDs struct {
 	vcpus map[int]int
 }
 
-func (conf *HypervisorConfig) CheckTemplateConfig() error {
-	if conf.BootToBeTemplate && conf.BootFromTemplate {
-		return fmt.Errorf("Cannot set both 'to be' and 'from' vm tempate")
-	}
-
-	if conf.BootToBeTemplate || conf.BootFromTemplate {
-		if conf.MemoryPath == "" {
-			return fmt.Errorf("Missing MemoryPath for vm template")
-		}
-
-		if conf.BootFromTemplate && conf.DevicesStatePath == "" {
-			return fmt.Errorf("Missing DevicesStatePath to Load from vm template")
-		}
-	}
-
-	return nil
-}
-
 // AddKernelParam allows the addition of new kernel parameters to an existing
 // hypervisor configuration.
 func (conf *HypervisorConfig) AddKernelParam(p Param) error {
@@ -1311,7 +1312,16 @@ type Hypervisor interface {
 	// just perform cleanup.
 	StopVM(ctx context.Context, waitOnly bool) error
 	PauseVM(ctx context.Context) error
-	SaveVM() error
+	// SaveVM snapshots the running VM into snapshotDir. It is a pure
+	// hypervisor operation: the caller chooses the destination and is
+	// responsible for any feature-specific post-processing (e.g. template
+	// memory-sharing adjustments).
+	SaveVM(snapshotDir string) error
+	// RestoreVM brings up a VM by restoring it from a snapshot previously
+	// written to snapshotDir. The restored VM is left in a paused state; the
+	// caller decides when to ResumeVM and what post-restore housekeeping to
+	// perform (e.g. reseeding the RNG, syncing the guest clock).
+	RestoreVM(ctx context.Context, snapshotDir string) error
 	ResumeVM(ctx context.Context) error
 	AddDevice(ctx context.Context, devInfo interface{}, devType DeviceType) error
 	HotplugAddDevice(ctx context.Context, devInfo interface{}, devType DeviceType) (interface{}, error)

src/runtime/virtcontainers/hypervisor.go

@@ -32,10 +32,6 @@ func validateHypervisorConfig(conf *HypervisorConfig) error {
 		return fmt.Errorf("Image and initrd path cannot be both set")
 	}
 
-	if err := conf.CheckTemplateConfig(); err != nil {
-		return err
-	}
-
 	if conf.NumVCPUsF == 0 {
 		conf.NumVCPUsF = defaultVCPUs
 	}

src/runtime/virtcontainers/hypervisor_config_linux.go

@@ -65,30 +65,6 @@ func TestHypervisorConfigSecureExecution(t *testing.T) {
 	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{

src/runtime/virtcontainers/hypervisor_config_linux_test.go

@@ -61,7 +61,11 @@ func (m *mockHypervisor) ResumeVM(ctx context.Context) error {
 	return nil
 }
 
-func (m *mockHypervisor) SaveVM() error {
+func (m *mockHypervisor) SaveVM(snapshotDir string) error {
+	return nil
+}
+
+func (m *mockHypervisor) RestoreVM(ctx context.Context, snapshotDir string) error {
 	return nil
 }