| Message ID | 20240628145807.1800474-1-maharmstone@fb.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs-progs: use libbtrfsutil for subvolume creation | expand |
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: ? 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. Thanks, Boris > > Omar Sandoval (3): > btrfs-progs: remove unused qgroup functions > btrfs-progs: use libbtrfsutil for btrfs subvolume create > btrfs-progs: use libbtrfsutil for btrfs subvolume snapshot > > cmds/qgroup.c | 64 ------------- > cmds/qgroup.h | 2 - > cmds/subvolume.c | 227 ++++++++++++++++++----------------------------- > 3 files changed, 86 insertions(+), 207 deletions(-) > > -- > 2.44.2 >
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"
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/ Omar Sandoval (3): btrfs-progs: remove unused qgroup functions btrfs-progs: use libbtrfsutil for btrfs subvolume create btrfs-progs: use libbtrfsutil for btrfs subvolume snapshot cmds/qgroup.c | 64 ------------- cmds/qgroup.h | 2 - cmds/subvolume.c | 227 ++++++++++++++++++----------------------------- 3 files changed, 86 insertions(+), 207 deletions(-)