[lxc-devel] [PATCH RFC] pivot_root: switch to a new mechanism
Stéphane Graber
stgraber at ubuntu.com
Sat Sep 20 01:36:28 UTC 2014
On Sat, Sep 20, 2014 at 01:10:04AM +0000, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgraber at ubuntu.com):
> > On Fri, Sep 19, 2014 at 10:21:49PM +0000, Serge Hallyn wrote:
> > > Quoting Stéphane Graber (stgraber at ubuntu.com):
> > > > On Fri, Sep 19, 2014 at 05:07:17PM +0000, Serge Hallyn wrote:
> > > > > This idea came from Andy Lutomirski. Instead of using a temporary
> > > > > directory for the pivot_root put-old, use "." both for new-root and
> > > > > old-root. Then fchdir into the old root temporarily in order to unmount
> > > > > the old-root, and finally chdir back into our '/'.
> > > > >
> > > > > This patch doesn't yet do it, but we can drop the lxc.pivotdir with this
> > > > > approach altogether.
> > >
> > > Should have mentioned this should be far better behavior with read-only
> > > rootfs.
> > >
> > > > Might as well do it then, I'd suggest it be dropped from default
> > > > configs, documentation and the config struct, however keep it valid in
> > > > the parser showing a WARNING or INFO when detected so that we don't
> > > > break old containers but point it out so they can drop it from their
> > > > config.
> > >
> > > Actually that's a good point - but in that case we may be better off
> > > simply mentioning that it is deprecated in the manpages, but not changing
> > > any of the rest. In particular, if we clone a container or update the
> > > pivotdir using API, and then migrate the container to a host with older
> > > lxc version, we wnat the lxc.pivotdir to still be there.
> >
> > If only everyone was using the new configs using lxc.include that
> > wouldn't be a problem at all :)
> >
> > So yeah, I guess we want:
> > - Drop from any documentation we have in master (I prefer that to
> > marking as deprecated since people may be parsing those somehow to build
> > a list of valid keys, also deprecated has different meanings for
> > different people, in this case it is "doesn't do anything at all").
>
> Isn't that going to confuse someone who comes new to lxc and sees the
> entry in a lxc.conf?
Well, if they have it in their config, they'll get the warning at
container startup time.
Newcomers are also likely to be using the containers which LXC itself
creates and most of those use lxc.include nowadays so they shouldn't see
any pivotdir in there.
>
> > - Have the parser issue a WARNING if it sees it, making it clear that
> > it is entirely ignored and that this WARNING will turn into a parsing
> > error in LXC 2.0. Possibly mention that the only use case for keeping
> > it set is if sharing containers with older hosts.
>
> Can do.
>
> > - Otherwise keep things as they are today, so store it in lxc_conf and
> > spit it back at write_config so we don't loose it when copying
> > containers.
> >
> >
> > Note that if this does end up working as well as we think, I'd consider
> > changing stable to using the pivot_root(".", ".") as the default when
> > pivotdir isn't set as a bugfix for read-only containers.
> >
> > >
> > > > > Only lightly tested. This might have subtle unforeseen side-effects
> > > > > hence this is only RFC.
> > > > >
> > > > > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> > > > > ---
> > > > > src/lxc/conf.c | 211 +++++++++++----------------------------------------------
> > > > > 1 file changed, 39 insertions(+), 172 deletions(-)
> > > > >
> > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > > > index 5e61c35..e136185 100644
> > > > > --- a/src/lxc/conf.c
> > > > > +++ b/src/lxc/conf.c
> > > > > @@ -1025,199 +1025,66 @@ static int setup_tty(const struct lxc_rootfs *rootfs,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > -static int setup_rootfs_pivot_root_cb(char *buffer, void *data)
> > > > > -{
> > > > > - struct lxc_list *mountlist, *listentry, *iterator;
> > > > > - char *pivotdir, *mountpoint, *mountentry, *saveptr = NULL;
> > > > > - int found;
> > > > > - void **cbparm;
> > > > > -
> > > > > - mountentry = buffer;
> > > > > - cbparm = (void **)data;
> > > > > -
> > > > > - mountlist = cbparm[0];
> > > > > - pivotdir = cbparm[1];
> > > > > -
> > > > > - /* parse entry, first field is mountname, ignore */
> > > > > - mountpoint = strtok_r(mountentry, " ", &saveptr);
> > > > > - if (!mountpoint)
> > > > > - return -1;
> > > > > -
> > > > > - /* second field is mountpoint */
> > > > > - mountpoint = strtok_r(NULL, " ", &saveptr);
> > > > > - if (!mountpoint)
> > > > > - return -1;
> > > > > -
> > > > > - /* only consider mountpoints below old root fs */
> > > > > - if (strncmp(mountpoint, pivotdir, strlen(pivotdir)))
> > > > > - return 0;
> > > > > -
> > > > > - /* filter duplicate mountpoints */
> > > > > - found = 0;
> > > > > - lxc_list_for_each(iterator, mountlist) {
> > > > > - if (!strcmp(iterator->elem, mountpoint)) {
> > > > > - found = 1;
> > > > > - break;
> > > > > - }
> > > > > - }
> > > > > - if (found)
> > > > > - return 0;
> > > > > -
> > > > > - /* add entry to list */
> > > > > - listentry = malloc(sizeof(*listentry));
> > > > > - if (!listentry) {
> > > > > - SYSERROR("malloc for mountpoint listentry failed");
> > > > > - return -1;
> > > > > - }
> > > > >
> > > > > - listentry->elem = strdup(mountpoint);
> > > > > - if (!listentry->elem) {
> > > > > - SYSERROR("strdup failed");
> > > > > - free(listentry);
> > > > > - return -1;
> > > > > - }
> > > > > - lxc_list_add_tail(mountlist, listentry);
> > > > > -
> > > > > - return 0;
> > > > > -}
> > > > > -
> > > > > -static int umount_oldrootfs(const char *oldrootfs)
> > > > > +static int setup_rootfs_pivot_root(const char *rootfs, const char *pivotdir)
> > > > > {
> > > > > - char path[MAXPATHLEN];
> > > > > - void *cbparm[2];
> > > > > - struct lxc_list mountlist, *iterator, *next;
> > > > > - int ok, still_mounted, last_still_mounted;
> > > > > - int rc;
> > > > > + int oldroot = -1, newroot = -1;
> > > > >
> > > > > - /* read and parse /proc/mounts in old root fs */
> > > > > - lxc_list_init(&mountlist);
> > > > > -
> > > > > - /* oldrootfs is on the top tree directory now */
> > > > > - rc = snprintf(path, sizeof(path), "/%s", oldrootfs);
> > > > > - if (rc >= sizeof(path)) {
> > > > > - ERROR("rootfs name too long");
> > > > > + oldroot = open("/", O_DIRECTORY | O_RDONLY);
> > > > > + if (oldroot < 0) {
> > > > > + SYSERROR("Error opening old-/ for fchdir");
> > > > > return -1;
> > > > > }
> > > > > - cbparm[0] = &mountlist;
> > > > > -
> > > > > - cbparm[1] = strdup(path);
> > > > > - if (!cbparm[1]) {
> > > > > - SYSERROR("strdup failed");
> > > > > - return -1;
> > > > > + newroot = open(rootfs, O_DIRECTORY | O_RDONLY);
> > > > > + if (newroot < 0) {
> > > > > + SYSERROR("Error opening new-/ for fchdir");
> > > > > + goto fail;
> > > > > }
> > > > >
> > > > > - rc = snprintf(path, sizeof(path), "%s/proc/mounts", oldrootfs);
> > > > > - if (rc >= sizeof(path)) {
> > > > > - ERROR("container proc/mounts name too long");
> > > > > - return -1;
> > > > > - }
> > > > > -
> > > > > - ok = lxc_file_for_each_line(path,
> > > > > - setup_rootfs_pivot_root_cb, &cbparm);
> > > > > - if (ok < 0) {
> > > > > - SYSERROR("failed to read or parse mount list '%s'", path);
> > > > > - return -1;
> > > > > - }
> > > > > -
> > > > > - /* umount filesystems until none left or list no longer shrinks */
> > > > > - still_mounted = 0;
> > > > > - do {
> > > > > - last_still_mounted = still_mounted;
> > > > > - still_mounted = 0;
> > > > > -
> > > > > - lxc_list_for_each_safe(iterator, &mountlist, next) {
> > > > > -
> > > > > - /* umount normally */
> > > > > - if (!umount(iterator->elem)) {
> > > > > - DEBUG("umounted '%s'", (char *)iterator->elem);
> > > > > - lxc_list_del(iterator);
> > > > > - continue;
> > > > > - }
> > > > > -
> > > > > - still_mounted++;
> > > > > - }
> > > > > -
> > > > > - } while (still_mounted > 0 && still_mounted != last_still_mounted);
> > > > > -
> > > > > -
> > > > > - lxc_list_for_each(iterator, &mountlist) {
> > > > > -
> > > > > - /* let's try a lazy umount */
> > > > > - if (!umount2(iterator->elem, MNT_DETACH)) {
> > > > > - INFO("lazy unmount of '%s'", (char *)iterator->elem);
> > > > > - continue;
> > > > > - }
> > > > > -
> > > > > - /* be more brutal (nfs) */
> > > > > - if (!umount2(iterator->elem, MNT_FORCE)) {
> > > > > - INFO("forced unmount of '%s'", (char *)iterator->elem);
> > > > > - continue;
> > > > > - }
> > > > > -
> > > > > - WARN("failed to unmount '%s'", (char *)iterator->elem);
> > > > > - }
> > > > > -
> > > > > - return 0;
> > > > > -}
> > > > > -
> > > > > -static int setup_rootfs_pivot_root(const char *rootfs, const char *pivotdir)
> > > > > -{
> > > > > - char path[MAXPATHLEN];
> > > > > - int remove_pivotdir = 0;
> > > > > - int rc;
> > > > > -
> > > > > /* change into new root fs */
> > > > > - if (chdir(rootfs)) {
> > > > > + if (fchdir(newroot)) {
> > > > > SYSERROR("can't chdir to new rootfs '%s'", rootfs);
> > > > > - return -1;
> > > > > - }
> > > > > -
> > > > > - if (!pivotdir)
> > > > > - pivotdir = "lxc_putold";
> > > > > -
> > > > > - /* compute the full path to pivotdir under rootfs */
> > > > > - rc = snprintf(path, sizeof(path), "%s/%s", rootfs, pivotdir);
> > > > > - if (rc >= sizeof(path)) {
> > > > > - ERROR("pivot dir name too long");
> > > > > - return -1;
> > > > > + goto fail;
> > > > > }
> > > > >
> > > > > - if (access(path, F_OK)) {
> > > > > -
> > > > > - if (mkdir_p(path, 0755) < 0) {
> > > > > - SYSERROR("failed to create pivotdir '%s'", path);
> > > > > - return -1;
> > > > > - }
> > > > > -
> > > > > - remove_pivotdir = 1;
> > > > > - DEBUG("created '%s' directory", path);
> > > > > - }
> > > > > -
> > > > > - DEBUG("mountpoint for old rootfs is '%s'", path);
> > > > > -
> > > > > /* pivot_root into our new root fs */
> > > > > - if (pivot_root(".", path)) {
> > > > > + if (pivot_root(".", ".")) {
> > > > > SYSERROR("pivot_root syscall failed");
> > > > > - return -1;
> > > > > + goto fail;
> > > > > }
> > > > >
> > > > > - if (chdir("/")) {
> > > > > - SYSERROR("can't chdir to / after pivot_root");
> > > > > - return -1;
> > > > > + /*
> > > > > + * at this point the old-root is mounted on top of our new-root
> > > > > + * To unmounted it we must not be chdir'd into it, so escape back
> > > > > + * to old-root
> > > > > + */
> > > > > + if (fchdir(oldroot) < 0) {
> > > > > + SYSERROR("Error entering oldroot");
> > > > > + goto fail;
> > > > > + }
> > > > > + if (umount2("/", MNT_DETACH) < 0) {
> > > > > + SYSERROR("Error detaching old root");
> > > > > + goto fail;
> > > > > }
> > > > >
> > > > > - DEBUG("pivot_root syscall to '%s' successful", rootfs);
> > > > > + if (fchdir(newroot) < 0) {
> > > > > + SYSERROR("Error re-entering newroot");
> > > > > + goto fail;
> > > > > + }
> > > > >
> > > > > - /* we switch from absolute path to relative path */
> > > > > - if (umount_oldrootfs(pivotdir))
> > > > > - return -1;
> > > > > + close(oldroot);
> > > > > + close(newroot);
> > > > >
> > > > > - /* remove temporary mount point, we don't consider the removing
> > > > > - * as fatal */
> > > > > - if (remove_pivotdir && rmdir(pivotdir))
> > > > > - WARN("can't remove mountpoint '%s': %m", pivotdir);
> > > > > + DEBUG("pivot_root syscall to '%s' successful", rootfs);
> > > > >
> > > > > return 0;
> > > > > +
> > > > > +fail:
> > > > > + if (oldroot != -1)
> > > > > + close(oldroot);
> > > > > + if (newroot != -1)
> > > > > + close(newroot);
> > > > > + return -1;
> > > > > }
> > > > >
> > > > > /*
> > > > > --
> > > > > 2.1.0
> > > > >
> > > > > _______________________________________________
> > > > > 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
> > >
> > >
> > >
> > > > _______________________________________________
> > > > lxc-devel mailing list
> > > > lxc-devel at lists.linuxcontainers.org
> > > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > >
> > > _______________________________________________
> > > 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
>
>
>
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
>
> _______________________________________________
> 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/20140919/fa409822/attachment-0001.sig>
More information about the lxc-devel
mailing list