Re: [PATCH v2 00/13] rename & split tests

From: Zorro Lang <zlang@redhat.com>
To: Christian Brauner <brauner@kernel.org>
Cc: fstests <fstests@vger.kernel.org>, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 00/13] rename & split tests
Date: Fri, 13 May 2022 04:19:29 +0800	[thread overview]
Message-ID: <20220512201929.5egujgnf66fn5yh4@zlang-mailbox> (raw)
In-Reply-To: <20220512165250.450989-1-brauner@kernel.org>

On Thu, May 12, 2022 at 06:52:37PM +0200, Christian Brauner wrote:
> From: "Christian Brauner (Microsoft)" <brauner@kernel.org>
> 
> Hey everyone,
> 
> Please note that this patch series contains patches that will be
> rejected by the fstests mailing list because of the amount of changes
> they contain. So tools like b4 will not be able to find the whole patch
> series on a mailing list. In case it's helpful I've added the
> "fstests.vfstest.for-next" tag which can be pulled. Otherwise it's
> possible to simply use the patch series as it appears in your inbox.

Thanks Christian! I've merged this patchset to my local testing branch, and
haven't found regressions for now. That's good to me.

Due to it *blocks* all other idmapped related patches, so if VFS (idmapped)
forks don't have objections, and if no big regressions either, I'd like to merge
this patchset at first, to free this *blocking*, then we can fix small issues
bit by bit. If anyone has review points, please point out ASAP :)

Thanks,
Zorro

> 
> All vfstests pass:
> 
> #### btrfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> btrfs/245 52s ...  54s
> generic/633 58s ...  51s
> generic/644 60s ...  49s
> generic/645 161s ...  143s
> generic/656 63s ...  55s
> Ran: btrfs/245 generic/633 generic/644 generic/645 generic/656
> Passed all 5 tests
> 
> #### ext4 ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- ext4
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- /dev/sda4
> MOUNT_OPTIONS -- -o acl,user_xattr /dev/sda4 /mnt/scratch
> 
> generic/633 47s ...  50s
> generic/644 46s ...  49s
> generic/645 135s ...  139s
> generic/656 53s ...  54s
> Ran: generic/633 generic/644 generic/645 generic/656
> Passed all 4 tests
> 
> #### xfs ####
> ubuntu@imp1-vm:~/src/git/xfstests$ sudo ./check -g idmapped
> FSTYP         -- xfs (debug)
> PLATFORM      -- Linux/x86_64 imp1-vm 5.18.0-rc4-fs-mnt-hold-writers-8a2e2350494f #107 SMP PREEMPT_DYNAMIC Mon May 9 12:12:34 UTC 2022
> MKFS_OPTIONS  -- -f /dev/sda4
> MOUNT_OPTIONS -- /dev/sda4 /mnt/scratch
> 
> generic/633 58s ...  58s
> generic/644 62s ...  60s
> generic/645 161s ...  161s
> generic/656 62s ...  63s
> xfs/152 133s ...  133s
> xfs/153 94s ...  92s
> Ran: generic/633 generic/644 generic/645 generic/656 xfs/152 xfs/153
> Passed all 6 tests
> 
> /* v2 */
> * This rebases the patchset on top the for-next branch.
> * Last week we merged 858a19c5e9b0 ("idmapped_mounts: Prepare for
>   support for more features"). The patch switched feature checking from
>   a boolean to a flag. It failed convert all tests. This adds a patch to
>   fix this in patch 01/13.
> * A patch has been added to remove an invalid test. The semantics for a
>   specific corner-case where we allowed a mount's idmapping to change
>   while there were active writers will be altered.
> 
> /* v1 */
> As announced multiple times already we need to rename and split the
> idmapped mount testsuite into separate source files and also give it a
> better name to reflect the fact that it covers a lot more than just
> idmapped mounts.
> 
> I have decided against compiling different binaries for now. Instead we
> compile a single vfstest binary that can be called with various command
> line switches to run the various test suites. This is not different than
> what we did for the idmapped-mounts binary. Of course, nothing prevents
> us from using multiple binaries in the future.
> 
> Thanks!
> Christian
> 
> Christian Brauner (13):
>   idmapped-mounts: make all tests set their required feature flags
>   src: rename idmapped-mounts folder
>   src/vfs: rename idmapped-mounts.c file
>   vfstest: rename struct t_idmapped_mounts
>   utils: add missing global.h include
>   utils: add struct vfstest_info
>   utils: move helpers into utils
>   missing: move sys_execveat() to missing.h
>   utils: add struct test_suite
>   vfstests: split idmapped mount tests into separate suite
>   vfstest: split out btrfs idmapped mounts test
>   vfstest: split out remaining idmapped mount tests
>   vfs/idmapped-mounts: remove invalid test
> 
>  .gitignore                                    |     4 +-
>  common/rc                                     |    32 +-
>  src/Makefile                                  |     2 +-
>  src/detached_mounts_propagation.c             |     2 +-
>  src/feature.c                                 |     2 +-
>  src/idmapped-mounts/idmapped-mounts.c         | 14625 ----------------
>  src/idmapped-mounts/utils.c                   |   425 -
>  src/idmapped-mounts/utils.h                   |   130 -
>  src/{idmapped-mounts => vfs}/Makefile         |    14 +-
>  src/vfs/btrfs-idmapped-mounts.c               |  3854 ++++
>  src/vfs/btrfs-idmapped-mounts.h               |    15 +
>  src/vfs/idmapped-mounts.c                     |  7747 ++++++++
>  src/vfs/idmapped-mounts.h                     |    18 +
>  src/{idmapped-mounts => vfs}/missing.h        |    11 +
>  src/{idmapped-mounts => vfs}/mount-idmapped.c |     0
>  src/vfs/utils.c                               |  1050 ++
>  src/vfs/utils.h                               |   373 +
>  src/vfs/vfstest.c                             |  2073 +++
>  tests/btrfs/245                               |     2 +-
>  tests/generic/633                             |     2 +-
>  tests/generic/644                             |     2 +-
>  tests/generic/645                             |     2 +-
>  tests/generic/656                             |     2 +-
>  tests/xfs/152                                 |     4 +-
>  tests/xfs/153                                 |     2 +-
>  25 files changed, 15177 insertions(+), 15216 deletions(-)
>  delete mode 100644 src/idmapped-mounts/idmapped-mounts.c
>  delete mode 100644 src/idmapped-mounts/utils.c
>  delete mode 100644 src/idmapped-mounts/utils.h
>  rename src/{idmapped-mounts => vfs}/Makefile (59%)
>  create mode 100644 src/vfs/btrfs-idmapped-mounts.c
>  create mode 100644 src/vfs/btrfs-idmapped-mounts.h
>  create mode 100644 src/vfs/idmapped-mounts.c
>  create mode 100644 src/vfs/idmapped-mounts.h
>  rename src/{idmapped-mounts => vfs}/missing.h (93%)
>  rename src/{idmapped-mounts => vfs}/mount-idmapped.c (100%)
>  create mode 100644 src/vfs/utils.c
>  create mode 100644 src/vfs/utils.h
>  create mode 100644 src/vfs/vfstest.c
> 
> 
> base-commit: 87cf32ad3fa234e3d5ec501e0f86516bef91d805
> -- 
> 2.34.1
> 