| Message ID | 20210322134522.916512-1-brauner@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | fstests: add idmapped mounts tests | expand |
On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > From: Christian Brauner <christian.brauner@ubuntu.com> > > Hey everyone, > > This series is available from: > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > https://github.com/brauner/xfstests/tree/idmapped_mounts I sent this series from my kernel.org mail address and patch 2/6 hasn't made it through this time too. So it seems that vger is rejecting it due to its size is my guess. I'll go poing the #korg folks to ask what's going on and whether that can be handled. Thanks! Christian
On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > Hey everyone, > > > > This series is available from: > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > made it through this time too. So it seems that vger is rejecting it due > to its size is my guess. I'll go poing the #korg folks to ask what's > going on and whether that can be handled. Ok, Konstantin confirmed that patch 2/6 got dropped because of it's size. Nothing to do about this now but just as an fyi. Thanks! Christian
On Mon, Mar 22, 2021 at 4:22 PM Christian Brauner <christian.brauner@ubuntu.com> wrote: > > On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > > > Hey everyone, > > > > > > This series is available from: > > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > > made it through this time too. So it seems that vger is rejecting it due > > to its size is my guess. I'll go poing the #korg folks to ask what's > > going on and whether that can be handled. > > Ok, Konstantin confirmed that patch 2/6 got dropped because of it's > size. Nothing to do about this now but just as an fyi. > Since 2/6 got dropped, I'll write a small nit here which is also relevant to the rest of the series: --- a/tests/generic/group +++ b/tests/generic/group @@ -634,3 +634,4 @@ 629 auto quick rw copy_range 630 auto quick rw dedupe clone 631 auto quick mount +632 auto atime attr cap io_uring mount perms quick rw unlink This is a mouthful of test tags, but that doesn't hurt. I would personally not bother with obscure tags like 'unlink' but whatever. Two things I would request are: 1. Keep 'auto quick' before all other tags. There is no strong rule about this format, but that's the common practice and it makes sense IMO because -g auto and -g quick are the far more commonly used groups of tests, so it's convenient to be able to 'eye grep' those tests in the group file. 2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) This would be used for running tests with -g idmapped for quick sanity when modifiying idmapped mounts related code Thanks, Amir.
On Wed, Mar 24, 2021 at 10:03:43AM +0200, Amir Goldstein wrote: > On Mon, Mar 22, 2021 at 4:22 PM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > > > On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > > > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > > > > > Hey everyone, > > > > > > > > This series is available from: > > > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > > > > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > > > made it through this time too. So it seems that vger is rejecting it due > > > to its size is my guess. I'll go poing the #korg folks to ask what's > > > going on and whether that can be handled. > > > > Ok, Konstantin confirmed that patch 2/6 got dropped because of it's > > size. Nothing to do about this now but just as an fyi. > > > > Since 2/6 got dropped, I'll write a small nit here which is also You could still pull it from above. I don't think resending would retain the patch afaict until vger has been ported by Konstantin. > relevant to the rest of the series: > > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -634,3 +634,4 @@ > 629 auto quick rw copy_range > 630 auto quick rw dedupe clone > 631 auto quick mount > +632 auto atime attr cap io_uring mount perms quick rw unlink > > This is a mouthful of test tags, but that doesn't hurt. > I would personally not bother with obscure tags like 'unlink' but whatever. > > Two things I would request are: > 1. Keep 'auto quick' before all other tags. There is no strong rule about > this format, but that's the common practice and it makes sense IMO > because -g auto and -g quick are the far more commonly used groups of > tests, so it's convenient to be able to 'eye grep' those tests in > the group file. > 2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) > This would be used for running tests with -g idmapped for quick sanity > when modifiying idmapped mounts related code Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in charge of applying it?) whether he can fix this up when applying or whether he wants a resend. Christian
On Wed, Mar 24, 2021 at 10:41 AM Christian Brauner <christian.brauner@ubuntu.com> wrote: > > On Wed, Mar 24, 2021 at 10:03:43AM +0200, Amir Goldstein wrote: > > On Mon, Mar 22, 2021 at 4:22 PM Christian Brauner > > <christian.brauner@ubuntu.com> wrote: > > > > > > On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > > > > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > > > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > > > > > > > Hey everyone, > > > > > > > > > > This series is available from: > > > > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > > > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > > > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > > > > > > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > > > > made it through this time too. So it seems that vger is rejecting it due > > > > to its size is my guess. I'll go poing the #korg folks to ask what's > > > > going on and whether that can be handled. > > > > > > Ok, Konstantin confirmed that patch 2/6 got dropped because of it's > > > size. Nothing to do about this now but just as an fyi. > > > > > > > Since 2/6 got dropped, I'll write a small nit here which is also > > You could still pull it from above. I don't think resending would retain > the patch afaict until vger has been ported by Konstantin. > > > relevant to the rest of the series: > > > > --- a/tests/generic/group > > +++ b/tests/generic/group > > @@ -634,3 +634,4 @@ > > 629 auto quick rw copy_range > > 630 auto quick rw dedupe clone > > 631 auto quick mount > > +632 auto atime attr cap io_uring mount perms quick rw unlink > > > > This is a mouthful of test tags, but that doesn't hurt. > > I would personally not bother with obscure tags like 'unlink' but whatever. > > > > Two things I would request are: > > 1. Keep 'auto quick' before all other tags. There is no strong rule about > > this format, but that's the common practice and it makes sense IMO > > because -g auto and -g quick are the far more commonly used groups of > > tests, so it's convenient to be able to 'eye grep' those tests in > > the group file. > > 2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) > > This would be used for running tests with -g idmapped for quick sanity > > when modifiying idmapped mounts related code > > Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in > charge of applying it?) He will. Meanwhile, found a bug in Makefile: --- a/src/idmapped-mounts/Makefile +++ b/src/idmapped-mounts/Makefile @@ -25,11 +25,11 @@ depend: .dep include $(BUILDRULES) -idmapped-mounts: +idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS) @echo " [CC] $@" $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS) $(LDFLAGS) $(LDLIBS) -mount-idmapped: +mount-idmapped: $(CFILES_MOUNT_IDMAPPED) @echo " [CC] $@" $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS) $(LDFLAGS) $(LDLIBS) --- And in parsing of /proc/<pid>/ns/user: --- a/src/idmapped-mounts/mount-idmapped.c +++ b/src/idmapped-mounts/mount-idmapped.c @@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type, pid_t pid, const char *buf, s int fd = -EBADF, setgroups_fd = -EBADF; int fret = -1; int ret; - char path[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + + char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + STRLITERALLEN("/setgroups") + 1]; if (geteuid() != 0 && map_type == ID_TYPE_GID) { @@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap) { int ret; pid_t pid; - char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + + char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + STRLITERALLEN("/ns/user") + 1]; pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS); @@ -364,7 +364,7 @@ int main(int argc, char *argv[]) while ((ret = getopt_long_only(argc, argv, "", longopts, &index)) != -1) { switch (ret) { case 'a': - if (strnequal(optarg, "/proc", STRLITERALLEN("/proc/"))) { + if (strnequal(optarg, "/proc/", STRLITERALLEN("/proc/"))) { fd_userns = open(optarg, O_RDONLY | O_CLOEXEC); --- Thanks, Amir.
On Wed, Mar 24, 2021 at 1:24 PM Amir Goldstein <amir73il@gmail.com> wrote: > > On Wed, Mar 24, 2021 at 10:41 AM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > > > On Wed, Mar 24, 2021 at 10:03:43AM +0200, Amir Goldstein wrote: > > > On Mon, Mar 22, 2021 at 4:22 PM Christian Brauner > > > <christian.brauner@ubuntu.com> wrote: > > > > > > > > On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > > > > > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > > > > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > > > > > > > > > Hey everyone, > > > > > > > > > > > > This series is available from: > > > > > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > > > > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > > > > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > > > > > > > > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > > > > > made it through this time too. So it seems that vger is rejecting it due > > > > > to its size is my guess. I'll go poing the #korg folks to ask what's > > > > > going on and whether that can be handled. > > > > > > > > Ok, Konstantin confirmed that patch 2/6 got dropped because of it's > > > > size. Nothing to do about this now but just as an fyi. > > > > > > > > > > Since 2/6 got dropped, I'll write a small nit here which is also > > > > You could still pull it from above. I don't think resending would retain > > the patch afaict until vger has been ported by Konstantin. > > > > > relevant to the rest of the series: > > > > > > --- a/tests/generic/group > > > +++ b/tests/generic/group > > > @@ -634,3 +634,4 @@ > > > 629 auto quick rw copy_range > > > 630 auto quick rw dedupe clone > > > 631 auto quick mount > > > +632 auto atime attr cap io_uring mount perms quick rw unlink > > > > > > This is a mouthful of test tags, but that doesn't hurt. > > > I would personally not bother with obscure tags like 'unlink' but whatever. > > > > > > Two things I would request are: > > > 1. Keep 'auto quick' before all other tags. There is no strong rule about > > > this format, but that's the common practice and it makes sense IMO > > > because -g auto and -g quick are the far more commonly used groups of > > > tests, so it's convenient to be able to 'eye grep' those tests in > > > the group file. > > > 2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) > > > This would be used for running tests with -g idmapped for quick sanity > > > when modifiying idmapped mounts related code > > > > Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in > > charge of applying it?) > > He will. > > Meanwhile, found a bug in Makefile: > > --- a/src/idmapped-mounts/Makefile > +++ b/src/idmapped-mounts/Makefile > @@ -25,11 +25,11 @@ depend: .dep > > include $(BUILDRULES) > > -idmapped-mounts: > +idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS) > @echo " [CC] $@" > $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS) > $(LDFLAGS) $(LDLIBS) > > -mount-idmapped: > +mount-idmapped: $(CFILES_MOUNT_IDMAPPED) > @echo " [CC] $@" > $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS) > $(LDFLAGS) $(LDLIBS) > --- > > And in parsing of /proc/<pid>/ns/user: > > --- a/src/idmapped-mounts/mount-idmapped.c > +++ b/src/idmapped-mounts/mount-idmapped.c > @@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type, > pid_t pid, const char *buf, s > int fd = -EBADF, setgroups_fd = -EBADF; > int fret = -1; > int ret; > - char path[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + > + char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + > STRLITERALLEN("/setgroups") + 1]; > > if (geteuid() != 0 && map_type == ID_TYPE_GID) { > @@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap) > { > int ret; > pid_t pid; > - char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + > + char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + > STRLITERALLEN("/ns/user") + 1]; > > pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS); > @@ -364,7 +364,7 @@ int main(int argc, char *argv[]) > while ((ret = getopt_long_only(argc, argv, "", longopts, > &index)) != -1) { > switch (ret) { > case 'a': > - if (strnequal(optarg, "/proc", > STRLITERALLEN("/proc/"))) { > + if (strnequal(optarg, "/proc/", > STRLITERALLEN("/proc/"))) { > fd_userns = open(optarg, O_RDONLY | O_CLOEXEC); > --- > And also: @@ -402,12 +402,15 @@ int main(int argc, char *argv[]) exit(EXIT_FAILURE); } - if (!list_empty(&active_map)) { + if (!list_empty(&active_map) || fd_userns > 0) { struct mount_attr attr = { .attr_set = MOUNT_ATTR_IDMAP, }; - attr.userns_fd = get_userns_fd_from_idmap(&active_map); + if (fd_userns > 0) + attr.userns_fd = fd_userns; + else + attr.userns_fd = get_userns_fd_from_idmap(&active_map); if (attr.userns_fd < 0) exit_log("%m - Failed to create user namespace\n"); --- It's a bug in the test helper program, not a bug in the test per-se because the test does not use the --map-mount=/proc/<pid>/ns/user option. Thanks, Amir.
On Wed, Mar 24, 2021 at 01:24:36PM +0200, Amir Goldstein wrote: > On Wed, Mar 24, 2021 at 10:41 AM Christian Brauner > <christian.brauner@ubuntu.com> wrote: > > > > On Wed, Mar 24, 2021 at 10:03:43AM +0200, Amir Goldstein wrote: > > > On Mon, Mar 22, 2021 at 4:22 PM Christian Brauner > > > <christian.brauner@ubuntu.com> wrote: > > > > > > > > On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > > > > > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > > > > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > > > > > > > > > Hey everyone, > > > > > > > > > > > > This series is available from: > > > > > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > > > > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > > > > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > > > > > > > > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > > > > > made it through this time too. So it seems that vger is rejecting it due > > > > > to its size is my guess. I'll go poing the #korg folks to ask what's > > > > > going on and whether that can be handled. > > > > > > > > Ok, Konstantin confirmed that patch 2/6 got dropped because of it's > > > > size. Nothing to do about this now but just as an fyi. > > > > > > > > > > Since 2/6 got dropped, I'll write a small nit here which is also > > > > You could still pull it from above. I don't think resending would retain > > the patch afaict until vger has been ported by Konstantin. > > > > > relevant to the rest of the series: > > > > > > --- a/tests/generic/group > > > +++ b/tests/generic/group > > > @@ -634,3 +634,4 @@ > > > 629 auto quick rw copy_range > > > 630 auto quick rw dedupe clone > > > 631 auto quick mount > > > +632 auto atime attr cap io_uring mount perms quick rw unlink > > > > > > This is a mouthful of test tags, but that doesn't hurt. > > > I would personally not bother with obscure tags like 'unlink' but whatever. > > > > > > Two things I would request are: > > > 1. Keep 'auto quick' before all other tags. There is no strong rule about > > > this format, but that's the common practice and it makes sense IMO > > > because -g auto and -g quick are the far more commonly used groups of > > > tests, so it's convenient to be able to 'eye grep' those tests in > > > the group file. > > > 2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) > > > This would be used for running tests with -g idmapped for quick sanity > > > when modifiying idmapped mounts related code > > > > Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in > > charge of applying it?) > > He will. > > Meanwhile, found a bug in Makefile: > > --- a/src/idmapped-mounts/Makefile > +++ b/src/idmapped-mounts/Makefile > @@ -25,11 +25,11 @@ depend: .dep > > include $(BUILDRULES) > > -idmapped-mounts: > +idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS) > @echo " [CC] $@" > $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS) > $(LDFLAGS) $(LDLIBS) > > -mount-idmapped: > +mount-idmapped: $(CFILES_MOUNT_IDMAPPED) > @echo " [CC] $@" > $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS) > $(LDFLAGS) $(LDLIBS) Stupid question maybe but what is the reason for this change? It builds fine without those lines. Building idmapped-mounts [CC] idmapped-mounts [CC] mount-idmapped > --- > > And in parsing of /proc/<pid>/ns/user: > > --- a/src/idmapped-mounts/mount-idmapped.c > +++ b/src/idmapped-mounts/mount-idmapped.c > @@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type, > pid_t pid, const char *buf, s > int fd = -EBADF, setgroups_fd = -EBADF; > int fret = -1; > int ret; > - char path[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + > + char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + > STRLITERALLEN("/setgroups") + 1]; Thanks. Though it's very unlikely for the kernel to assign a pid that is at least 100 billion. Even newest systems only bump the maximum pid number to 4m and we would've thankfully caught this in the snprintf()s below. > > if (geteuid() != 0 && map_type == ID_TYPE_GID) { > @@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap) > { > int ret; > pid_t pid; > - char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + > + char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + > STRLITERALLEN("/ns/user") + 1]; > > pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS); > @@ -364,7 +364,7 @@ int main(int argc, char *argv[]) > while ((ret = getopt_long_only(argc, argv, "", longopts, > &index)) != -1) { > switch (ret) { > case 'a': > - if (strnequal(optarg, "/proc", > STRLITERALLEN("/proc/"))) { > + if (strnequal(optarg, "/proc/", > STRLITERALLEN("/proc/"))) { > fd_userns = open(optarg, O_RDONLY | O_CLOEXEC); That's currently not used by the code and would've simply meant we failed. Will fix this up and push it to the tree I linked to earlier. Thank you! Christian
On Wed, Mar 24, 2021 at 03:33:53PM +0200, Amir Goldstein wrote: > On Wed, Mar 24, 2021 at 1:24 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > On Wed, Mar 24, 2021 at 10:41 AM Christian Brauner > > <christian.brauner@ubuntu.com> wrote: > > > > > > On Wed, Mar 24, 2021 at 10:03:43AM +0200, Amir Goldstein wrote: > > > > On Mon, Mar 22, 2021 at 4:22 PM Christian Brauner > > > > <christian.brauner@ubuntu.com> wrote: > > > > > > > > > > On Mon, Mar 22, 2021 at 02:50:02PM +0100, Christian Brauner wrote: > > > > > > On Mon, Mar 22, 2021 at 02:45:16PM +0100, Christian Brauner wrote: > > > > > > > From: Christian Brauner <christian.brauner@ubuntu.com> > > > > > > > > > > > > > > Hey everyone, > > > > > > > > > > > > > > This series is available from: > > > > > > > https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts > > > > > > > https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts > > > > > > > https://github.com/brauner/xfstests/tree/idmapped_mounts > > > > > > > > > > > > I sent this series from my kernel.org mail address and patch 2/6 hasn't > > > > > > made it through this time too. So it seems that vger is rejecting it due > > > > > > to its size is my guess. I'll go poing the #korg folks to ask what's > > > > > > going on and whether that can be handled. > > > > > > > > > > Ok, Konstantin confirmed that patch 2/6 got dropped because of it's > > > > > size. Nothing to do about this now but just as an fyi. > > > > > > > > > > > > > Since 2/6 got dropped, I'll write a small nit here which is also > > > > > > You could still pull it from above. I don't think resending would retain > > > the patch afaict until vger has been ported by Konstantin. > > > > > > > relevant to the rest of the series: > > > > > > > > --- a/tests/generic/group > > > > +++ b/tests/generic/group > > > > @@ -634,3 +634,4 @@ > > > > 629 auto quick rw copy_range > > > > 630 auto quick rw dedupe clone > > > > 631 auto quick mount > > > > +632 auto atime attr cap io_uring mount perms quick rw unlink > > > > > > > > This is a mouthful of test tags, but that doesn't hurt. > > > > I would personally not bother with obscure tags like 'unlink' but whatever. > > > > > > > > Two things I would request are: > > > > 1. Keep 'auto quick' before all other tags. There is no strong rule about > > > > this format, but that's the common practice and it makes sense IMO > > > > because -g auto and -g quick are the far more commonly used groups of > > > > tests, so it's convenient to be able to 'eye grep' those tests in > > > > the group file. > > > > 2. Please tags all the idmapped tests with a tag 'idmapped' (or whatever) > > > > This would be used for running tests with -g idmapped for quick sanity > > > > when modifiying idmapped mounts related code > > > > > > Ok, I'll wait for Eryu to respond (Since I assume he'll be the one in > > > charge of applying it?) > > > > He will. > > > > Meanwhile, found a bug in Makefile: > > > > --- a/src/idmapped-mounts/Makefile > > +++ b/src/idmapped-mounts/Makefile > > @@ -25,11 +25,11 @@ depend: .dep > > > > include $(BUILDRULES) > > > > -idmapped-mounts: > > +idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS) > > @echo " [CC] $@" > > $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS) > > $(LDFLAGS) $(LDLIBS) > > > > -mount-idmapped: > > +mount-idmapped: $(CFILES_MOUNT_IDMAPPED) > > @echo " [CC] $@" > > $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS) > > $(LDFLAGS) $(LDLIBS) > > --- > > > > And in parsing of /proc/<pid>/ns/user: > > > > --- a/src/idmapped-mounts/mount-idmapped.c > > +++ b/src/idmapped-mounts/mount-idmapped.c > > @@ -175,7 +175,7 @@ static int write_id_mapping(idmap_type_t map_type, > > pid_t pid, const char *buf, s > > int fd = -EBADF, setgroups_fd = -EBADF; > > int fret = -1; > > int ret; > > - char path[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + > > + char path[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + > > STRLITERALLEN("/setgroups") + 1]; > > > > if (geteuid() != 0 && map_type == ID_TYPE_GID) { > > @@ -273,7 +273,7 @@ static int get_userns_fd_from_idmap(struct list *idmap) > > { > > int ret; > > pid_t pid; > > - char path_ns[STRLITERALLEN("/proc") + INTTYPE_TO_STRLEN(pid_t) + > > + char path_ns[STRLITERALLEN("/proc/") + INTTYPE_TO_STRLEN(pid_t) + > > STRLITERALLEN("/ns/user") + 1]; > > > > pid = do_clone(get_userns_fd_cb, NULL, CLONE_NEWUSER | CLONE_NEWNS); > > @@ -364,7 +364,7 @@ int main(int argc, char *argv[]) > > while ((ret = getopt_long_only(argc, argv, "", longopts, > > &index)) != -1) { > > switch (ret) { > > case 'a': > > - if (strnequal(optarg, "/proc", > > STRLITERALLEN("/proc/"))) { > > + if (strnequal(optarg, "/proc/", > > STRLITERALLEN("/proc/"))) { > > fd_userns = open(optarg, O_RDONLY | O_CLOEXEC); > > --- > > > > And also: > > @@ -402,12 +402,15 @@ int main(int argc, char *argv[]) > exit(EXIT_FAILURE); > } > > - if (!list_empty(&active_map)) { > + if (!list_empty(&active_map) || fd_userns > 0) { > struct mount_attr attr = { > .attr_set = MOUNT_ATTR_IDMAP, > }; > > - attr.userns_fd = get_userns_fd_from_idmap(&active_map); > + if (fd_userns > 0) > + attr.userns_fd = fd_userns; > + else > + attr.userns_fd = get_userns_fd_from_idmap(&active_map); > if (attr.userns_fd < 0) > exit_log("%m - Failed to create user namespace\n"); > --- > > It's a bug in the test helper program, not a bug in the test per-se because > the test does not use the > --map-mount=/proc/<pid>/ns/user option. Thanks! Will fix and also re-order the patches. Christian
> > Meanwhile, found a bug in Makefile: > > > > --- a/src/idmapped-mounts/Makefile > > +++ b/src/idmapped-mounts/Makefile > > @@ -25,11 +25,11 @@ depend: .dep > > > > include $(BUILDRULES) > > > > -idmapped-mounts: > > +idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS) > > @echo " [CC] $@" > > $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS) > > $(LDFLAGS) $(LDLIBS) > > > > -mount-idmapped: > > +mount-idmapped: $(CFILES_MOUNT_IDMAPPED) > > @echo " [CC] $@" > > $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS) > > $(LDFLAGS) $(LDLIBS) > > Stupid question maybe but what is the reason for this change? > It builds fine without those lines. > > Building idmapped-mounts > [CC] idmapped-mounts > [CC] mount-idmapped > My mistake. You are right. It should be fine as is. I may have messed up something else while building. You may take the fix or leave it - as you wish. Thanks, Amir.
On Wed, Mar 24, 2021 at 04:01:18PM +0200, Amir Goldstein wrote: > > > Meanwhile, found a bug in Makefile: > > > > > > --- a/src/idmapped-mounts/Makefile > > > +++ b/src/idmapped-mounts/Makefile > > > @@ -25,11 +25,11 @@ depend: .dep > > > > > > include $(BUILDRULES) > > > > > > -idmapped-mounts: > > > +idmapped-mounts: $(CFILES_IDMAPPED_MOUNTS) > > > @echo " [CC] $@" > > > $(Q)$(LTLINK) $(CFILES_IDMAPPED_MOUNTS) -o $@ $(CFLAGS) > > > $(LDFLAGS) $(LDLIBS) > > > > > > -mount-idmapped: > > > +mount-idmapped: $(CFILES_MOUNT_IDMAPPED) > > > @echo " [CC] $@" > > > $(Q)$(LTLINK) $(CFILES_MOUNT_IDMAPPED) -o $@ $(CFLAGS) > > > $(LDFLAGS) $(LDLIBS) > > > > Stupid question maybe but what is the reason for this change? > > It builds fine without those lines. > > > > Building idmapped-mounts > > [CC] idmapped-mounts > > [CC] mount-idmapped > > > > My mistake. You are right. It should be fine as is. > I may have messed up something else while building. > > You may take the fix or leave it - as you wish. Eh, it kinda makes it more obvious to have the lines there and they don't hurt so I've added your change. Christian
From: Christian Brauner <christian.brauner@ubuntu.com> Hey everyone, This series is available from: https://git.kernel.org/brauner/xfstests-dev/h/idmapped_mounts https://gitlab.com/brauner/xfstests/-/tree/idmapped_mounts https://github.com/brauner/xfstests/tree/idmapped_mounts /* v10 */ Reworked according to Eryu's comments. /* v9 */ Rebased onto current master. ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check generic/631 FSTYP -- xfs (non-debug) PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021 MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1 MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch generic/631 files ... 10s Ran: generic/631 Passed all 1 tests ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check generic/632 FSTYP -- xfs (non-debug) PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021 MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1 MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch generic/632 14s Ran: generic/632 Passed all 1 tests ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check xfs/529 FSTYP -- xfs (non-debug) PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021 MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1 MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch xfs/529 files ... 43s Ran: xfs/529 Passed all 1 tests ubuntu@f1-vm:~/src/git/xfstests$ sudo ./check xfs/530 FSTYP -- xfs (non-debug) PLATFORM -- Linux/x86_64 f1-vm 5.12.0-rc3-idmapped-mounts-inode-helpers #351 SMP Sat Mar 20 10:32:48 UTC 2021 MKFS_OPTIONS -- -f -bsize=4096 /dev/loop1 MOUNT_OPTIONS -- /dev/loop1 /mnt/scratch xfs/530 files ... 25s Ran: xfs/530 Passed all 1 tests Thanks! Christian Christian Brauner (6): generic/631: add test for detached mount propagation generic/632: add fstests for idmapped mounts common/rc: add _scratch_{u}mount_idmapped() helpers common/quota: move _qsetup() helper to common code xfs/529: quotas and idmapped mounts xfs/530: quotas on idmapped mounts .gitignore | 3 + common/quota | 20 + common/rc | 60 + configure.ac | 2 + include/builddefs.in | 1 + m4/Makefile | 1 + m4/package_libcap.m4 | 4 + src/Makefile | 8 +- src/detached_mounts_propagation.c | 189 + src/feature.c | 40 +- src/idmapped-mounts/Makefile | 41 + src/idmapped-mounts/idmapped-mounts.c | 8761 +++++++++++++++++++++++++ src/idmapped-mounts/missing.h | 151 + src/idmapped-mounts/mount-idmapped.c | 428 ++ src/idmapped-mounts/utils.c | 134 + src/idmapped-mounts/utils.h | 30 + tests/generic/631 | 43 + tests/generic/631.out | 2 + tests/generic/632 | 42 + tests/generic/632.out | 2 + tests/generic/group | 2 + tests/xfs/050 | 19 - tests/xfs/299 | 19 - tests/xfs/529 | 377 ++ tests/xfs/529.out | 657 ++ tests/xfs/530 | 211 + tests/xfs/530.out | 129 + tests/xfs/group | 2 + 28 files changed, 11335 insertions(+), 43 deletions(-) create mode 100644 m4/package_libcap.m4 create mode 100644 src/detached_mounts_propagation.c create mode 100644 src/idmapped-mounts/Makefile create mode 100644 src/idmapped-mounts/idmapped-mounts.c create mode 100644 src/idmapped-mounts/missing.h create mode 100644 src/idmapped-mounts/mount-idmapped.c create mode 100644 src/idmapped-mounts/utils.c create mode 100644 src/idmapped-mounts/utils.h create mode 100644 tests/generic/631 create mode 100644 tests/generic/631.out create mode 100644 tests/generic/632 create mode 100644 tests/generic/632.out create mode 100644 tests/xfs/529 create mode 100644 tests/xfs/529.out create mode 100644 tests/xfs/530 create mode 100644 tests/xfs/530.out base-commit: f6ddaf130d5b0817278afe441fdde52f464f321b