Re: [PATCH 0/8] btrfs-progs: add subvol list options for sane path behavior

[Omar Sandoval <osandov@osandov.com>]

On Thu, Jul 04, 2024 at 01:40:54AM +0200, David Sterba wrote:
> On Mon, Jul 01, 2024 at 02:52:02PM -0700, Omar Sandoval wrote:
> > On Tue, Jun 25, 2024 at 05:34:38PM +0200, David Sterba wrote:
> > > On Fri, Jun 21, 2024 at 11:53:29AM -0700, Omar Sandoval wrote:
> > > > From: Omar Sandoval <osandov@fb.com>
> > > > 
> > > > Hello,
> > > > 
> > > > btrfs subvol list's path handling has been a constant source of
> > > > confusion for users. None of -o, -a, or the default do what users
> > > > expect. This has been broken for a decade; see [1].
> > > > 
> > > > This series adds two new options, -O and -A, which do what users
> > > > actually want: list subvolumes below a path, or list all subvolumes,
> > > > with minimal path shenanigans. This approach is conservative and tries
> > > > to maintain backwards compatibility, but it's worth discussing whether
> > > > we should instead fix the existing options/default, or more noisily
> > > > deprecate the existing options.
> > > 
> > > I'm working on a replacement command of 'subvolume list', there seems to
> > > be no other sane way around that.
> > 
> > I love this idea. Do you have a work in progress anywhere?
> 
> Yes, the initial version is in my branch dev/subvol-list-new but I have
> recent updates adding more optional columns. The patches are not
> polished, I'll update the branch once it's done.
> 
> > > The command line options are indeed
> > > confusing and the output is maybe easy to parse but not nice to read.
> > > Changing meaning of the options would break too many things as everybody
> > > got used to the bad UI and output.
> > > 
> > > We can add the two new options but I'd rather do that only in the new
> > > command so we can let everybody migrate there.
> > > 
> > > > One additional benefit of this is that -O can be used by unprivileged
> > > > users.
> > > 
> > > This should be the default (and is supposed to be in the new command).
> > > 
> > > > Patch 1 fixes a libbtrfsutil bug I encountered while testing this.
> > > > Patches 2 and 3 fix libbtrfsutil's privilege checks to work in user
> > > > namespaces. Patches 4 and 5 remove some unused subvol list code. Patch 6
> > > > documents and tests the current, insane behavior. Patch 7 converts
> > > > subvol list to use libbtrfsutil. This is a revival of one of my old
> > > > patches [2], but is much easier now that libbtrfs has been pared down.
> > > > Patch 8 adds the new options, including documentation and tests.
> > > > 
> > > > Thanks!
> > > > Omar
> > > > 
> > > > 1: https://lore.kernel.org/all/bdd9af61-b408-c8d2-6697-84230b0bcf89@gmail.com/
> > > > 2: https://lore.kernel.org/all/6492726d6e89bf792627e4431f7ba7691f09c3d2.1518720598.git.osandov@fb.com/
> > > > 
> > > > Omar Sandoval (8):
> > > >   libbtrfsutil: fix accidentally closing fd passed to subvolume iterator
> > > 
> > > I've picked this patch now as it's a fix.
> > 
> > Patches 2 and 3 are also fixes, so those would be nice to have, too, if
> > you don't mind.
> 
> I picked only the first one for the 6.9.2 release that had little time
> for testing.

Ok, no problem.

> > My real motivation for this series is so that some internal workloads in
> > user namespaces can use subvol list. I'd love for the unprivileged use
> > case to be unblocked in the short term. Would it be better to add just
> > the -O option without -A and all of the big documentation changes? (This
> > still requires the libbtrfsutil rework.)
> 
> I think for the time being the saner -O and -A options can be added.
> Their functionality resemble the lower case options so it's still
> understandable and can be released without delays.

Thanks! Let me know if this series needs any updates.

> For the new command
> I'd like to do a public poll and comment round that will take some time.

That makes total sense. I'm happy to chime in when it's ready.

Thanks,
Omar