From c4a1d54d86ad1a3b4c08b8e900211b3c37aecef8 Mon Sep 17 00:00:00 2001 From: Nicolas MASSE Date: Wed, 2 Sep 2026 14:40:09 +0200 Subject: [PATCH] Make the snapshot name format queryable The snapshot format was hardcoded in snapshot-libvirt-domains, so any tool pruning zvirt's snapshots had to copy the same literal. On a mismatch, zfs-autobackup selects the right datasets, matches none of their snapshots, destroys nothing and still exits 0 - snapshots then accumulate forever while the retention job reports success. Move the format to a sourceable file, /usr/share/zvirt/snapshot-format, that snapshot-libvirt-domains sources instead of hardcoding, so producer and consumer cannot drift. Also expose it as --print-snapshot-format for consumers that would rather not source a file. Document the retention run, the deliberate absence of a {} placeholder (it is what allows a single prune run to cover every domain), and the systemd specifier escaping trap. While here, fix the lint target: bin/*.sh matched no file, since neither script carries a .sh extension. Closes #1 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XaqxX5X9VGwQSvNEBYyZq1 --- Makefile | 6 +++- README.md | 45 +++++++++++++++++++++++++++ packaging/zvirt.spec | 6 ++++ src/bin/snapshot-libvirt-domains | 52 ++++++++++++++++++++++++++++++-- src/share/zvirt/snapshot-format | 32 ++++++++++++++++++++ 5 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 src/share/zvirt/snapshot-format diff --git a/Makefile b/Makefile index 89695e1..1b6098c 100644 --- a/Makefile +++ b/Makefile @@ -25,11 +25,15 @@ install: @install -d $(PREFIX)/bin @install -m 755 src/bin/libvirt-hook $(PREFIX)/bin/libvirt-hook @install -m 755 src/bin/snapshot-libvirt-domains $(PREFIX)/bin/snapshot-libvirt-domains + @install -d $(PREFIX)/share/zvirt + @install -m 644 src/share/zvirt/snapshot-format $(PREFIX)/share/zvirt/snapshot-format uninstall: @echo "Uninstalling zvirt..." @rm -f $(PREFIX)/bin/libvirt-hook @rm -f $(PREFIX)/bin/snapshot-libvirt-domains + @rm -f $(PREFIX)/share/zvirt/snapshot-format + @rmdir --ignore-fail-on-non-empty $(PREFIX)/share/zvirt tarball: @echo "Creating release tarball..." @@ -87,4 +91,4 @@ clean: lint: prerequisites @echo "Linting..." - @cd src && shellcheck --severity=error bin/*.sh + @cd src && shellcheck --severity=error bin/* diff --git a/README.md b/README.md index 80c058c..ff4e976 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,51 @@ At the end, all components of a domain - Domain definition, TPM, NVRAM, VirtioFS - Support both crash-consistent and live snapshots. - Support batch mode (pause all domains, take snapshots, then resume all domains) +## Snapshot retention + +`snapshot-libvirt-domains` runs `zfs-autobackup` with `--no-thinning`: it only creates snapshots and +never deletes any. Retention is left to a separate `zfs-autobackup` run in snapshot-only mode: + +```console +$ zfs-autobackup --no-snapshot --keep-source 1w1d,1m1w,1y1m \ + --snapshot-format "$(snapshot-libvirt-domains --print-snapshot-format)" libvirt +``` + +That run **must** be given the very same snapshot name format zvirt snapshotted with: `zfs-autobackup` +only thins snapshots whose name parses against the format it was given. On a mismatch it selects the +right datasets, matches none of their snapshots, deletes nothing and still exits 0 with +`All operations completed successfully` — so never hardcode the format, always query it. + +Two ways to query it: + +```console +$ snapshot-libvirt-domains --print-snapshot-format +libvirt-%Y-%m-%d-%H:%M:%S + +$ . /usr/share/zvirt/snapshot-format && echo "$ZVIRT_SNAPSHOT_FORMAT" +libvirt-%Y-%m-%d-%H:%M:%S +``` + +`/usr/share/zvirt/snapshot-format` is the single source of truth: `snapshot-libvirt-domains` sources +it too, so the format cannot drift between the tool that writes the snapshots and the tool that +prunes them. + +The format deliberately carries no `{}` placeholder (`zfs-autobackup`'s default is +`{}-%Y%m%d%H%M%S`, where `{}` expands to the backup name). All domains therefore share a single +snapshot name pattern, which is what allows one prune run over the shared `autobackup:libvirt` +property to cover every domain at once, instead of one invocation per domain. + +### Pruning from a systemd unit + +`%Y`, `%m`, `%d`, `%H`, `%M` and `%S` are systemd specifiers: written literally in an `ExecStart=` +they are silently expanded into paths and hostnames, and `systemd-analyze verify` does not catch it. +Either double them (`%%Y`, `%%m`, ...) or — better — let the unit call a wrapper that queries the +format: + +```ini +ExecStart=/bin/bash -c '. /usr/share/zvirt/snapshot-format; exec /usr/bin/zfs-autobackup --no-snapshot --keep-source 1w1d,1m1w,1y1m --snapshot-format "$ZVIRT_SNAPSHOT_FORMAT" libvirt' +``` + ## License MIT License diff --git a/packaging/zvirt.spec b/packaging/zvirt.spec index c0eba5b..68097a7 100644 --- a/packaging/zvirt.spec +++ b/packaging/zvirt.spec @@ -47,6 +47,8 @@ pip install --root %{buildroot} --prefix %{_prefix} --no-compile --no-deps --no- %files %{_bindir}/libvirt-hook %{_bindir}/snapshot-libvirt-domains +%dir %{_datadir}/zvirt/ +%{_datadir}/zvirt/snapshot-format %{_bindir}/zfs-autobackup %{_bindir}/zfs-autoverify %{_bindir}/zfs-check @@ -54,6 +56,10 @@ pip install --root %{buildroot} --prefix %{_prefix} --no-compile --no-deps --no- %{python3_sitelib}/zfs_autobackup-*.dist-info/ %changelog +* Wed Sep 02 2026 Nicolas Massé - 0.0.7-1 +- Make the snapshot name format queryable (--print-snapshot-format) +- Ship /usr/share/zvirt/snapshot-format as the single source of truth + * Wed Apr 22 2026 Nicolas Massé - 0.0.6-1 - Switch to zfs-autobackup + hooks diff --git a/src/bin/snapshot-libvirt-domains b/src/bin/snapshot-libvirt-domains index e3cb29d..10c2662 100755 --- a/src/bin/snapshot-libvirt-domains +++ b/src/bin/snapshot-libvirt-domains @@ -4,7 +4,22 @@ set -Eeuo pipefail verbose=0 live=0 +print_snapshot_format=0 SCRIPT_DIR="$(dirname "$(readlink -f "$0")")" +SNAPSHOT_FORMAT_FILE="$(readlink -f "$SCRIPT_DIR/../share/zvirt/snapshot-format")" + +# The snapshot name format lives in a single sourceable file so that tools +# pruning zvirt's snapshots can read it instead of copying it. See that file. +if [ ! -r "$SNAPSHOT_FORMAT_FILE" ]; then + echo "Error: cannot read the snapshot format file: $SNAPSHOT_FORMAT_FILE" >&2 + exit 1 +fi +# shellcheck source=../share/zvirt/snapshot-format +source "$SNAPSHOT_FORMAT_FILE" +if [ -z "${ZVIRT_SNAPSHOT_FORMAT:-}" ]; then + echo "Error: ZVIRT_SNAPSHOT_FORMAT is not set in $SNAPSHOT_FORMAT_FILE" >&2 + exit 1 +fi function show_help () { cat << EOF @@ -12,11 +27,27 @@ Snapshots all libvirt domains on the host. It uses the zfs-autobackup tool to create snapshots of the domains' using the ZFS native tools. Usage: ${0##*/} [-l] [-h] [-v] + ${0##*/} --print-snapshot-format Options: -h display this help and exit -v verbose mode -l live snapshot mode (default is crash-consistent) + + --print-snapshot-format + print the snapshot name format used by zvirt and exit + +Snapshots are created with --no-thinning: retention is left to a separate +zfs-autobackup run in snapshot-only mode, which must be given the very same +snapshot format, otherwise it matches no snapshot, deletes nothing and still +exits successfully: + + ${0##*/} --print-snapshot-format + zfs-autobackup --no-snapshot --keep-source 1w1d,1m1w,1y1m \\ + --snapshot-format "\$(${0##*/} --print-snapshot-format)" libvirt + +The format can also be sourced from ${SNAPSHOT_FORMAT_FILE} +as \$ZVIRT_SNAPSHOT_FORMAT. EOF } @@ -29,7 +60,7 @@ function run () { OPTIND=1 # Reset in case getopts has been used previously in the shell. -while getopts "h?lv" opt; do +while getopts "h?lv-:" opt; do case "$opt" in h|\?) show_help @@ -39,6 +70,18 @@ while getopts "h?lv" opt; do ;; l) live=1 ;; + -) case "$OPTARG" in + help) show_help + exit 0 + ;; + print-snapshot-format) print_snapshot_format=1 + ;; + *) echo "Error: Unknown option: --$OPTARG" >&2 + show_help >&2 + exit 1 + ;; + esac + ;; *) show_help >&2 exit 1 ;; @@ -55,12 +98,17 @@ if [ $# -ne 0 ]; then exit 1 fi +if [ "$print_snapshot_format" -eq 1 ]; then + echo "$ZVIRT_SNAPSHOT_FORMAT" + exit 0 +fi + declare -a zfs_autobackup_args=() if [ "$verbose" -eq 1 ]; then zfs_autobackup_args+=("-v") fi zfs_autobackup_args+=("--no-send" "--no-thinning") -zfs_autobackup_args+=("--snapshot-format" "libvirt-%Y-%m-%d-%H:%M:%S") +zfs_autobackup_args+=("--snapshot-format" "$ZVIRT_SNAPSHOT_FORMAT") declare -a virsh_args=() if [ "$live" -eq 1 ]; then diff --git a/src/share/zvirt/snapshot-format b/src/share/zvirt/snapshot-format new file mode 100644 index 0000000..8c27272 --- /dev/null +++ b/src/share/zvirt/snapshot-format @@ -0,0 +1,32 @@ +# shellcheck shell=bash +# Snapshot name format used by zvirt. +# +# This file is the single source of truth: snapshot-libvirt-domains sources it +# instead of hardcoding the format, so any tool that prunes zvirt's snapshots +# can source it too and cannot drift from what was actually written. +# +# Consumers: +# +# . /usr/share/zvirt/snapshot-format +# zfs-autobackup --no-snapshot --keep-source 1w1d,1m1w,1y1m \ +# --snapshot-format "$ZVIRT_SNAPSHOT_FORMAT" libvirt +# +# or, without sourcing: +# +# snapshot-libvirt-domains --print-snapshot-format +# +# A mismatched format is silently harmless-looking: zfs-autobackup only thins +# snapshots whose name parses against the format it was given, so a wrong +# format destroys nothing and still exits 0. +# +# Note: the format carries no '{}' placeholder on purpose (zfs-autobackup's +# default is '{}-%Y%m%d%H%M%S', where '{}' expands to the backup name). All +# domains therefore share a single snapshot name pattern, which is what allows +# one prune run over the shared 'autobackup:libvirt' property to cover every +# domain at once, instead of one invocation per domain. Do not add '{}' here. +# +# Note for systemd units: '%Y', '%m', '%d', '%H', '%M' and '%S' are systemd +# specifiers and must be doubled in ExecStart= ('%%Y', '%%m', ...). Sourcing +# this file from a wrapper script avoids the problem entirely. + +ZVIRT_SNAPSHOT_FORMAT='libvirt-%Y-%m-%d-%H:%M:%S'