From 57869b17f8626e2f728fd3f117c59c7193ae5306 Mon Sep 17 00:00:00 2001 From: Georgiy Lebedev Date: Tue, 8 Aug 2023 14:05:05 +0300 Subject: [PATCH 1/2] Remove `addVsocksHandler` from `loadSnapshotHandlerList` According to the firecracker OpenAPI specification [1], creating vsock devices is only a pre-boot request, so adding vsocks after loading a snapshot fails. It also seems redundant, since the VM loaded from a snapshot restores vsocks anyways. Closes #506 1. https://github.com/firecracker-microvm/firecracker/blob/56aeeac51c00b449a45be4542b3e807d34690ba7/src/api_server/swagger/firecracker.yaml#L676-L678 Signed-off-by: Georgiy Lebedev --- handlers.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/handlers.go b/handlers.go index d9d022d8..50fa029c 100644 --- a/handlers.go +++ b/handlers.go @@ -321,7 +321,10 @@ var loadSnapshotHandlerList = HandlerList{}.Append( CreateLogFilesHandler, BootstrapLoggingHandler, LoadSnapshotHandler, - AddVsocksHandler, + // According to the firecracker OpenAPI specification, creating vsock devices is only a pre-boot request, so adding + // vsocks after loading a snapshot fails. It also seems redundant, since the VM loaded from a snapshot restores + // vsocks anyways (firecracker-microvm/firecracker-go-sdk#506). + // AddVsocksHandler, ) var defaultValidationHandlerList = HandlerList{}.Append( From 1afa908bf41c01058c15eff2b0db8580eca56e67 Mon Sep 17 00:00:00 2001 From: Georgiy Lebedev Date: Mon, 7 Aug 2023 13:28:45 +0300 Subject: [PATCH 2/2] Add `container_snapshot_path` to load snapshot request Forward the `container_snapshot_path` parameter of the firecracker load snapshot request. Signed-off-by: Georgiy Lebedev --- client/models/snapshot_load_params.go | 17 +++++++++++++++++ client/swagger.yaml | 5 +++++ machine.go | 17 +++++++++++------ opts.go | 3 ++- snapshot.go | 11 ++++++----- 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/client/models/snapshot_load_params.go b/client/models/snapshot_load_params.go index b16beaec..c5016193 100644 --- a/client/models/snapshot_load_params.go +++ b/client/models/snapshot_load_params.go @@ -33,6 +33,10 @@ type SnapshotLoadParams struct { // Enable support for incremental (diff) snapshots by tracking dirty guest pages. EnableDiffSnapshots bool `json:"enable_diff_snapshots,omitempty"` + // Path to the disk device backing the container snapshot. + // Required: true + ContainerSnapshotPath *string `json:"container_snapshot_path"` + // Configuration for the backend that handles memory load. If this field is specified, `mem_file_path` is forbidden. Either `mem_backend` or `mem_file_path` must be present at a time. MemBackend *MemoryBackend `json:"mem_backend,omitempty"` @@ -51,6 +55,10 @@ type SnapshotLoadParams struct { func (m *SnapshotLoadParams) Validate(formats strfmt.Registry) error { var res []error + if err := m.validateContainerSnapshotPath(formats); err != nil { + res = append(res, err) + } + if err := m.validateMemBackend(formats); err != nil { res = append(res, err) } @@ -65,6 +73,15 @@ func (m *SnapshotLoadParams) Validate(formats strfmt.Registry) error { return nil } +func (m *SnapshotLoadParams) validateContainerSnapshotPath(formats strfmt.Registry) error { + + if err := validate.Required("container_snapshot_path", "body", m.ContainerSnapshotPath); err != nil { + return err + } + + return nil +} + func (m *SnapshotLoadParams) validateMemBackend(formats strfmt.Registry) error { if swag.IsZero(m.MemBackend) { // not required diff --git a/client/swagger.yaml b/client/swagger.yaml index df3d1d6b..14b5f7a2 100644 --- a/client/swagger.yaml +++ b/client/swagger.yaml @@ -1188,6 +1188,7 @@ definitions: the two `mem_*` fields must be present in the body of the request. required: - snapshot_path + - container_snapshot_path properties: enable_diff_snapshots: type: boolean @@ -1212,6 +1213,10 @@ definitions: type: boolean description: When set to true, the vm is also resumed if the snapshot load is successful. + container_snapshot_path: + type: string + description: + Path to the disk device backing the container snapshot. TokenBucket: type: object diff --git a/machine.go b/machine.go index a812b986..9860e1a8 100644 --- a/machine.go +++ b/machine.go @@ -172,7 +172,7 @@ type Config struct { } func (cfg *Config) hasSnapshot() bool { - return cfg.Snapshot.GetMemBackendPath() != "" || cfg.Snapshot.SnapshotPath != "" + return cfg.Snapshot.GetMemBackendPath() != "" || cfg.Snapshot.SnapshotPath != "" || cfg.Snapshot.ContainerSnapshotPath != "" } // Validate will ensure that the required fields are set and that @@ -243,6 +243,10 @@ func (cfg *Config) ValidateLoadSnapshot() error { return err } + if _, err := os.Stat(cfg.Snapshot.ContainerSnapshotPath); err != nil { + return err + } + return nil } @@ -1171,11 +1175,12 @@ func (m *Machine) CreateSnapshot(ctx context.Context, memFilePath, snapshotPath // loadSnapshot loads a snapshot of the VM func (m *Machine) loadSnapshot(ctx context.Context, snapshot *SnapshotConfig) error { snapshotParams := &models.SnapshotLoadParams{ - MemFilePath: snapshot.MemFilePath, - MemBackend: snapshot.MemBackend, - SnapshotPath: &snapshot.SnapshotPath, - EnableDiffSnapshots: snapshot.EnableDiffSnapshots, - ResumeVM: snapshot.ResumeVM, + MemFilePath: snapshot.MemFilePath, + MemBackend: snapshot.MemBackend, + SnapshotPath: &snapshot.SnapshotPath, + EnableDiffSnapshots: snapshot.EnableDiffSnapshots, + ResumeVM: snapshot.ResumeVM, + ContainerSnapshotPath: &snapshot.ContainerSnapshotPath, } if _, err := m.client.LoadSnapshot(ctx, snapshotParams); err != nil { diff --git a/opts.go b/opts.go index c3a70caf..027eb148 100644 --- a/opts.go +++ b/opts.go @@ -62,10 +62,11 @@ type WithSnapshotOpt func(*SnapshotConfig) // WithSnapshot( // "", snapshotPath, // WithMemoryBackend(models.MemoryBackendBackendTypeUffd, "uffd.sock")) -func WithSnapshot(memFilePath, snapshotPath string, opts ...WithSnapshotOpt) Opt { +func WithSnapshot(memFilePath, snapshotPath, ContainerSnapshotPath string, opts ...WithSnapshotOpt) Opt { return func(m *Machine) { m.Cfg.Snapshot.MemFilePath = memFilePath m.Cfg.Snapshot.SnapshotPath = snapshotPath + m.Cfg.Snapshot.ContainerSnapshotPath = ContainerSnapshotPath for _, opt := range opts { opt(&m.Cfg.Snapshot) diff --git a/snapshot.go b/snapshot.go index e27cf497..8ef8220b 100644 --- a/snapshot.go +++ b/snapshot.go @@ -16,11 +16,12 @@ package firecracker import "github.com/firecracker-microvm/firecracker-go-sdk/client/models" type SnapshotConfig struct { - MemFilePath string - MemBackend *models.MemoryBackend - SnapshotPath string - EnableDiffSnapshots bool - ResumeVM bool + MemFilePath string + MemBackend *models.MemoryBackend + SnapshotPath string + EnableDiffSnapshots bool + ResumeVM bool + ContainerSnapshotPath string } // GetMemBackendPath returns the effective memory backend path. If MemBackend