Re: [PATCH v3 1/5] idmapped-mounts: Reset errno to zero after detect fs_allow_idmap

[Zorro Lang <zlang@redhat.com>]

On Sat, May 07, 2022 at 01:33:33AM +0000, xuyang2018.jy@fujitsu.com wrote:
> Hi Zorro
> 
> Since  Christian doesn't send  a new patchset(for rename idmap-mount)
> based on lastest xfstests, should I send a v4 patch for the following
> patches today?
> "idmapped-mounts: Reset errno to zero after detect fs_allow_idmap"
> " idmapped-mounts: Add mknodat operation in setgid test"
> "idmapped-mounts: Add open with O_TMPFILE operation in setgid test"
> 
> So you can merge these three patches if you plan to announce a new
> xfstests version in this weekend.
> 
> What do you think about it?

Sure, you can send V4 of patch 1/5 ～ 3/5 (base on latest for-next branch
please), as they have been reviewed and tested. Christian's patch (about
refactor idmapped testing) might need more review, he just sent it out to
get some review points I think (cc Christian).

If you'd like to catch up the release of this weekend, please send your
v4 patch ASAP. Due to I need time to do regression test before pushing.
It'll wait for next week if too late.

Thanks,
Zorro

> 
> Best Regards
> Yang Xu
> > If we run case on old kernel that doesn't support mount_setattr and
> > then fail on our own function before call is_setgid/is_setuid function
> > to reset errno, run_test will print "Function not implement" error.
> > 
> > Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
> > ---
> >   src/idmapped-mounts/idmapped-mounts.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/idmapped-mounts/idmapped-mounts.c b/src/idmapped-mounts/idmapped-mounts.c
> > index 4cf6c3bb..8e6405c5 100644
> > --- a/src/idmapped-mounts/idmapped-mounts.c
> > +++ b/src/idmapped-mounts/idmapped-mounts.c
> > @@ -14070,6 +14070,8 @@ int main(int argc, char *argv[])
> >   		die("failed to open %s", t_mountpoint_scratch);
> > 
> >   	t_fs_allow_idmap = fs_allow_idmap();
> > +	/* don't copy ENOSYS errno to child process on older kernel */
> > +	errno = 0;
> >   	if (supported) {
> >   		/*
> >   		 * Caller just wants to know whether the filesystem we're on