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

2010-02-11 Thread Michel Normand
Le mercredi 10 février 2010 à 20:25 -0800, Sukadev Bhattiprolu a écrit :
> From: Sukadev Bhattiprolu 
> 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
>   CONTAINERPID TTY  TIME CMD
>  19525 pts/200:00:01 bash
>  20311 pts/200:00:00 lxc-ps
>  20312 pts/200: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/200:00:01 bash
>   20339 pts/200:00:00 lxc-ps
>   20340 pts/200:00:00 ps
> 
>   $ ./lxc-ps --lxc -n foo
>   CONTAINERPID TTY  TIME CMD
>   foo19646 pts/100:00:00 lxc-init
>   foo19647 pts/100:00:00 bash
> 
> Signed-off-by: Sukadev Bhattiprolu 
> ---
>  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


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

2010-02-11 Thread 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 
> > 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
> > CONTAINERPID TTY  TIME CMD
> >19525 pts/200:00:01 bash
> >20311 pts/200:00:00 lxc-ps
> >20312 pts/200: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/200:00:01 bash
> > 20339 pts/200:00:00 lxc-ps
> > 20340 pts/200:00:00 ps
> > 
> > $ ./lxc-ps --lxc -n foo
> > CONTAINERPID TTY  TIME CMD
> > foo19646 pts/100:00:00 lxc-init
> > foo19647 pts/100:00:00 bash
> > 
> > Signed-off-by: Sukadev Bhattiprolu 
> > ---
> >  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



--
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


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

2010-02-11 Thread Serge E. Hallyn
Quoting Michel Normand (norm...@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 
> > > 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
> > >   CONTAINERPID TTY  TIME CMD
> > >  19525 pts/200:00:01 bash
> > >  20311 pts/200:00:00 lxc-ps
> > >  20312 pts/200: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/200:00:01 bash
> > >   20339 pts/200:00:00 lxc-ps
> > >   20340 pts/200:00:00 ps
> > > 
> > >   $ ./lxc-ps --lxc -n foo
> > >   CONTAINERPID TTY  TIME CMD
> > >   foo19646 pts/100:00:00 lxc-init
> > >   foo19647 pts/100:00:00 bash
> > > 
> > > Signed-off-by: Sukadev Bhattiprolu 
> > > ---
> > >  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
> contain...@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

--
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


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

2010-02-11 Thread Michel Normand
Le jeudi 11 février 2010 à 10:19 -0800, Sukadev Bhattiprolu a écrit :
> Serge E. Hallyn [se...@us.ibm.com] wrote:
> | Quoting Michel Normand (norm...@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 
> | > > > 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
> | > > >   CONTAINERPID TTY  TIME CMD
> | > > >  19525 pts/200:00:01 bash
> | > > >  20311 pts/200:00:00 lxc-ps
> | > > >  20312 pts/200: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



--
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