[lxc-devel] [PATCH RFC] pivot_root: switch to a new mechanism

Serge Hallyn serge.hallyn at ubuntu.com
Fri Sep 19 22:21:49 UTC 2014


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.

> > 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



More information about the lxc-devel mailing list