diff --git a/README.md b/README.md index ff4e976..fe92b50 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,37 @@ 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) +## Selecting the domains to snapshot + +`snapshot-libvirt-domains` iterates over the libvirt domains and hands each one to `zfs-autobackup` +under the backup name `libvirt-`, that is, the ZFS property `autobackup:libvirt-`. +Which datasets that property selects is `zfs-autobackup`'s decision, not zvirt's: it looks the +property up on every dataset, resolves inheritance, and tests its **value**: + +| Value | Selected | +| -------- | ------------------------------------------------ | +| `true` | yes, on the dataset and everything inheriting it | +| `false` | no, explicitly excluded | +| `child` | only the datasets that *inherit* it, not this one | +| `parent` | only this dataset, not the ones inheriting it | + +Inherited properties count, so the whole fleet can be configured on the parent dataset of all the +domains: + +```console +$ zfs set autobackup:libvirt-quay=true data/domains/quay # this domain and its children +$ zfs set autobackup:libvirt=true data/domains # the shared property, for pruning +``` + +A domain that carries the property nowhere is skipped with a message on stderr and the run carries +on with the next one, exiting 0 — configuring only some domains is a legitimate setup. + +> [!WARNING] +> The property selects **datasets**, not domains, and zvirt does not check that the selection +> actually covers a domain's storage. Set it on the domain's root dataset — set on a child only, the +> run reports success while the disks above it are never snapshotted. `zfs-autobackup --test +> --no-send --no-thinning libvirt-` prints the datasets it would select. + ## Snapshot retention `snapshot-libvirt-domains` runs `zfs-autobackup` with `--no-thinning`: it only creates snapshots and diff --git a/packaging/zvirt.spec b/packaging/zvirt.spec index 71b8513..d8a6dff 100644 --- a/packaging/zvirt.spec +++ b/packaging/zvirt.spec @@ -56,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.8-1 +- Let zfs-autobackup decide which domains are configured, so that an inherited + autobackup:libvirt- property is honoured + * 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 diff --git a/src/bin/snapshot-libvirt-domains b/src/bin/snapshot-libvirt-domains index 10c2662..4f935fd 100755 --- a/src/bin/snapshot-libvirt-domains +++ b/src/bin/snapshot-libvirt-domains @@ -58,6 +58,18 @@ function run () { "$@" } +# Same as run(), but keeps the command's stderr in the given file instead of +# letting it through: the caller needs to read it before deciding what it means. +function run_capturing_stderr () { + local stderr_file="$1" + shift + + if [ "$verbose" -eq 1 ]; then + echo "$*" >&2 + fi + "$@" 2>"$stderr_file" +} + OPTIND=1 # Reset in case getopts has been used previously in the shell. while getopts "h?lv-:" opt; do @@ -116,12 +128,11 @@ if [ "$live" -eq 1 ]; then else virsh_args+=("--all") fi +# zfs-autobackup's stderr is kept aside for the length of one domain, see below. +stderr_file="$(mktemp)" +trap 'rm -f "$stderr_file"' EXIT + for domain in $(virsh list --name "${virsh_args[@]}"); do - if [ "$(zfs get -t filesystem,volume autobackup:libvirt-${domain} -o value -H -s local)" == "" ]; then - echo "Skipping domain ${domain} because it is not configured for autobackup" >&2 - continue - fi - declare -a zfs_autobackup_hooks_args=() if [ "$live" -eq 1 ]; then zfs_autobackup_hooks_args+=("-l" "-r" "/var/lib/libvirt/images/${domain}") @@ -130,8 +141,36 @@ for domain in $(virsh list --name "${virsh_args[@]}"); do zfs_autobackup_hooks_args+=("-v") fi - run zfs-autobackup "${zfs_autobackup_args[@]}" \ + # Whether a domain is configured for autobackup is zfs-autobackup's question to + # answer, not ours: it looks the autobackup:libvirt- property up on + # every dataset, resolves inheritance and tests the *value* (true, false, + # child, parent). A 'zfs get -s local' guard here could only test the presence + # of a *local* property, pool-wide: it skipped every domain configured by + # inheritance from a parent dataset - the normal way to configure a fleet - + # and, having no dataset operand, let through any domain whose property + # happened to be set on some unrelated dataset. + # + # zfs-autobackup reports "nothing selected" as exit 255 with a distinctive + # message, so keep its stderr aside long enough to tell that case (skip this + # domain, carry on with the others) from a genuine failure (abort). + rc=0 + run_capturing_stderr "$stderr_file" \ + zfs-autobackup "${zfs_autobackup_args[@]}" \ --pre-snapshot-cmd "$SCRIPT_DIR/libvirt-hook ${zfs_autobackup_hooks_args[*]} -k pre $domain" \ --post-snapshot-cmd "$SCRIPT_DIR/libvirt-hook ${zfs_autobackup_hooks_args[*]} -k post $domain" \ - "libvirt-${domain}" + "libvirt-${domain}" || rc=$? + + if [ "$rc" -ne 0 ] && grep -qF "No source filesystems selected" "$stderr_file"; then + # Not an error here: the domain simply carries no autobackup property. + # zfs-autobackup's own wording is swallowed on purpose, it reads as a + # failure while nothing failed. + echo "Skipping domain ${domain} because it is not configured for autobackup" >&2 + continue + fi + + cat "$stderr_file" >&2 + if [ "$rc" -ne 0 ]; then + echo "Error: zfs-autobackup failed on domain ${domain} with exit code ${rc}" >&2 + exit "$rc" + fi done