| Message ID | cover.1543294426.git.misono.tomohiro@jp.fujitsu.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs-progs: sub: Relax the privileges of "subvolume list/show" | expand |
Misono Tomohiro - 27.11.18, 06:24: > Importantly, in order to make output consistent for both root and > non-privileged user, this changes the behavior of "subvolume list": > - (default) Only list in subvolume under the specified path. > Path needs to be a subvolume. Does that work recursively? I wound find it quite unexpected if I did btrfs subvol list in or on the root directory of a BTRFS filesystem would not display any subvolumes on that filesystem no matter where they are. Thanks,
Hi, > -----Original Message----- > From: Martin Steigerwald [mailto:martin@lichtvoll.de] > Sent: Tuesday, November 27, 2018 6:48 PM > To: Misono, Tomohiro <misono.tomohiro@fujitsu.com> > Cc: linux-btrfs@vger.kernel.org > Subject: Re: [PATCH RESEND 0/8] btrfs-progs: sub: Relax the privileges > of "subvolume list/show" > > Misono Tomohiro - 27.11.18, 06:24: > > Importantly, in order to make output consistent for both root and > > non-privileged user, this changes the behavior of "subvolume list": > > - (default) Only list in subvolume under the specified path. > > Path needs to be a subvolume. > > Does that work recursively? Not in this version. Previous version has -f option which recursively search and list subbvolumes (only if they have the same btrfs fsid): https://lore.kernel.org/linux-btrfs/84d06767762b4285ddefec0392ee16e2d7e06f62.1529310485.git.misono.tomohiro@jp.fujitsu.com/ However, current "sub list" command already has many options and we cannot add new one randomly, I drop the patches which add new options in this version. (In other word, previous version can be divided in two parts: 1. Relax the privileges of sub list/show 2. Add new option to sub list And this version contains only 1). > > I wound find it quite unexpected if I did btrfs subvol list in or on the > root directory of a BTRFS filesystem would not display any subvolumes > on > that filesystem no matter where they are. Yes, I think output of -f option is more readable in such case. If agreement has been made about how "sub list" should really work, I could update/send patches again. Thanks, Misono
On Tue, Nov 27, 2018 at 02:24:41PM +0900, Misono Tomohiro wrote: > Hello, > > This is basically the resend of > "[PATCH v2 00/20] btrfs-progs: Rework of "subvolume list/show" and relax the > root privileges of them" [1] > which I submitted in June. The aim of this series is to allow non-privileged user > to use basic subvolume functionality (create/list/snapshot/delete; this allows "list") > > They were once in devel branch with some whitespace/comment modification by david. > I rebased them to current devel branch. > > github: https://github.com/t-msn/btrfs-progs/tree/rework-sub-list > > Basic logic/code is the same as before. Some differences are: > - Use latest libbtrfsutil from Omar [2] (thus drop first part of patches). > As a result, "sub list" cannot accept an ordinary directry to be > specified (which is allowed in previous version) > - Drop patches which add new options to "sub list" > - Use 'nobody' as non-privileged test user just like libbtrfsutil test > - Update comments > > Importantly, in order to make output consistent for both root and non-privileged > user, this changes the behavior of "subvolume list": > - (default) Only list in subvolume under the specified path. > Path needs to be a subvolume. > - (-a) filter is dropped. i.e. its output is the same as the > default behavior of "sub list" in progs <= 4.19 > > Therefore, existent scripts may need to update to add -a option > (I believe nobody uses current -a option). > If anyone thinks this is not good, please let me know. I think there are a few options in the case that the path isn't a subvolume: 1. List all subvolumes in the filesystem with randomly mangled paths, which is what we currently do. 2. Error out, which is what this version of the series does. 3. List all subvolumes under the containing subvolume, which is what the previous version does. 4. List all subvolumes under the containing subvolume that are underneath the given path. Option 1 won't work well for unprivileged users. Option 2 (this series) is definitely going to break people's workflows/scripts. Option 3 is unintuitive. In my opinion, option 4 is the nicest, but it may also break scripts that expect all subvolumes to be printed. There's also an option 5, which is to keep the behavior the same for root (like what my previous patch [1] did) and implement option 4 for unprivileged users. I think 4 and 5 are the two main choices: do we want to preserve backwards compatibility as carefully as possible (at the cost of consistency), or do we want to risk it and improve the interface? 1: https://github.com/osandov/btrfs-progs/commit/fb61c21aeb998b12c1d02532639083d7f40c41e0
> -----Original Message----- > From: Omar Sandoval [mailto:osandov@osandov.com] > Sent: Friday, December 7, 2018 10:02 AM > To: Misono, Tomohiro/味曽野 智礼 <misono.tomohiro@fujitsu.com> > Cc: linux-btrfs@vger.kernel.org > Subject: Re: [PATCH RESEND 0/8] btrfs-progs: sub: Relax the privileges of > "subvolume list/show" > > On Tue, Nov 27, 2018 at 02:24:41PM +0900, Misono Tomohiro wrote: > > Hello, > > > > This is basically the resend of > > "[PATCH v2 00/20] btrfs-progs: Rework of "subvolume list/show" and relax > the > > root privileges of them" [1] > > which I submitted in June. The aim of this series is to allow non-privileged > user > > to use basic subvolume functionality (create/list/snapshot/delete; this > allows "list") > > > > They were once in devel branch with some whitespace/comment modification by > david. > > I rebased them to current devel branch. > > > > github: https://github.com/t-msn/btrfs-progs/tree/rework-sub-list > > > > Basic logic/code is the same as before. Some differences are: > > - Use latest libbtrfsutil from Omar [2] (thus drop first part of patches). > > As a result, "sub list" cannot accept an ordinary directry to be > > specified (which is allowed in previous version) > > - Drop patches which add new options to "sub list" > > - Use 'nobody' as non-privileged test user just like libbtrfsutil test > > - Update comments > > > > Importantly, in order to make output consistent for both root and non-privileged > > user, this changes the behavior of "subvolume list": > > - (default) Only list in subvolume under the specified path. > > Path needs to be a subvolume. > > - (-a) filter is dropped. i.e. its output is the same as the > > default behavior of "sub list" in progs <= 4.19 > > > > Therefore, existent scripts may need to update to add -a option > > (I believe nobody uses current -a option). > > If anyone thinks this is not good, please let me know. > > I think there are a few options in the case that the path isn't a > subvolume: > > 1. List all subvolumes in the filesystem with randomly mangled paths, > which is what we currently do. > 2. Error out, which is what this version of the series does. > 3. List all subvolumes under the containing subvolume, which is what the > previous version does. > 4. List all subvolumes under the containing subvolume that are > underneath the given path. > > Option 1 won't work well for unprivileged users. Option 2 (this series) > is definitely going to break people's workflows/scripts. Option 3 is > unintuitive. In my opinion, option 4 is the nicest, but it may also > break scripts that expect all subvolumes to be printed. I think my previous version is 4, or am I misunderstanding? In previous version, output could be: $ mkfs.btrfs -f $DEV $ mount $DEV /mnt $ btrfs subvolume create /mnt/sub1 $ btrfs subvolume create /mnt/sub1/sub2 $ mkdir /mnt/sub1/dir $ btrfs subvolume create /mnt/sub1/dir/sub3 $ btrfs subvolume list /mnt ID 256 gen 8 top level 5 path sub1 ID 258 gen 7 top level 256 path sub1/sub2 ID 259 gen 8 top level 256 path sub1/dir/sub3 $ btrfs subvolume list /mnt/sub1 ID 256 gen 8 top level 5 path . ID 258 gen 7 top level 256 path sub2 ID 259 gen 8 top level 256 path dir/sub3 $ btrfs subvolume list /mnt/sub1/dir ID 259 gen 8 top level 256 path sub3 https://lore.kernel.org/linux-btrfs/6af6dac8829d3ba405e3c53baffd828c9c428ef6.1529310485.git.misono.tomohiro@jp.fujitsu.com/ > > There's also an option 5, which is to keep the behavior the same for > root (like what my previous patch [1] did) and implement option 4 for > unprivileged users. > > I think 4 and 5 are the two main choices: do we want to preserve > backwards compatibility as carefully as possible (at the cost of > consistency), or do we want to risk it and improve the interface? > > 1: > https://github.com/osandov/btrfs-progs/commit/fb61c21aeb998b12c1d0253263908 > 3d7f40c41e0
Hello, This is basically the resend of "[PATCH v2 00/20] btrfs-progs: Rework of "subvolume list/show" and relax the root privileges of them" [1] which I submitted in June. The aim of this series is to allow non-privileged user to use basic subvolume functionality (create/list/snapshot/delete; this allows "list") They were once in devel branch with some whitespace/comment modification by david. I rebased them to current devel branch. github: https://github.com/t-msn/btrfs-progs/tree/rework-sub-list Basic logic/code is the same as before. Some differences are: - Use latest libbtrfsutil from Omar [2] (thus drop first part of patches). As a result, "sub list" cannot accept an ordinary directry to be specified (which is allowed in previous version) - Drop patches which add new options to "sub list" - Use 'nobody' as non-privileged test user just like libbtrfsutil test - Update comments Importantly, in order to make output consistent for both root and non-privileged user, this changes the behavior of "subvolume list": - (default) Only list in subvolume under the specified path. Path needs to be a subvolume. - (-a) filter is dropped. i.e. its output is the same as the default behavior of "sub list" in progs <= 4.19 Therefore, existent scripts may need to update to add -a option (I believe nobody uses current -a option). If anyone thinks this is not good, please let me know. Behavior summary from cover letter in [1] ==== * Behavior summary of new "sub list" - default (no option) - lists subvolumes below the specified path (inc. path itself) - If new ioctls exists non-privileged user can call it (subvolumes to which the user cannot access will be skipped) - -a - updated to remove filter. i.e. the output is the same as current progs without option (require root privileges) [Example] $ mkfs.btrfs -f $DEV $ mkfs.btrfs -f $DEV2 $ mount $DEV $MNT $ btrfs subvolume create $MNT/AAA $ btrfs subvolume create $MNT/BBB $ btrfs subvolume create $MNT/CCC $ btrfs subvolume create $MNT/DDD $ mkdir $MNT/AAA/bbb $ mkdir $MNT/AAA/ccc $ mkdir $MNT/AAA/other $ umount $MNT $ mount -o subvol=AAA $DEV $MNT $ mount -o subvol=BBB $DEV $MNT/bbb $ mount -o subvol=CCC $DEV $MNT/ccc $ mount -o $DEV2 $MNT/other $ btrfs subvolume list $MNT # print subvolumes below the path ID 256 gen 10 top level 5 path . $ btrfs subvolume list -a $MNT # print all subvolumes in the fs. the same output as progs<=4.19 without option ID 256 gen 10 top level 5 path AAA ID 258 gen 7 top level 5 path BBB ID 259 gen 8 top level 5 path CCC ID 260 gen 9 top level 5 path DDD * Behavior summary of new "sub show" - No change for root's output - If new ioctls exists, non-privileged user can call it - In that case, path to be shown is absolute path (for root, it is relative to top-level subvolume) Also, snapshots to be shown are to which the user can access from current mount point. (for root, all snapshots in the fs) === [1] https://lore.kernel.org/linux-btrfs/cover.1529310485.git.misono.tomohiro@jp.fujitsu.com/ [2] https://lore.kernel.org/linux-btrfs/cover.1542181521.git.osandov@fb.com/ Thanks, Misono Misono Tomohiro (8): btrfs-progs: sub list: Use libbtrfsuitl for subvolume list btrfs-progs: sub list: factor out main part of btrfs_list_subvols btrfs-progs: sub list: Change the default behavior of "subvolume list" and allow non-privileged user to call it btrfs-progs: sub list: Update -a option and remove meaningless filter btrfs-progs: utils: Fallback to open without O_NOATIME flag in find_mount_root(): btrfs-progs: sub show: Allow non-privileged user to call "subvolume show" btrfs-progs: test: Add helper function to check if test user exists btrfs-porgs: test: Add cli-test/009 to check subvolume list for both root and normal user Documentation/btrfs-subvolume.asciidoc | 25 +- cmds-subvolume.c | 1149 +++++++++++++++++++- tests/cli-tests/009-subvolume-list/test.sh | 130 +++ tests/common | 10 + utils.c | 3 + 5 files changed, 1266 insertions(+), 51 deletions(-) create mode 100755 tests/cli-tests/009-subvolume-list/test.sh