| Message ID | 20220201142230.20243-1-andrea.cervesato@suse.de |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1] Extract libclone from testcases/kernel/containers | expand |
Hi Andrea, [ Cc Richie, Li ] https://lore.kernel.org/ltp/20220201142230.20243-1-andrea.cervesato@suse.de/ > libclone has been added to the libs folder and updated with the new > LTP API. This library will be used by containers tests, which will be > updated to the new LTP API as well. I suppose you want to use it for containers (still use legacy API), which already use it. I remember Richie not considering libclone.h as a good source and suggesting to use SAFE_CLONE() for simple cases. https://lore.kernel.org/ltp/878s7k59tk.fsf@suse.de/ We probably need some wrapper for containers, but we should recheck, whether we want to take a different approach. Code in the functions is really a bit weird. ... > +#ifndef SYS_unshare > +#ifdef __NR_unshare > +#define SYS_unshare __NR_unshare > +#elif __i386__ > +#define SYS_unshare 310 > +#elif __ia64__ > +#define SYS_unshare 1296 > +#elif __x86_64__ > +#define SYS_unshare 272 > +#elif __s390x__ || __s390__ > +#define SYS_unshare 303 > +#elif __powerpc__ > +#define SYS_unshare 282 > +#else > +#error "unshare not supported on this architecure." > +#endif > +#endif nit: we usually put space before define for a readability. #ifndef SYS_unshare #ifdef __NR_unshare # define SYS_unshare __NR_unshare #elif __i386__ # define SYS_unshare 310 #elif __ia64__ # define SYS_unshare 1296 ... Kind regards, Petr
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Andrea, > > [ Cc Richie, Li ] > https://lore.kernel.org/ltp/20220201142230.20243-1-andrea.cervesato@suse.de/ > >> libclone has been added to the libs folder and updated with the new >> LTP API. This library will be used by containers tests, which will be >> updated to the new LTP API as well. > I suppose you want to use it for containers (still use legacy API), which > already use it. I remember Richie not considering libclone.h as a good source > and suggesting to use SAFE_CLONE() for simple cases. > > https://lore.kernel.org/ltp/878s7k59tk.fsf@suse.de/ > > We probably need some wrapper for containers, but we should recheck, whether > we want to take a different approach. Code in the functions is really a bit weird. > Yeah tst_clone.{c,h} is the new library which uses clone3 + a compatability layer if clone3 is not available. This avoids reinventing a cloning API to some extent because the clone3 interface is nice IMO. Also IMO tests should be rewritten to use tst_clone, I just haven't had chance to do that. BTW we need to test cloning into a CGroup, so I'll probably add that soon.
Hi! On 2/3/22 06:31, Richard Palethorpe wrote: > Hello, > > Petr Vorel<pvorel@suse.cz> writes: > >> Hi Andrea, >> >> [ Cc Richie, Li ] >> https://lore.kernel.org/ltp/20220201142230.20243-1-andrea.cervesato@suse.de/ >> >>> libclone has been added to the libs folder and updated with the new >>> LTP API. This library will be used by containers tests, which will be >>> updated to the new LTP API as well. >> I suppose you want to use it for containers (still use legacy API), which >> already use it. I remember Richie not considering libclone.h as a good source >> and suggesting to use SAFE_CLONE() for simple cases. >> >> https://lore.kernel.org/ltp/878s7k59tk.fsf@suse.de/ >> >> We probably need some wrapper for containers, but we should recheck, whether >> we want to take a different approach. Code in the functions is really a bit weird. >> > Yeah tst_clone.{c,h} is the new library which uses clone3 + a > compatability layer if clone3 is not available. This avoids reinventing > a cloning API to some extent because the clone3 interface is nice IMO. > > Also IMO tests should be rewritten to use tst_clone, I just haven't had > chance to do that. libclone does also a several other things, such as executing a process under unshared namespace and that is used by mountns testcases. So maybe we can just call it in an another way or find a way to recycle the libclone code. > BTW we need to test cloning into a CGroup, so I'll probably add that > soon. >
Hi, an update about the last comments: the unshared clone is only used by mountns tests suite, so we can add that to the common utilities and get back to tst_clone for the other testing suites. I'm going to do that. Andrea On 2/3/22 10:22, Andrea Cervesato wrote: > > Hi! > > On 2/3/22 06:31, Richard Palethorpe wrote: >> Hello, >> >> Petr Vorel<pvorel@suse.cz> writes: >> >>> Hi Andrea, >>> >>> [ Cc Richie, Li ] >>> https://lore.kernel.org/ltp/20220201142230.20243-1-andrea.cervesato@suse.de/ >>> >>>> libclone has been added to the libs folder and updated with the new >>>> LTP API. This library will be used by containers tests, which will be >>>> updated to the new LTP API as well. >>> I suppose you want to use it for containers (still use legacy API), which >>> already use it. I remember Richie not considering libclone.h as a good source >>> and suggesting to use SAFE_CLONE() for simple cases. >>> >>> https://lore.kernel.org/ltp/878s7k59tk.fsf@suse.de/ >>> >>> We probably need some wrapper for containers, but we should recheck, whether >>> we want to take a different approach. Code in the functions is really a bit weird. >>> >> Yeah tst_clone.{c,h} is the new library which uses clone3 + a >> compatability layer if clone3 is not available. This avoids reinventing >> a cloning API to some extent because the clone3 interface is nice IMO. >> >> Also IMO tests should be rewritten to use tst_clone, I just haven't had >> chance to do that. > libclone does also a several other things, such as executing a process > under unshared namespace and that is used by mountns testcases. So > maybe we can just call it in an another way or find a way to recycle > the libclone code. >> BTW we need to test cloning into a CGroup, so I'll probably add that >> soon. >>
> Hi, > an update about the last comments: the unshared clone is only used by > mountns tests suite, so we can add that to the common utilities and get back > to tst_clone for the other testing suites. I'm going to do that. Thanks! Thus setting this as rejected. Kind regards, Petr > Andrea
diff --git a/include/libclone.h b/include/libclone.h new file mode 100644 index 000000000..9ffa35165 --- /dev/null +++ b/include/libclone.h @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) International Business Machines Corp., 2007 + * Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +#ifndef __LIBCLONE_H +#define __LIBCLONE_H + +#include "tst_test.h" +#include "lapi/syscalls.h" +#include "lapi/namespaces_constants.h" + +#define T_UNSHARE 0 +#define T_CLONE 1 +#define T_NONE 2 + +#ifndef SYS_unshare +#ifdef __NR_unshare +#define SYS_unshare __NR_unshare +#elif __i386__ +#define SYS_unshare 310 +#elif __ia64__ +#define SYS_unshare 1296 +#elif __x86_64__ +#define SYS_unshare 272 +#elif __s390x__ || __s390__ +#define SYS_unshare 303 +#elif __powerpc__ +#define SYS_unshare 282 +#else +#error "unshare not supported on this architecure." +#endif +#endif + +#ifndef __NR_unshare +#define __NR_unshare SYS_unshare +#endif + +/* + * Run fn1 in a unshared environmnent, and fn2 in the original context + * Fn2 may be NULL. + */ + +int tst_clone_tests(unsigned long clone_flags, int (*fn1)(void *arg), + void *arg1, int (*fn2)(void *arg), void *arg2); + +int tst_unshare_tests(unsigned long clone_flags, int (*fn1)(void *arg), + void *arg1, int (*fn2)(void *arg), void *arg2); + +int tst_fork_tests(int (*fn1)(void *arg), void *arg1, int (*fn2)(void *arg), + void *arg2); + +int tst_clone_unshare_test(int use_clone, unsigned long clone_flags, + int (*fn1)(void *arg), void *arg1); + +int tst_clone_unshare_tests(int use_clone, unsigned long clone_flags, + int (*fn1)(void *arg), void *arg1, + int (*fn2)(void *arg), void *arg2); + +#endif diff --git a/libs/libltpclone/Makefile b/libs/libltpclone/Makefile new file mode 100644 index 000000000..c869f6cf8 --- /dev/null +++ b/libs/libltpclone/Makefile @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) International Business Machines Corp., 2007 +# Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + +top_srcdir ?= ../.. + +include $(top_srcdir)/include/mk/env_pre.mk + +INTERNAL_LIB = libltpclone.a + +include $(top_srcdir)/include/mk/lib.mk +include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/libs/libltpclone/libclone.c b/libs/libltpclone/libclone.c new file mode 100644 index 000000000..869e8ac63 --- /dev/null +++ b/libs/libltpclone/libclone.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) International Business Machines Corp., 2007 + * Copyright (C) 2021 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com> + */ + +#include <stdio.h> +#include <stdlib.h> +#include <sched.h> +#include <unistd.h> +#include <string.h> +#include <errno.h> +#include <libgen.h> +#include <signal.h> + +#define TST_NO_DEFAULT_MAIN + +#include "libclone.h" +#include "tst_test.h" +#include "lapi/syscalls.h" +#include "lapi/namespaces_constants.h" + +int tst_clone_tests(unsigned long clone_flags, int (*fn1)(void *arg), + void *arg1, int (*fn2)(void *arg), void *arg2) +{ + int ret; + + ret = ltp_clone_quick(clone_flags | SIGCHLD, fn1, arg1); + + if (ret == -1) { + return -1; + } + if (fn2) + ret = fn2(arg2); + else + ret = 0; + + return ret; +} + +int tst_unshare_tests(unsigned long clone_flags, int (*fn1)(void *arg), + void *arg1, int (*fn2)(void *arg), void *arg2) +{ + int pid, ret = 0; + int retpipe[2]; + char buf[2]; + + SAFE_PIPE(retpipe); + + pid = fork(); + if (pid < 0) { + SAFE_CLOSE(retpipe[0]); + SAFE_CLOSE(retpipe[1]); + tst_brk(TBROK, "fork"); + } + + if (!pid) { + SAFE_CLOSE(retpipe[0]); + + ret = tst_syscall(SYS_unshare, clone_flags); + if (ret == -1) { + SAFE_WRITE(1, retpipe[1], "0", 2); + SAFE_CLOSE(retpipe[1]); + exit(1); + } else { + SAFE_WRITE(1, retpipe[1], "1", 2); + } + + SAFE_CLOSE(retpipe[1]); + + ret = fn1(arg1); + exit(ret); + } + + SAFE_CLOSE(retpipe[1]); + SAFE_READ(1, retpipe[0], &buf, 2); + SAFE_CLOSE(retpipe[0]); + + if (*buf == '0') + return -1; + + if (fn2) + ret = fn2(arg2); + + return ret; +} + +int tst_plain_tests(int (*fn1)(void *arg), void *arg1, int (*fn2)(void *arg), + void *arg2) +{ + int ret = 0, pid; + + pid = SAFE_FORK(); + if (!pid) + exit(fn1(arg1)); + + if (fn2) + ret = fn2(arg2); + + return ret; +} + +int tst_clone_unshare_test(int use_clone, unsigned long clone_flags, + int (*fn1)(void *arg), void *arg1) +{ + int ret = 0; + + switch (use_clone) { + case T_NONE: + ret = tst_plain_tests(fn1, arg1, NULL, NULL); + break; + case T_CLONE: + ret = tst_clone_tests(clone_flags, fn1, arg1, NULL, NULL); + break; + case T_UNSHARE: + ret = tst_unshare_tests(clone_flags, fn1, arg1, NULL, NULL); + break; + default: + ret = -1; + tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__, + use_clone); + break; + } + + return ret; +} + +/* + * Run fn1 in a unshared environmnent, and fn2 in the original context + */ +int tst_clone_unshare_tests(int use_clone, unsigned long clone_flags, + int (*fn1)(void *arg), void *arg1, + int (*fn2)(void *arg), void *arg2) +{ + int ret = 0; + + switch (use_clone) { + case T_NONE: + ret = tst_plain_tests(fn1, arg1, fn2, arg2); + break; + case T_CLONE: + ret = tst_clone_tests(clone_flags, fn1, arg1, fn2, arg2); + break; + case T_UNSHARE: + ret = tst_unshare_tests(clone_flags, fn1, arg1, fn2, arg2); + break; + default: + ret = -1; + tst_brk(TBROK, "%s: bad use_clone option: %d", __FUNCTION__, + use_clone); + break; + } + + return ret; +}
libclone has been added to the libs folder and updated with the new LTP API. This library will be used by containers tests, which will be updated to the new LTP API as well. Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de> --- include/libclone.h | 61 ++++++++++++++ libs/libltpclone/Makefile | 12 +++ libs/libltpclone/libclone.c | 155 ++++++++++++++++++++++++++++++++++++ 3 files changed, 228 insertions(+) create mode 100644 include/libclone.h create mode 100644 libs/libltpclone/Makefile create mode 100644 libs/libltpclone/libclone.c