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

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
>  #define _GNU_SOURCE
>  
> +#include "config.h"
>  #include "tst_test.h"
>  #include "lapi/mount.h"
>  #include "lapi/syscalls.h"
> @@ -26,7 +27,11 @@ static uint64_t mnt_ids[MNT_SIZE];
>  static struct tcase {
>  	int req_usage;
>  	uint32_t size;
> +#ifdef HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
> +	uint32_t mnt_ns_fd;
> +#else
>  	uint32_t spare;
> +#endif
>  	uint64_t mnt_id;
>  	uint64_t param;
>  	uint64_t *mnt_ids;
> @@ -73,12 +78,21 @@ static struct tcase {
>  	{
>  		.req_usage = 1,
>  		.size = MNT_ID_REQ_SIZE_VER0,
> +#ifdef HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
> +		.mnt_ns_fd = -1,
> +#else
>  		.spare = -1,
> +#endif
>  		.mnt_id = LSMT_ROOT,
>  		.mnt_ids = mnt_ids,
>  		.nr_mnt_ids = MNT_SIZE,
> +#ifdef HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
> +		.exp_errno = EBADF,
> +		.msg = "invalid mnt_id_req.mnt_ns_fd bad file descriptor",
> +#else
>  		.exp_errno = EINVAL,
>  		.msg = "invalid mnt_id_req.spare",
> +#endif
>
>  	},
>  	{
>  		.req_usage = 1,
> @@ -122,7 +136,17 @@ static void run(unsigned int n)
>  		req->mnt_id = tc->mnt_id;
>  		req->param = tc->param;
>  		req->size = tc->size;
> -		req->spare = tc->spare;
> +#ifdef HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
> +		if ((tst_kvercmp(6, 18, 0)) >= 0)
> +			req->mnt_ns_fd = tc->mnt_ns_fd;
> +		else
> +			tst_brk(TCONF, "Skipping test, kernel version should be >= 6.18");
> +#else
> +		if ((tst_kvercmp(6, 18, 0)) >= 0)
> +			tst_brk(TCONF, "Skipping test, kernel version should be < 6.18");
> +		else
> +			req->spare = tc->spare;
> +#endif

I do not like this solution. Here we disable the test on newer kernel if
it was compiled on older headers and vice versa.

There are actually two problems to be solved and they are independent of
each other and we shouldn't mix these two.

First problem is that the mnt_ns_fd is not defined on older headers.
That should be fixed by a fallback defintion in lapi/. The spare in
lapi/mount.h in struct mnt_id_req should be renamed to mnt_ns_fd. And we
should use that structure if mnt_id_req is not defined in system
headers. I guess that we can use typedef for that and do something as:

struct mnt_id_req_fallback {
	uint32_t size;
	...
};

#if !defined(HAVE_STRUCT_MNT_ID_REQ) || !HAVE_STRUCT_MNT_ID_REQ_MNT_NS_FD
typedef struct mnt_id_req_fallback mnt_id_req
#else
typedef struct mnd_id_req mnt_id_req
#endif

And then use mnt_id_req in test with the guarantee that the mnt_ns_fd is
always there.

The second problem is the expected errno. That should be just set once
based on the kernel version in the test setup().

>  	}
>  
>  	TST_EXP_FAIL(tst_syscall(__NR_listmount, req, tc->mnt_ids,
> -- 
> 2.51.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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