diff mbox series

[6/8] btrfs-progs: subvol list: document and test actual behavior of paths

Message ID c015f1943def5e892b0aac540d1ce5a3d143ed6b.1718995160.git.osandov@fb.com (mailing list archive)
State New
Headers show
Series btrfs-progs: add subvol list options for sane path behavior | expand

Commit Message

Omar Sandoval June 21, 2024, 6:53 p.m. UTC
From: Omar Sandoval <osandov@fb.com>

The way btrfs subvol list prints paths and what the -o and -a flags do
are all nonsense.

Apparently, very early versions of Btrfs had a concept of a "top level"
of subvolumes rather than the root filesystem tree that we have today;
see commit 4ff9e2af1721 ("Add btrfs-list for listing subvolumes"). The
original subvol list code tracked the ID of that top level subvolume.
Eventually, 5 became the only possibility for the top level, and -o, -a,
and path printing were based on that. Commit 4f5ebb3ef553 ("Btrfs-progs:
fix to make list specified directory's subvolumes work") broke this and
changed the top level to be the same as the parent subvolume ID, which
gave us the illogical behavior we have today.

It has been this way for a decade, so we're probably stuck with it. But
let's at least document precisely what these all do in preparation for
adding sensible options. Let's also add tests in preparation for the
upcoming changes.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 Documentation/btrfs-subvolume.rst             |  20 ++-
 cmds/subvolume-list.c                         |  13 +-
 .../026-subvolume-list-path-filtering/test.sh | 118 ++++++++++++++++++
 3 files changed, 143 insertions(+), 8 deletions(-)
 create mode 100755 tests/cli-tests/026-subvolume-list-path-filtering/test.sh
diff mbox series

Patch

diff --git a/Documentation/btrfs-subvolume.rst b/Documentation/btrfs-subvolume.rst
index d1e89f15..fe84ab1c 100644
--- a/Documentation/btrfs-subvolume.rst
+++ b/Documentation/btrfs-subvolume.rst
@@ -134,7 +134,8 @@  list [options] [-G [\+|-]<value>] [-C [+|-]<value>] [--sort=rootid,gen,ogen,path
 
         where *ID* is subvolume's (root)id, *generation* is an internal counter which is
         updated every transaction, *parent_ID* is the same as the parent subvolume's id,
-        and *path* is the relative path of the subvolume to the top level subvolume.
+        and *path* is the path of the subvolume. The exact meaning of *path*
+        depends on the **Path filtering** option used.
         The subvolume's ID may be used by the subvolume set-default command,
         or at mount time via the *subvolid=* option.
 
@@ -143,11 +144,20 @@  list [options] [-G [\+|-]<value>] [-C [+|-]<value>] [--sort=rootid,gen,ogen,path
         Path filtering:
 
         -o
-                Print only subvolumes below specified <path>. Note that this is not a
-                recursive command, and won't show nested subvolumes under <path>.
+                Print only the immediate children subvolumes of the subvolume
+                containing <path>. Paths are printed relative to the root of
+                the filesystem.
         -a
-                print all the subvolumes in the filesystem and distinguish between
-                absolute and relative path with respect to the given *path*.
+                Print all subvolumes in the filesystem other than the root
+                subvolume. Paths are printed relative to the root of the
+                filesystem, except that subvolumes that are not an immediate
+                child of the subvolume containing <path> are prefixed with
+                "<FS_TREE>/".
+
+        If none of these are given, print all subvolumes in the filesystem
+        other than the root subvolume. Paths below the subvolume containing
+        <path> are printed relative to that subvolume, and other paths are
+        printed relative to the root of the filesystem.
 
         Field selection:
 
diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
index cfe165f9..3b32a5ff 100644
--- a/cmds/subvolume-list.c
+++ b/cmds/subvolume-list.c
@@ -57,9 +57,16 @@  static const char * const cmd_subvolume_list_usage[] = {
 	"List subvolumes and snapshots in the filesystem.",
 	"",
 	"Path filtering:",
-	OPTLINE("-o", "print only subvolumes below specified path"),
-	OPTLINE("-a", "print all the subvolumes in the filesystem and "
-		"distinguish absolute and relative path with respect to the given <path>"),
+	OPTLINE("-o", "print only the immediate children subvolumes of the "
+		"subvolume containing <path>"),
+	OPTLINE("-a", "print all subvolumes in the filesystem other than the "
+		"root subvolume, and prefix subvolumes that are not an "
+		"immediate child of the subvolume containing <path> with "
+		"\"<FS_TREE>/\""),
+	"",
+	"If none of these are given, print all subvolumes other than the root",
+	"subvolume relative to the subvolume containing <path> if below it,",
+	"otherwise relative to the root of the filesystem.",
 	"",
 	"Field selection:",
 	OPTLINE("-p", "print parent ID"),
diff --git a/tests/cli-tests/026-subvolume-list-path-filtering/test.sh b/tests/cli-tests/026-subvolume-list-path-filtering/test.sh
new file mode 100755
index 00000000..1b272ddc
--- /dev/null
+++ b/tests/cli-tests/026-subvolume-list-path-filtering/test.sh
@@ -0,0 +1,118 @@ 
+#!/bin/bash
+# Test how btrfs subvolume list prints paths, including all of the weird
+# accidental behavior.
+
+source "$TEST_TOP/common" || exit
+
+check_prereq btrfs
+check_prereq mkfs.btrfs
+
+setup_root_helper
+prepare_test_dev
+
+run_check_mkfs_test_dev
+run_check_mount_test_dev
+
+cd "$TEST_MNT"
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "a"
+run_check $SUDO_HELPER mkdir "a/b"
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "a/b/c"
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "a/b/c/d"
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "a/e"
+
+subvol_list_paths() {
+	run_check_stdout $SUDO_HELPER "$TOP/btrfs" subvolume list "$@" | sed 's/.*path //'
+}
+
+expect_subvol_list_paths() {
+	diff -u - <(subvol_list_paths "$@") || _fail "wrong output from btrfs subvolume list $*"
+}
+
+### No options ###
+
+# Paths are relative to the given subvolume if they are beneath it and relative
+# to the root of the filesystem otherwise.
+expect_subvol_list_paths . << EOF
+a
+a/b/c
+a/b/c/d
+a/e
+EOF
+
+expect_subvol_list_paths a << EOF
+a
+b/c
+b/c/d
+e
+EOF
+
+expect_subvol_list_paths a/b/c << EOF
+a
+a/b/c
+d
+a/e
+EOF
+
+# If passed a directory, it's treated as the containing subvolume.
+expect_subvol_list_paths a/b << EOF
+a
+b/c
+b/c/d
+e
+EOF
+
+### -a ###
+
+# Paths are relative to the root of the filesystem. Subvolumes that are not an
+# immediate child of the passed subvolume are prefixed with <FS_TREE>/.
+expect_subvol_list_paths -a . << EOF
+a
+<FS_TREE>/a/b/c
+<FS_TREE>/a/b/c/d
+<FS_TREE>/a/e
+EOF
+
+expect_subvol_list_paths -a a << EOF
+<FS_TREE>/a
+a/b/c
+<FS_TREE>/a/b/c/d
+a/e
+EOF
+
+# If passed a directory, it's treated as the containing subvolume.
+expect_subvol_list_paths -a a/b << EOF
+<FS_TREE>/a
+a/b/c
+<FS_TREE>/a/b/c/d
+a/e
+EOF
+
+### -o ###
+
+# Only immediate children of the passed subvolume are printed, and they are
+# printed relative to the root of the filesystem.
+expect_subvol_list_paths -o . << EOF
+a
+EOF
+
+expect_subvol_list_paths -o a << EOF
+a/b/c
+a/e
+EOF
+
+# If passed a directory, it's treated as the containing subvolume.
+expect_subvol_list_paths -o a/b << EOF
+a/b/c
+a/e
+EOF
+
+expect_subvol_list_paths -o a/b/c << EOF
+a/b/c/d
+EOF
+
+expect_subvol_list_paths -o a/b/c/d << EOF
+EOF
+
+cd ..
+run_check_umount_test_dev