On Thu, May 8, 2025 at 9:52 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
>
> On 5/7/25 1:43 PM, Amir Goldstein wrote:
> > Add helper to utils and use it in statmount userns tests.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > .../filesystems/statmount/statmount_test_ns.c | 60 +----------------
> > tools/testing/selftests/filesystems/utils.c | 65 +++++++++++++++++++
> > tools/testing/selftests/filesystems/utils.h | 1 +
> > 3 files changed, 68 insertions(+), 58 deletions(-)
> >
> > diff --git a/tools/testing/selftests/filesystems/statmount/statmount_test_ns.c b/tools/testing/selftests/filesystems/statmount/statmount_test_ns.c
> > index 375a52101d08..3c5bc2e33821 100644
> > --- a/tools/testing/selftests/filesystems/statmount/statmount_test_ns.c
> > +++ b/tools/testing/selftests/filesystems/statmount/statmount_test_ns.c
> > @@ -79,66 +79,10 @@ static int get_mnt_ns_id(const char *mnt_ns, uint64_t *mnt_ns_id)
> > return NSID_PASS;
> > }
> >
> > -static int write_file(const char *path, const char *val)
> > -{
> > - int fd = open(path, O_WRONLY);
> > - size_t len = strlen(val);
> > - int ret;
> > -
> > - if (fd == -1) {
> > - ksft_print_msg("opening %s for write: %s\n", path, strerror(errno));
> > - return NSID_ERROR;
> > - }
> > -
> > - ret = write(fd, val, len);
> > - if (ret == -1) {
> > - ksft_print_msg("writing to %s: %s\n", path, strerror(errno));
> > - return NSID_ERROR;
> > - }
> > - if (ret != len) {
> > - ksft_print_msg("short write to %s\n", path);
> > - return NSID_ERROR;
> > - }
> > -
> > - ret = close(fd);
> > - if (ret == -1) {
> > - ksft_print_msg("closing %s\n", path);
> > - return NSID_ERROR;
> > - }
> > -
> > - return NSID_PASS;
> > -}
> > -
> > static int setup_namespace(void)
> > {
> > - int ret;
> > - char buf[32];
> > - uid_t uid = getuid();
> > - gid_t gid = getgid();
> > -
> > - ret = unshare(CLONE_NEWNS|CLONE_NEWUSER|CLONE_NEWPID);
> > - if (ret == -1)
> > - ksft_exit_fail_msg("unsharing mountns and userns: %s\n",
> > - strerror(errno));
> > -
> > - sprintf(buf, "0 %d 1", uid);
> > - ret = write_file("/proc/self/uid_map", buf);
> > - if (ret != NSID_PASS)
> > - return ret;
> > - ret = write_file("/proc/self/setgroups", "deny");
> > - if (ret != NSID_PASS)
> > - return ret;
> > - sprintf(buf, "0 %d 1", gid);
> > - ret = write_file("/proc/self/gid_map", buf);
> > - if (ret != NSID_PASS)
> > - return ret;
> > -
> > - ret = mount("", "/", NULL, MS_REC|MS_PRIVATE, NULL);
> > - if (ret == -1) {
> > - ksft_print_msg("making mount tree private: %s\n",
> > - strerror(errno));
> > + if (setup_userns() != 0)
> > return NSID_ERROR;
> > - }
> >
> > return NSID_PASS;
> > }
> > @@ -200,7 +144,7 @@ static void test_statmount_mnt_ns_id(void)
> > return;
> > }
> >
> > - ret = setup_namespace();
> > + ret = setup_userns();
> > if (ret != NSID_PASS)
> > exit(ret);
> > ret = _test_statmount_mnt_ns_id();
> > diff --git a/tools/testing/selftests/filesystems/utils.c b/tools/testing/selftests/filesystems/utils.c
> > index 9b5419e6f28d..9dab197ddd9c 100644
> > --- a/tools/testing/selftests/filesystems/utils.c
> > +++ b/tools/testing/selftests/filesystems/utils.c
> > @@ -18,6 +18,7 @@
> > #include <sys/types.h>
> > #include <sys/wait.h>
> > #include <sys/xattr.h>
> > +#include <sys/mount.h>
> >
> > #include "utils.h"
> >
> > @@ -447,6 +448,70 @@ static int create_userns_hierarchy(struct userns_hierarchy *h)
> > return fret;
> > }
> >
> > +static int write_file(const char *path, const char *val)
> > +{
> > + int fd = open(path, O_WRONLY);
> > + size_t len = strlen(val);
> > + int ret;
> > +
> > + if (fd == -1) {
> > + syserror("opening %s for write: %s\n", path, strerror(errno));
>
> While I have no opinion about ksft_print_msg() vs. syserror(), I do
> think it's worth a mention in the commit log: there is some reason
> that you changed to syserror() throughout. Could you write down
> what that was?
Very good question.
I admit I did not put much thought into this. I was blindly following
Christian's lead in utils.c.
I will revert those syserror back to ksft_print_msg().
>
> In any case, it looks correct, so with an update commit message, please
> feel free to add:
>
>
> Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>
>
Thanks for the review!
Amir.
