[CRIU] [PATCH] zdtm: add a small program to create a zdtm container

[Serge Hallyn]

Quoting Andrew Vagin (avagin at parallels.com):
> On Wed, Aug 13, 2014 at 11:13:11AM +0400, Andrew Vagin wrote:
> > On Tue, Aug 12, 2014 at 04:33:30PM +0000, Serge Hallyn wrote:
> > > Quoting Andrey Vagin (avagin at openvz.org):
> > > > I didn't find a way how to do that with help "unshare".
> > > > It's simpler to write this program. It looks better than tricks in
> > > > zdtm.sh.
> > > > 
> > > > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > > 
> > > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> > > 
> > > Would it be worth verifying that the waited-upon process has the expected pid?
> > 
> > Serge, could you look at the attached version. If you will not have
> > objections, I will send it with your acked-by.

Hi,

so is there any concern about system("ip ...") and corrupted paths, or do
we trust the caller?

Otherwise, it looks ok to me, unfamiliar as I am with the context code.

thanks,
-serge

> > Thanks.
> > 
> 
> ...
> 
> > --- a/test/Makefile
> > +++ b/test/Makefile
> > @@ -28,7 +28,13 @@ fault-injection: .FORCE
> >  zdtm_ns: $(shell echo "$(TST)" | tr ' ' '\n' | grep -P $(EXP))
> >  zdtm_nons: $(shell echo "$(TST)" | tr ' ' '\n' | grep -vP $(EXP))
> >  
> > +zdtm_ct: zdtm_ct.c
> > +
> > +clean:
> > +	rm -rf zdtm_ct
> > +	$(MAKE) -C zdtm $@
> > +
> >  $(TST):
> > -	./zdtm.sh --ct ${ZDTM_ARGS} $(@) &> $(subst /,_,$@).log || \
> > +	./zdtm.sh ${ZDTM_ARGS} $(@) &> $(subst /,_,$@).log || \
> 
> I don't know when I removed the --ct option. It is still required.
> The updated patch is attached. Sorry for inconvenience.
> 
> >  	{ flock Makefile cat $(subst /,_,$@).log; exit 1; }
> >  .PHONY: zdtm_ns

> From 9d1b5ac214e1fab3d77b55043fcb80f4b5bde4f9 Mon Sep 17 00:00:00 2001
> From: Andrey Vagin <avagin at openvz.org>
> Date: Tue, 12 Aug 2014 19:23:35 +0400
> Subject: [PATCH] zdtm: add a small program to create a zdtm container (v3)
> 
> I didn't find a way how to do that with help "unshare".
> It's simpler to write this program. It looks better than tricks in
> zdtm.sh.
> 
> v2: proxify exit status
> v3: typo fix
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> ---
>  test/.gitignore |  1 +
>  test/Makefile   |  6 ++++++
>  test/zdtm.sh    | 14 ++-----------
>  test/zdtm_ct.c  | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 71 insertions(+), 12 deletions(-)
>  create mode 100644 test/zdtm_ct.c
> 
> diff --git a/test/.gitignore b/test/.gitignore
> index 9f00af7..1a6f6d2 100644
> --- a/test/.gitignore
> +++ b/test/.gitignore
> @@ -4,3 +4,4 @@
>  /dev
>  /dump
>  /*.log
> +/zdtm_ct
> diff --git a/test/Makefile b/test/Makefile
> index 5d04805..fdb96ee 100644
> --- a/test/Makefile
> +++ b/test/Makefile
> @@ -28,6 +28,12 @@ fault-injection: .FORCE
>  zdtm_ns: $(shell echo "$(TST)" | tr ' ' '\n' | grep -P $(EXP))
>  zdtm_nons: $(shell echo "$(TST)" | tr ' ' '\n' | grep -vP $(EXP))
>  
> +zdtm_ct: zdtm_ct.c
> +
> +clean:
> +	rm -rf zdtm_ct
> +	$(MAKE) -C zdtm $@
> +
>  $(TST):
>  	./zdtm.sh --ct ${ZDTM_ARGS} $(@) &> $(subst /,_,$@).log || \
>  	{ flock Makefile cat $(subst /,_,$@).log; exit 1; }
> diff --git a/test/zdtm.sh b/test/zdtm.sh
> index 395b256..2064866 100755
> --- a/test/zdtm.sh
> +++ b/test/zdtm.sh
> @@ -874,21 +874,11 @@ while :; do
>  		[ -z "$ZDTM_SH_IN_CT" ] && {
>  			export ZDTM_SH_IN_CT=1
>  			shift
> -			args="$@"
>  			# pidns is used to avoid conflicts
>  			# mntns is used to mount /proc
>  			# net is used to avoid conflicts of parasite sockets
> -			unshare --pid --mount --ipc --net -- bash -c "
> -				(
> -					ip link set up dev lo &&
> -					mount --make-rprivate / &&
> -					umount -l /proc &&
> -					mount -t proc proc /proc/ &&
> -					umount -l /dev/pts &&
> -					mount -t devpts zdtm_pts /dev/pts -o newinstance,ptmxmode=666 &&
> -					mount --bind /dev/pts/ptmx /dev/ptmx &&
> -					./zdtm.sh $args
> -				)"
> +			make zdtm_ct &&
> +			./zdtm_ct ./zdtm.sh "$@"
>  			exit
>  		}
>  		shift
> diff --git a/test/zdtm_ct.c b/test/zdtm_ct.c
> new file mode 100644
> index 0000000..4524ab8
> --- /dev/null
> +++ b/test/zdtm_ct.c
> @@ -0,0 +1,62 @@
> +#define _GNU_SOURCE
> +#include <sched.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/mount.h>
> +
> +int main(int argc, char **argv)
> +{
> +	pid_t pid;
> +	int status;
> +
> +	/*
> +	 *pidns is used to avoid conflicts
> +	 * mntns is used to mount /proc
> +	 * net is used to avoid conflicts of parasite sockets
> +	*/
> +	if (unshare(CLONE_NEWNS | CLONE_NEWPID | CLONE_NEWNET | CLONE_NEWIPC))
> +		return 1;
> +	pid = fork();
> +	if (pid == 0) {
> +		if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL)) {
> +			fprintf(stderr, "mount(/, S_REC | MS_PRIVATE)): %m");
> +			return 1;
> +		}
> +		umount2("/proc", MNT_DETACH);
> +		umount2("/dev/pts", MNT_DETACH);
> +		if (mount("zdtm_proc", "/proc", "proc", 0, NULL)) {
> +			fprintf(stderr, "mount(/proc): %m");
> +			return 1;
> +		}
> +		if (mount("zdtm_devpts", "/dev/pts", "devpts", 0,
> +					"newinstance,ptmxmode=0666")) {
> +			fprintf(stderr, "mount(pts): %m");
> +			return 1;
> +		}
> +		if (mount("/dev/pts/ptmx", "/dev/ptmx", NULL, MS_BIND, NULL)) {
> +			fprintf(stderr, "mount(ptmx): %m");
> +			return 1;
> +		}
> +		if (system("ip link set up dev lo"))
> +			return 1;
> +		execv(argv[1], argv + 1);
> +		fprintf(stderr, "execve: %m");
> +		return 1;
> +	}
> +
> +	if (waitpid(pid, &status, 0) != pid) {
> +		fprintf(stderr, "waitpid: %m");
> +		return 1;
> +	}
> +
> +	if (WIFEXITED(status))
> +		return WEXITSTATUS(status);
> +	else if (WIFSIGNALED(status))
> +		kill(getpid(), WTERMSIG(status));
> +	else
> +		fprintf(stderr, "Unexpected exit status: %x\n", status);
> +
> +	return 1;
> +}
> -- 
> 1.9.3
>