[PATCH lxc 1/2] lxc-unshare: accept multiple -s options

[PATCH lxc 1/2] lxc-unshare: accept multiple -s options

[Serge E. Hallyn]

(also remove -f from usage as it is not actually supported)

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 src/lxc/lxc_unshare.c |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/lxc/lxc_unshare.c b/src/lxc/lxc_unshare.c
index 10654f7..8db1cb7 100644
--- a/src/lxc/lxc_unshare.c
+++ b/src/lxc/lxc_unshare.c
@@ -48,7 +48,7 @@ void usage(char *cmd)
 	fprintf(stderr, "\t -s flags: Ored list of flags to unshare:\n" \
 			"\t           MOUNT, PID, UTSNAME, IPC, USER, NETWORK\n");
 	fprintf(stderr, "\t -u <id> : new id to be set if -s USER is specified\n");
-	fprintf(stderr, "\t if -f or -s PID is specified, <command> is mandatory)\n");
+	fprintf(stderr, "\t if -s PID is specified, <command> is mandatory)\n");
 	_exit(1);
 }
 
@@ -154,6 +154,29 @@ static int do_start(void *arg)
 	return 1;
 }
 
+void extend_namespaces(char **n, char *optarg)
+{
+	char *namespaces = *n;
+	int cont = 0, newlen = strlen(optarg) + 1; /* +1 for trailing \0 */
+
+	if (namespaces) {
+		cont = 1;
+		newlen += strlen(namespaces) + 1; /* +1 for '|' */
+	}
+
+	namespaces = realloc(namespaces, newlen);
+	if (!namespaces) {
+		perror("realloc");
+		exit(1);
+	}
+	if (cont)
+		sprintf(namespaces+strlen(namespaces), "|%s", optarg);
+	else
+		sprintf(namespaces, "%s", optarg);
+	namespaces[newlen-1] = '\0';
+	*n = namespaces;
+}
+
 int main(int argc, char *argv[])
 {
 	int opt, status;
@@ -174,7 +197,8 @@ int main(int argc, char *argv[])
 	while ((opt = getopt(argc, argv, "s:u:")) != -1) {
 		switch (opt) {
 		case 's':
-			namespaces = optarg;
+			extend_namespaces(&namespaces, optarg);
+			printf("namespaces is %s\n", namespaces);
 			break;
 		case 'u':
 			uid = lookup_user(optarg);
-- 
1.6.3.3

[PATCH lxc 2/2] lxc-unshare: make CLONE_NEWPID imply CLONE_NEWNS

[Serge E. Hallyn]

I would like to also automatically have /proc remounted, but
that would require digging deeper into lxc_clone.

Mind you perhaps having NEWPID imply NEWNS should be done there,
at src/lxc/namespace.c:lxc_clone anyway.  I'm starting here...
Won't be offended if it's rejected on those grounds :)

Signed-off-by: Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 src/lxc/lxc_unshare.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/lxc/lxc_unshare.c b/src/lxc/lxc_unshare.c
index 8db1cb7..8531b59 100644
--- a/src/lxc/lxc_unshare.c
+++ b/src/lxc/lxc_unshare.c
@@ -49,6 +49,7 @@ void usage(char *cmd)
 			"\t           MOUNT, PID, UTSNAME, IPC, USER, NETWORK\n");
 	fprintf(stderr, "\t -u <id> : new id to be set if -s USER is specified\n");
 	fprintf(stderr, "\t if -s PID is specified, <command> is mandatory)\n");
+	fprintf(stderr, "\t If -s PID is specified, then -s MOUNT is implied\n");
 	_exit(1);
 }
 
@@ -213,6 +214,9 @@ int main(int argc, char *argv[])
  	if (ret)
 		usage(argv[0]);
 
+	if (flags & CLONE_NEWPID)
+		flags |= CLONE_NEWNS;
+
 	if (!(flags & CLONE_NEWUSER) && uid != -1) {
 		ERROR("-u <uid> needs -s USER option");
 		return 1;
-- 
1.6.3.3

Re: [PATCH lxc 1/2] lxc-unshare: accept multiple -s options

[Daniel Lezcano]

On 05/13/2010 09:34 PM, Serge E. Hallyn wrote:
> (also remove -f from usage as it is not actually supported)
>
> Signed-off-by: Serge E. Hallyn<serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>   src/lxc/lxc_unshare.c |   28 ++++++++++++++++++++++++++--
>   1 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/src/lxc/lxc_unshare.c b/src/lxc/lxc_unshare.c
> index 10654f7..8db1cb7 100644
> --- a/src/lxc/lxc_unshare.c
> +++ b/src/lxc/lxc_unshare.c
> @@ -48,7 +48,7 @@ void usage(char *cmd)
>   	fprintf(stderr, "\t -s flags: Ored list of flags to unshare:\n" \
>   			"\t           MOUNT, PID, UTSNAME, IPC, USER, NETWORK\n");
>   	fprintf(stderr, "\t -u<id>  : new id to be set if -s USER is specified\n");
> -	fprintf(stderr, "\t if -f or -s PID is specified,<command>  is mandatory)\n");
> +	fprintf(stderr, "\t if -s PID is specified,<command>  is mandatory)\n");
>   	_exit(1);
>   }
>
> @@ -154,6 +154,29 @@ static int do_start(void *arg)
>   	return 1;
>   }
>
> +void extend_namespaces(char **n, char *optarg)
> +{
> +	char *namespaces = *n;
> +	int cont = 0, newlen = strlen(optarg) + 1; /* +1 for trailing \0 */
> +
> +	if (namespaces) {
> +		cont = 1;
> +		newlen += strlen(namespaces) + 1; /* +1 for '|' */
> +	}
> +
> +	namespaces = realloc(namespaces, newlen);
> +	if (!namespaces) {
> +		perror("realloc");
> +		exit(1);
> +	}
> +	if (cont)
> +		sprintf(namespaces+strlen(namespaces), "|%s", optarg);
> +	else
> +		sprintf(namespaces, "%s", optarg);
> +	namespaces[newlen-1] = '\0';
> +	*n = namespaces;
> +}
> +
>   int main(int argc, char *argv[])
>   {
>   	int opt, status;
> @@ -174,7 +197,8 @@ int main(int argc, char *argv[])
>   	while ((opt = getopt(argc, argv, "s:u:")) != -1) {
>   		switch (opt) {
>   		case 's':
> -			namespaces = optarg;
> +			extend_namespaces(&namespaces, optarg);
> +			printf("namespaces is %s\n", namespaces);
>   			break;
>   		case 'u':
>   			uid = lookup_user(optarg);

Hi Serge,

Sorry for the delay.

Is it possible to just kill the "Ored" option format and have only 
multiple "-s" options ? That will simplificate the code a lot.

Thanks
   -- Daniel

Re: [PATCH lxc 2/2] lxc-unshare: make CLONE_NEWPID imply CLONE_NEWNS

[Daniel Lezcano]

On 05/13/2010 09:34 PM, Serge E. Hallyn wrote:
> I would like to also automatically have /proc remounted, but
> that would require digging deeper into lxc_clone.

You should not make that automatically, especially in lxc-clone because 
this function is just for cloning a process in a new namespace, nothing 
more. We may want to access /proc after cloning, for example to reach 
/proc/<pid>/ns/*. The automatic mount, should be done in your child 
reaper (like lxc-init), otherwise let the container init to run the 
services and mount /proc.

In the case of lxc_unshare, you can add a new option to remount /proc 
when there is the pidns or the mountns options.

Otherwise, adding the NEWNS with the NEWPID makes sense for me.

Don't forget lxc_unshare is a simple tool, it is not supposed to replace 
lxc-start/lxc-execute, at least it should do a bit more than the 
"unshare" command.

> Mind you perhaps having NEWPID imply NEWNS should be done there,
> at src/lxc/namespace.c:lxc_clone anyway.  I'm starting here...
> Won't be offended if it's rejected on those grounds :)
>
> Signed-off-by: Serge E. Hallyn<serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> ---
>   src/lxc/lxc_unshare.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/src/lxc/lxc_unshare.c b/src/lxc/lxc_unshare.c
> index 8db1cb7..8531b59 100644
> --- a/src/lxc/lxc_unshare.c
> +++ b/src/lxc/lxc_unshare.c
> @@ -49,6 +49,7 @@ void usage(char *cmd)
>   			"\t           MOUNT, PID, UTSNAME, IPC, USER, NETWORK\n");
>   	fprintf(stderr, "\t -u<id>  : new id to be set if -s USER is specified\n");
>   	fprintf(stderr, "\t if -s PID is specified,<command>  is mandatory)\n");
> +	fprintf(stderr, "\t If -s PID is specified, then -s MOUNT is implied\n");
>   	_exit(1);
>   }
>
> @@ -213,6 +214,9 @@ int main(int argc, char *argv[])
>    	if (ret)
>   		usage(argv[0]);
>
> +	if (flags&  CLONE_NEWPID)
> +		flags |= CLONE_NEWNS;
> +
>   	if (!(flags&  CLONE_NEWUSER)&&  uid != -1) {
>   		ERROR("-u<uid>  needs -s USER option");
>   		return 1;

Re: [PATCH lxc 1/2] lxc-unshare: accept multiple -s options

[Serge E. Hallyn]

Quoting Daniel Lezcano (dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
> On 05/13/2010 09:34 PM, Serge E. Hallyn wrote:
> >(also remove -f from usage as it is not actually supported)
> >
> >Signed-off-by: Serge E. Hallyn<serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> >---
> >  src/lxc/lxc_unshare.c |   28 ++++++++++++++++++++++++++--
> >  1 files changed, 26 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/lxc/lxc_unshare.c b/src/lxc/lxc_unshare.c
> >index 10654f7..8db1cb7 100644
> >--- a/src/lxc/lxc_unshare.c
> >+++ b/src/lxc/lxc_unshare.c
> >@@ -48,7 +48,7 @@ void usage(char *cmd)
> >  	fprintf(stderr, "\t -s flags: Ored list of flags to unshare:\n" \
> >  			"\t           MOUNT, PID, UTSNAME, IPC, USER, NETWORK\n");
> >  	fprintf(stderr, "\t -u<id>  : new id to be set if -s USER is specified\n");
> >-	fprintf(stderr, "\t if -f or -s PID is specified,<command>  is mandatory)\n");
> >+	fprintf(stderr, "\t if -s PID is specified,<command>  is mandatory)\n");
> >  	_exit(1);
> >  }
> >
> >@@ -154,6 +154,29 @@ static int do_start(void *arg)
> >  	return 1;
> >  }
> >
> >+void extend_namespaces(char **n, char *optarg)
> >+{
> >+	char *namespaces = *n;
> >+	int cont = 0, newlen = strlen(optarg) + 1; /* +1 for trailing \0 */
> >+
> >+	if (namespaces) {
> >+		cont = 1;
> >+		newlen += strlen(namespaces) + 1; /* +1 for '|' */
> >+	}
> >+
> >+	namespaces = realloc(namespaces, newlen);
> >+	if (!namespaces) {
> >+		perror("realloc");
> >+		exit(1);
> >+	}
> >+	if (cont)
> >+		sprintf(namespaces+strlen(namespaces), "|%s", optarg);
> >+	else
> >+		sprintf(namespaces, "%s", optarg);
> >+	namespaces[newlen-1] = '\0';
> >+	*n = namespaces;
> >+}
> >+
> >  int main(int argc, char *argv[])
> >  {
> >  	int opt, status;
> >@@ -174,7 +197,8 @@ int main(int argc, char *argv[])
> >  	while ((opt = getopt(argc, argv, "s:u:")) != -1) {
> >  		switch (opt) {
> >  		case 's':
> >-			namespaces = optarg;
> >+			extend_namespaces(&namespaces, optarg);
> >+			printf("namespaces is %s\n", namespaces);
> >  			break;
> >  		case 'u':
> >  			uid = lookup_user(optarg);
> 
> Hi Serge,
> 
> Sorry for the delay.
> 
> Is it possible to just kill the "Ored" option format and have only
> multiple "-s" options ? That will simplificate the code a lot.

Agreed I think that'd be better

-serge