[lxc-devel] [PATCH 2/2] lxc-attach: Implement --clear-env and --keep-env

[Serge Hallyn]

Quoting Michael H. Warfield (mhw at WittsEnd.com):
> On Sun, 2013-03-31 at 22:50 -0500, Serge Hallyn wrote:
> > Quoting Christian Seiler (christian at iwakd.de):
> > > This patch introduces the --clear-env and --keep-env options for
> > > lxc-attach, that allows the user to specify whether the environment
> > > should be passed on inside the container or not.
> > > 
> > > This is to be expanded upon in later versions, this patch only
> > > introduces the most basic functionality.
> > > 
> > > Signed-off-by: Christian Seiler <christian at iwakd.de>
> 
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> > but,
> 
> Below...
> 
> <snip>
>  
> > > -int lxc_attach_set_environment()
> > > +int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char** extra_env, char** extra_keep)
> > >  {
> > > -	if (clearenv()) {
> > > -		SYSERROR("failed to clear environment");
> > > -		/* don't error out though */
> > > +	/* TODO: implement extra_env, extra_keep
> > > +	 * Rationale:
> > > +	 *  - extra_env is an array of strings of the form
> > > +	 *    "VAR=VALUE", which are to be set (after clearing or not,
> > > +	 *    depending on the value of the policy variable)
> > > +	 *  - extra_keep is an array of strings of the form
> > > +	 *    "VAR", which are extra environment variables to be kept
> > > +	 *    around after clearing (if that is done, otherwise, the
> > > +	 *    remain anyway)
> > > +	 */
> > > +	(void) extra_env;
> > > +	(void) extra_keep;
> 
> > Sorry, what are these?
> 
> If I may be so bold...
> 
> Recurse back to some of the discussions several of us (including you and
> me, both) have had over the last couple of months.  This whole thing
> about clearing the environment included discussions about why we would
> want to convey environment variables and how it would be configured and
> under what circumstances (lxc-start, lxc-execute, lxc-attach).
> 
> IMHO, It looks like these are intended to be environment variables to be
> flagged to keep (not to be cleared but to be copied over from the
> calling environment) and variables to be added (fixed configured
> variables, not in the calling environment but intended to be present in
> the resulting environment).  I can see a rational for both of them and I
> recall them coming up in those past discussions.
> 
> He probably didn't need to define them until he was ready to implement
> them but I see where it's going and I could agree with it even where I
> don't need it myself.

Right I'm not objecting to the arguments being passed, but I'm wondering
why he specifically has:

> > > +	(void) extra_env;
> > > +	(void) extra_keep;

in the function there.

-serge