[lxc-devel] [PATCH] Issue #278: lxc-start-ephemeral: add --cdir option for cow-mounts

Stéphane Graber stgraber at ubuntu.com
Mon Dec 1 19:04:53 UTC 2014


On Thu, Nov 27, 2014 at 04:11:43AM +1100, overlay fs wrote:
> This is a copy of patch version 3 for issue #278 on the issue-tracker:
> 
> -Allow multiple bind-mounts (--bdir) and multiple cow-mounts (--cdir).
> 
> -Further fixes to permissions throughout lxc-start-ephemeral
> (annotated in the code).
> 
> -Reduce start-up time by ~5 seconds; only wait for a network ip
> address if really need it, ie if we are running a command.
> 
> 
> Signed-off by: Oleg Freedholm <overlayfs at gmail.com>

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

But I pushed with some changes:
1) Had to fix your patch by hand because your mail client messed it up
   (wrapped the long lines resulting in an invalid diff).
2) Removed all of the "edit:" lines. We already have diffs and your
   commit description, no need to add all that stuff to the code.
3) Added safeguards in case of failure of getfacl/setfacl
4) Reverted the start-up time change as that'd have caused regressions
   for sure. lxc-start-ephemeral guarantess that by the time it returns the
   container has network connectivity. People using it so far have been
   able to rely on that both for interactive work and for scripted use
   cases, let's not break that.

> 
> --- /usr/bin/lxc-start-ephemeral    2014-11-21 17:48:49.000000000 +1100
> +++ lxc-start-ephemeral 2014-11-27 00:30:42.095429007 +1100
> @@ -84,9 +84,14 @@
>  parser.add_argument("--name", "-n", type=str,
>                      help=_("name of the target container"))
> 
> -parser.add_argument("--bdir", "-b", type=str,
> +# edit: insert action="append"
> +parser.add_argument("--bdir", "-b", type=str, action="append", default=[],
>                      help=_("directory to bind mount into container"))
> 
> +# edit: add cdir
> +parser.add_argument("--cdir", "-c", type=str, action="append", default=[],
> +                    help=_("directory to cow mount into container"))
> +
>  parser.add_argument("--user", "-u", type=str,
>                      help=_("the user to run the command as"))
> 
> @@ -156,6 +161,14 @@
>  else:
>      dest_path = tempfile.mkdtemp(prefix="%s-" % args.orig, dir=lxc_path)
>  os.mkdir(os.path.join(dest_path, "rootfs"))
> +# edit: set the permissions for an ephemeral container to the default
> permissions for a non-ephemeral container, o770.
> +#     : if the permissions are not set here, then they vary greatly,
> depending upon the arguments.
> +#     : sometimes permissions are too tight, so that the
> (unprivileged) host user cannot list the container's host directory.
> +#     : in this case, lxc-start-ephemeral fails to cleanup the
> container upon termination.
> +#     :           eg lxc-start-ephemeral -o trusty
> +#     : othertimes permissions are too loose, so that every host user
> can list the container's host directory.
> +#     :           eg lxc-start-ephemeral -o trusty -n trusty_ephemeral
> +os.chmod(dest_path, 0o770)
> 
>  # Setup the new container's configuration
>  dest = lxc.Container(os.path.basename(dest_path), args.lxcpath)
> @@ -206,6 +219,16 @@
>                  # Setup an overlay for anything remaining
>                  overlay_dirs += [(fields[0], dest_mount)]
> 
> +# edit: Setup an overlay for each cow mount
> +for entry in args.cdir:
> +    if not os.path.exists(entry):
> +        print(_("Path '%s' doesn't exist, won't be cow-mounted.") %
> +              entry)
> +    else:
> +        src_path = os.path.abspath(entry)
> +        dst_path = "%s/rootfs/%s" % (dest_path, src_path)
> +        overlay_dirs += [(src_path, dst_path)]
> +
>  # Generate pre-mount script
>  with open(os.path.join(dest_path, "pre-mount"), "w+") as fd:
>      os.fchmod(fd.fileno(), 0o755)
> @@ -223,6 +246,17 @@
>          if args.storage_type == "tmpfs":
>              fd.write("mount -n -t tmpfs -o mode=0755 none %s\n" % (target))
> 
> +        # edit: attempt to fix permissions (setfacl) and optionally
> ownership (chown)
> +        #    - this is complicated, because we are inside an id_map,
> and this confuses tools such as setfacl & chown.
> +        #    - fixing permissions is essential.  Without the fix, an
> unprivileged user in the container
> +        #      cannot write to the top level of '--cdir' (though they
> can write to subdirectories of --cdir).
> +        #      setfacl seems to solve the problem.
> +        #    - fixing ownership is optional, since acl permissions
> trump ownership.
> +        #      chown behaves strangely under the id_map, so it has
> been commented out.
> +        ###fd.write("chown --no-dereference --reference=%s %s %s\n" %
> (entry[0], target, entry[1]))
> +        fd.write("getfacl -a %s | setfacl --set-file=- %s\n" %
> (entry[0], target))
> +        fd.write("getfacl -a %s | setfacl --set-file=- %s\n" %
> (entry[0], entry[1]))
> +
>          if args.union_type == "overlayfs":
>              fd.write("mount -n -t overlayfs"
>                       " -oupperdir=%s,lowerdir=%s none %s\n" % (
> @@ -242,13 +276,13 @@
>                           entry[1]))
>          count += 1
> 
> -    if args.bdir:
> -        if not os.path.exists(args.bdir):
> +    for entry in args.bdir:
> +        if not os.path.exists(entry):
>              print(_("Path '%s' doesn't exist, won't be bind-mounted.") %
> -                  args.bdir)
> +                  entry)
>          else:
> -            src_path = os.path.abspath(args.bdir)
> -            dst_path = "%s/rootfs/%s" % (dest_path, os.path.abspath(args.bdir))
> +            src_path = os.path.abspath(entry)
> +            dst_path = "%s/rootfs/%s" % (dest_path, os.path.abspath(entry))
>              fd.write("mkdir -p %s\nmount -n --bind %s %s\n" % (
>                       dst_path, src_path, dst_path))
> 
> @@ -295,7 +329,11 @@
>      sys.exit(0)
> 
>  # Try to get the IP addresses
> -ips = dest.get_ips(timeout=10)
> +# edit: Only wait for the IP address if we really need it, ie if we
> are executing a command.
> +#     : This takes ~5 seconds, perhaps because it takes that long to
> launch the network in the container.
> +ips = []         # edit: ... ensure ips is defined
> +if args.command: # edit: added if statement
> +    ips = dest.get_ips(timeout=10)
> 
>  # Deal with the case where we just print info about the container
>  if args.daemon:
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20141201/751913dc/attachment-0001.sig>


More information about the lxc-devel mailing list