Re: [PATCH 8/8] Allow pasta to take a command to execute

[Stefano Brivio <sbrivio@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

On Tue, 30 Aug 2022 11:16:05 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> On Mon, Aug 29, 2022 at 09:16:58PM +0200, Stefano Brivio wrote:
> > On Fri, 26 Aug 2022 14:58:39 +1000
> > David Gibson <david(a)gibson.dropbear.id.au> wrote:
> >   
> > > When not given an existing PID or network namspace to attach to, pasta
> > > spawns a shell.  Most commands which can spawn a shell in an altered
> > > environment can also run other commands in that same environment, which can
> > > be useful in automation.
> > > 
> > > Allow pasta to do the same thing; it can be given an arbitrary command to
> > > run in the network and user namespace which pasta creates.  If neither a
> > > command nor an existing PID or netns to attach to is given, continue to
> > > spawn a default shell, as before.
> > > 
> > > Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
> > > ---
> > >  conf.c  | 27 ++++++++++++++++++---------
> > >  passt.1 | 14 +++++++++-----
> > >  pasta.c | 33 +++++++++++++++++++++++----------
> > >  pasta.h |  2 +-
> > >  4 files changed, 51 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/conf.c b/conf.c
> > > index 2a18124..162c2dd 100644
> > > --- a/conf.c
> > > +++ b/conf.c
> > > @@ -550,7 +550,8 @@ static int conf_ns_pid(char *userns, char *netns, const char *arg)
> > >  		return 0;
> > >  	}
> > >  
> > > -	return -EINVAL;
> > > +	/* Not a PID, later code will treat as a command */
> > > +	return 0;
> > >  }
> > >  
> > >  /**
> > > @@ -1480,14 +1481,18 @@ void conf(struct ctx *c, int argc, char **argv)
> > >  
> > >  	check_root(c);
> > >  
> > > -	if (c->mode == MODE_PASTA && optind + 1 == argc) {
> > > -		ret = conf_ns_pid(userns, netns, argv[optind]);
> > > -		if (ret < 0)
> > > +	if (c->mode == MODE_PASTA) {
> > > +		if (*netns && optind != argc) {
> > > +			err("Both --netns and PID or command given");
> > >  			usage(argv[0]);
> > > -	} else if (c->mode == MODE_PASTA && *userns
> > > -		   && !*netns && optind == argc) {
> > > -		err("--userns requires --netns or PID");
> > > -		usage(argv[0]);
> > > +		} else if (optind + 1 == argc) {
> > > +			ret = conf_ns_pid(userns, netns, argv[optind]);
> > > +			if (ret < 0)
> > > +				usage(argv[0]);
> > > +		} else if (*userns && !*netns && optind == argc) {
> > > +			err("--userns requires --netns or PID");
> > > +			usage(argv[0]);
> > > +		}
> > >  	} else if (optind != argc) {
> > >  		usage(argv[0]);
> > >  	}
> > >
> > > [...]  
> > 
> > I haven't really looked into this yet, but I guess we should now
> > handle getopts return codes a bit differently, because this works:
> > 
> >   $ ./pasta --config-net -- sh -c 'sleep 1; ip li sh'
> >   1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> >       link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> >   2: enp9s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 65520 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 1000
> >       link/ether aa:3e:39:5f:c6:15 brd ff:ff:ff:ff:ff:ff
> > 
> > while this doesn't:
> > 
> >   $ ./pasta --config-net sh -c 'sleep 1; ip li sh'
> >   ./pasta: invalid option -- 'c'
> >   [...]
> > 
> > despite the fact that there's no ambiguity.  
> 
> You mean because pasta itself doesn't have a -c option?

Ah, no, I meant it's after 'sh', which is a non-option argument.
However,

> Attempting to
> account for that sounds like a bad idea.  Requiring -- when the
> command given has options of its own that aren't supposed to go to the
> wrapper is pretty common for these sorts of tools.  Basically the
> trade-off is that you either need to require that, or you have to
> require that all non-option arguments of the wrapper come last (old
> style POSIXish command line parsing, as opposed to GNUish
> conventions).  The latter is usually more awkward than the former.

...right, my assumption was exactly that, but it's probably not a good
one. Let's keep it this way then. I wonder, though, if in the man page:

       pasta [OPTION]... [COMMAND [ARG]...]

we should explicitly mention this, because from this synopsis line it
looks like it's enough to put any command, with any argument, at the
end. Or maybe it's already covered by typical GNUish conventions and
users are used to it.

-- 
Stefano