Let zfs-autobackup decide which domains are configured

Before handing a domain over, snapshot-libvirt-domains guarded it behind
'zfs get -s local autobackup:libvirt-<domain>' and tested the result for
non-emptiness. That answers neither of the questions that matter.

-s local drops exactly the inherited lines, while zfs-autobackup looks the
property up without -s and resolves inheritance (ZfsNode.selected_datasets
rewrites "inherited from <parent>" to the parent's source, ZfsDataset.is_selected
takes an inherited flag). Configuring the fleet once on the parent dataset - the
normal way to drive zfs-autobackup, and the only way to cover datasets that
already exist without walking them one by one - therefore made zvirt skip every
domain, while the very same backup names handed to zfs-autobackup by hand
snapshotted all of them.

The guard also had no dataset operand, so it asked "does any dataset in the
pools carry this property locally?"; and it tested presence where zfs-autobackup
tests value, so a local =false passed it, and child/parent were misjudged in
both directions.

Drop the guard and let zfs-autobackup answer, since it is the one selecting.
It reports "nothing selected" as exit 255 with a distinctive message, so keep
its stderr aside for the length of one domain, long enough to tell that case
(skip the domain, carry on with the fleet, exit 0) from a genuine failure
(replay the stderr, abort with its exit code). Its own wording is swallowed in
the skip case: it is printed as an error while nothing failed.

Document the selection in the README - the four property values, the fact that
inheritance counts, and the two properties involved (autobackup:libvirt-<domain>
to snapshot, autobackup:libvirt to prune).

This does not cover the case of a domain whose selection misses part of its
storage: the property selects datasets, not domains, and only zvirt knows what a
domain is made of. Tracked in #3.

Closes #2

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014BSA2qQpCr7cY4qDEX7kuX

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-<domain>`, that is, the ZFS property `autobackup:libvirt-<domain>`.
+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-<domain>` prints the datasets it would select.
+
 ## Snapshot retention
 
 `snapshot-libvirt-domains` runs `zfs-autobackup` with `--no-thinning`: it only creates snapshots and

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é <nicolas.masse@itix.fr> - 0.0.8-1
+- Let zfs-autobackup decide which domains are configured, so that an inherited
+  autobackup:libvirt-<domain> property is honoured
+
 * Wed Sep 02 2026 Nicolas Massé <nicolas.masse@itix.fr> - 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

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-<domain> 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