Re: [LTP] [PATCH v2] syscalls/statx01: Add exit condition when parsing /proc/self/mountinfo

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

"xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com> writes:

> Hi Richard
>
>
>> Hello,
>> 
>> Yang Xu <xuyang2018.jy@fujitsu.com> writes:
>> 
>>> When using user filesystem such as overlayfs, the current parsing way can't
>>> work well.
>>>
>>> 63 66 8:3 / /sysroot rw,relatime - ext4 /dev/sda3 rw,seclabel
>>> 43 66 8:3 /ostree/deploy/rhivos/var /var rw,relatime shared:3 - ext4 /dev/sda3 rw,seclabel
>>>
>>> So add the exit condition for statx.mnt_id check so it can skip the
>>> underflying filesystem and parse the correct user fileystem's mnt_id.
>>>
>>> Fixes: #1001
>>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>>> ---
>>> v1-v2: change the fail message
>>>   testcases/kernel/syscalls/statx/statx01.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/statx/statx01.c b/testcases/kernel/syscalls/statx/statx01.c
>>> index 60b50958b..e9677475a 100644
>>> --- a/testcases/kernel/syscalls/statx/statx01.c
>>> +++ b/testcases/kernel/syscalls/statx/statx01.c
>>> @@ -68,7 +68,8 @@ static void test_mnt_id(struct statx *buf)
>>>   		if (sscanf(line, "%"SCNu64" %*d %d:%d", &mnt_id, &line_mjr, &line_mnr) != 3)
>>>   			continue;
>>>   
>>> -		if (line_mjr == buf->stx_dev_major && line_mnr == buf->stx_dev_minor)
>>> +		if (line_mjr == buf->stx_dev_major && line_mnr == buf->stx_dev_minor &&
>>> +				mnt_id == buf->stx_mnt_id)
>>>   			break;
>>>   	}
>>>   
>>> @@ -80,8 +81,8 @@ static void test_mnt_id(struct statx *buf)
>>>   			mnt_id);
>>>   	else
>>>   		tst_res(TFAIL,
>>> -			"statx.stx_mnt_id(%"PRIu64") is different from mount_id(%"PRIu64") in /proc/self/mountinfo",
>>> -			(uint64_t)buf->stx_mnt_id, mnt_id);
>>> +			"statx.stx_mnt_id(%"PRIu64") doesn't exist in /proc/self/mountinfo",
>>> +			(uint64_t)buf->stx_mnt_id);
>> 
>> The mnt_id may exist in mountinfo, but not the triple (mnt_id,
>> dev_major, dev_minor). So really we should print all three here (unless
>> we already display that somewhere else).
>> 
>
> Yes, so how about the following changes:
>
> diff --git a/testcases/kernel/syscalls/statx/statx01.c 
> b/testcases/kernel/syscalls/statx/statx01.c
> index 60b50958b..8f3b01b32 100644
> --- a/testcases/kernel/syscalls/statx/statx01.c
> +++ b/testcases/kernel/syscalls/statx/statx01.c
> @@ -53,7 +53,7 @@ static void test_mnt_id(struct statx *buf)
>   {
>          FILE *file;
>          char line[PATH_MAX];
> -       int pid;
> +       int pid, flag = 0;
>          unsigned int line_mjr, line_mnr;
>          uint64_t mnt_id;
>
> @@ -68,20 +68,26 @@ static void test_mnt_id(struct statx *buf)
>                  if (sscanf(line, "%"SCNu64" %*d %d:%d", &mnt_id, 
> &line_mjr, &line_mnr) != 3)
>                          continue;
>
> -               if (line_mjr == buf->stx_dev_major && line_mnr == 
> buf->stx_dev_minor)
> -                       break;
> +               if (line_mjr == buf->stx_dev_major && line_mnr == 
> buf->stx_dev_minor) {
> +                       if (buf->stx_mnt_id == mnt_id) {
> +                               flag = 1;
> +                               break
> +                       }
> +                       tst_res(TINFO, "%s doesn't contain (%"PRIu64") 
> %d:%d",
> +                               line, (uint64_t)buf->stx_mnt_id, 
> buf->stx_dev_major, buf->stx_dev_minor);
> +               }
>          }
>
>          SAFE_FCLOSE(file);
>
> -       if (buf->stx_mnt_id == mnt_id)
> +       if (flag)
>                  tst_res(TPASS,
>                          "statx.stx_mnt_id equals to mount_id(%"PRIu64") 
> in /proc/self/mountinfo",
>                          mnt_id);
>          else
>                  tst_res(TFAIL,
> -                       "statx.stx_mnt_id(%"PRIu64") is different from 
> mount_id(%"PRIu64") in /proc/self/mountinfo",
> -                       (uint64_t)buf->stx_mnt_id, mnt_id);
> +                       "statx.stx_mnt_id(%"PRIu64") doesn't exist in 
> /proc/self/mountinfo",
> +                       (uint64_t)buf->stx_mnt_id);

Looks good, (except for the line wrap). With that

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>
>          pid = getpid();
>          snprintf(line, PATH_MAX, "/proc/%d/fdinfo/%d", pid, file_fd);
>
> Best Regards
> Yang Xu
>>>   
>>>   	pid = getpid();
>>>   	snprintf(line, PATH_MAX, "/proc/%d/fdinfo/%d", pid, file_fd);
>>> -- 
>>> 2.27.0
>> 
>> 


-- 
Thank you,
Richard.

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