[lxc-devel] [PATCH 1/6] Move lxcbr0 setup logic into lxc.net script

[Serge Hallyn]

Quoting Michael H. Warfield (mhw at WittsEnd.com):
> On Mon, 2014-08-11 at 15:37 +0000, Serge Hallyn wrote:
> > Quoting Michael H. Warfield (mhw at WittsEnd.com):
> > > On Thu, 2014-07-31 at 08:53 +0200, Martin Pitt wrote:
> > > > Factor this out of the lxc-net.conf upstart job, so that it can be used by
> > > > init.d scripts and systemd units, too.
> > > 
> > > Crap.  Never fails.  I was in Europe when this came out.
> > > 
> > > Looking at lxc.net, I would say it's going to break some existing setups
> > > (notably mine) where lxcbr0 is already setup.  Yes, we can set
> > > LXC_BRIDGE to no but we should also include some autodetect logic such
> > > that, if lxcbr0 already exists, this doesn't commit random acts of
> > > terrorism.
> 
> > That logic should already be there.  If /sys/class/net/lxcbr0 already
> > exists, then start will do nothing;  if /run/lxc/network_up does not
> > exist then stop will do nothing.
> 
> Yeah, I traced through that logic and finally realized that.  I'm a
> little nudgey about even calling "stop" from "start" in the case where
> the bridge already exists or when USE_LXC_BRIDGE is not true and then
> relying on "stop" to rely on the non-existence of network_up to exit
> without doing anything to the bridge.  If that's the case, why even call
> "stop" in the "start" function at all?  That had me confused.

Yeah, that's an upstart-ism which doesn't belong in the script (and
must continue to be done in upstart)

> I don't see the need for this logic at all in "start":
> 
> -- 
>         if [ -d /sys/class/net/${LXC_BRIDGE} ]; then
>                 if [ ! -f ${varrun}/network_up ]; then
>                         # bridge exists, but we didn't start it
>                         stop;
>                 fi
>                 exit 0;
>         fi
> -- 
> 
> If this bridge exists and we didn't start it, why even attempt to stop

We don't - the stop is to tell upstart not to continue running the
job.  It doesn't belong in this script.

-serge