lxc-autostart: rework boot and group handling

This adds new functionality to lxc-autostart.

*) The -g / --groups option is multiple cummulative entry.
	This may be mixed freely with the previous comma separated
	group list convention.  Groups are processed in the
	order they first appear in the aggregated group list.

*) The NULL group may be specified in the group list using either a
	leading comma, a trailing comma, or an embedded comma.

*) Booting proceeds in order of the groups specified on the command line
	then ordered by lxc.start.order and name collalating sequence.

*) Default host bootup is now specified as "-g onboot," meaning that first
	the "onboot" group is booted and then any remaining enabled
	containers in the NULL group are booted.

*) Adds documentation to lxc-autostart for -g processing order and
	combinations.

*) Parameterizes bootgroups, options, and shutdown delay in init scripts
	and services.

*) Update the various init scripts to use lxc-autostart in a similar way.

Reported-by: CDR <venefax@gmail.com>
Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
Signed-off-by: Michael H. Warfield <mhw@WittsEnd.com>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

.gitignore

@@ -112,6 +112,9 @@ config/missing
 config/libtool.m4
 config/lt*.m4
 config/bash/lxc
+config/init/systemd/lxc-autostart-helper
+config/init/systemd/lxc.service
+config/init/sysvinit/lxc
 
 doc/*.1
 doc/*.5

config/init/systemd/Makefile.am

@@ -5,7 +5,17 @@ EXTRA_DIST = \
 if INIT_SCRIPT_SYSTEMD
 SYSTEMD_UNIT_DIR = $(prefix)/lib/systemd/system
 
-install-systemd: lxc.service lxc-devsetup
+lxc-autostart-helper: ../sysvinit/lxc.in $(top_builddir)/config.status
+	$(AM_V_GEN)sed                                          \
+	    -e 's|[@]SYSCONFDIR[@]|$(sysconfdir)|g'             \
+	    -e 's|[@]LOCALSTATEDIR[@]|$(localstatedir)|g'       \
+	    -e 's|[@]BINDIR[@]|$(bindir)|g'                     \
+	    < $< > $@-t &&                                      \
+	    chmod a+x $@-t &&                                   \
+	    mv $@-t $@
+BUILT_SOURCES = lxc-autostart-helper
+
+install-systemd: lxc.service lxc-devsetup lxc-autostart-helper
 	$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
 	$(INSTALL_DATA) lxc.service $(DESTDIR)$(SYSTEMD_UNIT_DIR)/
 
@@ -13,7 +23,7 @@ uninstall-systemd:
 	rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/lxc.service
 	rmdir $(DESTDIR)$(SYSTEMD_UNIT_DIR) || :
 
-pkglibexec_SCRIPTS = lxc-devsetup
+pkglibexec_SCRIPTS = lxc-devsetup lxc-autostart-helper
 
 install-data-local: install-systemd
 uninstall-local: uninstall-systemd

config/init/systemd/lxc.service.in

@@ -1,13 +1,13 @@
 [Unit]
 Description=LXC Container Initialization and Autoboot Code
-After=syslog.target
+After=syslog.target network.target
 
 [Service]
 Type=oneshot
 RemainAfterExit=yes
-ExecStartPre=/usr/libexec/lxc/lxc-devsetup
+ExecStartPre=@libexecdir@/lxc/lxc-devsetup
-ExecStart=/usr/libexec/lxc/lxc-startup start
+ExecStart=@libexecdir@/lxc/lxc-autostart-helper start
-ExecStop=/usr/libexec/lxc/lxc-startup stop
+ExecStop=@libexecdir@/lxc/lxc-autostart-helper stop
 # Environment=BOOTUP=serial
 # Environment=CONSOLETYPE=serial
 StandardOutput=syslog

config/init/sysvinit/lxc

@@ -1,66 +0,0 @@
-#!/bin/sh
-#
-# lxc Start/Stop LXC autoboot containers
-#
-# chkconfig: 345 99 01
-# description: Starts/Stops all LXC containers configured for autostart.
-#
-### BEGIN INIT INFO
-# Provides: lxc
-# Default-Start: 3 4 5
-# Default-Stop: 0 1 6
-# Short-Description: Bring up/down LXC autostart containers
-# Description: Bring up/down LXC autostart containers
-### END INIT INFO
-
-# Source function library.
-. /etc/init.d/functions
-
-# Check for needed utility program
-[ -x /usr/bin/lxc-autostart ] || exit 1
-
-# If libvirtd is providing the bridge, it might not be
-# immediately available, so wait a bit for it before starting
-# up the containers or else any that use the bridge will fail
-# to start
-wait_for_bridge()
-{
-    [ -f /etc/lxc/default.conf ] || { return 0; }
-
-    BRNAME=`grep '^[ 	]*lxc.network.link' /etc/lxc/default.conf | sed 's/^.*=[ 	]*//'`
-    if [ -z "$BRNAME" ]; then
-	return 0
-    fi
-
-    for try in `seq 1 30`; do
-	ifconfig -a |grep "^$BRNAME" >/dev/null 2>&1
-	if [ $? = 0 ]; then
-	    return
-	fi
-	sleep 1
-    done
-}
-
-# See how we were called.
-case "$1" in
-  start)
-	[ ! -f /var/lock/subsys/lxc ] || { exit 0; }
-
-	# Start containers
-	wait_for_bridge
-	action $"Starting LXC containers: " /usr/bin/lxc-autostart
-	touch /var/lock/subsys/lxc
-	;;
-  stop)
-	action $"Stopping LXC containers: " /usr/bin/lxc-autostart -a -A -s
-	rm -f /var/lock/subsys/lxc
-	;;
-  restart|reload|force-reload)
-	$0 stop
-	$0 start
-	;;
-  *)
-	echo $"Usage: $0 {start|stop|restart|reload|force-reload}"
-	exit 2
-esac
-exit $?

config/init/sysvinit/lxc.in

@@ -0,0 +1,124 @@
+#!/bin/sh
+#
+# lxc Start/Stop LXC autoboot containers
+#
+# chkconfig: 345 99 01
+# description: Starts/Stops all LXC containers configured for autostart.
+#
+### BEGIN INIT INFO
+# Provides: lxc
+# Default-Start: 3 4 5
+# Default-Stop: 0 1 6
+# Short-Description: Bring up/down LXC autostart containers
+# Description: Bring up/down LXC autostart containers
+### END INIT INFO
+
+sysconfdir="@SYSCONFDIR@"
+bindir="@BINDIR@"
+localstatedir="@LOCALSTATEDIR@"
+
+# These can be overridden in @SYSCONFDIR@/sysconfig/lxc
+
+# BOOTGROUPS - What groups should start on bootup?
+#	Comma separated list of groups.
+#	Leading comma, trailing comma or embedded double
+#	comma indicates when the NULL group should be run.
+# Example (default): boot the onboot group first then the NULL group
+BOOTGROUPS="onboot,"
+
+# SHUTDOWNDELAY - Wait time for a container to shut down.
+#	ner shutdown can result in lengthy system
+#	shutdown times.  Even 5 seconds per container can be
+#	too long.
+SHUTDOWNDELAY=5
+
+# OPTIONS can be used for anything else.
+#	If you want to boot everything then
+#	options can be "-a" or "-a -A".
+OPTIONS=
+
+# STOPOPTS are stop options.  The can be used for anything else to stop.
+#	If you want to kill containers fast, use -k
+STOPOPTS="-a -A -s"
+
+# Source function library.
+test ! -r "$sysconfdir"/rc.d/init.d/functions ||
+        . "$sysconfdir"/rc.d/init.d/functions
+
+# Source any configurable options
+test ! -r "$sysconfdir"/sysconfig/lxc ||
+        . "$sysconfdir"/sysconfig/lxc
+
+# Check for needed utility program
+[ -x "$bindir"/lxc-autostart ] || exit 1
+
+# If libvirtd is providing the bridge, it might not be
+# immediately available, so wait a bit for it before starting
+# up the containers or else any that use the bridge will fail
+# to start
+wait_for_bridge()
+{
+    [ -f "$sysconfdir"/lxc/default.conf ] || { return 0; }
+
+    which ifconfig >/dev/null 2>&1
+    if [ $? = 0 ]; then
+        cmd="ifconfig -a"
+    else
+        which ip >/dev/null 2>&1
+        if [ $? = 0 ]; then
+            cmd="ip link list"
+        fi
+    fi
+    [ -n cmd ] || { return 0; }
+
+    BRNAME=`grep '^[ 	]*lxc.network.link' "$sysconfdir"/lxc/default.conf | sed 's/^.*=[ 	]*//'`
+    if [ -z "$BRNAME" ]; then
+	return 0
+    fi
+
+    for try in `seq 1 30`; do
+	eval $cmd |grep "^$BRNAME" >/dev/null 2>&1
+	if [ $? = 0 ]; then
+	    return
+	fi
+	sleep 1
+    done
+}
+
+# See how we were called.
+case "$1" in
+  start)
+	[ ! -f "$localstatedir"/lock/subsys/lxc ] || { exit 0; }
+
+	if [ -n "$BOOTGROUPS" ]
+	then
+		BOOTGROUPS="-g $BOOTGROUPS"
+	fi
+
+	# Start containers
+	wait_for_bridge
+	# Start autoboot containers first then the NULL group "onboot,".
+	action $"Starting LXC autoboot containers: " /usr/bin/lxc-autostart $OPTIONS $BOOTGROUPS
+	touch "$localstatedir"/lock/subsys/lxc
+	;;
+  stop)
+	if [ -n "$SHUTDOWNDELAY" ]
+	then
+		SHUTDOWNDELAY="-t $SHUTDOWNDELAY"
+	fi
+
+	# The stop is serialized and can take excessive time.  We need to avoid
+	# delaying the system shutdown / reboot as much as we can since it's not
+	# parallelized...  Even 5 second timout may be too long.
+	action $"Stopping LXC containers: " "$bindir"/lxc-autostart $STOPOPTS $SHUTDOWNDELAY
+	rm -f "$localstatedir"/lock/subsys/lxc
+	;;
+  restart|reload|force-reload)
+	$0 stop
+	$0 start
+	;;
+  *)
+	echo $"Usage: $0 {start|stop|restart|reload|force-reload}"
+	exit 2
+esac
+exit $?

config/init/upstart/lxc.conf

@@ -6,6 +6,30 @@ stop on starting rc RUNLEVEL=[016]
 
 env LXC_AUTO="false"
 
+# These can be overridden in /etc/default/lxc
+
+# BOOTGROUPS - What groups should start on bootup?
+#	Comma separated list of groups.
+#	Leading comma, trailing comma or embedded double
+#	comma indicates when the NULL group should be run.
+# Example (default): boot the onboot group first then the NULL group
+env BOOTGROUPS="onboot,"
+
+# SHUTDOWNDELAY - Wait time for a container to shut down.
+#	Container shutdown can result in lengthy system
+#	shutdown times.  Even 5 seconds per container can be
+#	too long.
+env SHUTDOWNDELAY=5
+
+# OPTIONS can be used for anything else.
+#	If you want to boot everything then
+#	options can be "-a" or "-a -A".
+env OPTIONS=
+
+# STOPOPTS are stop options.  The can be used for anything else to stop.
+#	If you want to kill containers fast, use -k
+env STOPOPTS="-a -A -s"
+
 pre-start script
 	[ -f /etc/default/lxc ] && . /etc/default/lxc
 
@@ -20,12 +44,28 @@ pre-start script
 
 	[ "x$LXC_AUTO" = "xtrue" ] || exit 0
 
-	lxc-autostart -L | while read line; do
+	if [ -n "$BOOTGROUPS" ]
+	then
+		BOOTGROUPS="-g $BOOTGROUPS"
+	fi
+
+	# Process the "onboot" group first then the NULL group.
+	lxc-autostart -L $OPTIONS $BOOTGROUPS | while read line; do
 		set -- $line
 		(start lxc-instance NAME=$1 && sleep $2) || true
 	done
 end script
 
+# The stop is serialized and can take excessive time.  We need to avoid
+# delaying the system shutdown / reboot as much as we can since it's not
+# parallelized...  Even 5 second timout may be too long.
 post-stop script
-	lxc-autostart -a -A -s || true
+	[ -f /etc/default/lxc ] && . /etc/default/lxc
+
+	if [ -n "$SHUTDOWNDELAY" ]
+	then
+		SHUTDOWNDELAY="-t $SHUTDOWNDELAY"
+	fi
+
+	lxc-autostart $STOPOPTS $SHUTDOWNDELAY || true
 end script

configure.ac

@@ -575,7 +575,9 @@ AC_CONFIG_FILES([
 	config/bash/lxc
 	config/init/Makefile
 	config/init/sysvinit/Makefile
+	config/init/sysvinit/lxc
 	config/init/systemd/Makefile
+	config/init/systemd/lxc.service
 	config/init/upstart/Makefile
 	config/etc/Makefile
 	config/templates/Makefile

doc/lxc-autostart.sgml.in

@@ -154,8 +154,17 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
                 </term>
                 <listitem>
                     <para>
-                        Comma separate list of groups to select
+                        Comma separated list of groups to select
-                        (defaults to those without a lxc.group).
+                        (defaults to those without a lxc.group - the NULL group).
+                        This option may be specified multiple times
+                        and the arguments concatentated.  The NULL or
+                        empty group may be specified as a leading comma,
+                        trailing comma, embedded double comma, or empty
+                        argument where the NULL group should be processed.
+                        Groups are processed in the order specified on the
+                        command line.  Multiple invocations of the -g option
+                        may be freely intermixed with the comma separated
+                        lists and will be combined in specified order.
                     </para>
                 </listitem>
             </varlistentry>
@@ -185,6 +194,69 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
         </variablelist>
     </refsect1>
 
+    <refsect1>
+        <title>Autostart and System Boot</title>
+
+        <para>
+            The <command>lxc-autostart</command> command is used as part of the
+            LXC system service, when enabled to run on host system at bootup and at
+            shutdown.  It's used to select which containers to start in what order
+            and how much to delay between each startup when the host system boots.
+        </para>
+
+        <para>
+            Each container can be part of any number of groups or no group at all.
+            Two groups are special. One is the NULL group, i.e. the container does
+            not belong to any group. The other group is the "onboot" group.
+        </para>
+
+        <para>
+            When the system boots with the LXC service enabled, it will first
+            attempt to boot any containers with lxc.start.auto == 1 that is a member
+            of the "onboot" group. The startup will be in order of lxc.start.order.
+            If an lxc.start.delay has been specified, that delay will be honored
+            before attempting to start the next container to give the current
+            container time to begin initialization and reduce overloading the host
+            system. After starting the members of the "onboot" group, the LXC system
+            will proceed to boot containers with lxc.start.auto == 1 which are not
+            members of any group (the NULL group) and proceed as with the onboot
+            group.
+        </para>
+    </refsect1>
+
+    <refsect1>
+        <title>Startup Group Examples</title>
+        <variablelist>
+            <varlistentry>
+                <term>
+                    <option>-g "onboot,"</option>
+                </term>
+                <listitem>
+                    <para>
+                        Start the "onboot" group first then the NULL group.
+                    </para>
+                    <para>
+                        This is the equivalent of: <option>-g onboot -g ""</option>.
+                    </para>
+                </listitem>
+            </varlistentry>
+            <varlistentry>
+                <term>
+                    <option>-g "dns,web,,onboot"</option>
+                </term>
+                <listitem>
+                    <para>
+                        Starts the "dns" group first, the "web" group second, then
+                        the NULL group followed by the "onboot" group.
+                    </para>
+                    <para>
+                        This is the equivalent of: <option>-g dns,web -g ,onboot</option> or <option>-g dns -g web -g "" -g onboot</option>.
+                    </para>
+                </listitem>
+            </varlistentry>
+        </variablelist>
+    </refsect1>
+
     &seealso;
 
     <refsect1>

doc/lxc.container.conf.sgml.in

@@ -1486,6 +1486,29 @@ mknod errno 0
         </varlistentry>
       </variablelist>
     </refsect2>
+
+    <refsect2>
+    <title>Autostart and System Boot</title>
+    <para>
+          Each container can be part of any number of groups or no group at all.
+          Two groups are special. One is the NULL group, i.e. the container does
+          not belong to any group. The other group is the "onboot" group.
+    </para>
+
+    <para>
+          When the system boots with the LXC service enabled, it will first
+          attempt to boot any containers with lxc.start.auto == 1 that is a member
+          of the "onboot" group. The startup will be in order of lxc.start.order.
+          If an lxc.start.delay has been specified, that delay will be honored
+          before attempting to start the next container to give the current
+          container time to begin initialization and reduce overloading the host
+          system. After starting the members of the "onboot" group, the LXC system
+          will proceed to boot containers with lxc.start.auto == 1 which are not
+          members of any group (the NULL group) and proceed as with the onboot
+          group.
+    </para>
+
+    </refsect2>
   </refsect1>
 
   <refsect1>

lxc.spec.in

@@ -154,6 +154,7 @@ rm -rf %{buildroot}
 %attr(4111,root,root) %{_libexecdir}/%{name}/lxc-user-nic
 %if %{with_systemd}
 %attr(555,root,root) %{_libexecdir}/%{name}/lxc-devsetup
+%attr(555,root,root) %{_libexecdir}/%{name}/lxc-autostart-helper
 %endif
 
 %if %{with_python}

src/lxc/lxc_autostart.c

@@ -28,6 +28,9 @@
 #include "log.h"
 
 lxc_log_define(lxc_autostart_ui, lxc);
+static struct lxc_list *accumulate_list(char *input, char *delimiter, struct lxc_list *str_list);
+
+struct lxc_list *cmd_groups_list = NULL;
 
 static int my_parser(struct lxc_arguments* args, int c, char* arg)
 {
@@ -38,7 +41,7 @@ static int my_parser(struct lxc_arguments* args, int c, char* arg)
 	case 's': args->shutdown = 1; break;
 	case 'a': args->all = 1; break;
 	case 'A': args->ignore_auto = 1; break;
-	case 'g': args->groups = arg; break;
+	case 'g': cmd_groups_list = accumulate_list( arg, ",", cmd_groups_list); break;
 	case 't': args->timeout = atoi(arg); break;
 	}
 	return 0;
@@ -79,6 +82,29 @@ Options:\n\
 	.timeout = 60,
 };
 
+int list_contains_entry( char *str_ptr, struct lxc_list *p1 ) {
+	struct lxc_list *it1;
+
+	/*
+	 * If the entry is NULL or the empty string and the list
+	 * is NULL, we have a match
+	 */
+	if (! p1 && ! str_ptr)
+		return 1;
+	if (! p1 && ! *str_ptr)
+		return 1;
+
+	if (!p1)
+		return 0;
+
+	lxc_list_for_each(it1, p1) {
+		if (strcmp(it1->elem, str_ptr) == 0)
+			return 1;
+	}
+
+	return 0;
+}
+
 int lists_contain_common_entry(struct lxc_list *p1, struct lxc_list *p2) {
 	struct lxc_list *it1;
 	struct lxc_list *it2;
@@ -102,6 +128,78 @@ int lists_contain_common_entry(struct lxc_list *p1, struct lxc_list *p2) {
 	return 0;
 }
 
+/*
+ * This is a variation of get_list below it.
+ * This version allows two additional features.
+ * If a list is passed to it, it adds to it.
+ * It allows for empty entries (i.e. "group1,,group2") generating
+ * 	and empty list entry.
+ */
+static struct lxc_list *accumulate_list(char *input, char *delimiter, struct lxc_list *str_list) {
+	char *workstr = NULL;
+	char *workptr = NULL;
+	char *next_ptr = NULL;
+	struct lxc_list *worklist;
+	struct lxc_list *workstr_list;
+
+	workstr = strdup(input);
+	if (!workstr) {
+		return NULL;
+	}
+
+	workstr_list = str_list;
+	if ( ! workstr_list ) {
+		workstr_list = malloc(sizeof(*workstr_list));
+		lxc_list_init(workstr_list);
+	}
+
+	for (workptr = workstr; workptr; workptr = next_ptr) {
+		/*
+		 * We can't use strtok_r here because it collapses
+		 * multiple delimiters into 1 making empty fields
+		 * impossible...
+		 */
+		/* token = strtok_r(workptr, delimiter, &sptr); */
+		next_ptr = strchr( workptr, *delimiter );
+
+		if( next_ptr ) {
+			*next_ptr++ = '\0';
+		}
+
+		/*
+		 * At this point, we'd like to check to see if this
+		 * group is already contained in the list and ignore
+		 * it if it is...  This also helps us with any
+		 * corner cases where a string begins or ends with a
+		 * delimiter.
+		 */
+
+		if ( list_contains_entry( workptr, workstr_list ) ) {
+			if ( *workptr ) {
+				fprintf(stderr, "Duplicate group \"%s\" in list - ignoring\n", workptr );
+			} else {
+				fprintf(stderr, "Duilicate NULL group in list - ignoring\n" );
+			}
+		} else {
+			worklist = malloc(sizeof(*worklist));
+			if (!worklist)
+				break;
+
+			worklist->elem = strdup(workptr);
+			if (!worklist->elem) {
+				free(worklist);
+				break;
+			}
+
+			lxc_list_add_tail(workstr_list, worklist);
+		}
+	}
+
+	free(workstr);
+
+	return workstr_list;
+}
+
 static struct lxc_list *get_list(char *input, char *delimiter) {
 	char *workstr = NULL;
 	char *workptr = NULL;
@@ -209,15 +307,29 @@ static int cmporder(const void *p1, const void *p2) {
 		return (c1_order - c2_order) * -1;
 }
 
+static int toss_list( struct lxc_list *c_groups_list ) {
+	struct lxc_list *it, *next;
+
+	if (c_groups_list) {
+		lxc_list_for_each_safe(it, c_groups_list, next) {
+			lxc_list_del(it);
+			free(it->elem);
+			free(it);
+		}
+		free(c_groups_list);
+	}
+
+	return 1;
+}
+
 int main(int argc, char *argv[])
 {
 	int count = 0;
 	int i = 0;
 	int ret = 0;
 	struct lxc_container **containers = NULL;
-	struct lxc_list *cmd_groups_list = NULL;
+	struct lxc_list **c_groups_lists = NULL;
-	struct lxc_list *c_groups_list = NULL;
+	struct lxc_list *cmd_group;
-	struct lxc_list *it, *next;
 	char *const default_start_args[] = {
 		"/sbin/init",
 		NULL,
@@ -236,46 +348,80 @@ int main(int argc, char *argv[])
 	if (count < 0)
 		return 1;
 
+	if (!my_args.all) {
+		/* Allocate an array for our container group lists */
+		c_groups_lists = calloc( count, sizeof( struct lxc_list * ) );
+	}
+
 	qsort(&containers[0], count, sizeof(struct lxc_container *), cmporder);
 
-	if (my_args.groups && !my_args.all)
+	if (cmd_groups_list && my_args.all) {
-		cmd_groups_list = get_list((char*)my_args.groups, ",");
+		fprintf(stderr, "Specifying -a (all) with -g (groups) doesn't make sense. All option overrides.");
+	}
 
+	if (!cmd_groups_list) {
+		/*
+		 * We need a default cmd_groups_list even for the -a
+		 * case in order to force a pass through the loop for
+		 * the NULL group.  This, someday, could be taken from
+		 * a config file somewhere...
+		 */
+		cmd_groups_list = accumulate_list( "" , ",", NULL );
+	}
+
+	lxc_list_for_each(cmd_group, cmd_groups_list) {
+
+		/*
+		 * Prograpmmers Note:
+		 * Because we may take several passes through the container list
+		 * We'll switch on if the container pointer is NULL and if we process a
+		 * container (run it or decide to ignore it) and call lxc_container_put
+		 * then we'll NULL it out and not check it again.
+		 */
 		for (i = 0; i < count; i++) {
 			struct lxc_container *c = containers[i];
 
+			if (!c)
+				/* Skip - must have been already processed */
+				continue;
+
+			/*
+			 * We haven't loaded the container groups yet so
+			 * these next two checks don't need to free them
+			 * if they fail.  They'll fail on the first pass.
+			 */
 			if (!c->may_control(c)) {
-			lxc_container_put(c);
+				/* We're done with this container */
+				if ( lxc_container_put(c) > 0 )
+					containers[i] = NULL;
 				continue;
 			}
 
 			if (!my_args.ignore_auto &&
 			    get_config_integer(c, "lxc.start.auto") != 1) {
-			lxc_container_put(c);
+				/* We're done with this container */
+				if ( lxc_container_put(c) > 0 )
+					containers[i] = NULL;
 				continue;
 			}
 
 			if (!my_args.all) {
 				/* Filter by group */
-			c_groups_list = get_config_list(c, "lxc.group");
+				if( ! c_groups_lists[i] ) {
-
+					/* Now we're loading up a container's groups */
-			ret = lists_contain_common_entry(cmd_groups_list, c_groups_list);
+					c_groups_lists[i] = get_config_list(c, "lxc.group");
-
-			if (c_groups_list) {
-				lxc_list_for_each_safe(it, c_groups_list, next) {
-					lxc_list_del(it);
-					free(it->elem);
-					free(it);
-				}
-				free(c_groups_list);
 				}
 
+				ret = list_contains_entry(cmd_group->elem, c_groups_lists[i]);
+
 				if ( ret == 0 ) {
-				lxc_container_put(c);
+					/* Not in the target group this pass */
+					/* Leave in the list for subsequent passes */
 					continue;
 				}
 			}
 
+			/* We have a candidate continer to process */
 			c->want_daemonize(c, 1);
 
 			if (my_args.shutdown) {
@@ -332,17 +478,37 @@ int main(int argc, char *argv[])
 				}
 			}
 
-
+			/*
-		lxc_container_put(c);
+			 * If we get this far and we haven't hit any skip "continue"
+			 * then we're done with this container...  We can dump any
+			 * c_groups_list and the container itself.
+			 */
+			if ( lxc_container_put(c) > 0 ) {
+				containers[i] = NULL;
 			}
+			if ( c_groups_lists && c_groups_lists[i] ) {
+				toss_list(c_groups_lists[i]);
+				c_groups_lists[i] = NULL;
+			}
+		}
+
+	}
+
+	/* clean up any lingering detritus */
+	for (i = 0; i < count; i++) {
+		if ( containers[i] ) {
+			lxc_container_put(containers[i]);
+		}
+		if ( c_groups_lists && c_groups_lists[i] ) {
+			toss_list(c_groups_lists[i]);
+		}
+	}
+
+	if ( c_groups_lists )
+		free(c_groups_lists);
 
 	if ( cmd_groups_list ) {
-		lxc_list_for_each_safe(it, cmd_groups_list, next) {
+		toss_list( cmd_groups_list );
-			lxc_list_del(it);
-			free(it->elem);
-			free(it);
-		}
-		free(cmd_groups_list);
 	}
 
 	free(containers);