[libvirt] [PATCH 1/2] LXC: fix the problem that libvirt lxc fail to start on latest kernel
After kernel commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942 vfs: Lock in place mounts from more privileged users, unprivileged user has no rights to move the mounts that inherited from parent mountns. we use this feature to move the /stateDir/domain-name.{dev, devpts} to the /dev/ and /dev/pts directroy of container. this commit breaks libvirt lxc. this patch do the moving on host side, we are privileged user at this moment. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 81 +----------------------------------------------- src/lxc/lxc_controller.c | 53 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 80 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2bdf957..61283e4 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -953,76 +953,6 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED, } #endif -static int lxcContainerMountFSDev(virDomainDefPtr def, - const char *stateDir) -{ - int ret = -1; - char *path = NULL; - - VIR_DEBUG("Mount /dev/ stateDir=%s", stateDir); - - if ((ret = virAsprintf(&path, "/.oldroot/%s/%s.dev", - stateDir, def->name)) < 0) - return ret; - - if (virFileMakePath("/dev") < 0) { - virReportSystemError(errno, "%s", - _("Cannot create /dev")); - goto cleanup; - } - - VIR_DEBUG("Trying to move %s to /dev", path); - - if (mount(path, "/dev", NULL, MS_MOVE, NULL) < 0) { - virReportSystemError(errno, - _("Failed to mount %s on /dev"), - path); - goto cleanup; - } - - ret = 0; - -cleanup: - VIR_FREE(path); - return ret; -} - -static int lxcContainerMountFSDevPTS(virDomainDefPtr def, - const char *stateDir) -{ - int ret; - char *path = NULL; - - VIR_DEBUG("Mount /dev/pts stateDir=%s", stateDir); - - if ((ret = virAsprintf(&path, - "/.oldroot/%s/%s.devpts", - stateDir, - def->name)) < 0) - return ret; - - if (virFileMakePath("/dev/pts") < 0) { - virReportSystemError(errno, "%s", - _("Cannot create /dev/pts")); - goto cleanup; - } - - VIR_DEBUG("Trying to move %s to /dev/pts", path); - - if ((ret = mount(path, "/dev/pts", - NULL, MS_MOVE, NULL)) < 0) { - virReportSystemError(errno, - _("Failed to mount %s on /dev/pts"), - path); - goto cleanup; - } - -cleanup: - VIR_FREE(path); - - return ret; -} - static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths) { size_t i; @@ -1683,14 +1613,6 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (virCgroupIsolateMount(cgroup, "/.oldroot/", sec_mount_options) < 0) goto cleanup; - /* Mounts /dev */ - if (lxcContainerMountFSDev(vmDef, stateDir) < 0) - goto cleanup; - - /* Mounts /dev/pts */ - if (lxcContainerMountFSDevPTS(vmDef, stateDir) < 0) - goto cleanup; - /* Setup device nodes in /dev/ */ if (lxcContainerSetupDevices(ttyPaths, nttyPaths) < 0) goto cleanup; @@ -1853,8 +1775,7 @@ static int lxcContainerChild(void *data) const char *tty = argv->ttyPaths[0]; if (STRPREFIX(tty, "/dev/pts/")) tty += strlen("/dev/pts/"); - if (virAsprintf(&ttyPath, "%s/%s.devpts/%s", - LXC_STATE_DIR, vmDef->name, tty) < 0) + if (virAsprintf(&ttyPath, "%s/dev/pts/%s", root->src, tty) < 0) goto cleanup; } else if (VIR_STRDUP(ttyPath, "/dev/null") < 0) { goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c013147..f7b4127 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2020,6 +2020,56 @@ cleanup: } +static int +virLXCControllerMoveMount(char *name, char *root, + const char *s, const char *d) +{ + int ret = -1; + char *src = NULL; + char *dst = NULL; + + if ((ret = virAsprintf(&src, "%s/%s.%s", + LXC_STATE_DIR, name, s)) < 0) + return ret; + + if ((ret = virAsprintf(&dst, "%s%s", root, d)) < 0) + goto cleanup; + + if (virFileMakePath(dst) < 0) { + virReportSystemError(errno, _("Cannot create %s"), dst); + goto cleanup; + } + + if (mount(src, dst, NULL, MS_MOVE, NULL) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on %s"), + src, dst); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(src); + VIR_FREE(dst); + return ret; +} + +static int +virLXCControllerMoveMounts(virDomainDefPtr def) +{ + virDomainFSDefPtr root = virDomainGetRootFilesystem(def); + + if (virLXCControllerMoveMount(def->name, root->src, + "dev", "/dev") < 0) + return -1; + + if (virLXCControllerMoveMount(def->name, root->src, + "devpts", "/dev/pts") < 0) + return -1; + + return 0; +} + static void virLXCControllerEventSend(virLXCControllerPtr ctrl, int procnr, @@ -2167,6 +2217,9 @@ virLXCControllerRun(virLXCControllerPtr ctrl) if (virLXCControllerSetupConsoles(ctrl, containerTTYPaths) < 0) goto cleanup; + if (virLXCControllerMoveMounts(ctrl->def) < 0) + goto cleanup; + if (lxcSetPersonality(ctrl->def) < 0) goto cleanup; -- 1.8.3.1
Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942 vfs: Lock in place mounts from more privileged users, unprivileged user has no rights to umount the mounts that inherited from parent mountns. right now, I have no good idea to fix this problem, we need to do more research. this patch just skip unmounting these mounts for shared root. BTW, I think when libvirt lxc enables user namespace, the configuation that shares root with host is very rara. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 61283e4..8003594 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1591,7 +1591,9 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerPivotRoot(root) < 0) goto cleanup; - if (STREQ(root->src, "/") && + /* FIXME: we should find a way to unmount these mounts for container + * even user namespace is enabled. */ + if (STREQ(root->src, "/") && (!vmDef->idmap.nuidmap) && lxcContainerUnmountForSharedRoot(stateDir, vmDef->name) < 0) goto cleanup; -- 1.8.3.1
On Tue, Nov 19, 2013 at 05:53:21PM +0800, Gao feng wrote:
Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942 vfs: Lock in place mounts from more privileged users,
unprivileged user has no rights to umount the mounts that inherited from parent mountns.
right now, I have no good idea to fix this problem, we need to do more research. this patch just skip unmounting these mounts for shared root.
BTW, I think when libvirt lxc enables user namespace, the configuation that shares root with host is very rara.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 61283e4..8003594 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1591,7 +1591,9 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerPivotRoot(root) < 0) goto cleanup;
- if (STREQ(root->src, "/") && + /* FIXME: we should find a way to unmount these mounts for container + * even user namespace is enabled. */ + if (STREQ(root->src, "/") && (!vmDef->idmap.nuidmap) && lxcContainerUnmountForSharedRoot(stateDir, vmDef->name) < 0) goto cleanup;
If unmounting fails for these few temporary filesystems, then how is unmount succeeding for everything under /.oldroot after we do the pivot root ? Does the pivot_root() confuse the kernel into thinking stuff under /.oldroot was owned by this process & thus allowed to be unmounted ? Or is it falling back to the MNT_DETACH scenario instead ? Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 11/19/2013 11:00 PM, Daniel P. Berrange wrote:
On Tue, Nov 19, 2013 at 05:53:21PM +0800, Gao feng wrote:
Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942 vfs: Lock in place mounts from more privileged users,
unprivileged user has no rights to umount the mounts that inherited from parent mountns.
right now, I have no good idea to fix this problem, we need to do more research. this patch just skip unmounting these mounts for shared root.
BTW, I think when libvirt lxc enables user namespace, the configuation that shares root with host is very rara.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 61283e4..8003594 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1591,7 +1591,9 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerPivotRoot(root) < 0) goto cleanup;
- if (STREQ(root->src, "/") && + /* FIXME: we should find a way to unmount these mounts for container + * even user namespace is enabled. */ + if (STREQ(root->src, "/") && (!vmDef->idmap.nuidmap) && lxcContainerUnmountForSharedRoot(stateDir, vmDef->name) < 0) goto cleanup;
If unmounting fails for these few temporary filesystems, then how is unmount succeeding for everything under /.oldroot after we do the pivot root ? Does the pivot_root() confuse the kernel into thinking stuff under /.oldroot was owned by this process & thus allowed to be unmounted ? Or is it falling back to the MNT_DETACH scenario instead ?
./oldroot is mounted in container, so container has rights to umount it, but for sub mounts under ./oldroot, container has no rights to umount them, so it falls back to MNT_DETACH scenario. hmm, So I think for the [PATCH 1/2], use MS_BIND instead of MS_MOVE may be a simple way, since even we move the /dev /dev/pts on host side, the unmount ./oldroot will fall back to MNT_DETACH scenario too.
On 11/20/2013 09:50 AM, Gao feng wrote:
On 11/19/2013 11:00 PM, Daniel P. Berrange wrote:
On Tue, Nov 19, 2013 at 05:53:21PM +0800, Gao feng wrote:
Also after commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942 vfs: Lock in place mounts from more privileged users,
unprivileged user has no rights to umount the mounts that inherited from parent mountns.
right now, I have no good idea to fix this problem, we need to do more research. this patch just skip unmounting these mounts for shared root.
BTW, I think when libvirt lxc enables user namespace, the configuation that shares root with host is very rara.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 61283e4..8003594 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1591,7 +1591,9 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, if (lxcContainerPivotRoot(root) < 0) goto cleanup;
- if (STREQ(root->src, "/") && + /* FIXME: we should find a way to unmount these mounts for container + * even user namespace is enabled. */ + if (STREQ(root->src, "/") && (!vmDef->idmap.nuidmap) && lxcContainerUnmountForSharedRoot(stateDir, vmDef->name) < 0) goto cleanup;
If unmounting fails for these few temporary filesystems, then how is unmount succeeding for everything under /.oldroot after we do the pivot root ? Does the pivot_root() confuse the kernel into thinking stuff under /.oldroot was owned by this process & thus allowed to be unmounted ? Or is it falling back to the MNT_DETACH scenario instead ?
./oldroot is mounted in container, so container has rights to umount it, but for sub mounts under ./oldroot, container has no rights to umount them, so it falls back to MNT_DETACH scenario.
hmm, So I think for the [PATCH 1/2], use MS_BIND instead of MS_MOVE may be a simple way, since even we move the /dev /dev/pts on host side, the unmount ./oldroot will fall back to MNT_DETACH scenario too.
Maybe in furture we can find a way to unmount all sub mounts under ./oldroot and unmount the temporary mounts, but now, I have no idea how to implement this. I will repost this patchset. Thanks!
On Tue, Nov 19, 2013 at 05:53:20PM +0800, Gao feng wrote:
After kernel commit 5ff9d8a65ce80efb509ce4e8051394e9ed2cd942 vfs: Lock in place mounts from more privileged users,
unprivileged user has no rights to move the mounts that inherited from parent mountns. we use this feature to move the /stateDir/domain-name.{dev, devpts} to the /dev/ and /dev/pts directroy of container. this commit breaks libvirt lxc.
this patch do the moving on host side, we are privileged user at this moment.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 81 +----------------------------------------------- src/lxc/lxc_controller.c | 53 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 80 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2bdf957..61283e4 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -953,76 +953,6 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def ATTRIBUTE_UNUSED, } #endif
-static int lxcContainerMountFSDev(virDomainDefPtr def, - const char *stateDir) -{ - int ret = -1; - char *path = NULL; - - VIR_DEBUG("Mount /dev/ stateDir=%s", stateDir); - - if ((ret = virAsprintf(&path, "/.oldroot/%s/%s.dev", - stateDir, def->name)) < 0) - return ret; - - if (virFileMakePath("/dev") < 0) { - virReportSystemError(errno, "%s", - _("Cannot create /dev")); - goto cleanup; - } - - VIR_DEBUG("Trying to move %s to /dev", path); - - if (mount(path, "/dev", NULL, MS_MOVE, NULL) < 0) {
I wonder if we used MS_BIND instead of MS_MOVE would we avoid the problem completely, and thus not need to move this code around ? Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Gao feng