Support Btrfs subvolume paths in "source_directories" even when the subvolume is mounted elsewhere (#1043).

Reviewed-on: https://projects.torsion.org/borgmatic-collective/borgmatic/pulls/1113

NEWS

@@ -1,5 +1,7 @@
 2.0.7.dev0
  * #1032: Fix a bug in which a Borg archive gets created even when a database hook fails.
+ * #1043: Support Btrfs subvolume paths in "source_directories" even when the subvolume is mounted
+   elsewhere.
  * #1083: Add "debug_passphrase"/"display_passphrase" options and a "{unixtime}" placeholder in
    support of Borg 2 features.
  * #1099: Clarify documentation on command hooks order of execution.

borgmatic/hooks/data_source/btrfs.py

@@ -1,5 +1,6 @@
 import collections
 import glob
+import itertools
 import json
 import logging
 import os
@@ -21,9 +22,44 @@ def use_streaming(hook_config, config):  # pragma: no cover
     return False
 
 
-def get_subvolume_mount_points(findmnt_command):
+def get_contained_subvolume_paths(btrfs_command, subvolume_path):
     '''
-    Given a findmnt command to run, get all sorted Btrfs subvolume mount points.
+    Given the path of a Btrfs subvolume, return it in a sequence along with the paths of its
+    contained subvolumes.
+
+    If the btrfs command errors, log that error and return an empty sequence.
+    '''
+    try:
+        btrfs_output = borgmatic.execute.execute_command_and_capture_output(
+            tuple(btrfs_command.split(' '))
+            + (
+                'subvolume',
+                'list',
+                subvolume_path,
+            ),
+            close_fds=True,
+        )
+    except subprocess.CalledProcessError as error:
+        logger.debug(
+            f'Ignoring Btrfs subvolume {subvolume_path} because of error listing its subvolumes: {error}'
+        )
+
+        return ()
+
+    return (subvolume_path,) + tuple(
+        os.path.join(subvolume_path, line.split(' ')[-1])
+        for line in btrfs_output.splitlines()
+        if line.strip()
+    )
+
+
+FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION = 'subvolid=5'
+
+
+def get_all_subvolume_paths(btrfs_command, findmnt_command):
+    '''
+    Given btrfs and findmnt commands to run, get the sorted paths for all Btrfs subvolumes on the
+    system.
     '''
     findmnt_output = borgmatic.execute.execute_command_and_capture_output(
         tuple(findmnt_command.split(' '))
@@ -38,7 +74,22 @@ def get_subvolume_mount_points(findmnt_command):
 
     try:
         return tuple(
-            sorted(filesystem['target'] for filesystem in json.loads(findmnt_output)['filesystems'])
+            sorted(
+                itertools.chain.from_iterable(
+                    # If findmnt gave us a Btrfs root filesystem, list the subvolumes within it.
+                    # This is necessary because findmnt only returns a subvolume's mount point
+                    # rather than its original subvolume path (which can differ). For instance,
+                    # a subvolume might exist at /mnt/subvolume but be mounted at /home/myuser.
+                    # findmnt is still useful though because it's a global way to discover all
+                    # Btrfs subvolumes—even if we have to do some additional legwork ourselves.
+                    (
+                        get_contained_subvolume_paths(btrfs_command, filesystem['target'])
+                        if FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION in filesystem['options'].split(',')
+                        else (filesystem['target'],)
+                    )
+                    for filesystem in json.loads(findmnt_output)['filesystems']
+                )
+            )
         )
     except json.JSONDecodeError as error:
         raise ValueError(f'Invalid {findmnt_command} JSON output: {error}')
@@ -50,6 +101,12 @@ Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'),
 
 
 def get_subvolume_property(btrfs_command, subvolume_path, property_name):
+    '''
+    Given a btrfs command, a subvolume path, and a property name to lookup, return the value of the
+    corresponding property.
+
+    Raise subprocess.CalledProcessError if the btrfs command errors.
+    '''
     output = borgmatic.execute.execute_command_and_capture_output(
         tuple(btrfs_command.split(' '))
         + (
@@ -74,19 +131,24 @@ def get_subvolume_property(btrfs_command, subvolume_path, property_name):
     }.get(value, value)
 
 
-def omit_read_only_subvolume_mount_points(btrfs_command, subvolume_paths):
+def omit_read_only_subvolume_paths(btrfs_command, subvolume_paths):
     '''
-    Given a Btrfs command to run and a sequence of Btrfs subvolume mount points, filter them down to
-    just those that are read-write. The idea is that Btrfs can't actually snapshot a read-only
-    subvolume, so we should just ignore them.
+    Given a Btrfs command to run and a sequence of Btrfs subvolume paths, filter them down to just
+    those that are read-write. The idea is that Btrfs can't actually snapshot a read-only subvolume,
+    so we should just ignore them.
     '''
     retained_subvolume_paths = []
 
     for subvolume_path in subvolume_paths:
-        if get_subvolume_property(btrfs_command, subvolume_path, 'ro'):
-            logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only')
-        else:
-            retained_subvolume_paths.append(subvolume_path)
+        try:
+            if get_subvolume_property(btrfs_command, subvolume_path, 'ro'):
+                logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only')
+            else:
+                retained_subvolume_paths.append(subvolume_path)
+        except subprocess.CalledProcessError as error:
+            logger.debug(
+                f'Error determining read-only status of Btrfs subvolume {subvolume_path}: {error}'
+            )
 
     return tuple(retained_subvolume_paths)
 
@@ -94,32 +156,32 @@ def omit_read_only_subvolume_mount_points(btrfs_command, subvolume_paths):
 def get_subvolumes(btrfs_command, findmnt_command, patterns=None):
     '''
     Given a Btrfs command to run and a sequence of configured patterns, find the intersection
-    between the current Btrfs filesystem and subvolume mount points and the paths of any patterns.
-    The idea is that these pattern paths represent the requested subvolumes to snapshot.
+    between the current Btrfs filesystem and subvolume paths and the paths of any patterns.  The
+    idea is that these pattern paths represent the requested subvolumes to snapshot.
 
     Only include subvolumes that contain at least one root pattern sourced from borgmatic
     configuration (as opposed to generated elsewhere in borgmatic). But if patterns is None, then
     return all subvolumes instead, sorted by path.
 
-    Return the result as a sequence of matching subvolume mount points.
+    Return the result as a sequence of matching Subvolume instances.
     '''
     candidate_patterns = set(patterns or ())
     subvolumes = []
 
-    # For each subvolume mount point, match it against the given patterns to find the subvolumes to
+    # For each subvolume path, match it against the given patterns to find the subvolumes to
     # backup. Sort the subvolumes from longest to shortest mount points, so longer mount points get
     # a whack at the candidate pattern piñata before their parents do. (Patterns are consumed during
     # this process, so no two subvolumes end up with the same contained patterns.)
-    for mount_point in reversed(
-        omit_read_only_subvolume_mount_points(
-            btrfs_command, get_subvolume_mount_points(findmnt_command)
+    for subvolume_path in reversed(
+        omit_read_only_subvolume_paths(
+            btrfs_command, get_all_subvolume_paths(btrfs_command, findmnt_command)
         )
     ):
         subvolumes.extend(
-            Subvolume(mount_point, contained_patterns)
+            Subvolume(subvolume_path, contained_patterns)
             for contained_patterns in (
                 borgmatic.hooks.data_source.snapshot.get_contained_patterns(
-                    mount_point, candidate_patterns
+                    subvolume_path, candidate_patterns
                 ),
             )
             if patterns is None
@@ -244,9 +306,9 @@ def dump_data_sources(
     '''
     Given a Btrfs configuration dict, a configuration dict, the borgmatic configuration file paths,
     the borgmatic runtime directory, the configured patterns, and whether this is a dry run,
-    auto-detect and snapshot any Btrfs subvolume mount points listed in the given patterns. Also
-    update those patterns, replacing subvolume mount points with corresponding snapshot directories
-    so they get stored in the Borg archive instead.
+    auto-detect and snapshot any Btrfs subvolume paths listed in the given patterns. Also update
+    those patterns, replacing subvolume paths with corresponding snapshot directories so they get
+    stored in the Borg archive instead.
 
     Return an empty sequence, since there are no ongoing dump processes from this hook.
 
@@ -328,8 +390,8 @@ def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, d
         logger.debug(error)
         return
 
-    # Reversing the sorted subvolumes ensures that we remove longer mount point paths of child
-    # subvolumes before the shorter mount point paths of parent subvolumes.
+    # Reversing the sorted subvolumes ensures that we remove longer paths of child subvolumes before
+    # the shorter paths of parent subvolumes.
     for subvolume in reversed(all_subvolumes):
         subvolume_snapshots_glob = borgmatic.config.paths.replace_temporary_subdirectory_with_glob(
             os.path.normpath(make_snapshot_path(subvolume.path)),

docs/how-to/snapshot-your-filesystems.md

@@ -144,9 +144,8 @@ class="minilink minilink-addedin">Beta feature</span> borgmatic supports taking
 snapshots with the [Btrfs filesystem](https://btrfs.readthedocs.io/) and sending
 those snapshots to Borg for backup.
 
-To use this feature, first you need one or more Btrfs subvolumes on mounted
-filesystems. Then, enable Btrfs within borgmatic by adding the following line to
-your configuration file:
+To use this feature, first you need one or more Btrfs subvolumes. Then, enable
+Btrfs within borgmatic by adding the following line to your configuration file:
 
 ```yaml
 btrfs:
@@ -169,13 +168,18 @@ feedback](https://torsion.org/borgmatic/#issues) you have on this feature.
 
 #### Subvolume discovery
 
-For any read-write subvolume you'd like backed up, add its mount point path to
+For any read-write subvolume you'd like backed up, add its subvolume path to
 borgmatic's `source_directories` option. Btrfs does not support snapshotting
 read-only subvolumes.
 
-<span class="minilink minilink-addedin">New in version 1.9.6</span> Or include
-the mount point as a root pattern with borgmatic's `patterns` or `patterns_from`
-options.
+<span class="minilink minilink-addedin">New in version 2.0.7</span> The path can
+be either the path of the subvolume itself or the mount point where the
+subvolume is mounted. Prior to version 2.0.7, the subvolume path could not be
+used if the subvolume was mounted elsewhere; only the mount point could be used.
+
+<span class="minilink minilink-addedin">New in version 1.9.6</span> Instead of
+using `source_directories`, you can include the subvolume path as a root pattern
+with borgmatic's `patterns` or `patterns_from` options.
 
 During a backup, borgmatic snapshots these subvolumes (non-recursively) and
 includes the snapshotted files in the paths sent to Borg. borgmatic is also
@@ -184,25 +188,24 @@ responsible for cleaning up (deleting) these snapshots after a backup completes.
 borgmatic is smart enough to look at the parent (and grandparent, etc.)
 directories of each of your `source_directories` to discover any subvolumes. For
 instance, let's say you add `/var/log` and `/var/lib` to your source
-directories, but `/var` is a subvolume mount point. borgmatic will discover that
-and snapshot `/var` accordingly. This also works even with nested subvolumes;
+directories, but `/var` is a subvolume path. borgmatic will discover that and
+snapshot `/var` accordingly. This also works even with nested subvolumes;
 borgmatic selects the subvolume that's the "closest" parent to your source
 directories.
 
 <span class="minilink minilink-addedin">New in version 1.9.6</span> When using
 [patterns](https://borgbackup.readthedocs.io/en/stable/usage/help.html#borg-help-patterns),
 the initial portion of a pattern's path that you intend borgmatic to match
-against a subvolume mount point can't have globs or other non-literal characters
-in it—or it won't actually match. For instance, a subvolume mount point of
-`/var` would match a pattern of `+ fm:/var/*/data`, but borgmatic isn't
-currently smart enough to match `/var` to a pattern like `+ fm:/v*/lib/data`.
+against a subvolume path can't have globs or other non-literal characters in
+it—or it won't actually match. For instance, a subvolume path of `/var` would
+match a pattern of `+ fm:/var/*/data`, but borgmatic isn't currently smart
+enough to match `/var` to a pattern like `+ fm:/v*/lib/data`.
 
 Additionally, borgmatic rewrites the snapshot file paths so that they appear at
 their original subvolume locations in a Borg archive. For instance, if your
-subvolume is mounted at `/var/subvolume`, then the snapshotted files will appear
-in an archive at `/var/subvolume` as well—even if borgmatic has to mount the
-snapshot somewhere in `/var/subvolume/.borgmatic-snapshot-1234/` to perform the
-backup.
+subvolume path is `/var/subvolume`, then the snapshotted files will appear in an
+archive at `/var/subvolume` as well—even if borgmatic has to mount the snapshot
+somewhere in `/var/subvolume/.borgmatic-snapshot-1234/` to perform the backup.
 
 <span class="minilink minilink-addedin">With Borg version 1.2 and
 earlier</span>Snapshotted files are instead stored at a path dependent on the

test_requirements.txt

@@ -1,30 +1,30 @@
 appdirs==1.4.4
-apprise==1.8.0
-attrs==23.2.0
-black==24.4.2
-certifi==2024.7.4
+apprise==1.9.3
+attrs==25.3.0
+black==25.1.0
+certifi==2025.6.15
 chardet==5.2.0
-click==8.1.7
-codespell==2.2.6
-coverage==7.5.1
-flake8==7.0.0
+click==8.2.1
+codespell==2.4.1
+coverage==7.9.1
+flake8==7.3.0
 flake8-quotes==3.4.0
 flake8-use-fstring==1.4
 flake8-variables-names==0.0.6
 flexmock==0.12.1
-idna==3.7
-isort==5.13.2
-jsonschema==4.22.0
-Markdown==3.6
+idna==3.10
+isort==6.0.1
+jsonschema==4.24.0
+Markdown==3.8.2
 mccabe==0.7.0
-packaging==24.0
+packaging==25.0
 pathspec==0.12.1
-pluggy==1.5.0
+pluggy==1.6.0
 py==1.11.0
-pycodestyle==2.11.1
-pyflakes==3.2.0
-pytest==8.2.1
-pytest-cov==5.0.0
+pycodestyle==2.14.0
+pyflakes==3.4.0
+pytest==8.4.1
+pytest-cov==6.2.1
 PyYAML>5.0.0
 regex
 requests==2.32.4

tests/unit/hooks/data_source/test_btrfs.py

@@ -5,7 +5,31 @@ from borgmatic.borg.pattern import Pattern, Pattern_source, Pattern_style, Patte
 from borgmatic.hooks.data_source import btrfs as module
 
 
-def test_get_subvolume_mount_points_parses_findmnt_output():
+def test_get_contained_subvolume_paths_parses_btrfs_output():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_return(
+        'ID 256 gen 28 top level 5 path @sub\nID 258 gen 17 top level 5 path snap\n\n'
+    )
+
+    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == (
+        '/mnt0',
+        '/mnt0/@sub',
+        '/mnt0/snap',
+    )
+
+
+def test_get_contained_subvolume_paths_swallows_called_process_error():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_raise(
+        module.subprocess.CalledProcessError(1, 'btrfs')
+    )
+
+    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == ()
+
+
+def test_get_all_subvolume_paths_parses_findmnt_output():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return(
@@ -22,31 +46,53 @@ def test_get_subvolume_mount_points_parses_findmnt_output():
                  "source": "/dev/loop0",
                  "fstype": "btrfs",
                  "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/"
+              },
+              {
+                 "target": "/mnt2",
+                 "source": "/dev/loop0",
+                 "fstype": "btrfs",
+                 "options": "rw,relatime,ssd,space_cache=v2,subvolid=256,subvol=/"
               }
            ]
         }
         '''
     )
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs', '/mnt0'
+    ).and_return(('/mnt0',))
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs', '/mnt1'
+    ).and_return(('/mnt1', '/mnt1/sub'))
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs', '/mnt2'
+    ).never()
 
-    assert module.get_subvolume_mount_points('findmnt') == ('/mnt0', '/mnt1')
+    assert module.get_all_subvolume_paths('btrfs', 'findmnt') == (
+        '/mnt0',
+        '/mnt1',
+        '/mnt1/sub',
+        '/mnt2',
+    )
 
 
-def test_get_subvolume_mount_points_with_invalid_findmnt_json_errors():
+def test_get_all_subvolume_paths_with_invalid_findmnt_json_errors():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return('{')
+    flexmock(module).should_receive('get_contained_subvolume_paths').never()
 
     with pytest.raises(ValueError):
-        module.get_subvolume_mount_points('findmnt')
+        module.get_all_subvolume_paths('btrfs', 'findmnt')
 
 
-def test_get_subvolume_mount_points_with_findmnt_json_missing_filesystems_errors():
+def test_get_all_subvolume_paths_with_findmnt_json_missing_filesystems_errors():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return('{"wtf": "something is wrong here"}')
+    flexmock(module).should_receive('get_contained_subvolume_paths').never()
 
     with pytest.raises(ValueError):
-        module.get_subvolume_mount_points('findmnt')
+        module.get_all_subvolume_paths('btrfs', 'findmnt')
 
 
 def test_get_subvolume_property_with_invalid_btrfs_output_errors():
@@ -82,7 +128,7 @@ def test_get_subvolume_property_passes_through_general_value():
     assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value'
 
 
-def test_omit_read_only_subvolume_mount_points_filters_out_read_only():
+def test_omit_read_only_subvolume_paths_filters_out_read_only_subvolumes():
     flexmock(module).should_receive('get_subvolume_property').with_args(
         'btrfs', '/foo', 'ro'
     ).and_return(False)
@@ -93,17 +139,29 @@ def test_omit_read_only_subvolume_mount_points_filters_out_read_only():
         'btrfs', '/baz', 'ro'
     ).and_return(False)
 
-    assert module.omit_read_only_subvolume_mount_points('btrfs', ('/foo', '/bar', '/baz')) == (
+    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == (
         '/foo',
         '/baz',
     )
 
 
+def test_omit_read_only_subvolume_paths_filters_out_erroring_subvolumes():
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs', '/foo', 'ro'
+    ).and_raise(module.subprocess.CalledProcessError(1, 'btrfs'))
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs', '/bar', 'ro'
+    ).and_return(True)
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs', '/baz', 'ro'
+    ).and_return(False)
+
+    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == ('/baz',)
+
+
 def test_get_subvolumes_collects_subvolumes_matching_patterns():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     contained_pattern = Pattern(
         '/mnt1',
@@ -128,10 +186,8 @@ def test_get_subvolumes_collects_subvolumes_matching_patterns():
 
 
 def test_get_subvolumes_skips_non_root_patterns():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns'
@@ -162,10 +218,8 @@ def test_get_subvolumes_skips_non_root_patterns():
 
 
 def test_get_subvolumes_skips_non_config_patterns():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns'
@@ -196,10 +250,8 @@ def test_get_subvolumes_skips_non_config_patterns():
 
 
 def test_get_subvolumes_without_patterns_collects_all_subvolumes():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns'