Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3d85a72
api: update validate_size method to allow negative numbers
Guiiix Jun 25, 2025
73153cd
storage: add a new volume property snapshots_disabled
Guiiix Jun 25, 2025
1264ab1
storage: snapshots_disabled for file storage
Guiiix Jun 25, 2025
e872a76
storage: snapshots_disabled for lvm storage
Guiiix Jun 25, 2025
058eb92
storage: snapshots_disabled for reflink storage
Guiiix Jun 25, 2025
4b10f70
storage: snapshots_disabled for zfs storage
Guiiix Jun 25, 2025
464f079
storage: prevent setting disable snapshots when VM is running
Guiiix Jun 11, 2025
926ea32
prevent re-enabling snapshots when running VM
Guiiix Jun 15, 2025
f9c37cb
backup: prevent backup of running VM when private volume snapshots ar…
Guiiix Jun 11, 2025
f578702
storage: prevent setting revisions_to_keep < -1
Guiiix Jun 15, 2025
93addf1
storage: add a running state file for volumes
Guiiix Jun 17, 2025
1ad5da3
storage: add a running state file for volumes
Guiiix Jul 1, 2025
dd8308c
prevent startup if source volume snapshot disabled and is running
Guiiix Jul 1, 2025
0c1bd5a
prevent startup of vm having snapshots disabled and running DispVM
Guiiix Jun 19, 2025
8ef309c
don't allow to enable or disable snapshots if dispvm base on current …
Guiiix Jun 19, 2025
a3adbac
lint
Guiiix Jul 1, 2025
48d5c48
storage: prevent cloning when volumes without snapshot is running
Guiiix Jul 1, 2025
ee41bc0
storage: prune reflinks files when snapshots are disabled
Guiiix Jul 1, 2025
f0b01f6
use dirty file instead of clean file
Guiiix Jul 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions qubes/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,15 +245,21 @@ def enforce(predicate):
if not predicate:
raise PermissionDenied()

def validate_size(self, untrusted_size: bytes) -> int:
def validate_size(
self, untrusted_size: bytes, allow_negative: bool = False
) -> int:
self.enforce(isinstance(untrusted_size, bytes))
coefficient = 1
if allow_negative and untrusted_size.startswith(b"-"):
coefficient = -1
untrusted_size = untrusted_size[1:]
if not untrusted_size.isdigit():
raise qubes.exc.ProtocolError("Size must be ASCII digits (only)")
if len(untrusted_size) >= 20:
raise qubes.exc.ProtocolError("Sizes limited to 19 decimal digits")
if untrusted_size[0] == 48 and untrusted_size != b"0":
raise qubes.exc.ProtocolError("Spurious leading zeros not allowed")
return int(untrusted_size)
return int(untrusted_size) * coefficient


class QubesDaemonProtocol(asyncio.Protocol):
Expand Down
13 changes: 7 additions & 6 deletions qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ async def vm_volume_clone_to(self, untrusted_payload):
src_volume=src_volume, dst_volume=dst_volume
)
self.dest.volumes[self.arg] = await qubes.utils.coro_maybe(
dst_volume.import_volume(src_volume)
self.dest.storage.import_volume(dst_volume, src_volume)
)
self.app.save()

Expand Down Expand Up @@ -550,11 +550,9 @@ async def vm_volume_clear(self):
)
async def vm_volume_set_revisions_to_keep(self, untrusted_payload):
self.enforce(self.arg in self.dest.volumes.keys())
newvalue = self.validate_size(untrusted_payload)

newvalue = self.validate_size(untrusted_payload, allow_negative=True)
self.fire_event_for_permission(newvalue=newvalue)

self.dest.volumes[self.arg].revisions_to_keep = newvalue
self.dest.storage.set_revisions_to_keep(self.arg, newvalue)
self.app.save()

@qubes.api.method("admin.vm.volume.Set.rw", scope="local", write=True)
Expand Down Expand Up @@ -820,7 +818,10 @@ async def pool_set_revisions_to_keep(self, untrusted_payload):
self.enforce(self.dest.name == "dom0")
self.enforce(self.arg in self.app.pools.keys())
pool = self.app.pools[self.arg]
newvalue = self.validate_size(untrusted_payload)
newvalue = self.validate_size(untrusted_payload, allow_negative=True)

if newvalue < -1:
raise qubes.api.ProtocolError("Invalid value for revisions_to_keep")

self.fire_event_for_permission(newvalue=newvalue)

Expand Down
27 changes: 23 additions & 4 deletions qubes/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,20 @@
summary_line += fmt.format(size_to_human(vm_size))

if qid != 0 and vm_info.vm.is_running():
summary_line += (
" <-- The VM is running, backup will contain "
"its state from before its start!"
)
if [
volume
for volume in vm_info.vm.volumes.values()
if volume.snapshots_disabled
]:
summary_line += (
" <-- The VM is running and private volume snapshots "
"are disabled. Backup will fail!"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash this commit into "backup: prevent backup of running VM when private volume snapshots are disabled"?

)
else:
summary_line += (

Check warning on line 530 in qubes/backup.py

View check run for this annotation

Codecov / codecov/patch

qubes/backup.py#L530

Added line #L530 was not covered by tests
" <-- The VM is running, backup will "
"contain its state from before its start!"
)

summary += summary_line + "\n"

Expand Down Expand Up @@ -823,6 +833,15 @@
backup_app.domains[qid].features["backup-content"] = True
backup_app.domains[qid].features["backup-path"] = vm_info.subdir
backup_app.domains[qid].features["backup-size"] = vm_info.size

# VM running private volumes without snapshoting them
# (revision_to_keep = -1) must be powered off to be backup
if backup_app.domains[qid].is_running:
for volume in backup_app.domains[qid].volumes.values():
if volume.snapshots_disabled:
raise qubes.exc.QubesVMNotHaltedError(

Check warning on line 842 in qubes/backup.py

View check run for this annotation

Codecov / codecov/patch

qubes/backup.py#L839-L842

Added lines #L839 - L842 were not covered by tests
backup_app.domains[qid]
)
backup_app.save()
del backup_app

Expand Down
72 changes: 71 additions & 1 deletion qubes/storage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
from qubes.exc import StoragePoolException

STORAGE_ENTRY_POINT = "qubes.storage"
VOLUME_STATE_DIR = "/var/run/qubes/"
VOLUME_STATE_PREFIX = "volume-running-"
_am_root = os.getuid() == 0

BYTES_TO_ZERO = 1 << 16
Expand Down Expand Up @@ -96,7 +98,7 @@
snap_on_start=False,
source=None,
ephemeral=None,
**kwargs
**kwargs,
):
"""Initialize a volume.

Expand Down Expand Up @@ -513,6 +515,27 @@
msg = msg.format(str(self.__class__.__name__), method_name)
return NotImplementedError(msg)

@property
def snapshots_disabled(self) -> bool:
return (
self.revisions_to_keep == -1
and not self.snap_on_start
and self.save_on_stop
)

@property
def state_file(self) -> str:
return os.path.join(

Check warning on line 528 in qubes/storage/__init__.py

View check run for this annotation

Codecov / codecov/patch

qubes/storage/__init__.py#L528

Added line #L528 was not covered by tests
VOLUME_STATE_DIR,
VOLUME_STATE_PREFIX
+ f"{self.pool.name}:{self.vid}".replace("-", "--").replace(
"/", "-"
),
)

def is_running(self) -> bool:
return os.path.exists(self.state_file)

Check warning on line 537 in qubes/storage/__init__.py

View check run for this annotation

Codecov / codecov/patch

qubes/storage/__init__.py#L537

Added line #L537 was not covered by tests


class Storage:
"""Class for handling VM virtual disks.
Expand Down Expand Up @@ -786,8 +809,37 @@
else:
yield block_dev

def set_revisions_to_keep(self, volume, value):
if value < -1:
raise qubes.exc.QubesValueError(
"Invalid value for revisions_to_keep"
)

currentvalue = self.vm.volumes[volume].revisions_to_keep
enabling_disabling_snapshots = value != currentvalue and (
currentvalue == -1 or value == -1
)
if self.vm.is_running() and enabling_disabling_snapshots:
raise qubes.exc.QubesVMNotHaltedError(self.vm)

if self.vm.klass == "AppVM" and enabling_disabling_snapshots:
for vm in self.vm.dispvms:
if vm.is_running():
raise qubes.exc.QubesVMNotHaltedError(vm)

self.vm.volumes[volume].revisions_to_keep = value

async def start(self):
"""Execute the start method on each volume"""
for vol in self.vm.volumes.values():
if (
vol.source
and vol.source.snapshots_disabled
and vol.source.is_running()
):
raise qubes.exc.QubesVMError(
self.vm, f"Volume {vol.source.vid} is running"
)
await qubes.utils.void_coros_maybe(
# pylint: disable=line-too-long
(
Expand All @@ -800,6 +852,10 @@
for name, vol in self.vm.volumes.items()
)

for vol in self.vm.volumes.values():
with open(vol.state_file, "w", encoding="ascii"):
pass

Check warning on line 857 in qubes/storage/__init__.py

View check run for this annotation

Codecov / codecov/patch

qubes/storage/__init__.py#L855-L857

Added lines #L855 - L857 were not covered by tests

async def stop(self):
"""Execute the stop method on each volume"""
await qubes.utils.void_coros_maybe(
Expand All @@ -813,6 +869,8 @@
)
for name, vol in self.vm.volumes.items()
)
for vol in self.vm.volumes.values():
qubes.utils.remove_file(vol.state_file)

Check warning on line 873 in qubes/storage/__init__.py

View check run for this annotation

Codecov / codecov/patch

qubes/storage/__init__.py#L872-L873

Added lines #L872 - L873 were not covered by tests

def unused_frontend(self):
"""Find an unused device name"""
Expand Down Expand Up @@ -863,6 +921,18 @@
self.get_volume(volume).import_data_end(success=success)
)

async def import_volume(self, dst_volume: Volume, src_volume: Volume):
"""Helper function to import data from another volume"""
if src_volume.is_running() and src_volume.snapshots_disabled:
raise StoragePoolException(
f"Volume {src_volume.vid} must be stopped before importing its "
f"data"
)

return await qubes.utils.coro_maybe(
dst_volume.import_volume(src_volume)
)


class VolumesCollection:
"""Convenient collection wrapper for pool.get_volume and
Expand Down
17 changes: 12 additions & 5 deletions qubes/storage/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def revisions_to_keep(self):

@revisions_to_keep.setter
def revisions_to_keep(self, value):
if not isinstance(value, int) or value > 1 or value < 0:
if not isinstance(value, int) or value > 1 or value < -1:
raise NotImplementedError(
"FileVolume supports maximum 1 volume revision to keep"
)
Expand Down Expand Up @@ -490,9 +490,14 @@ async def start(self):
# shutdown routine wasn't called (power interrupt or so)
_remove_if_exists(self.path_cow)
if not os.path.exists(self.path_cow):
create_sparse_file(self.path_cow, self.size)
if not self.snapshots_disabled:
create_sparse_file(self.path_cow, self.size)
if not self.snap_on_start:
_check_path(self.path)

if self.snapshots_disabled:
return self

if hasattr(self, "path_source_cow"):
if not os.path.exists(self.path_source_cow):
create_sparse_file(self.path_source_cow, self.size)
Expand Down Expand Up @@ -524,10 +529,12 @@ async def stop(self):
self._export_lock is not FileVolume._marker_exported
), "trying to stop exported file volume?"
if self.save_on_stop or self.snap_on_start:
await self._destroy_blockdev()
if not self.snapshots_disabled:
await self._destroy_blockdev()
if self.save_on_stop:
if self.rw:
self.commit()
if not self.snapshots_disabled:
self.commit()
else:
_remove_if_exists(self.path_cow)
elif self.snap_on_start:
Expand Down Expand Up @@ -569,7 +576,7 @@ def script(self):

def _block_device_path(self):
if not self.snap_on_start:
if not self.save_on_stop:
if not self.save_on_stop or self.snapshots_disabled:
return self.path
return "-".join(
[
Expand Down
15 changes: 10 additions & 5 deletions qubes/storage/lvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,10 @@
try:
if self.snap_on_start or self.save_on_stop:
if not self.save_on_stop or not self.is_dirty():
await self._snapshot()
if self.snapshots_disabled and self.revisions:
await self._remove_revisions(self.revisions)

Check warning on line 784 in qubes/storage/lvm.py

View check run for this annotation

Codecov / codecov/patch

qubes/storage/lvm.py#L784

Added line #L784 was not covered by tests
if not self.snapshots_disabled:
await self._snapshot()
else:
await self._activate()
else:
Expand All @@ -795,7 +798,8 @@
changed = True
try:
if self.save_on_stop:
changed = await self._commit()
if not self.snapshots_disabled:
changed = await self._commit()
elif self.snap_on_start:
changed = await self._remove_if_exists(self._vid_snap)
else:
Expand Down Expand Up @@ -828,9 +832,10 @@
the libvirt XML template as <disk>.
"""
if self.snap_on_start or self.save_on_stop:
snap_path = "/dev/mapper/" + self._vid_snap.replace(
"-", "--"
).replace("/", "-")
vid = self.vid if self.snapshots_disabled else self._vid_snap
snap_path = "/dev/mapper/" + vid.replace("-", "--").replace(
"/", "-"
)
return qubes.storage.BlockDevice(
snap_path, self.name, None, self.rw, self.domain, self.devtype
)
Expand Down
14 changes: 13 additions & 1 deletion qubes/storage/reflink.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@
def _update_precache(self):
_remove_file(self._path_precache)
yield
if self.snapshots_disabled:
return
_copy_file(self._path_clean, self._path_precache, copy_mtime=True)

def _remove_stale_precache(self):
Expand Down Expand Up @@ -263,6 +265,13 @@
@_coroutinized
def start(self): # pylint: disable=invalid-overridden-method
self._remove_incomplete_images()
if self.snapshots_disabled:
self._prune_revisions(keep=0)
_remove_file(self._path_precache)
if not self.is_dirty():
_rename_file(self._path_clean, self._path_dirty)

return self
if not self.is_dirty():
if self.snap_on_start:
_remove_file(self._path_clean)
Expand All @@ -283,7 +292,10 @@
@_coroutinized
def stop(self): # pylint: disable=invalid-overridden-method
if self.is_dirty():
self._commit(self._path_dirty)
if self.snapshots_disabled:
_rename_file(self._path_dirty, self._path_clean)
else:
self._commit(self._path_dirty)

Check warning on line 298 in qubes/storage/reflink.py

View check run for this annotation

Codecov / codecov/patch

qubes/storage/reflink.py#L298

Added line #L298 was not covered by tests
elif not self.save_on_stop:
if not self.snap_on_start:
self._size = self.size # preserve manual resize of image
Expand Down
Loading