Skip to content

Commit d7f6f88

Browse files
committed
refactor(test): Eliminate Snapshot.is_diff
We were using this property as a proxy for two things: "Does this snapshot rebase rebasing before resumptions?" and "do I need to enable dirty page tracking to create another snapshot of this type?". With mincore-snapshots, these two questions are no longer equivalent (mincore snapshots need rebase, but do _not_ need dirty page tracking), so untangle this property into two properties. Signed-off-by: Patrick Roy <[email protected]>
1 parent 1affb3a commit d7f6f88

File tree

1 file changed

+17
-9
lines changed

1 file changed

+17
-9
lines changed

tests/framework/microvm.py

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ def __repr__(self):
5656
cls_name = self.__class__.__name__
5757
return f"{cls_name}.{self.name}"
5858

59+
@property
60+
def needs_rebase(self) -> bool:
61+
"""Does this snapshot type need rebasing on top of a base snapshot before restoration?"""
62+
return self == SnapshotType.DIFF
63+
64+
@property
65+
def needs_dirty_page_tracking(self) -> bool:
66+
"""Does taking this snapshot type require dirty page tracking to be enabled?"""
67+
return self == SnapshotType.DIFF
68+
5969

6070
def hardlink_or_copy(src, dst):
6171
"""If src and dst are in the same device, hardlink. Otherwise, copy."""
@@ -79,15 +89,10 @@ class Snapshot:
7989
snapshot_type: SnapshotType
8090
meta: dict
8191

82-
@property
83-
def is_diff(self) -> bool:
84-
"""Is this a DIFF snapshot?"""
85-
return self.snapshot_type == SnapshotType.DIFF
86-
8792
def rebase_snapshot(self, base, use_snapshot_editor=False):
8893
"""Rebases current incremental snapshot onto a specified base layer."""
89-
if not self.is_diff:
90-
raise ValueError("Can only rebase DIFF snapshots")
94+
if not self.snapshot_type.needs_rebase:
95+
raise ValueError(f"Cannot rebase {self.snapshot_type}")
9196
if use_snapshot_editor:
9297
build_tools.run_snap_editor_rebase(base.mem, self.mem)
9398
else:
@@ -1067,7 +1072,7 @@ def restore_from_snapshot(
10671072
self.api.snapshot_load.put(
10681073
mem_backend=mem_backend,
10691074
snapshot_path=str(jailed_vmstate),
1070-
enable_diff_snapshots=jailed_snapshot.is_diff,
1075+
enable_diff_snapshots=jailed_snapshot.snapshot_type.needs_dirty_page_tracking,
10711076
resume_vm=resume,
10721077
**optional_kwargs,
10731078
)
@@ -1224,7 +1229,10 @@ def build_n_from_snapshot(
12241229
if incremental:
12251230
# When doing diff snapshots, we continuously overwrite the same base snapshot file from the first
12261231
# iteration in-place with successive snapshots, so don't delete it!
1227-
if last_snapshot is not None and not last_snapshot.is_diff:
1232+
if (
1233+
last_snapshot is not None
1234+
and not last_snapshot.snapshot_type.needs_rebase
1235+
):
12281236
last_snapshot.delete()
12291237

12301238
next_snapshot = microvm.make_snapshot(current_snapshot.snapshot_type)

0 commit comments

Comments
 (0)