[lxc-devel] Patch: pivot_root, new version
Daniel Lezcano
daniel.lezcano at free.fr
Wed Jan 6 22:05:18 UTC 2010
Michael Holzt wrote:
> Hi Cedric!
>
> On Mi, 06 Jan 2010, Cedric Le Goater wrote:
>
>
>> 2 minor comments on allocations.
>>
>
> Thanks for pointing this out. I have done some minor work on the patch
> anyway, so a new version is attached.
>
Thanks Michael,
here some questions / comments :
> diff -Naur lxc-0.6.4/src/lxc/conf.c lxc-0.6.4-pivot/src/lxc/conf.c
> --- lxc-0.6.4/src/lxc/conf.c 2009-11-24 09:47:26.000000000 +0100
> +++ lxc-0.6.4-pivot/src/lxc/conf.c 2010-01-06 19:35:48.000000000 +0100
> @@ -65,6 +65,8 @@
> #define MS_REC 16384
> #endif
>
> +extern int pivot_root(const char * new_root, const char * put_old);
> +
> typedef int (*instanciate_cb)(struct lxc_netdev *);
>
> struct mount_opt {
> @@ -334,7 +336,168 @@
> return 0;
> }
>
> -static int setup_rootfs(const char *rootfs)
> +static int setup_rootfs_pivot_root_cb(void *buffer, void *data)
> +{
> + struct lxc_list *mountlist, *listentry, *iterator;
> + char *pivotdir, *mountpoint, *mountentry;
> + int found;
> + void **cbparm;
> +
> + mountentry = buffer;
> + cbparm = (void **)data;
> +
> + mountlist = cbparm[0];
> + pivotdir = cbparm[1];
> +
> + /* parse entry, first field is mountname, ignore */
> + mountpoint = strtok(mountentry, " ");
> + if (!mountpoint)
> + return -1;
> +
> + /* second field is mountpoint */
> + mountpoint = strtok(NULL, " ");
> + 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");
> + return -1
>
";" is missing
> + }
> + lxc_list_add_tail(mountlist, listentry);
> +
> + return 0;
> +}
> +
> +
> +static int setup_rootfs_pivot_root(const char *rootfs, const char *pivotdir)
> +{
> + char path[MAXPATHLEN], buffer[MAXPATHLEN];
> + void *cbparm[2];
> + struct lxc_list mountlist, *iterator;
> + int ok, still_mounted, last_still_mounted;
> +
> + /* pivot_root into our new root fs */
> + snprintf(path, sizeof(path), ".%s", pivotdir);
> +
> + if (chdir(rootfs)) {
> + SYSERROR("can't chroot to new rootfs '%s'", rootfs);
> + return -1;
> + }
> +
> + if (pivot_root(".", path)) {
> + SYSERROR("pivot_root syscall failed");
> + return -1;
> + }
> +
> + if (chdir("/")) {
> + SYSERROR("can't chroot to / after pivot_root");
> + return -1;
> + }
> +
> + DEBUG("pivot_root syscall successful");
> +
> + /* read and parse /proc/mounts in old root fs */
> + lxc_list_init(&mountlist);
> +
> + snprintf(path, sizeof(path), "%s/", pivotdir);
> + cbparm[0] = &mountlist;
> + cbparm[1] = strdup(path);
> +
> + if (!cbparm[1]) {
> + SYSERROR("strdup failed");
> + return -1;
> + }
> +
> + snprintf(path, sizeof(path), "/%s/proc/mounts", pivotdir);
> + ok = lxc_file_for_each_line(path,
> + setup_rootfs_pivot_root_cb,
> + buffer, sizeof(buffer), &cbparm);
> + if (ok < 0) {
> + SYSERROR("failed to read or parse mount list '%s'", path);
> + return -1;
> + }
>
Why the setup_rootfs_pivot_root_cb does not directly umount instead of
creating a list and then browse the list to umount ?
> +
> + /* 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(iterator, &mountlist) {
> +
> + if (!umount(iterator->elem)) {
> + DEBUG("umounted '%s'", (char *)iterator->elem);
> + lxc_list_del(iterator);
> + continue;
> + }
> +
> + if (errno != EBUSY) {
> + SYSERROR("failed to umount '%s'", (char *)iterator->elem);
> + return -1;
> + }
> +
> + still_mounted++;
> + }
> + } while (still_mounted > 0 && still_mounted != last_still_mounted);
> +
> + if (still_mounted) {
> + ERROR("could not umount %d mounts", still_mounted);
> + return -1;
> + }
> +
> + /* umount old root fs */
> + if (umount(pivotdir)) {
> + SYSERROR("could not unmount old rootfs");
> + return -1;
> + }
> + DEBUG("umounted '%s'", pivotdir);
> +
> +
> + INFO("pivoted to '%s'", rootfs);
> + return 0;
> +}
> +
> +static int setup_rootfs_chroot(const char *rootfs)
> +{
> + if (chroot(rootfs)) {
> + SYSERROR("failed to set chroot %s", rootfs);
> + return -1;
> + }
> +
> + if (chdir(getenv("HOME")) && chdir("/")) {
> + SYSERROR("failed to change to home directory");
> + return -1;
> + }
> +
> + INFO("chrooted to '%s'", rootfs);
> + return 0;
> +}
> +
> +
> +static int setup_rootfs(const char *rootfs, const char *pivotdir)
> {
> char *tmpname;
> int ret = -1;
> @@ -358,18 +521,16 @@
> goto out;
> }
>
> - if (chroot(tmpname)) {
> - SYSERROR("failed to set chroot %s", tmpname);
> - goto out;
> + if (pivotdir && setup_rootfs_pivot_root(tmpname, pivotdir)) {
> + ERROR("failed to pivot_root to '%s' (put_old: '%s')", rootfs, pivotdir);
> + goto out;
> }
>
> - if (chdir(getenv("HOME")) && chdir("/")) {
> - SYSERROR("failed to change to home directory");
> + if (!pivotdir && setup_rootfs_chroot(tmpname)) {
> + ERROR("failed to chroot to '%s'", rootfs);
> goto out;
> }
> -
> - INFO("chrooted to '%s'", rootfs);
> -
> +
> ret = 0;
> out:
> rmdir(tmpname);
> @@ -816,6 +977,7 @@
> int lxc_conf_init(struct lxc_conf *conf)
> {
> conf->rootfs = NULL;
> + conf->pivotdir = NULL;
>
I think you convinced with your demonstration the chroot is insane for a
container, so IMO we can consider using always the pivot_root and
definitively get rid of the chroot only. Is it possible to not add the
pivotdir configuration variable and keep this internally by creating in
the /tmp/lxc-rootfs the pivotdir to be used ?
That will simplify the code a lot and will avoid the user/admin to be
puzzled with an extra configuration variable path, no ?
Thanks !
More information about the lxc-devel
mailing list