[PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Sukadev Bhattiprolu]

From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Date: Wed, 10 Feb 2010 20:09:17 -0800
Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

The contianer name is not printed in the lxc-ps output unless the --lxc
option is specified.  But the CONTAINER column is printed (and left
blank) even if the --lxc option is not specified.

	$ /usr/local/bin/lxc-ps -n foo
	CONTAINER    PID TTY          TIME CMD
		   19525 pts/2    00:00:01 bash
		   20311 pts/2    00:00:00 lxc-ps
		   20312 pts/2    00:00:00 ps

Like normal ps, it would be less confusing (IMHO) if the CONTAINER
column is skipped unless the --lxc option is specified.

	$ ./lxc-ps -n foo
	  PID TTY          TIME CMD
	19525 pts/2    00:00:01 bash
	20339 pts/2    00:00:00 lxc-ps
	20340 pts/2    00:00:00 ps

	$ ./lxc-ps --lxc -n foo
	CONTAINER    PID TTY          TIME CMD
	foo        19646 pts/1    00:00:00 lxc-init
	foo        19647 pts/1    00:00:00 bash

Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 src/lxc/lxc-ps.in |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/lxc/lxc-ps.in b/src/lxc/lxc-ps.in
index 249b104..e1aa655 100755
--- a/src/lxc/lxc-ps.in
+++ b/src/lxc/lxc-ps.in
@@ -124,7 +124,11 @@ sub get_container {
 }
 
 sub display_headers {
-    printf "%-10s %s", "CONTAINER", $PS_HEADERS;
+    if ($LXC_DISPLAY) {
+        printf "%-10s %s", "CONTAINER", $PS_HEADERS;
+    } else {
+        printf "%s", $PS_HEADERS;
+    }
 }
 
 sub display_usage {
@@ -195,7 +199,11 @@ for (@PS_LINES) {
     my $container = get_container $a[$PS_PID_INDEX];
     if ($LXC_DISPLAY == 2 and not $LXC_NAMES{$container}) {next;}
     if ($LXC_DISPLAY == 1 and $container eq '') {next;}
-    printf "%-10s %s", $container, $_;
+    if ($LXC_DISPLAY) {
+        printf "%-10s %s", $container, $_;
+    } else {
+        printf "%s", $_;
+    }
 }
 
 exit 0;
-- 
1.6.6.1

Re: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Michel Normand]

Le mercredi 10 février 2010 à 20:25 -0800, Sukadev Bhattiprolu a écrit :
> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Date: Wed, 10 Feb 2010 20:09:17 -0800
> Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column
> 
> The contianer name is not printed in the lxc-ps output unless the --lxc
> option is specified.  But the CONTAINER column is printed (and left
> blank) even if the --lxc option is not specified.
> 
> 	$ /usr/local/bin/lxc-ps -n foo
> 	CONTAINER    PID TTY          TIME CMD
> 		   19525 pts/2    00:00:01 bash
> 		   20311 pts/2    00:00:00 lxc-ps
> 		   20312 pts/2    00:00:00 ps

For me the error in the script is 
to report default pid of ps command,
rather than to report the pids of specified 'foo' 
container in this case.

So a patch should address this point,
rather to remove the container column.

---
Michel

> 
> Like normal ps, it would be less confusing (IMHO) if the CONTAINER
> column is skipped unless the --lxc option is specified.
> 
> 	$ ./lxc-ps -n foo
> 	  PID TTY          TIME CMD
> 	19525 pts/2    00:00:01 bash
> 	20339 pts/2    00:00:00 lxc-ps
> 	20340 pts/2    00:00:00 ps
> 
> 	$ ./lxc-ps --lxc -n foo
> 	CONTAINER    PID TTY          TIME CMD
> 	foo        19646 pts/1    00:00:00 lxc-init
> 	foo        19647 pts/1    00:00:00 bash
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  src/lxc/lxc-ps.in |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lxc/lxc-ps.in b/src/lxc/lxc-ps.in
> index 249b104..e1aa655 100755
> --- a/src/lxc/lxc-ps.in
> +++ b/src/lxc/lxc-ps.in
> @@ -124,7 +124,11 @@ sub get_container {
>  }
>  
>  sub display_headers {
> -    printf "%-10s %s", "CONTAINER", $PS_HEADERS;
> +    if ($LXC_DISPLAY) {
> +        printf "%-10s %s", "CONTAINER", $PS_HEADERS;
> +    } else {
> +        printf "%s", $PS_HEADERS;
> +    }
>  }
>  
>  sub display_usage {
> @@ -195,7 +199,11 @@ for (@PS_LINES) {
>      my $container = get_container $a[$PS_PID_INDEX];
>      if ($LXC_DISPLAY == 2 and not $LXC_NAMES{$container}) {next;}
>      if ($LXC_DISPLAY == 1 and $container eq '') {next;}
> -    printf "%-10s %s", $container, $_;
> +    if ($LXC_DISPLAY) {
> +        printf "%-10s %s", $container, $_;
> +    } else {
> +        printf "%s", $_;
> +    }
>  }
>  
>  exit 0;


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [lxc-devel] [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Michel Normand]

Le jeudi 11 février 2010 à 11:08 +0100, Michel Normand a écrit :
> Le mercredi 10 février 2010 à 20:25 -0800, Sukadev Bhattiprolu a écrit :
> > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > Date: Wed, 10 Feb 2010 20:09:17 -0800
> > Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column
> > 
> > The contianer name is not printed in the lxc-ps output unless the --lxc
> > option is specified.  But the CONTAINER column is printed (and left
> > blank) even if the --lxc option is not specified.
> > 
> > 	$ /usr/local/bin/lxc-ps -n foo
> > 	CONTAINER    PID TTY          TIME CMD
> > 		   19525 pts/2    00:00:01 bash
> > 		   20311 pts/2    00:00:00 lxc-ps
> > 		   20312 pts/2    00:00:00 ps
> 
> For me the error in the script is 
> to report default pid of ps command,
> rather than to report the pids of specified 'foo' 
> container in this case.
> 
> So a patch should address this point,
> rather to remove the container column.

In fact there is no error in the script :)
The -n option for lxc-ps is a ps option ! 
and not the --name option that I assume you should have specified
as detailed in the online help of lxc-ps.

---
Michel

> 
> ---
> Michel
> 
> > 
> > Like normal ps, it would be less confusing (IMHO) if the CONTAINER
> > column is skipped unless the --lxc option is specified.
> > 
> > 	$ ./lxc-ps -n foo
> > 	  PID TTY          TIME CMD
> > 	19525 pts/2    00:00:01 bash
> > 	20339 pts/2    00:00:00 lxc-ps
> > 	20340 pts/2    00:00:00 ps
> > 
> > 	$ ./lxc-ps --lxc -n foo
> > 	CONTAINER    PID TTY          TIME CMD
> > 	foo        19646 pts/1    00:00:00 lxc-init
> > 	foo        19647 pts/1    00:00:00 bash
> > 
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > ---
> >  src/lxc/lxc-ps.in |   12 ++++++++++--
> >  1 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/lxc/lxc-ps.in b/src/lxc/lxc-ps.in
> > index 249b104..e1aa655 100755
> > --- a/src/lxc/lxc-ps.in
> > +++ b/src/lxc/lxc-ps.in
> > @@ -124,7 +124,11 @@ sub get_container {
> >  }
> >  
> >  sub display_headers {
> > -    printf "%-10s %s", "CONTAINER", $PS_HEADERS;
> > +    if ($LXC_DISPLAY) {
> > +        printf "%-10s %s", "CONTAINER", $PS_HEADERS;
> > +    } else {
> > +        printf "%s", $PS_HEADERS;
> > +    }
> >  }
> >  
> >  sub display_usage {
> > @@ -195,7 +199,11 @@ for (@PS_LINES) {
> >      my $container = get_container $a[$PS_PID_INDEX];
> >      if ($LXC_DISPLAY == 2 and not $LXC_NAMES{$container}) {next;}
> >      if ($LXC_DISPLAY == 1 and $container eq '') {next;}
> > -    printf "%-10s %s", $container, $_;
> > +    if ($LXC_DISPLAY) {
> > +        printf "%-10s %s", $container, $_;
> > +    } else {
> > +        printf "%s", $_;
> > +    }
> >  }
> >  
> >  exit 0;
> 
> 
> 
> ------------------------------------------------------------------------------
> SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
> Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
> http://p.sf.net/sfu/solaris-dev2dev
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [lxc-devel] [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Serge E. Hallyn]

Quoting Michel Normand (normand-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
> Le jeudi 11 février 2010 à 11:08 +0100, Michel Normand a écrit :
> > Le mercredi 10 février 2010 à 20:25 -0800, Sukadev Bhattiprolu a écrit :
> > > From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > > Date: Wed, 10 Feb 2010 20:09:17 -0800
> > > Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column
> > > 
> > > The contianer name is not printed in the lxc-ps output unless the --lxc
> > > option is specified.  But the CONTAINER column is printed (and left
> > > blank) even if the --lxc option is not specified.
> > > 
> > > 	$ /usr/local/bin/lxc-ps -n foo
> > > 	CONTAINER    PID TTY          TIME CMD
> > > 		   19525 pts/2    00:00:01 bash
> > > 		   20311 pts/2    00:00:00 lxc-ps
> > > 		   20312 pts/2    00:00:00 ps
> > 
> > For me the error in the script is 
> > to report default pid of ps command,
> > rather than to report the pids of specified 'foo' 
> > container in this case.
> > 
> > So a patch should address this point,
> > rather to remove the container column.
> 
> In fact there is no error in the script :)
> The -n option for lxc-ps is a ps option ! 

heh, good catch - given -n is pretty ubiquitous in lxc
commands it should probably be used here :)

And a '--' should be allowed to separate lxc-ps options from
/bin/ps ones.


> and not the --name option that I assume you should have specified
> as detailed in the online help of lxc-ps.
> 
> ---
> Michel
> 
> > 
> > ---
> > Michel
> > 
> > > 
> > > Like normal ps, it would be less confusing (IMHO) if the CONTAINER
> > > column is skipped unless the --lxc option is specified.
> > > 
> > > 	$ ./lxc-ps -n foo
> > > 	  PID TTY          TIME CMD
> > > 	19525 pts/2    00:00:01 bash
> > > 	20339 pts/2    00:00:00 lxc-ps
> > > 	20340 pts/2    00:00:00 ps
> > > 
> > > 	$ ./lxc-ps --lxc -n foo
> > > 	CONTAINER    PID TTY          TIME CMD
> > > 	foo        19646 pts/1    00:00:00 lxc-init
> > > 	foo        19647 pts/1    00:00:00 bash
> > > 
> > > Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > > ---
> > >  src/lxc/lxc-ps.in |   12 ++++++++++--
> > >  1 files changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/lxc/lxc-ps.in b/src/lxc/lxc-ps.in
> > > index 249b104..e1aa655 100755
> > > --- a/src/lxc/lxc-ps.in
> > > +++ b/src/lxc/lxc-ps.in
> > > @@ -124,7 +124,11 @@ sub get_container {
> > >  }
> > >  
> > >  sub display_headers {
> > > -    printf "%-10s %s", "CONTAINER", $PS_HEADERS;
> > > +    if ($LXC_DISPLAY) {
> > > +        printf "%-10s %s", "CONTAINER", $PS_HEADERS;
> > > +    } else {
> > > +        printf "%s", $PS_HEADERS;
> > > +    }
> > >  }
> > >  
> > >  sub display_usage {
> > > @@ -195,7 +199,11 @@ for (@PS_LINES) {
> > >      my $container = get_container $a[$PS_PID_INDEX];
> > >      if ($LXC_DISPLAY == 2 and not $LXC_NAMES{$container}) {next;}
> > >      if ($LXC_DISPLAY == 1 and $container eq '') {next;}
> > > -    printf "%-10s %s", $container, $_;
> > > +    if ($LXC_DISPLAY) {
> > > +        printf "%-10s %s", $container, $_;
> > > +    } else {
> > > +        printf "%s", $_;
> > > +    }
> > >  }
> > >  
> > >  exit 0;
> > 
> > 
> > 
> > ------------------------------------------------------------------------------
> > SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
> > Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
> > http://p.sf.net/sfu/solaris-dev2dev
> > _______________________________________________
> > Lxc-devel mailing list
> > Lxc-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
> 
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [lxc-devel] [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Sukadev Bhattiprolu]

Serge E. Hallyn [serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org] wrote:
| Quoting Michel Normand (normand-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
| > Le jeudi 11 février 2010 à 11:08 +0100, Michel Normand a écrit :
| > > Le mercredi 10 février 2010 à 20:25 -0800, Sukadev Bhattiprolu a écrit :
| > > > From: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
| > > > Date: Wed, 10 Feb 2010 20:09:17 -0800
| > > > Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column
| > > > 
| > > > The contianer name is not printed in the lxc-ps output unless the --lxc
| > > > option is specified.  But the CONTAINER column is printed (and left
| > > > blank) even if the --lxc option is not specified.
| > > > 
| > > > 	$ /usr/local/bin/lxc-ps -n foo
| > > > 	CONTAINER    PID TTY          TIME CMD
| > > > 		   19525 pts/2    00:00:01 bash
| > > > 		   20311 pts/2    00:00:00 lxc-ps
| > > > 		   20312 pts/2    00:00:00 ps
| > > 
| > > For me the error in the script is 
| > > to report default pid of ps command,
| > > rather than to report the pids of specified 'foo' 
| > > container in this case.
| > > 
| > > So a patch should address this point,
| > > rather to remove the container column.
| > 
| > In fact there is no error in the script :)
| > The -n option for lxc-ps is a ps option ! 

Ah, good point. Like Serge pointed, I got the -n mixed up with other
lxc- commands.

Even so, I still think the blank CONTAINER column is misleading.
Does it mean that the listed processes are not part of any container ?
Like /bin/ps which prints, say the PGID column, only when selected,
wouldn't it make sense to print the CONTAINER column only when necessary ?

Thanks,

Sukadev

Re: [lxc-devel] [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Michel Normand]

Le jeudi 11 février 2010 à 10:19 -0800, Sukadev Bhattiprolu a écrit :
> Serge E. Hallyn [serue@us.ibm.com] wrote:
> | Quoting Michel Normand (normand@fr.ibm.com):
> | > Le jeudi 11 février 2010 à 11:08 +0100, Michel Normand a écrit :
> | > > Le mercredi 10 février 2010 à 20:25 -0800, Sukadev Bhattiprolu a écrit :
> | > > > From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> | > > > Date: Wed, 10 Feb 2010 20:09:17 -0800
> | > > > Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column
> | > > > 
> | > > > The contianer name is not printed in the lxc-ps output unless the --lxc
> | > > > option is specified.  But the CONTAINER column is printed (and left
> | > > > blank) even if the --lxc option is not specified.
> | > > > 
> | > > > 	$ /usr/local/bin/lxc-ps -n foo
> | > > > 	CONTAINER    PID TTY          TIME CMD
> | > > > 		   19525 pts/2    00:00:01 bash
> | > > > 		   20311 pts/2    00:00:00 lxc-ps
> | > > > 		   20312 pts/2    00:00:00 ps
> | > > 
> | > > For me the error in the script is 
> | > > to report default pid of ps command,
> | > > rather than to report the pids of specified 'foo' 
> | > > container in this case.
> | > > 
> | > > So a patch should address this point,
> | > > rather to remove the container column.
> | > 
> | > In fact there is no error in the script :)
> | > The -n option for lxc-ps is a ps option ! 
> 
> Ah, good point. Like Serge pointed, I got the -n mixed up with other
> lxc- commands.
> 
> Even so, I still think the blank CONTAINER column is misleading.
> Does it mean that the listed processes are not part of any container ?
> Like /bin/ps which prints, say the PGID column, only wuiuihen selected,
> wouldn't it make sense to print the CONTAINER column only when necessary ?
> 

The purpose of the CONTAINER column printed in any case,
is to be able to have the relations of processes in containers
versus their parents outside of containers.
when you do a request like lxc-ps axf

---
Michel

> Thanks,
> 
> Sukadev


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [lxc-devel] [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Cedric Le Goater]

>> In fact there is no error in the script :)
>> The -n option for lxc-ps is a ps option !
>
> heh, good catch - given -n is pretty ubiquitous in lxc
> commands it should probably be used here :)
>
> And a '--' should be allowed to separate lxc-ps options from
> /bin/ps ones.

lxc-ps is just an enhanced ps and, ultimately, lxc-ps should disappear and
be replaced by a new ps option, like the -z on solaris. this is similar to
the -Z for listing the security context of a file the with ls

I agree with suka that -n shouldn't be used as a short option for --name.
because the result is confusing.

C.  

Re: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column

[Cedric Le Goater]

On 02/11/2010 05:25 AM, Sukadev Bhattiprolu wrote:
> From: Sukadev Bhattiprolu<sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Date: Wed, 10 Feb 2010 20:09:17 -0800
> Subject: [PATCH 1/1] lxc-ps: Nit - Avoid blank CONTAINER column
>
> The contianer name is not printed in the lxc-ps output unless the --lxc
> option is specified.  But the CONTAINER column is printed (and left
> blank) even if the --lxc option is not specified.
>
> 	$ /usr/local/bin/lxc-ps -n foo
> 	CONTAINER    PID TTY          TIME CMD
> 		   19525 pts/2    00:00:01 bash
> 		   20311 pts/2    00:00:00 lxc-ps
> 		   20312 pts/2    00:00:00 ps
>
> Like normal ps, it would be less confusing (IMHO) if the CONTAINER
> column is skipped unless the --lxc option is specified.
>
> 	$ ./lxc-ps -n foo
> 	  PID TTY          TIME CMD
> 	19525 pts/2    00:00:01 bash
> 	20339 pts/2    00:00:00 lxc-ps
> 	20340 pts/2    00:00:00 ps
>
> 	$ ./lxc-ps --lxc -n foo
> 	CONTAINER    PID TTY          TIME CMD
> 	foo        19646 pts/1    00:00:00 lxc-init
> 	foo        19647 pts/1    00:00:00 bash

it would be real nice to have but without the --lxc.

can we kill the short option '-n', which conflicts with the ps below, and make
sure that only '--name'  triggers the extra CONTAINER column ?

C.