Skip to content

Commit 3b9264e

Browse files
committed
Fix snapshot compression review follow-ups
1 parent 460c265 commit 3b9264e

5 files changed

Lines changed: 61 additions & 11 deletions

File tree

cmd/api/config/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ func (c *Config) Validate() error {
489489
}
490490
}
491491
}
492+
c.Snapshot.CompressionDefault.Algorithm = algorithm
492493
}
493494
if c.Hypervisor.Memory.KernelPageInitMode != "performance" && c.Hypervisor.Memory.KernelPageInitMode != "hardened" {
494495
return fmt.Errorf("hypervisor.memory.kernel_page_init_mode must be one of {performance,hardened}, got %q", c.Hypervisor.Memory.KernelPageInitMode)

cmd/api/config/config_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,15 @@ func TestValidateRejectsInvalidVMLabelBudget(t *testing.T) {
8787
func TestValidateAllowsLZ4CompressionDefaultWithImplicitLevel(t *testing.T) {
8888
cfg := defaultConfig()
8989
cfg.Snapshot.CompressionDefault.Enabled = true
90-
cfg.Snapshot.CompressionDefault.Algorithm = "lz4"
90+
cfg.Snapshot.CompressionDefault.Algorithm = "LZ4"
9191
cfg.Snapshot.CompressionDefault.Level = nil
9292

9393
if err := cfg.Validate(); err != nil {
9494
t.Fatalf("expected lz4 compression default to validate, got %v", err)
9595
}
96+
if cfg.Snapshot.CompressionDefault.Algorithm != "lz4" {
97+
t.Fatalf("expected algorithm to normalize to lowercase, got %q", cfg.Snapshot.CompressionDefault.Algorithm)
98+
}
9699
}
97100

98101
func TestValidateAllowsExplicitLZ4CompressionLevelRange(t *testing.T) {

lib/instances/snapshot.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,9 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS
393393
if target != nil {
394394
m.recordSnapshotCompressionPreemption(ctx, snapshotCompressionPreemptionForkSnapshot, *target)
395395
}
396+
if err := m.ensureSnapshotMemoryReady(ctx, m.paths.SnapshotGuestDir(snapshotID), "", rec.StoredMetadata.HypervisorType); err != nil {
397+
return nil, fmt.Errorf("prepare snapshot memory for fork: %w", err)
398+
}
396399

397400
if err := forkvm.CopyGuestDirectory(m.paths.SnapshotGuestDir(snapshotID), dstDir); err != nil {
398401
if errors.Is(err, forkvm.ErrSparseCopyUnsupported) {

lib/instances/snapshot_compression.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ func (m *manager) startCompressionJob(ctx context.Context, target compressionTar
320320
var uncompressedSize int64
321321
var compressedSize int64
322322
metricsCtx := context.Background()
323+
log := logger.FromContext(ctx)
323324

324325
defer func() {
325326
m.recordSnapshotCompressionJob(metricsCtx, target, result, start, uncompressedSize, compressedSize)
@@ -329,7 +330,6 @@ func (m *manager) startCompressionJob(ctx context.Context, target compressionTar
329330
close(job.done)
330331
}()
331332

332-
log := logger.FromContext(ctx)
333333
rawPath, ok := findRawSnapshotMemoryFile(target.SnapshotDir)
334334
if !ok {
335335
if compressedPath, algorithm, found := findCompressedSnapshotMemoryFile(target.SnapshotDir); found && target.SnapshotID != "" {
@@ -339,8 +339,8 @@ func (m *manager) startCompressionJob(ctx context.Context, target compressionTar
339339
size := st.Size()
340340
compressedSizeBytes = &size
341341
}
342-
if metaErr := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateCompressed, "", &cfg, compressedSizeBytes, nil); metaErr != nil {
343-
log.ErrorContext(ctx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "error", metaErr)
342+
if err := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateCompressed, "", &cfg, compressedSizeBytes, nil); err != nil {
343+
log.ErrorContext(jobCtx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "snapshot_dir", target.SnapshotDir, "state", snapshotstore.SnapshotCompressionStateCompressed, "error", err)
344344
}
345345
}
346346
return
@@ -352,25 +352,25 @@ func (m *manager) startCompressionJob(ctx context.Context, target compressionTar
352352
if errors.Is(err, context.Canceled) {
353353
result = snapshotCompressionResultCanceled
354354
if target.SnapshotID != "" {
355-
if metaErr := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateNone, "", nil, nil, nil); metaErr != nil {
356-
log.ErrorContext(ctx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "error", metaErr)
355+
if err := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateNone, "", nil, nil, nil); err != nil {
356+
log.ErrorContext(jobCtx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "snapshot_dir", target.SnapshotDir, "state", snapshotstore.SnapshotCompressionStateNone, "error", err)
357357
}
358358
}
359359
return
360360
}
361361
result = snapshotCompressionResultFailed
362362
if target.SnapshotID != "" {
363-
if metaErr := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateError, err.Error(), &target.Policy, nil, nil); metaErr != nil {
364-
log.ErrorContext(ctx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "error", metaErr)
363+
if metadataErr := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateError, err.Error(), &target.Policy, nil, nil); metadataErr != nil {
364+
log.ErrorContext(jobCtx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "snapshot_dir", target.SnapshotDir, "state", snapshotstore.SnapshotCompressionStateError, "error", metadataErr)
365365
}
366366
}
367-
log.WarnContext(ctx, "snapshot compression failed", "snapshot_dir", target.SnapshotDir, "error", err)
367+
log.WarnContext(jobCtx, "snapshot compression failed", "snapshot_dir", target.SnapshotDir, "error", err)
368368
return
369369
}
370370

371371
if target.SnapshotID != "" {
372-
if metaErr := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateCompressed, "", &target.Policy, &compressedSize, &uncompressedSize); metaErr != nil {
373-
log.ErrorContext(ctx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "error", metaErr)
372+
if err := m.updateSnapshotCompressionMetadata(target.SnapshotID, snapshotstore.SnapshotCompressionStateCompressed, "", &target.Policy, &compressedSize, &uncompressedSize); err != nil {
373+
log.ErrorContext(jobCtx, "failed to update snapshot compression metadata", "snapshot_id", target.SnapshotID, "snapshot_dir", target.SnapshotDir, "state", snapshotstore.SnapshotCompressionStateCompressed, "error", err)
374374
}
375375
}
376376
}()

lib/instances/snapshot_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,49 @@ func TestCreateStandbySnapshotFromCompressedSourceCopiesRawMemory(t *testing.T)
237237
assert.False(t, ok, "snapshot copy should not inherit compressed memory artifacts from the source standby instance")
238238
}
239239

240+
func TestForkSnapshotFromCompressedSourceCopiesRawMemory(t *testing.T) {
241+
t.Parallel()
242+
243+
mgr, _ := setupTestManager(t)
244+
ctx := context.Background()
245+
246+
hvType := mgr.defaultHypervisor
247+
sourceID := "snapshot-fork-compressed-src"
248+
createStandbySnapshotSourceFixture(t, mgr, sourceID, "snapshot-fork-compressed-src", hvType)
249+
250+
snap, err := mgr.CreateSnapshot(ctx, sourceID, CreateSnapshotRequest{
251+
Kind: SnapshotKindStandby,
252+
Name: "standby-for-fork-compressed",
253+
})
254+
require.NoError(t, err)
255+
256+
snapshotDir := mgr.paths.SnapshotGuestDir(snap.Id)
257+
rawPath := filepath.Join(snapshotDir, "memory-ranges")
258+
snapshotConfigPath := filepath.Join(snapshotDir, "snapshots", "snapshot-latest", "config.json")
259+
require.NoError(t, os.MkdirAll(filepath.Dir(snapshotConfigPath), 0o755))
260+
require.NoError(t, os.WriteFile(snapshotConfigPath, []byte(`{}`), 0o644))
261+
require.NoError(t, os.WriteFile(rawPath, []byte("some guest memory"), 0o644))
262+
_, _, err = compressSnapshotMemoryFile(ctx, rawPath, snapshotstore.SnapshotCompressionConfig{
263+
Enabled: true,
264+
Algorithm: snapshotstore.SnapshotCompressionAlgorithmZstd,
265+
Level: intPtr(1),
266+
})
267+
require.NoError(t, err)
268+
269+
forked, err := mgr.ForkSnapshot(ctx, snap.Id, ForkSnapshotRequest{
270+
Name: "snapshot-fork-compressed",
271+
TargetState: StateStopped,
272+
})
273+
require.NoError(t, err)
274+
t.Cleanup(func() { _ = mgr.DeleteInstance(context.Background(), forked.Id) })
275+
276+
forkSnapshotDir := mgr.paths.InstanceDir(forked.Id)
277+
_, ok := findRawSnapshotMemoryFile(forkSnapshotDir)
278+
assert.True(t, ok, "forked snapshot payload should contain raw memory after preparing a compressed snapshot source")
279+
_, _, ok = findCompressedSnapshotMemoryFile(forkSnapshotDir)
280+
assert.False(t, ok, "forked snapshot payload should not retain compressed memory artifacts from the source snapshot")
281+
}
282+
240283
func createStoppedSnapshotSourceFixture(t *testing.T, mgr *manager, id, name string, hvType hypervisor.Type) {
241284
t.Helper()
242285
require.NoError(t, mgr.ensureDirectories(id))

0 commit comments

Comments
 (0)