[lxc-devel] [PATCH] Refactoring lxc-autostart boot process and group handling.

Michael H. Warfield mhw at WittsEnd.com
Fri May 16 18:07:31 UTC 2014


Before anyone else spots it...  I did miss one spot where I failed to
toss a list (cmd_group_lists) on exit.  So, some memory checkers will
complain about orphaned memory or leaks (even though it's on exit).
I'll fix that and add some doco once this has been reviewed further.

Mike

On Thu, 2014-05-15 at 16:51 -0400, Michael H. Warfield wrote:
> Ok...
> 
> Here it is.  The roll up of Dwight's earlier patch, my previous 3
> patches plus, now, the rework to lxc-autostart to support multiple
> invocations of the -g option, allow ordinal inclusion of the NULL group,
> and process containers first in order of the groups specified on the
> command line and their membership in those groups, subject to all other
> constraints (lxc.start.auto) and orderings (lxc.start.order).
> 
> I have also noticed that host shutdown time can be very exhorbatently
> long.  I added parameters to set a shutdown time of -t 5 seconds to
> speed that process up (may still not be fast enough).  Problem here is
> that while startup returns quickly while a container initializes and we
> need a delay, shutdown is largely serial and not parallelized at all.
> We may need to address that in some way moving forward.
> 
> Here's the grand patch:
> 
> -- 
> Refactoring lxc-autostart boot process and group handling.
> 
> This is a rollup of 4 earlier patches patching the systemd
> init to use the sysvinit script, adding an onboot group to the
> boot set, updating upstart to include the onboot group, and adding
> documentation for the special boot groups.
> 
> This also 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.org 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.
> 
> From the previous 4 individual patches:
> 
> Reported-by: CDR <venefax at gmail.com>
> Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> 
> - reuse the sysvinit script to ensure that if the lxc is configured to use
>   a bridge setup by libvirt, the bridge will be available before starting
>   the container
> 
> - made the sysvinit script check for the existance of ifconfig, and fall
>   back to ip link list if available
> 
> - made the lxc service also dependant on the network.target
> 
> - autoconfized the paths in the service file and sysvinit script
> 
> - v2: rename script lxc-autostart to lxc-autostart-helper to avoid confusion
> 
> From: Michael H. Warfield <mhw at WittsEnd.com>
> 
> - This adds a non-null group (onboot) to the sysvinit startup script
> for autobooting containers.  This allows for containers which are
> in other groups to be included in the autoboot process.
> 
> This script is used by both the sysvinit systems and the systemd
> systems.
> 
> From: Michael H. Warfield <mhw at WittsEnd.com>
> 
> - Add the feature to the Upstart init script to boot the "onboot"
> group dependent on the start.auto = 1 flag.  This brings the
> the Upstart behavior into congruence with the sysvinit script
> for SysV Init and Systemd.
> 
> From: Michael H. Warfield <mhw at WittsEnd.com>
> 
> Added sections to lxc-autostart and lxc.container.config to document
> the behavior of the LXC service at host system boot time with regards
> to the lxc.group and lxc.start.auto parameters.
> 
> Signed-off-by: Michael H. Warfield <mhw at WittsEnd.com>
> ---
>  .gitignore                         |   3 +
>  config/init/systemd/Makefile.am    |  14 +-
>  config/init/systemd/lxc.service    |  17 --
>  config/init/systemd/lxc.service.in |  17 ++
>  config/init/sysvinit/lxc           |  66 --------
>  config/init/sysvinit/lxc.in        |  86 ++++++++++
>  config/init/upstart/lxc.conf       |   8 +-
>  configure.ac                       |   2 +
>  doc/lxc-autostart.sgml.in          |  30 ++++
>  doc/lxc.container.conf.sgml.in     |  23 +++
>  lxc.spec.in                        |   1 +
>  src/lxc/lxc_autostart.c            | 331 +++++++++++++++++++++++++++----------
>  12 files changed, 427 insertions(+), 171 deletions(-)
>  delete mode 100644 config/init/systemd/lxc.service
>  create mode 100644 config/init/systemd/lxc.service.in
>  delete mode 100755 config/init/sysvinit/lxc
>  create mode 100644 config/init/sysvinit/lxc.in
> 
> diff --git a/.gitignore b/.gitignore
> index 8145f81..2b478cd 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -111,6 +111,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
> diff --git a/config/init/systemd/Makefile.am b/config/init/systemd/Makefile.am
> index de5ee50..fc374c5 100644
> --- a/config/init/systemd/Makefile.am
> +++ b/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
> diff --git a/config/init/systemd/lxc.service b/config/init/systemd/lxc.service
> deleted file mode 100644
> index aa20b91..0000000
> --- a/config/init/systemd/lxc.service
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -[Unit]
> -Description=LXC Container Initialization and Autoboot Code
> -After=syslog.target
> -
> -[Service]
> -Type=oneshot
> -RemainAfterExit=yes
> -ExecStartPre=/usr/libexec/lxc/lxc-devsetup
> -ExecStart=/usr/libexec/lxc/lxc-startup start
> -ExecStop=/usr/libexec/lxc/lxc-startup stop
> -# Environment=BOOTUP=serial
> -# Environment=CONSOLETYPE=serial
> -StandardOutput=syslog
> -StandardError=syslog
> -
> -[Install]
> -WantedBy=multi-user.target
> diff --git a/config/init/systemd/lxc.service.in b/config/init/systemd/lxc.service.in
> new file mode 100644
> index 0000000..5f155b6
> --- /dev/null
> +++ b/config/init/systemd/lxc.service.in
> @@ -0,0 +1,17 @@
> +[Unit]
> +Description=LXC Container Initialization and Autoboot Code
> +After=syslog.target network.target
> +
> +[Service]
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStartPre=@libexecdir@/lxc/lxc-devsetup
> +ExecStart=@libexecdir@/lxc/lxc-autostart-helper start
> +ExecStop=@libexecdir@/lxc/lxc-autostart-helper stop
> +# Environment=BOOTUP=serial
> +# Environment=CONSOLETYPE=serial
> +StandardOutput=syslog
> +StandardError=syslog
> +
> +[Install]
> +WantedBy=multi-user.target
> diff --git a/config/init/sysvinit/lxc b/config/init/sysvinit/lxc
> deleted file mode 100755
> index 5ab3c46..0000000
> --- a/config/init/sysvinit/lxc
> +++ /dev/null
> @@ -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 $?
> diff --git a/config/init/sysvinit/lxc.in b/config/init/sysvinit/lxc.in
> new file mode 100644
> index 0000000..e784285
> --- /dev/null
> +++ b/config/init/sysvinit/lxc.in
> @@ -0,0 +1,86 @@
> +#!/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@"
> +
> +# Source function library.
> +test ! -r "$sysconfdir"/rc.d/init.d/functions ||
> +        . "$sysconfdir"/rc.d/init.d/functions
> +
> +# 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; }
> +
> +	# Start containers
> +	wait_for_bridge
> +	# Start autoboot containers first then the NULL group "onboot,".
> +	action $"Starting LXC autoboot containers: " /usr/bin/lxc-autostart -g "onboot,"
> +	touch "$localstatedir"/lock/subsys/lxc
> +	;;
> +  stop)
> +	# 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...  5 second timout may be too long.
> +	action $"Stopping LXC containers: " "$bindir"/lxc-autostart -a -A -s -t 5
> +	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 $?
> diff --git a/config/init/upstart/lxc.conf b/config/init/upstart/lxc.conf
> index d5131ad..b9f8174 100644
> --- a/config/init/upstart/lxc.conf
> +++ b/config/init/upstart/lxc.conf
> @@ -20,12 +20,16 @@ pre-start script
>  
>  	[ "x$LXC_AUTO" = "xtrue" ] || exit 0
>  
> -	lxc-autostart -L | while read line; do
> +	# Process the "onboot" group first then the NULL group.
> +	lxc-autostart -L -g "onboot," | 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...  5 second timout may be too long.
>  post-stop script
> -	lxc-autostart -a -A -s || true
> +	lxc-autostart -a -A -s -t 5 || true
>  end script
> diff --git a/configure.ac b/configure.ac
> index 8865bc8..a7fce11 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -573,7 +573,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
> diff --git a/doc/lxc-autostart.sgml.in b/doc/lxc-autostart.sgml.in
> index 3d423dd..116d7bc 100644
> --- a/doc/lxc-autostart.sgml.in
> +++ b/doc/lxc-autostart.sgml.in
> @@ -185,6 +185,36 @@ 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>
> +
>      &seealso;
>  
>      <refsect1>
> diff --git a/doc/lxc.container.conf.sgml.in b/doc/lxc.container.conf.sgml.in
> index 6e96889..bbae448 100644
> --- a/doc/lxc.container.conf.sgml.in
> +++ b/doc/lxc.container.conf.sgml.in
> @@ -1464,6 +1464,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>
> diff --git a/lxc.spec.in b/lxc.spec.in
> index 2717c83..57912a1 100644
> --- a/lxc.spec.in
> +++ b/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}
> diff --git a/src/lxc/lxc_autostart.c b/src/lxc/lxc_autostart.c
> index 1e0c608..34964c0 100644
> --- a/src/lxc/lxc_autostart.c
> +++ b/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_list = NULL;
> -	struct lxc_list *it, *next;
> +	struct lxc_list **c_groups_lists = NULL;
> +	struct lxc_list *cmd_group;
>  	char *const default_start_args[] = {
>  		"/sbin/init",
>  		NULL,
> @@ -236,115 +348,166 @@ int main(int argc, char *argv[])
>  	if (count < 0)
>  		return 1;
>  
> -	qsort(&containers[0], count, sizeof(struct lxc_container *), cmporder);
> +	if (!my_args.all) {
> +		/* Allocate an array for our container group lists */
> +		c_groups_lists = calloc( count, sizeof( struct lxc_list * ) );
> +	}
>  
> -	if (my_args.groups && !my_args.all)
> -		cmd_groups_list = get_list((char*)my_args.groups, ",");
> +	qsort(&containers[0], count, sizeof(struct lxc_container *), cmporder);
>  
> -	for (i = 0; i < count; i++) {
> -		struct lxc_container *c = containers[i];
> +	if (cmd_groups_list && my_args.all) {
> +		fprintf(stderr, "Specifying -a (all) with -g (groups) doesn't make sense. All option overrides.");
> +	}
>  
> -		if (!c->may_control(c)) {
> -			lxc_container_put(c);
> -			continue;
> -		}
> +	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 );
> +	}
>  
> -		if (!my_args.ignore_auto &&
> -		    get_config_integer(c, "lxc.start.auto") != 1) {
> -			lxc_container_put(c);
> -			continue;
> -		}
> +	lxc_list_for_each(cmd_group, cmd_groups_list) {
>  
> -		if (!my_args.all) {
> -			/* Filter by group */
> -			c_groups_list = get_config_list(c, "lxc.group");
> +		/*
> +		 * 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];
>  
> -			ret = lists_contain_common_entry(cmd_groups_list, c_groups_list);
> +			if (!c)
> +				/* Skip - must have been already processed */
> +				continue;
>  
> -			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);
> +			/*
> +			 * 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)) {
> +				/* We're done with this container */
> +				if ( lxc_container_put(c) > 0 )
> +					containers[i] = NULL;
> +				continue;
>  			}
>  
> -			if (ret == 0) {
> -				lxc_container_put(c);
> +			if (!my_args.ignore_auto &&
> +			    get_config_integer(c, "lxc.start.auto") != 1) {
> +				/* We're done with this container */
> +				if ( lxc_container_put(c) > 0 )
> +					containers[i] = NULL;
>  				continue;
>  			}
> -		}
>  
> -		c->want_daemonize(c, 1);
> -
> -		if (my_args.shutdown) {
> -			/* Shutdown the container */
> -			if (c->is_running(c)) {
> -				if (my_args.list)
> -					printf("%s\n", c->name);
> -				else {
> -					if (!c->shutdown(c, my_args.timeout)) {
> -						if (!c->stop(c)) {
> -							fprintf(stderr, "Error shutting down container: %s\n", c->name);
> +			if (!my_args.all) {
> +				/* Filter by group */
> +				if( ! c_groups_lists[i] ) {
> +					/* Now we're loading up a container's groups */
> +					c_groups_lists[i] = get_config_list(c, "lxc.group");
> +				}
> +
> +				ret = list_contains_entry(cmd_group->elem, c_groups_lists[i]);
> +
> +				if ( ret == 0 ) {
> +					/* 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) {
> +				/* Shutdown the container */
> +				if (c->is_running(c)) {
> +					if (my_args.list)
> +						printf("%s\n", c->name);
> +					else {
> +						if (!c->shutdown(c, my_args.timeout)) {
> +							if (!c->stop(c)) {
> +								fprintf(stderr, "Error shutting down container: %s\n", c->name);
> +							}
>  						}
>  					}
>  				}
>  			}
> -		}
> -		else if (my_args.hardstop) {
> -			/* Kill the container */
> -			if (c->is_running(c)) {
> -				if (my_args.list)
> -					printf("%s\n", c->name);
> -				else {
> -					if (!c->stop(c))
> -						fprintf(stderr, "Error killing container: %s\n", c->name);
> +			else if (my_args.hardstop) {
> +				/* Kill the container */
> +				if (c->is_running(c)) {
> +					if (my_args.list)
> +						printf("%s\n", c->name);
> +					else {
> +						if (!c->stop(c))
> +							fprintf(stderr, "Error killing container: %s\n", c->name);
> +					}
>  				}
>  			}
> -		}
> -		else if (my_args.reboot) {
> -			/* Reboot the container */
> -			if (c->is_running(c)) {
> -				if (my_args.list)
> -					printf("%s %d\n", c->name,
> -					       get_config_integer(c, "lxc.start.delay"));
> -				else {
> -					if (!c->reboot(c))
> -						fprintf(stderr, "Error rebooting container: %s\n", c->name);
> -					else
> -						sleep(get_config_integer(c, "lxc.start.delay"));
> +			else if (my_args.reboot) {
> +				/* Reboot the container */
> +				if (c->is_running(c)) {
> +					if (my_args.list)
> +						printf("%s %d\n", c->name,
> +						       get_config_integer(c, "lxc.start.delay"));
> +					else {
> +						if (!c->reboot(c))
> +							fprintf(stderr, "Error rebooting container: %s\n", c->name);
> +						else
> +							sleep(get_config_integer(c, "lxc.start.delay"));
> +					}
>  				}
>  			}
> -		}
> -		else {
> -			/* Start the container */
> -			if (!c->is_running(c)) {
> -				if (my_args.list)
> -					printf("%s %d\n", c->name,
> -					       get_config_integer(c, "lxc.start.delay"));
> -				else {
> -					if (!c->start(c, 0, default_start_args))
> -						fprintf(stderr, "Error starting container: %s\n", c->name);
> -					else
> -						sleep(get_config_integer(c, "lxc.start.delay"));
> +			else {
> +				/* Start the container */
> +				if (!c->is_running(c)) {
> +					if (my_args.list)
> +						printf("%s %d\n", c->name,
> +						       get_config_integer(c, "lxc.start.delay"));
> +					else {
> +						if (!c->start(c, 0, default_start_args))
> +							fprintf(stderr, "Error starting container: %s\n", c->name);
> +						else
> +							sleep(get_config_integer(c, "lxc.start.delay"));
> +					}
>  				}
>  			}
> -		}
>  
> +			/*
> +			 * 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;
> +			}
> +		}
>  
> -		lxc_container_put(c);
>  	}
>  
> -	if (cmd_groups_list) {
> -		lxc_list_for_each_safe(it, cmd_groups_list, next) {
> -			lxc_list_del(it);
> -			free(it->elem);
> -			free(it);
> +	/* 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]);
>  		}
> -		free(cmd_groups_list);
>  	}
>  
> +	if ( c_groups_lists )
> +		free(c_groups_lists);
> +
> +	free(cmd_groups_list);
>  	free(containers);
>  
>  	return 0;
> -- 
> 1.9.0
> 
> 

-- 
Michael H. Warfield (AI4NB) | (770) 978-7061 |  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/20140516/5c1aa78d/attachment.sig>


More information about the lxc-devel mailing list