From 7b163f60102d587d0f67f1adc96655670c866017 Mon Sep 17 00:00:00 2001 From: Omar Sandoval Date: Fri, 21 Jun 2024 11:53:37 -0700 Subject: [PATCH] btrfs-progs: subvol list: add sane -O and -A options Now that we've documented the current nonsensical behavior, add a couple of options that actually make sense: -O lists all subvolumes below a path (which is what people think -o does), and -A lists all subvolumes with no path munging (which is what people think the default or -a do). -O can even be used by unprivileged users. -O and -A also renames the "top level" in the default output to what it actually is now: the "parent". Signed-off-by: Omar Sandoval --- Documentation/btrfs-subvolume.rst | 17 ++++ cmds/subvolume-list.c | 77 +++++++++++++++---- .../026-subvolume-list-path-filtering/test.sh | 38 +++++++++ 3 files changed, 117 insertions(+), 15 deletions(-) diff --git a/Documentation/btrfs-subvolume.rst b/Documentation/btrfs-subvolume.rst index c5f635fb..a7bc4572 100644 --- a/Documentation/btrfs-subvolume.rst +++ b/Documentation/btrfs-subvolume.rst @@ -145,6 +145,9 @@ list [options] [-G [\+|-]] [-C [+|-]] [--sort=rootid,gen,ogen,path updated every transaction, *parent_ID* is the same as the parent subvolume's id, and *path* is the path of the subvolume. The exact meaning of *path* depends on the **Path filtering** option used. + + If -O or -A is given, "top level" is replaced by "parent". + The subvolume's ID may be used by the subvolume set-default command, or at mount time via the *subvolid=* option. @@ -152,6 +155,20 @@ list [options] [-G [\+|-]] [-C [+|-]] [--sort=rootid,gen,ogen,path Path filtering: + -O + Print and all subvolumes below it, recursively. + must be a subvolume. Paths are printed relative to . + + This may be used by unprivileged users, in which case this only + lists subvolumes that the user has access to. + -A + Print all subvolumes in the filesystem. Paths are printed + relative to the root of the filesystem. + + You likely always want either -O or -A. The -o and -a options and the + default if no path filtering options are given have very confusing, + accidental behavior that is only kept for backwards compatibility. + -o Print only the immediate children subvolumes of the subvolume containing . Paths are printed relative to the root of diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c index fa5082af..4dde9fbe 100644 --- a/cmds/subvolume-list.c +++ b/cmds/subvolume-list.c @@ -59,6 +59,13 @@ static const char * const cmd_subvolume_list_usage[] = { "List subvolumes and snapshots in the filesystem.", "", "Path filtering:", + OPTLINE("-O", "print all subvolumes below relative to "), + OPTLINE("-A", "print all subvolumes in the filesystem relative to the " + "root of the filesystem"), + "", + "You likely always want either -O or -A. The -o and -a options and the", + "default are confusing and only kept for backwards compatibility.", + "", OPTLINE("-o", "print only the immediate children subvolumes of the " "subvolume containing "), OPTLINE("-a", "print all subvolumes in the filesystem other than the " @@ -866,9 +873,11 @@ out: return subvols; } -static struct subvol_list *btrfs_list_subvols(int fd, +static struct subvol_list *btrfs_list_subvols(int fd, bool include_top, + bool below, struct btrfs_list_filter_set *filter_set) { + u64 top_id = below ? 0 : BTRFS_FS_TREE_OBJECTID; struct subvol_list *subvols; size_t capacity = 4; struct btrfs_util_subvolume_iterator *iter; @@ -883,15 +892,28 @@ static struct subvol_list *btrfs_list_subvols(int fd, } subvols->num = 0; - err = btrfs_util_create_subvolume_iterator_fd(fd, - BTRFS_FS_TREE_OBJECTID, 0, - &iter); + err = btrfs_util_create_subvolume_iterator_fd(fd, top_id, 0, &iter); if (err) { iter = NULL; error_btrfs_util(err); goto out; } + if (include_top) { + err = btrfs_util_subvolume_info_fd(fd, top_id, + &subvols->subvols[0].info); + if (err) { + error_btrfs_util(err); + goto out; + } + subvols->subvols[0].path = strdup(""); + if (!subvols->subvols[0].path) { + error("out of memory"); + goto out; + } + subvols->num++; + } + for (;;) { struct root_info subvol; @@ -945,14 +967,15 @@ out: static int btrfs_list_subvols_print(int fd, struct btrfs_list_filter_set *filter_set, struct btrfs_list_comparer_set *comp_set, - enum btrfs_list_layout layout) + enum btrfs_list_layout layout, bool include_top, + bool below) { struct subvol_list *subvols; if (filter_set->only_deleted) subvols = btrfs_list_deleted_subvols(fd, filter_set); else - subvols = btrfs_list_subvols(fd, filter_set); + subvols = btrfs_list_subvols(fd, include_top, below, filter_set); if (!subvols) return -1; @@ -1097,8 +1120,10 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg char *top_path = NULL; int ret = -1, uerr = 0; char *subvol; + bool is_list_below = false; bool is_list_all = false; - bool is_only_in_path = false; + bool is_old_a_option = false; + bool is_old_o_option = false; enum btrfs_list_layout layout = BTRFS_LIST_LAYOUT_DEFAULT; filter_set = btrfs_list_alloc_filter_set(); @@ -1113,7 +1138,7 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg }; c = getopt_long(argc, argv, - "acdgopqsurRG:C:t", long_options, NULL); + "acdgopqsurRG:C:tOA", long_options, NULL); if (c < 0) break; @@ -1122,7 +1147,7 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg btrfs_list_setup_print_column(BTRFS_LIST_PARENT); break; case 'a': - is_list_all = true; + is_old_a_option = true; break; case 'c': btrfs_list_setup_print_column(BTRFS_LIST_OGENERATION); @@ -1134,7 +1159,7 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg btrfs_list_setup_print_column(BTRFS_LIST_GENERATION); break; case 'o': - is_only_in_path = true; + is_old_o_option = true; break; case 't': layout = BTRFS_LIST_LAYOUT_TABLE; @@ -1187,6 +1212,12 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg goto out; } break; + case 'O': + is_list_below = true; + break; + case 'A': + is_list_all = true; + break; default: uerr = 1; @@ -1197,6 +1228,19 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg if (check_argc_exact(argc - optind, 1)) goto out; + /* + * The path filtering options and -d don't make sense together. For -O + * and -A, we're strict about not combining them with each other or with + * -o, -a, or -d. The -o, -a, and -d options are older and have never + * been restricted, so although they produce dubious results when + * combined, we allow it for backwards compatibility. + */ + if (is_list_below + is_list_all + + (is_old_a_option || is_old_o_option || filter_set->only_deleted) > 1) { + error("-O, -A, -o, -a, and -d are mutually exclusive"); + goto out; + } + subvol = argv[optind]; fd = btrfs_open_dir(subvol); if (fd < 0) { @@ -1216,15 +1260,15 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg goto out; } - if (is_list_all) + if (is_old_a_option) btrfs_list_setup_filter(&filter_set, BTRFS_LIST_FILTER_FULL_PATH, top_id); - else if (is_only_in_path) + else if (is_old_o_option) btrfs_list_setup_filter(&filter_set, BTRFS_LIST_FILTER_TOPID_EQUAL, top_id); - else if (!filter_set->only_deleted) { + else if (!is_list_below && !is_list_all && !filter_set->only_deleted) { enum btrfs_util_error err; err = btrfs_util_subvolume_get_path_fd(fd, top_id, &top_path); @@ -1241,13 +1285,16 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg /* by default we shall print the following columns*/ btrfs_list_setup_print_column(BTRFS_LIST_OBJECTID); btrfs_list_setup_print_column(BTRFS_LIST_GENERATION); - btrfs_list_setup_print_column(BTRFS_LIST_TOP_LEVEL); + btrfs_list_setup_print_column(is_list_below || is_list_all ? + BTRFS_LIST_PARENT : BTRFS_LIST_TOP_LEVEL); btrfs_list_setup_print_column(BTRFS_LIST_PATH); if (bconf.output_format == CMD_FORMAT_JSON) layout = BTRFS_LIST_LAYOUT_JSON; - ret = btrfs_list_subvols_print(fd, filter_set, comparer_set, layout); + ret = btrfs_list_subvols_print(fd, filter_set, comparer_set, layout, + is_list_below || is_list_all, + is_list_below); out: free(top_path); diff --git a/tests/cli-tests/026-subvolume-list-path-filtering/test.sh b/tests/cli-tests/026-subvolume-list-path-filtering/test.sh index 1b272ddc..a65ba91f 100755 --- a/tests/cli-tests/026-subvolume-list-path-filtering/test.sh +++ b/tests/cli-tests/026-subvolume-list-path-filtering/test.sh @@ -114,5 +114,43 @@ EOF expect_subvol_list_paths -o a/b/c/d << EOF EOF +### -A ### + +# Paths are always relative to the root of the filesystem. +for path in . a/b a/b/c; do + expect_subvol_list_paths -A "$path" << EOF + +a +a/b/c +a/b/c/d +a/e +EOF +done + +### -O ### + +# Paths are relative to the given path. +expect_subvol_list_paths -O . << EOF + +a +a/b/c +a/b/c/d +a/e +EOF + +expect_subvol_list_paths -O a << EOF + +b/c +b/c/d +e +EOF + +expect_subvol_list_paths -O a/e << EOF + +EOF + +run_mustfail "btrfs subvol list -O allowed non-subvolume" \ + $SUDO_HELPER "$TOP/btrfs" subvolume list -O a/b + cd .. run_check_umount_test_dev