Re: [PATCH 0/3] btrfs-progs: use libbtrfsutil for subvolume creation

[David Sterba <dsterba@suse.cz>]

On Wed, Jul 24, 2024 at 11:05:37AM -0700, Boris Burkov wrote:
> On Fri, Jun 28, 2024 at 03:56:46PM +0100, Mark Harmstone wrote:
> > From: Mark Harmstone <maharmstone@meta.com>
> > 
> > These patches are a resending of Omar Sandoval's patch from 2018, which
> > appears to have been overlooked [0], split up and rebased against the
> > current code.
> > 
> > We change btrfs subvol create and btrfs subvol snapshot so that they use
> > libbtrfsutil rather than calling the ioctl directly.
> > 
> > [0] https://lore.kernel.org/linux-btrfs/ab09ba595157b7fb6606814730508cae4da48caf.1516991902.git.osandov@fb.com/
> 
> The series looks good to me, you can add
> Reviewed-by: Boris Burkov <boris@bur.io>
> to it.
> 
> Two high level questions:
> 1. I think you put duplicate Signed-off-by: and Co-authored-by: lines on
> each patch. Did you mean to put Omar as the Co-authored-by: ?

I think Omar's signed-off could be there in case the code is the same
except some minor adjustments to fix conflicts, or co-authored-by.

> 2. Have you run fstests with this patch applied? We have recently had
> some test failures from stdout in create subvol/snapshot changing, so I
> would like to avoid that fiasco again.

If the output changed then a filter needs to be added to fstests. I
don't see any for subvolume creation, only _filter_btrfs_subvol_delete.

Besides, the tests of btrfs-progs don't pass,
https://github.com/kdave/btrfs-progs/actions/runs/10114811661

misc test 033-filename-length-limit fails, subvolume length,
"unexpected success: subvolume with name 256 bytes long succeeded"

and cli test 019-subvolume-create-parents
"unexpected success: was able to create subvolume without an intermediate directory"