One of the current flaws in the LXC driver is that when doing the better- chroot-than-chroot mode setup, /dev/pts leaks the parent OS TTYs into the container. As of 2.6.29 it is possible to create new instances of /dev/pts by passing the 'newinstance' flag to mount. In this way /dev/pts inside the container will be totally independant of the parent OS. This is a kind of fun / tricky thing to get right, because the monitor process (libvirt_lxc) actually needs 2 ttys, one in the host's /dev/pts (which virsh console connects to), and the other in the container's /dev/pts (which acts as its stdin/out/err). This means that libvirt_lxc has to mount the new devpts instance on the containers root filesystem before starting the container itself. To avoid this then appearing in the host OS, we make libvirt_lxc call unshare(CLONE_NEWNS). This also has the nice advantage of adding a guarentee that this new devpts instance is cleaned up with libvirt_lxc exits. Now in the container startup sequence, instead of mounting a devpts instance, it just moves the instance that libvirt_lxc previously setup into its desired location. I have arranged this so that if the host OS kernel lacks support for the 'newinstance' flag with devpts, it'll just use a regular shared instance. If this happens, then /var/log/libvirt/lxc/$NAME.log should warn you 16:55:00.454: warning : kernel does not support private devpts, using shared devpts Also the previous patch to pivot_root() setup added a chroot() call. For some wierd reason, with this all the host OS's mounts are still visible in the container's /proc/mounts, even though they are not actually accessible (since we pivot'd onto a new root) This change seemed to fix that problem with no ill-effects. - if (chroot(oldroot) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to chroot into tmpfs")); - goto err; - } - - if (chdir("/new") < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to chdir into /new on tmpfs")); + if (chdir(newroot) < 0) { + virReportSystemError(NULL, errno, + _("failed to chroot into %s"), newroot); goto err; } There was also a call added to umount the .oldroot location, but this was being done too early, because later methods still needed to access various bits under /.oldroot - eg to mount other (non-root) filesystems from the host OS in the container. So I'm removing this chunk: if (chdir("/") < 0) goto err; - if (umount2(".oldroot", MNT_DETACH) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to lazily unmount old root")); - goto err; - } - This showed up a different problem though. Our call later on which would unmount /.oldroot was failing because there were some open file descriptors on /var/ still. This was because we didn't setup stdin/out until after we'd dealt with all the mounts. So my patch also makes sure that the first thing we do is close all open file descriptors and move stdin/out/err immdiately onto the TTY allocated for this container. That ensures all the mounts are able to be released. I've tested this patch on 2.6.27 (lacking 'newinstance') and on 2.6.29 (with 'newinstance' for devpts). It seemed to work correctly in both cases, but would appreciate someone else confirming.... domain_conf.c | 15 ++++ domain_conf.h | 2 libvirt_private.syms | 1 lxc_container.c | 175 +++++++++++++++++++++++++++++++-------------------- lxc_container.h | 1 lxc_controller.c | 87 +++++++++++++++++++++++-- util.c | 22 ++++-- util.h | 4 + 8 files changed, 230 insertions(+), 77 deletions(-) Daniel Index: src/domain_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/domain_conf.c,v retrieving revision 1.75 diff -u -p -r1.75 domain_conf.c --- src/domain_conf.c 3 Apr 2009 14:10:17 -0000 1.75 +++ src/domain_conf.c 15 Apr 2009 15:44:52 -0000 @@ -3855,6 +3857,21 @@ const char *virDomainDefDefaultEmulator( return emulator; } +virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def) +{ + int i; + + for (i = 0 ; i < def->nfss ; i++) { + if (def->fss[i]->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + + if (STREQ(def->fss[i]->dst, "/")) + return def->fss[i]; + } + + return NULL; +} + void virDomainObjLock(virDomainObjPtr obj) { Index: src/domain_conf.h =================================================================== RCS file: /data/cvs/libvirt/src/domain_conf.h,v retrieving revision 1.40 diff -u -p -r1.40 domain_conf.h --- src/domain_conf.h 3 Mar 2009 16:53:13 -0000 1.40 +++ src/domain_conf.h 15 Apr 2009 15:44:52 -0000 @@ -635,6 +635,8 @@ const char *virDomainDefDefaultEmulator( virDomainDefPtr def, virCapsPtr caps); +virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def); + void virDomainObjLock(virDomainObjPtr obj); void virDomainObjUnlock(virDomainObjPtr obj); Index: src/libvirt_private.syms =================================================================== RCS file: /data/cvs/libvirt/src/libvirt_private.syms,v retrieving revision 1.28 diff -u -p -r1.28 libvirt_private.syms --- src/libvirt_private.syms 3 Apr 2009 10:55:51 -0000 1.28 +++ src/libvirt_private.syms 15 Apr 2009 15:44:52 -0000 @@ -78,6 +78,7 @@ virDomainDiskQSort; virDomainFindByID; virDomainFindByName; virDomainFindByUUID; +virDomainGetRootFilesystem; virDomainGraphicsTypeFromString; virDomainGraphicsDefFree; virDomainInputDefFree; Index: src/lxc_container.c =================================================================== RCS file: /data/cvs/libvirt/src/lxc_container.c,v retrieving revision 1.23 diff -u -p -r1.23 lxc_container.c --- src/lxc_container.c 14 Apr 2009 17:51:12 -0000 1.23 +++ src/lxc_container.c 15 Apr 2009 15:44:52 -0000 @@ -306,13 +306,13 @@ static int lxcContainerPivotRoot(virDoma /* Create a tmpfs root since old and new roots must be * on separate filesystems */ - if (mount("", oldroot, "tmpfs", 0, NULL) < 0) { + if (mount("tmprootfs", oldroot, "tmpfs", 0, NULL) < 0) { virReportSystemError(NULL, errno, _("failed to mount empty tmpfs at %s"), oldroot); goto err; } - + /* Create a directory called 'new' in tmpfs */ if (virAsprintf(&newroot, "%s/new", oldroot) < 0) { virReportOOMError(NULL); @@ -336,15 +336,9 @@ static int lxcContainerPivotRoot(virDoma /* Now we chroot into the tmpfs, then pivot into the * root->src bind-mounted onto '/new' */ - if (chroot(oldroot) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to chroot into tmpfs")); - goto err; - } - - if (chdir("/new") < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to chdir into /new on tmpfs")); + if (chdir(newroot) < 0) { + virReportSystemError(NULL, errno, + _("failed to chroot into %s"), newroot); goto err; } @@ -360,12 +354,6 @@ static int lxcContainerPivotRoot(virDoma if (chdir("/") < 0) goto err; - if (umount2(".oldroot", MNT_DETACH) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to lazily unmount old root")); - goto err; - } - VIR_FREE(oldroot); VIR_FREE(newroot); @@ -377,50 +365,77 @@ err: return -1; } -static int lxcContainerPopulateDevices(void) + +static int lxcContainerMountBasicFS(virDomainFSDefPtr root) { - int i; - int rc; const struct { - int maj; - int min; - mode_t mode; - const char *path; - } devs[] = { - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL, 0666, "/dev/null" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO, 0666, "/dev/zero" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL, 0666, "/dev/full" }, - { LXC_DEV_MAJ_TTY, LXC_DEV_MIN_CONSOLE, 0600, "/dev/console" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM, 0666, "/dev/random" }, - { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM, 0666, "/dev/urandom" }, + const char *src; + const char *dst; + const char *type; + } mnts[] = { + { "/dev", "/dev", "tmpfs" }, + { "/proc", "/proc", "proc" }, + { "/sys", "/sys", "sysfs" }, +#if WITH_SELINUX + { "none", "/selinux", "selinuxfs" }, +#endif }; + int i, rc; + char *devpts; - if ((rc = virFileMakePath("/dev")) < 0) { - virReportSystemError(NULL, rc, "%s", - _("cannot create /dev/")); + if (virAsprintf(&devpts, "/.oldroot%s/dev/pts", root->src) < 0) { + virReportOOMError(NULL); return -1; } - if (mount("none", "/dev", "tmpfs", 0, NULL) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to mount /dev tmpfs")); - return -1; + + for (i = 0 ; i < ARRAY_CARDINALITY(mnts) ; i++) { + if (virFileMakePath(mnts[i].dst) < 0) { + virReportSystemError(NULL, errno, + _("failed to mkdir %s"), + mnts[i].src); + return -1; + } + if (mount(mnts[i].src, mnts[i].dst, mnts[i].type, 0, NULL) < 0) { + virReportSystemError(NULL, errno, + _("failed to mount %s on %s"), + mnts[i].type, mnts[i].type); + return -1; + } } - /* Move old devpts into container, since we have to - connect to the master ptmx which was opened in - the parent. - XXX This sucks, we need to figure out how to get our - own private devpts for isolation - */ + if ((rc = virFileMakePath("/dev/pts") < 0)) { virReportSystemError(NULL, rc, "%s", _("cannot create /dev/pts")); return -1; } - if (mount("devpts", "/dev/pts", "devpts", 0, NULL) < 0) { + + VIR_DEBUG("Trying to move %s to %s", devpts, "/dev/pts"); + if ((rc = mount(devpts, "/dev/pts", NULL, MS_MOVE, NULL)) < 0) { virReportSystemError(NULL, errno, "%s", _("failed to mount /dev/pts in container")); return -1; } + VIR_FREE(devpts); + + return 0; +} + +static int lxcContainerPopulateDevices(void) +{ + int i; + const struct { + int maj; + int min; + mode_t mode; + const char *path; + } devs[] = { + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_NULL, 0666, "/dev/null" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_ZERO, 0666, "/dev/zero" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_FULL, 0666, "/dev/full" }, + { LXC_DEV_MAJ_TTY, LXC_DEV_MIN_CONSOLE, 0600, "/dev/console" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_RANDOM, 0666, "/dev/random" }, + { LXC_DEV_MAJ_MEMORY, LXC_DEV_MIN_URANDOM, 0666, "/dev/urandom" }, + }; /* Populate /dev/ with a few important bits */ for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { @@ -434,6 +449,23 @@ static int lxcContainerPopulateDevices(v } } + if (access("/dev/pts/ptmx", W_OK) == 0) { + if (symlink("/dev/pts/ptmx", "/dev/ptmx") < 0) { + virReportSystemError(NULL, errno, "%s", + _("failed to create symlink /dev/ptmx to /dev/pts/ptmx")); + return -1; + } + } else { + dev_t dev = makedev(LXC_DEV_MAJ_TTY, LXC_DEV_MIN_PTMX); + if (mknod("/dev/ptmx", 0, dev) < 0 || + chmod("/dev/ptmx", 0666)) { + virReportSystemError(NULL, errno, "%s", + _("failed to make device /dev/ptmx")); + return -1; + } + } + + return 0; } @@ -493,6 +525,7 @@ static int lxcContainerUnmountOldFS(void return -1; } while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) { + VIR_DEBUG("Got %s", mntent.mnt_dir); if (!STRPREFIX(mntent.mnt_dir, "/.oldroot")) continue; @@ -513,6 +546,7 @@ static int lxcContainerUnmountOldFS(void lxcContainerChildMountSort); for (i = 0 ; i < nmounts ; i++) { + VIR_DEBUG("Umount %s", mounts[i]); if (umount(mounts[i]) < 0) { virReportSystemError(NULL, errno, _("failed to unmount '%s'"), @@ -534,22 +568,23 @@ static int lxcContainerUnmountOldFS(void static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, virDomainFSDefPtr root) { + /* Gives us a private root, leaving all parent OS mounts on /.oldroot */ if (lxcContainerPivotRoot(root) < 0) return -1; - if (virFileMakePath("/proc") < 0 || - mount("none", "/proc", "proc", 0, NULL) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to mount /proc")); + /* Mounts the core /proc, /sys, /dev, /dev/pts filesystems */ + if (lxcContainerMountBasicFS(root) < 0) return -1; - } + /* Populates device nodes in /dev/ */ if (lxcContainerPopulateDevices() < 0) return -1; + /* Sets up any non-root mounts from guest config */ if (lxcContainerMountNewFS(vmDef) < 0) return -1; + /* Gets rid of all remaining mounts from host OS, including /.oldroot itself */ if (lxcContainerUnmountOldFS() < 0) return -1; @@ -595,18 +630,9 @@ static int lxcContainerSetupExtraMounts( return 0; } -static int lxcContainerSetupMounts(virDomainDefPtr vmDef) +static int lxcContainerSetupMounts(virDomainDefPtr vmDef, + virDomainFSDefPtr root) { - int i; - virDomainFSDefPtr root = NULL; - - for (i = 0 ; i < vmDef->nfss ; i++) { - if (vmDef->fss[i]->type != VIR_DOMAIN_FS_TYPE_MOUNT) - continue; - if (STREQ(vmDef->fss[i]->dst, "/")) - root = vmDef->fss[i]; - } - if (root) return lxcContainerSetupPivotRoot(vmDef, root); else @@ -630,6 +656,8 @@ static int lxcContainerChild( void *data lxc_child_argv_t *argv = data; virDomainDefPtr vmDef = argv->config; int ttyfd; + char *ttyPath; + virDomainFSDefPtr root; if (NULL == vmDef) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -637,16 +665,28 @@ static int lxcContainerChild( void *data return -1; } - if (lxcContainerSetupMounts(vmDef) < 0) - return -1; + root = virDomainGetRootFilesystem(vmDef); - ttyfd = open(argv->ttyPath, O_RDWR|O_NOCTTY); + if (root) { + if (virAsprintf(&ttyPath, "%s%s", root->src, argv->ttyPath) < 0) { + virReportOOMError(NULL); + return -1; + } + } else { + if (!(ttyPath = strdup(argv->ttyPath))) { + virReportOOMError(NULL); + return -1; + } + } + + ttyfd = open(ttyPath, O_RDWR|O_NOCTTY); if (ttyfd < 0) { virReportSystemError(NULL, errno, - _("failed to open %s"), - argv->ttyPath); + _("failed to open tty %s"), + ttyPath); return -1; } + VIR_FREE(ttyPath); if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) { close(ttyfd); @@ -654,6 +694,9 @@ static int lxcContainerChild( void *data } close(ttyfd); + if (lxcContainerSetupMounts(vmDef, root) < 0) + return -1; + /* Wait for interface devices to show up */ if (lxcContainerWaitForContinue(argv->monitor) < 0) return -1; Index: src/lxc_container.h =================================================================== RCS file: /data/cvs/libvirt/src/lxc_container.h,v retrieving revision 1.9 diff -u -p -r1.9 lxc_container.h --- src/lxc_container.h 21 Oct 2008 16:46:47 -0000 1.9 +++ src/lxc_container.h 15 Apr 2009 15:44:52 -0000 @@ -39,6 +39,7 @@ enum { #define LXC_DEV_MAJ_TTY 5 #define LXC_DEV_MIN_CONSOLE 1 +#define LXC_DEV_MIN_PTMX 2 #define LXC_DEV_MAJ_PTY 136 Index: src/lxc_controller.c =================================================================== RCS file: /data/cvs/libvirt/src/lxc_controller.c,v retrieving revision 1.15 diff -u -p -r1.15 lxc_controller.c --- src/lxc_controller.c 3 Feb 2009 13:09:00 -0000 1.15 +++ src/lxc_controller.c 15 Apr 2009 15:44:52 -0000 @@ -33,6 +33,7 @@ #include <fcntl.h> #include <signal.h> #include <getopt.h> +#include <sys/mount.h> #include "virterror_internal.h" #include "logging.h" @@ -440,6 +441,9 @@ lxcControllerRun(virDomainDefPtr def, int containerPty; char *containerPtyPath; pid_t container = -1; + virDomainFSDefPtr root; + char *devpts = NULL; + char *devptmx = NULL; if (socketpair(PF_UNIX, SOCK_STREAM, 0, control) < 0) { virReportSystemError(NULL, errno, "%s", @@ -447,14 +451,83 @@ lxcControllerRun(virDomainDefPtr def, goto cleanup; } - if (virFileOpenTty(&containerPty, - &containerPtyPath, - 0) < 0) { - virReportSystemError(NULL, errno, "%s", - _("failed to allocate tty")); - goto cleanup; + root = virDomainGetRootFilesystem(def); + + /* + * If doing a chroot style setup, we need to prepare + * a private /dev/pts for the child now, which they + * will later move into position. + * + * This is complex because 'virsh console' needs to + * use /dev/pts from the host OS, and the guest OS + * needs to use /dev/pts from the guest. + * + * This means that we (libvirt_lxc) need to see and + * use both /dev/pts instances. We're running in the + * host OS context though and don't want to expose + * the guest OS /dev/pts there. + * + * Thus we call unshare(CLONE_NS) so that we can see + * the guest's new /dev/pts, without it becoming + * visible to the host OS. + */ + if (root) { + VIR_DEBUG0("Setting up private /dev/pts"); + if (unshare(CLONE_NEWNS) < 0) { + virReportSystemError(NULL, errno, "%s", + _("cannot unshare mount namespace")); + goto cleanup; + } + + if (virAsprintf(&devpts, "%s/dev/pts", root->src) < 0 || + virAsprintf(&devptmx, "%s/dev/pts/ptmx", root->src) < 0) { + virReportOOMError(NULL); + goto cleanup; + } + + if (virFileMakePath(devpts) < 0) { + virReportSystemError(NULL, errno, + _("failed to make path %s"), + devpts); + goto cleanup; + } + + VIR_DEBUG("Mouting 'devpts' on %s", devpts); + if (mount("devpts", devpts, "devpts", 0, "newinstance,ptmxmode=0666") < 0) { + virReportSystemError(NULL, errno, + _("failed to mount devpts on %s"), + devpts); + goto cleanup; + } + + if (access(devptmx, R_OK) < 0) { + VIR_WARN0("kernel does not support private devpts, using shared devpts"); + VIR_FREE(devptmx); + } } + if (devptmx) { + VIR_DEBUG("Opening tty on private %s", devptmx); + if (virFileOpenTtyAt(devptmx, + &containerPty, + &containerPtyPath, + 0) < 0) { + virReportSystemError(NULL, errno, "%s", + _("failed to allocate tty")); + goto cleanup; + } + } else { + VIR_DEBUG0("Opening tty on shared /dev/ptmx"); + if (virFileOpenTty(&containerPty, + &containerPtyPath, + 0) < 0) { + virReportSystemError(NULL, errno, "%s", + _("failed to allocate tty")); + goto cleanup; + } + } + + if (lxcSetContainerResources(def) < 0) goto cleanup; @@ -476,6 +549,8 @@ lxcControllerRun(virDomainDefPtr def, rc = lxcControllerMain(monitor, client, appPty, containerPty); cleanup: + VIR_FREE(devptmx); + VIR_FREE(devpts); if (control[0] != -1) close(control[0]); if (control[1] != -1) Index: src/util.c =================================================================== RCS file: /data/cvs/libvirt/src/util.c,v retrieving revision 1.99 diff -u -p -r1.99 util.c --- src/util.c 2 Apr 2009 18:42:33 -0000 1.99 +++ src/util.c 15 Apr 2009 15:44:52 -0000 @@ -1050,14 +1050,25 @@ int virFileBuildPath(const char *dir, } -#ifdef __linux__ int virFileOpenTty(int *ttymaster, char **ttyName, int rawmode) { + return virFileOpenTtyAt("/dev/ptmx", + ttymaster, + ttyName, + rawmode); +} + +#ifdef __linux__ +int virFileOpenTtyAt(const char *ptmx, + int *ttymaster, + char **ttyName, + int rawmode) +{ int rc = -1; - if ((*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0) + if ((*ttymaster = open(ptmx, O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0) goto cleanup; if (unlockpt(*ttymaster) < 0) @@ -1100,9 +1111,10 @@ cleanup: } #else -int virFileOpenTty(int *ttymaster ATTRIBUTE_UNUSED, - char **ttyName ATTRIBUTE_UNUSED, - int rawmode ATTRIBUTE_UNUSED) +int virFileOpenTtyAt(const char *ptmx ATTRIBUTE_UNUSED, + int *ttymaster ATTRIBUTE_UNUSED, + char **ttyName ATTRIBUTE_UNUSED, + int rawmode ATTRIBUTE_UNUSED) { return -1; } Index: src/util.h =================================================================== RCS file: /data/cvs/libvirt/src/util.h,v retrieving revision 1.46 diff -u -p -r1.46 util.h --- src/util.h 1 Apr 2009 10:26:22 -0000 1.46 +++ src/util.h 15 Apr 2009 15:44:52 -0000 @@ -103,6 +103,10 @@ int virFileBuildPath(const char *dir, int virFileOpenTty(int *ttymaster, char **ttyName, int rawmode); +int virFileOpenTtyAt(const char *ptmx, + int *ttymaster, + char **ttyName, + int rawmode); char* virFilePid(const char *dir, const char *name); -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|