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

Michael H. Warfield mhw at WittsEnd.com
Mon Apr 1 13:44:30 UTC 2013


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.

> > +
> > +	if (policy == LXC_ATTACH_CLEAR_ENV) {
> > +		if (clearenv()) {
> > +			SYSERROR("failed to clear environment");
> > +			/* don't error out though */
> > +		}
> >  	}
> >  
> >  	if (putenv("container=lxc")) {
> > diff --git a/src/lxc/attach.h b/src/lxc/attach.h
> > index 404ff4c..151445a 100644
> > --- a/src/lxc/attach.h
> > +++ b/src/lxc/attach.h
> > @@ -34,10 +34,15 @@ struct lxc_proc_context_info {
> >  
> >  extern struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid);
> >  
> > +typedef enum lxc_attach_env_policy_t {
> > +	LXC_ATTACH_KEEP_ENV,
> > +	LXC_ATTACH_CLEAR_ENV
> > +} lxc_attach_env_policy_t;
> > +
> >  extern int lxc_attach_to_ns(pid_t other_pid, int which);
> >  extern int lxc_attach_remount_sys_proc();
> >  extern int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx);
> > -extern int lxc_attach_set_environment();
> > +extern int lxc_attach_set_environment(enum lxc_attach_env_policy_t policy, char** extra_env, char** extra_keep);
> >  
> >  extern char *lxc_attach_getpwshell(uid_t uid);
> >  
> > diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> > index 7fd76ee..77039fb 100644
> > --- a/src/lxc/lxc_attach.c
> > +++ b/src/lxc/lxc_attach.c
> > @@ -55,6 +55,9 @@ static const struct option my_longopts[] = {
> >  	{"arch", required_argument, 0, 'a'},
> >  	{"namespaces", required_argument, 0, 's'},
> >  	{"remount-sys-proc", no_argument, 0, 'R'},
> > +	/* TODO: decide upon short option names */
> > +	{"clear-env", no_argument, 0, 500},
> > +	{"keep-env", no_argument, 0, 501},
> >  	LXC_COMMON_OPTIONS
> >  };
> >  
> > @@ -62,6 +65,7 @@ static int elevated_privileges = 0;
> >  static signed long new_personality = -1;
> >  static int namespace_flags = -1;
> >  static int remount_sys_proc = 0;
> > +static lxc_attach_env_policy_t env_policy = LXC_ATTACH_KEEP_ENV;
> >  
> >  static int my_parser(struct lxc_arguments* args, int c, char* arg)
> >  {
> > @@ -85,6 +89,12 @@ static int my_parser(struct lxc_arguments* args, int c, char* arg)
> >  		/* -s implies -e */
> >  		elevated_privileges = 1;
> >  		break;
> > +        case 500: /* clear-env */
> > +                env_policy = LXC_ATTACH_CLEAR_ENV;
> > +                break;
> > +        case 501: /* keep-env */
> > +                env_policy = LXC_ATTACH_KEEP_ENV;
> > +                break;
> >  	}
> >  
> >  	return 0;
> > @@ -116,7 +126,15 @@ Options :\n\
> >                      Remount /sys and /proc if not attaching to the\n\
> >                      mount namespace when using -s in order to properly\n\
> >                      reflect the correct namespace context. See the\n\
> > -                    lxc-attach(1) manual page for details.\n",
> > +                    lxc-attach(1) manual page for details.\n\
> > +      --clear-env\n\
> > +                    Clear all environment variables before attaching.\n\
> > +                    The attached shell/program will start with only\n\
> > +                    container=lxc set.\n\
> > +      --keep-env\n\
> > +                    Keep all current enivornment variables. This\n\
> > +                    is the current default behaviour, but is likely to\n\
> > +                    change in the future.\n",
> >  	.options  = my_longopts,
> >  	.parser   = my_parser,
> >  	.checker  = NULL,
> > @@ -411,7 +429,7 @@ int main(int argc, char *argv[])
> >  			return -1;
> >  		}
> >  
> > -		if (lxc_attach_set_environment()) {
> > +		if (lxc_attach_set_environment(env_policy, NULL, NULL)) {
> >  			ERROR("could not set environment");
> >  			return -1;
> >  		}
> > -- 
> > 1.7.10.4
> > 
> > 
> > ------------------------------------------------------------------------------
> > Own the Future-Intel(R) Level Up Game Demo Contest 2013
> > Rise to greatness in Intel's independent game demo contest. Compete 
> > for recognition, cash, and the chance to get your game on Steam. 
> > $5K grand prize plus 10 genre and skill prizes. Submit your demo 
> > by 6/6/13. http://altfarm.mediaplex.com/ad/ck/12124-176961-30367-2
> > _______________________________________________
> > Lxc-devel mailing list
> > Lxc-devel at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
> 
> ------------------------------------------------------------------------------
> Own the Future-Intel® Level Up Game Demo Contest 2013
> Rise to greatness in Intel's independent game demo contest.
> Compete for recognition, cash, and the chance to get your game 
> on Steam. $5K grand prize plus 10 genre and skill prizes. 
> Submit your demo by 6/6/13. http://p.sf.net/sfu/intel_levelupd2d
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel
> 

-- 
Michael H. Warfield (AI4NB) | (770) 985-6132 |  mhw at WittsEnd.com
   /\/\|=mhw=|\/\/          | (678) 463-0932 |  http://www.wittsend.com/mhw/
   NIC whois: MHW9          | An optimist believes we live in the best of all
 PGP Key: 0x674627FF        | possible worlds.  A pessimist is sure of it!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 482 bytes
Desc: This is a digitally signed message part
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20130401/f2cb05de/attachment.pgp>


More information about the lxc-devel mailing list