Re: [LTP] [PATCH v5] listmount04.c: Update case support mnt_id_req.mnt_ns_fd

[Petr Vorel <pvorel@suse.cz>]

> Hi!
> > > -#ifndef HAVE_STRUCT_MNT_ID_REQ
> > > -struct mnt_id_req {
> > > +struct mnt_id_req_fallback {
> > >  	uint32_t size;
> > > -	uint32_t spare;
> > > +	uint32_t mnt_ns_fd;
> > >  	uint64_t mnt_id;
> > >  	uint64_t param;
> > >  	uint64_t mnt_ns_id;
> > >  };
> > > +
> > > +#if !defined(HAVE_STRUCT_MNT_ID_REQ) || !HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
> > Shouldn't be !HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
> > !defined(HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD) ?

> > Also, this patchset introduces compilation error on old systems:

> > In file included from listmount03.c:13:0:
> > listmount.h:20:4: error: 'struct mnt_id_req' has no member named 'mnt_id'
> >    .mnt_id = mnt_id,

> > Obviously some definition is wrong.

> That looks like we need configure check for mnt_id too however that
> happens in a different test.

Yes:


Do we still need the check for the struct mnt_id_req now? I'd say no:

-AC_CHECK_TYPES([struct mnt_id_req],,,[#include <sys/mount.h>])
+AC_CHECK_MEMBERS([struct mnt_id_req.mnt_ns_fd, struct mnt_id_req.mnt_id],,,[#include <sys/mount.h>])

-#if !defined(HAVE_STRUCT_MNT_ID_REQ) || !HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
+#if !defined(HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD) || !defined(HAVE_STRUCT_MNT_ID_REQ_MNT_ID)

But we also need to change listmount.h to use the struct fallback. Full diff is
at the end.

> > https://github.com/pevik/ltp/actions/runs/20166087670/job/57889711762

> > (CI does not work, therefore I pushed you patchset.)

> > > +typedef struct mnt_id_req_fallback mnt_id_req;
> > > +#else
> > > +typedef struct mnt_id_req mnt_id_req;
> > >  #endif

> > ...
> > > +static void setup(void)
> > > +{
> > > +	if (tst_kvercmp(6, 18, 0) >= 0) {
> > > +		tcases[4].exp_errno = EBADF;
> > > +		tcases[4].msg = "invalid mnt_id_req.mnt_ns_fd (EBADF)";

> > Hardcoding array index is asking for problems in the future.

> > That's why I suggested more complicated code in previous version.
> > https://lore.kernel.org/ltp/20251211115111.GA63924@pevik/

> Another solution would be to define testcases for both for EINVAL and
> EBADF and add a flag which should be enabled on newer kernel:

> #define BEFORE_6_18 1
> #define AFTER_6_18 2

> ...
> 	{
>                 .req_usage = 1,
>                 .size = MNT_ID_REQ_SIZE_VER0,
>                 .spare = -1,
>                 .mnt_id = LSMT_ROOT,
>                 .mnt_ids = mnt_ids,
>                 .nr_mnt_ids = MNT_SIZE,
>                 .exp_errno = EINVAL,
>                 .msg = "invalid mnt_id_req.spare",
> 		.kver = BEFORE_6_18,
>         },
> 	{
>                 .req_usage = 1,
>                 .size = MNT_ID_REQ_SIZE_VER0,
>                 .spare = -1,
>                 .mnt_id = LSMT_ROOT,
>                 .mnt_ids = mnt_ids,
>                 .nr_mnt_ids = MNT_SIZE,
>                 .exp_errno = EBADF,
>                 .msg = "invalid mnt_id_req.mnt_nf_fd",
> 		.kver = AFTER_6_18,
>         },
> ...

> static void run(...)
> {
> 	...

> 	if (tc->kver && tc->kver != kver)
> 		return;

Yeah that would work as well, but I suppose TCONF will be needed
due check for results.

> 	...
> }

> static void setup(void)
> {
> 	if (tst_kvercmp(6, 18, 0) >= 0)
> 		kver = AFTER_6_18;
> 	else
> 		kver = BEFORE_6_18;
> }

Kind regards,
Petr

Diff without kernel version change.

+++ configure.ac
@@ -262,8 +262,8 @@ AC_CHECK_TYPES([struct cachestat_range],,,[#include <sys/mman.h>])
 AC_CHECK_TYPES([struct cachestat],,,[#include <sys/mman.h>])
 
 # Defined in <linux/mount.h>, but include/lapi/mount.h includes <sys/mount.h> */
-AC_CHECK_TYPES([struct mnt_id_req],,,[#include <sys/mount.h>])
-AC_CHECK_MEMBERS([struct mnt_id_req.mnt_ns_fd],,,[#include <sys/mount.h>])
+#AC_CHECK_TYPES([struct mnt_id_req],,,[#include <sys/mount.h>])
+AC_CHECK_MEMBERS([struct mnt_id_req.mnt_ns_fd, struct mnt_id_req.mnt_id],,,[#include <sys/mount.h>])
 AC_CHECK_TYPES([struct statmount],,,[#include <sys/mount.h>])
 AC_CHECK_MEMBERS([struct statmount.mnt_ns_id],,,[#include <unistd.h>
 #include <linux/mount.h>])
diff --git include/lapi/mount.h include/lapi/mount.h
index 7b1c4ca0fe..8bde0e563a 100644
--- include/lapi/mount.h
+++ include/lapi/mount.h
@@ -53,7 +53,7 @@ struct mnt_id_req_fallback {
 	uint64_t mnt_ns_id;
 };
 
-#if !defined(HAVE_STRUCT_MNT_ID_REQ) || !HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
+#if !defined(HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD) || !defined(HAVE_STRUCT_MNT_ID_REQ_MNT_NS_ID)
 typedef struct mnt_id_req_fallback mnt_id_req;
 #else
 typedef struct mnt_id_req mnt_id_req;
diff --git testcases/kernel/syscalls/listmount/listmount.h testcases/kernel/syscalls/listmount/listmount.h
index aad927f714..74f334c74e 100644
--- testcases/kernel/syscalls/listmount/listmount.h
+++ testcases/kernel/syscalls/listmount/listmount.h
@@ -8,6 +8,7 @@
 
 #define _GNU_SOURCE
 
+#include "config.h"
 #include "tst_test.h"
 #include "lapi/mount.h"
 #include "lapi/syscalls.h"
@@ -15,11 +16,10 @@
 static inline ssize_t listmount(uint64_t mnt_id, uint64_t last_mnt_id,
 			 uint64_t list[], size_t num, unsigned int flags)
 {
-	struct mnt_id_req req = {
-		.size = MNT_ID_REQ_SIZE_VER0,
-		.mnt_id = mnt_id,
-		.param = last_mnt_id,
-	};
+	mnt_id_req *req = NULL;
+	req->size = MNT_ID_REQ_SIZE_VER0;
+	req->mnt_id = mnt_id;
+	req->param = last_mnt_id;
 
 	return tst_syscall(__NR_listmount, &req, list, num, flags);
 }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp