On Thu, May 8, 2025 at 9:43 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
>
> On 5/7/25 1:43 PM, Amir Goldstein wrote:
> > Add helper to utils and use it in mount-notify and statmount tests.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > .../filesystems/mount-notify/Makefile | 3 ++
> > .../mount-notify/mount-notify_test.c | 13 ++-------
> > .../selftests/filesystems/statmount/Makefile | 3 ++
> > .../filesystems/statmount/statmount_test_ns.c | 28 +++----------------
> > tools/testing/selftests/filesystems/utils.c | 20 +++++++++++++
> > tools/testing/selftests/filesystems/utils.h | 2 ++
> > 6 files changed, 34 insertions(+), 35 deletions(-)
> >
> > diff --git a/tools/testing/selftests/filesystems/mount-notify/Makefile b/tools/testing/selftests/filesystems/mount-notify/Makefile
> > index 41ebfe558a0a..55a2e5399e8a 100644
> > --- a/tools/testing/selftests/filesystems/mount-notify/Makefile
> > +++ b/tools/testing/selftests/filesystems/mount-notify/Makefile
> > @@ -1,7 +1,10 @@
> > # SPDX-License-Identifier: GPL-2.0-or-later
> >
> > CFLAGS += -Wall -O2 -g $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
> > +LDLIBS += -lcap
>
> This addition of -lcap goes completely unmentioned in the commit log.
> I'm guessing you are fixing things up to build, so this definitely
> deserves an explanation there.
>
This is needed because we are linking with utils.c code.
See below.
I guess we could have built a filesystems selftests utils library,
but that seems like an operkill?
> >
> > TEST_GEN_PROGS := mount-notify_test
> >
> > include ../../lib.mk
> > +
> > +$(OUTPUT)/mount-notify_test: ../utils.c
> > diff --git a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> > index 4f0f325379b5..63ce708d93ed 100644
> > --- a/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> > +++ b/tools/testing/selftests/filesystems/mount-notify/mount-notify_test.c
> > @@ -13,6 +13,7 @@
> >
> > #include "../../kselftest_harness.h"
> > #include "../statmount/statmount.h"
> > +#include "../utils.h"
> >
> > // Needed for linux/fanotify.h
> > #ifndef __kernel_fsid_t
> > @@ -23,16 +24,6 @@ typedef struct {
> >
> > #include <sys/fanotify.h>
> >
> > -static uint64_t get_mnt_id(struct __test_metadata *const _metadata,
> > - const char *path)
> > -{
> > - struct statx sx;
> > -
> > - ASSERT_EQ(statx(AT_FDCWD, path, 0, STATX_MNT_ID_UNIQUE, &sx), 0);
> > - ASSERT_TRUE(!!(sx.stx_mask & STATX_MNT_ID_UNIQUE));
> > - return sx.stx_mnt_id;
> > -}
> > -
> > static const char root_mntpoint_templ[] = "/tmp/mount-notify_test_root.XXXXXX";
> >
> > static const int mark_cmds[] = {
> > @@ -81,7 +72,7 @@ FIXTURE_SETUP(fanotify)
> >
> > ASSERT_EQ(mkdir("b", 0700), 0);
> >
> > - self->root_id = get_mnt_id(_metadata, "/");
> > + self->root_id = get_unique_mnt_id("/");
> > ASSERT_NE(self->root_id, 0);
> >
> > for (i = 0; i < NUM_FAN_FDS; i++) {
> > diff --git a/tools/testing/selftests/filesystems/statmount/Makefile b/tools/testing/selftests/filesystems/statmount/Makefile
> > index 19adebfc2620..8e354fe99b44 100644
> > --- a/tools/testing/selftests/filesystems/statmount/Makefile
> > +++ b/tools/testing/selftests/filesystems/statmount/Makefile
> > @@ -1,7 +1,10 @@
> > # SPDX-License-Identifier: GPL-2.0-or-later
> >
> > CFLAGS += -Wall -O2 -g $(KHDR_INCLUDES) $(TOOLS_INCLUDES)
> > +LDLIBS += -lcap
>
> And here.
>
> >
> > TEST_GEN_PROGS := statmount_test statmount_test_ns listmount_test
> >
> > include ../../lib.mk
> > +
> > +$(OUTPUT)/statmount_test_ns: ../utils.c
>
> This is surprising: a new Makefile target, without removing an old one.
> And it's still listed in TEST_GEN_PROGS...
>
> Why did you feel the need to add this target?
I am not a makefile expert, but this states that statmount_test_ns
needs to link with utils.c code.
I copied it from overlayfs/Makefile.
Is there a different way to express this build dependency?
Thanks,
Amir.
