[lxc-devel] Possible bug in lxc-netstat w/ patch; feature patch

[Serge Hallyn]

Quoting Andrew Gilbert (andrewg800 at gmail.com):
> Hi,
> I think I've discovered a bug in lxc-netstat when trying to have it run 
> netstat with the '-n' option. I've attached a patch for this below 
> (patch 1). I've also attached a patch that tries to Do The Right Thing 
> if someone uses '-n' without the '--' before it (patch 2). I'd be happy 
> to resend the patches as separate emails if that would be better.
> --Andrew G

Hi,

thanks, first patch confirmed and

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

from me.  But yes, please do resend as separate patches (threaded,
1/2 and 2/2) and (more importantly) do include a signed-off-by.
(If there were more than 2 emails I'd suggest using git-send-email,
but with two that's a bit over the top)

(Question below on second patch)

> 
> S T E P S   T O   R E P R O D U C E:
> 1. Start a container.
> 2. Use lxc-netstat -n <container-name> -- -n -a to try to get numeric 
> IPs in netstat output.
> 3. Get error "./lxc-netstat: 75: shift: can't shift that many."
> 
> It appears that after the execution of lxc-unshare, the call to 
> lxc-netstat does not include the '--' separator to prevent the netstat 
> arguments being interpreted as lxc-netstat arguments. This only would 
> manifest as a bug if the option is one that is valid for lxc-netstat as 
> well as netstat, as the lone -n option is.
> 
> P A T C H   1:
>  From 78ea721f2a6b0cc01cffba93109bd8d8202eeb98 Mon Sep 17 00:00:00 2001
> From: Andrew Gilbert <andrewg800 at gmail.com>
> Date: Fri, 21 Jun 2013 11:24:37 -0700
> Subject: [PATCH] Add double-dash to lxc-netstat re-call arguments
> 
> When lxc-netstat was called by lxc-unshare, it would be given the
> arguments intended for netstat from the first invocation, but without
> anything to separate them from the arguments intended for lxc-netstat.
> This meant that netstat arguments like -n would result in lxc-netstat
> trying to process them
> ---
>   src/lxc/lxc-netstat.in | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/lxc/lxc-netstat.in b/src/lxc/lxc-netstat.in
> index 2fa2d23..229c214 100644
> --- a/src/lxc/lxc-netstat.in
> +++ b/src/lxc/lxc-netstat.in
> @@ -93,7 +93,7 @@ if [ -z "$name" ]; then
>   fi
> 
>   if [ -z "$exec" ]; then
> -    exec @BINDIR@/lxc-unshare -s MOUNT -- $0 -n $name --exec "$@"
> +    exec @BINDIR@/lxc-unshare -s MOUNT -- $0 -n $name --exec -- "$@"
>   fi
> 
>   if lxc-info -n $name --state-is 'STOPPED'; then
> -- 
> 1.8.1.2
> 
> P A T C H   2:
>  From e45138027e08d291f4fad1357bcf5cb1b610c402 Mon Sep 17 00:00:00 2001
> From: Andrew Gilbert <andrewg800 at gmail.com>
> Date: Fri, 21 Jun 2013 14:36:28 -0700
> Subject: [PATCH 2/2] Add -n differentiation to lxc-netstat
> 
> lxc-netstat now only processes an -n argument if it has not previously
> received a value for $name from --name or -n. If it _has_ received such
> a value, it stops processing arguments and leaves the -n for netstat.
> This does not apply to the use of --name after a name has been provided
> by --name or -n; the current behaviour continues.

Can you please give a test case here?  It sounds to me like you're
supporting

	lxc-netstat -n <container-name> -n -a

without the '--'.  I'm not sure we want to do that - the rules end
up more ambiguous than a simple 'use -- to pass arguments on'.

> ---
>   src/lxc/lxc-netstat.in | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lxc/lxc-netstat.in b/src/lxc/lxc-netstat.in
> index 229c214..4a239d9 100644
> --- a/src/lxc/lxc-netstat.in
> +++ b/src/lxc/lxc-netstat.in
> @@ -66,12 +66,23 @@ get_parent_cgroup()
>   }
> 
>   exec=""
> +name=""
> 
>   while true; do
>       case $1 in
>           -h|--help)
>               help; exit 1;;
> -        -n|--name)
> +        -n)
> +            # If we already have a value for $name, treat -n as being an
> +            # argument for netstat
> +            if [ -n "$name" ]
> +            then
> +                break
> +            else
> +                name="$2"; shift 2;
> +            fi
> +            ;;
> +        --name)
>               name=$2; shift 2;;
>           --exec)
>               exec="exec"; shift;;
> -- 
> 1.8.1.2
> 
> 
> 
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by Windows:
> 
> Build for Windows Store.
> 
> http://p.sf.net/sfu/windows-dev2dev
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel