[lxc-devel] [PATCH] Remove MAKEDEV call, add autodev hooks, add environment variables for hook scripts.

Michael H. Warfield mhw at WittsEnd.com
Wed Jan 9 20:38:07 UTC 2013


Ok, all!

Here's the patch I promised Serge in the "[PATCH] Support MS_SHARED /"
thread to address the mayhem that calling MAKEDEV was causing.

This is a straw-man patch.  This is the first patch I've submitted to
this project but I plagerized like hell, so I hope I got the coding
style matched.  Look it over and suggest changes.  It adds some features
and changes some features.  If you don't like what I did, no problem.  I
probably did somethings that some people may consider "suboptimal".  My
feelings won't be hurt if you say so.

This patch does several things...

1) Removes run_makedev() and the call to it from conf.c per discussion.

2) Adds an lxc.hook.autodev hook.

Note: This hook is very close (one routine level abstracted) from where
the run_makedev was called.  Anyone really rrreeeaaalllyyy needing
MAKEDEV can add it in with a small shim script to do whatever they want
under whatever distro they're using, so no functionality is lost there.

3) Added a number of environment variables for all the hook scripts to
reference to assist in execution.  Things like LXC_ROOTFS_MOUNT could be
very useful but others were added as well.  Room for more if anyone has
an itch.  All in one spot in lxc_start.c.

4) clearenv and putenv( "container=lxc" ) calls were moved to just after
the "start" hook in the container just prior to actually firing up the
container so we could use environment variables prior to that and have
them flushed them before firing up init.  Nice side effect is that you
can define environment variables and then call lxc-start and have them
show up in those hooks scripts.

5) I actually DID update the man page for lxc.conf!  I guess I lied when
I said I wouldn't get that done.

Now for the ugly side...

I don't think where I stuck all those "setenv" calls is the best place
in lxc_start.c but it's where all the variables I needed where set in
one place and I could get at them before the pre-start hook could be
called.  It could possibly be abstracted into a routine.  I did not
update lxc_execute or a couple of other places where the config file is
parsed and that's probably something that needs to be done.

Patch is below my signature block...

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

-- 
diff --git a/doc/lxc.conf.sgml.in b/doc/lxc.conf.sgml.in
index 96dea89..c635ed0 100644
--- a/doc/lxc.conf.sgml.in
+++ b/doc/lxc.conf.sgml.in
@@ -510,6 +510,10 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 	rootfs.  If lxc.autodev is set to 1, then after mounting the container's
 	rootfs LXC will mount a fresh tmpfs under <filename>/dev</filename>
 	(limited to 100k) and fill in a minimal set of initial devices.
+        This is generally required when starting a container containing
+        a "systemd" based "init" but may be optional at other times.  Addional
+        devices in the containers /dev directory may be created through the
+        use of the <option>lxc.hook.autodev</option> hook.
       </para>
       <variablelist>
 	<varlistentry>
@@ -737,6 +741,27 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
       <variablelist>
 	<varlistentry>
 	  <term>
+	    <option>lxc.hook.autodev</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      A hook to be run in the container's namespace after
+	      mounting has been done and after any mount hooks have
+	      run, but before the pivot_root, if
+	      <option>lxc.autodev</option> == 1.
+	      The purpose of this hook is to assist in populating the
+	      /dev directory of the container when using the autodev
+	      option for systemd based containers.  The container's /dev
+	      directory is relative to the
+	      ${<option>LXC_ROOTFS_MOUNT</option>} environment
+	      variable available when the hook is run.
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
 	    <option>lxc.hook.start</option>
 	  </term>
 	  <listitem>
@@ -763,6 +788,129 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
       </variablelist>
     </refsect2>
 
+    <refsect2>
+      <title>Startup hooks Environment Variables</title>
+      <para>
+        A number of environment variables are made available to the startup
+        hooks to provide configuration information and assist in the
+        functioning of the hooks.  Not all variables are valid in all
+        contexts.  In particular, all paths are relative to the host system
+        and, as such, not valid during the <option>lxc.hook.start</option> hook.
+      </para>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_NAME</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      The LXC name of the container.  Useful for logging messages
+	      in commmon log environments.  [<option>-n</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_CONFIG_FILE</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      Host relative path to the container configuration file.  This
+	      gives the container to reference the original, top level,
+	      configuration file for the container in order to locate any
+	      addotional configuration information not otherwise made
+	      available.  [<option>-f</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_LOGPATH</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      Host relative path to the stderr output of the invoking
+	      command if not NULL.  [<option>-o</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_LOGLEVEL</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      The logging level for the stderr output from the lxc command.
+	      [<option>-l</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_CONSOLE</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      The path to the console output of the container if not NULL.
+	      [<option>-c</option>] [<option>lxc.console</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_CONSOLE_LOGPATH</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      The path to the console log output of the container if not NULL.
+	      [<option>-L</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_ROOTFS_MOUNT</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      The mount location to which the container is initially bound.
+	      This will be the host relative path to the container rootfs
+	      for the container instance being started and is where changes
+	      should be made for that instance.
+	      [<option>lxc.rootfs.mount</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+      <variablelist>
+	<varlistentry>
+	  <term>
+	    <option>LXC_ROOTFS_PATH</option>
+	  </term>
+	  <listitem>
+	    <para>
+	      The host relative path to the container root which has been
+	      mounted to the rootfs.mount location.
+	      [<option>lxc.rootfs</option>]
+	    </para>
+	  </listitem>
+	</varlistentry>
+      </variablelist>
+
+    </refsect2>
+
   </refsect1>
 
   <refsect1>
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 1c02850..699011a 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -116,7 +116,7 @@ lxc_log_define(lxc_conf, lxc);
 #endif
 
 char *lxchook_names[NUM_LXC_HOOKS] = {
-	"pre-start", "pre-mount", "mount", "start", "post-stop" };
+	"pre-start", "pre-mount", "mount", "autodev", "start", "post-stop" };
 
 extern int pivot_root(const char * new_root, const char * put_old);
 
@@ -913,33 +913,6 @@ static int mount_autodev(char *root)
 	return 0;
 }
 
-/*
- * Try to run MAKEDEV console in the container.  If something fails,
- * continue anyway as it should not be detrimental to the container.
- * This makes sure that things like /dev/vcs* exist.
- * (Pass devpath in to reduce stack usage)
- */
-static void run_makedev(char *devpath)
-{
-	int curd;
-	int ret;
-
-	curd = open(".", O_RDONLY);
-	if (curd < 0)
-		return;
-	ret = chdir(devpath);
-	if (ret) {
-		close(curd);
-		return;
-	}
-	if (run_buffer("/sbin/MAKEDEV console"))
-		INFO("Error running MAKEDEV console in %s", devpath);
-	ret = fchdir(curd);
-	if (ret)
-		INFO("Error returning to original directory: expect breakage");
-	close(curd);
-}
-
 struct lxc_devs {
 	char *name;
 	mode_t mode;
@@ -971,8 +944,7 @@ static int setup_autodev(char *root)
 	if (ret < 0 || ret >= MAXPATHLEN) {
 		ERROR("Error calculating container /dev location");
 		return -1;
-	} else
-		run_makedev(path);
+	}
 
 	INFO("Populating /dev under %s\n", root);
 	cmask = umask(S_IXUSR | S_IXGRP | S_IXOTH);
@@ -2553,6 +2525,10 @@ int lxc_setup(const char *name, struct lxc_conf *lxc_conf)
 	}
 
 	if (lxc_conf->autodev) {
+		if (run_lxc_hooks(name, "autodev", lxc_conf)) {
+			ERROR("failed to run autodev hooks for container '%s'.", name);
+			return -1;
+		}
 		if (setup_autodev(lxc_conf->rootfs.mount)) {
 			ERROR("failed to populate /dev in the container");
 			return -1;
@@ -2628,6 +2604,8 @@ int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf)
 		which = LXCHOOK_PREMOUNT;
 	else if (strcmp(hook, "mount") == 0)
 		which = LXCHOOK_MOUNT;
+	else if (strcmp(hook, "autodev") == 0)
+		which = LXCHOOK_AUTODEV;
 	else if (strcmp(hook, "start") == 0)
 		which = LXCHOOK_START;
 	else if (strcmp(hook, "post-stop") == 0)
diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index b576893..19e156c 100644
--- a/src/lxc/conf.h
+++ b/src/lxc/conf.h
@@ -213,8 +213,8 @@ struct lxc_rootfs {
 #endif
  */
 enum lxchooks {
-	LXCHOOK_PRESTART, LXCHOOK_PREMOUNT, LXCHOOK_MOUNT, LXCHOOK_START,
-	LXCHOOK_POSTSTOP, NUM_LXC_HOOKS};
+	LXCHOOK_PRESTART, LXCHOOK_PREMOUNT, LXCHOOK_MOUNT, LXCHOOK_AUTODEV,
+	LXCHOOK_START, LXCHOOK_POSTSTOP, NUM_LXC_HOOKS};
 extern char *lxchook_names[NUM_LXC_HOOKS];
 
 struct saved_nic {
diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 1d87227..2143af6 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -104,6 +104,7 @@ static struct lxc_config_t config[] = {
 	{ "lxc.hook.pre-start",       config_hook                 },
 	{ "lxc.hook.pre-mount",       config_hook                 },
 	{ "lxc.hook.mount",           config_hook                 },
+	{ "lxc.hook.autodev",         config_hook                 },
 	{ "lxc.hook.start",           config_hook                 },
 	{ "lxc.hook.post-stop",       config_hook                 },
 	{ "lxc.network.type",         config_network_type         },
@@ -821,6 +822,8 @@ static int config_hook(const char *key, const char *value,
 		return add_hook(lxc_conf, LXCHOOK_PRESTART, copy);
 	else if (strcmp(key, "lxc.hook.pre-mount") == 0)
 		return add_hook(lxc_conf, LXCHOOK_PREMOUNT, copy);
+	else if (strcmp(key, "lxc.hook.autodev") == 0)
+		return add_hook(lxc_conf, LXCHOOK_AUTODEV, copy);
 	else if (strcmp(key, "lxc.hook.mount") == 0)
 		return add_hook(lxc_conf, LXCHOOK_MOUNT, copy);
 	else if (strcmp(key, "lxc.hook.start") == 0)
diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
index 184fb04..2d66494 100644
--- a/src/lxc/lxc_start.c
+++ b/src/lxc/lxc_start.c
@@ -168,15 +168,6 @@ int main(int argc, char *argv[])
 			 my_args.progname, my_args.quiet))
 		return err;
 
-	if (clearenv()) {
-		SYSERROR("failed to clear environment");
-		/* don't error out though */
-	}
-	if (putenv("container=lxc")) {
-		SYSERROR("failed to set environment variable");
-		return err;
-	}
-
 	/* rcfile is specified in the cli option */
 	if (my_args.rcfile)
 		rcfile = (char *)my_args.rcfile;
@@ -234,6 +225,43 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	/* Start of environment variables for hooks */
+
+	/* Tell any hooks where the config file was that we read */
+	if (setenv("LXC_CONFIG_FILE", rcfile, 1)) {
+		SYSERROR("failed to set environment variable for config path");
+	}
+	/* This logic needs a better home.  We've read and processed
+	 * the config file and loaded the defines.  Now we can set
+	 * some of them into environment variables for the various
+	 * hooks.  Needs to be done early to catch pre-start. */
+	if (setenv("LXC_NAME", my_args.name, 1)) {
+		SYSERROR("failed to set environment variable for container name");
+	}
+	if (setenv("LXC_LOGPATH", my_args.log_file, 1)) {
+		SYSERROR("failed to set environment variable for console log");
+	}
+	if (setenv("LXC_ROOTFS_MOUNT", conf->rootfs.mount, 1)) {
+		SYSERROR("failed to set environment variable for rootfs mount");
+	}
+	if (setenv("LXC_ROOTFS_PATH", conf->rootfs.path, 1)) {
+		SYSERROR("failed to set environment variable for rootfs mount");
+	}
+	if (setenv("LXC_CONSOLE", conf->console.path, 1)) {
+		SYSERROR("failed to set environment variable for console path");
+	}
+	if (setenv("LXC_CONSOLE_LOGPATH", conf->console.log_path, 1)) {
+		SYSERROR("failed to set environment variable for console log");
+	}
+	{
+		char loglevel[16];
+		snprintf( loglevel, 14, "%d", conf->loglevel );
+		if (setenv("LXC_LOGLEVEL", loglevel, 1)) {
+			SYSERROR("failed to set environment variable for log level mount");
+		}
+	}
+	/* End of environment variables for hooks */
+
 	if (my_args.daemonize) {
 		/* do an early check for needed privs, since otherwise the
 		 * user won't see the error */
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 82a74d8..ae9a282 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -575,6 +575,21 @@ static int do_start(void *data)
 		goto out_warn_father;
 	}
 
+	/* The clearenv() and putenv() calls have been moved here
+	 * to allow us to use enviroment variables passed to the various
+	 * hooks, such as the start hook above.  Not all of the
+	 * variables like CONFIG_PATH or ROOTFS are valid in this
+	 * context but others are. */
+	if (clearenv()) {
+		SYSERROR("failed to clear environment");
+		/* don't error out though */
+	}
+
+	if (putenv("container=lxc")) {
+		SYSERROR("failed to set environment variable");
+		return -1;
+	}
+
 	close(handler->sigfd);
 
 	/* after this call, we are in error because this

-------------- 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/20130109/7aa01336/attachment.pgp>


More information about the lxc-devel mailing list