On Thu, May 08, 2025 at 02:08:16PM +0200, Amir Goldstein wrote:
> 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().
I think I copied parts of that over from the work I did for xfstests
testing.
