[lxc-devel] [PATCH RFC] pivot_root: switch to a new mechanism
Stéphane Graber
stgraber at ubuntu.com
Fri Sep 19 22:30:20 UTC 2014
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").
- 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.
- 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
-------------- 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/e5f025fc/attachment.sig>
More information about the lxc-devel
mailing list