| Message ID | 1670901258-1995-1-git-send-email-xuyang2018.jy@fujitsu.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] syscalls/statx01: Add exit condition when parsing /proc/self/mountinfo | expand |
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). > > pid = getpid(); > snprintf(line, PATH_MAX, "/proc/%d/fdinfo/%d", pid, file_fd); > -- > 2.27.0
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); 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 > >
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 >> >>
Hi Richard > 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> Yes, I remove the line wrap and merged, thanks for your review. Best Regards Yang Xu > >> >> 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 >>> >>> > >
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); pid = getpid(); snprintf(line, PATH_MAX, "/proc/%d/fdinfo/%d", pid, file_fd);
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(-)