Re: [PATCH user-cr 2/2] add nsexeccwp to test clone-with-pids

["Serge E. Hallyn" <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>]

Quoting Nathan Lynch (ntl-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org):
> Okay, here's the violence I've committed against your code to get eclone
> working on powerpc (tested 32-bit userspace against 64-bit kernel).
> 
> ./nsexeccwp -z 300 /bin/bash -c 'echo $$'
> [debugging cruft elided]
> 300
> 
> This is meant not for inclusion but for discussion at this point.  I
> made some changes that will certainly break the builds for other
> architectures.
> 
> Note that I have generic code initializing clone_args with the true
> stack base and size and passing that to the architecture code.  The
> architecture code (e.g. clone_ppc.c) is responsible for calculating the
> stack pointer to pass to the kernel.  The architecture code is also
> responsible for clearing clone_args.child_stack_size and updating
> clone_args.child_stack, adjusting for alignment and arguments if
> appropriate.  In this way, we can accommodate ia64 and parisc and keep
> platform details in platform-specific code.

...

> diff --git a/clone_ppc.c b/clone_ppc.c
> index 49797fd..9e19fae 100644
> --- a/clone_ppc.c
> +++ b/clone_ppc.c
> @@ -10,14 +10,25 @@
>  
>  #define _GNU_SOURCE
>  
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
>  #include <unistd.h>
>  #include <errno.h>
>  #include <sys/types.h>
>  #include <sys/syscall.h>
>  #include <asm/unistd.h>
>  
> +#include "eclone.h"
> +
>  struct target_pid_set;
>  
> +struct pid_set {
> +	size_t nr_pids;
> +	pid_t *pids;
> +};

You shouldn't need the pid_set any more right?

...

> @@ -305,12 +294,17 @@ int main(int argc, char *argv[])
>  			return -1;
>  		}
>  
> +		memset(&clone_args, 0, sizeof(clone_args));
> +		clone_args.child_stack = (unsigned long)stack;
> +		clone_args.child_stack_size = stacksize;
> +		clone_args.nr_pids = nr_pids;
> +
>  		printf("about to clone with %lx\n", flags);
>  		if (chosen_pid)
>  			printf("Will choose pid %d\n", chosen_pid);
> +		printf("argv = %p\n", argv);
>  		flags |= SIGCHLD;
> -		pid = clone_with_pids(do_child, stack, flags, &pid_set,
> -					(void *)argv);
> +		pid = eclone(do_child, argv, flags, &clone_args, &chosen_pid);
>  		if (pid == -1) {
>  			perror("clone");
>  			return -1;

Yup, of course I agree with switching to a clean eclone passing
the clone_args and no struct pid_set, i was just trying to
minimize (to 0 :) the changes required for now in restart.c.

If you don't mind sending the patch to update restart.c as
well as this (minus some debugging) when you're ready, I'll
port clone_s390x.c to your precise api.

thanks,
-serge