Other drivers (like qemu) would like to know if the namespaces are available therefore it makes sense to move this function to a shared module. At the same time, this function had some default namespaces that are checked with every call. It is not necessary - let callers pass just those namespaces they are interested in. With the move the function is renamed to virProcessNamespaceAvailable. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/lxc/lxc_container.c | 44 +---------------------------- src/lxc/lxc_container.h | 2 -- src/lxc/lxc_driver.c | 7 +++-- src/util/virprocess.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 10 +++++++ 6 files changed, 89 insertions(+), 47 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9c74d35c4..d02d23b35 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2275,6 +2275,7 @@ virProcessGetPids; virProcessGetStartTime; virProcessKill; virProcessKillPainfully; +virProcessNamespaceAvailable; virProcessRunInMountNamespace; virProcessSchedPolicyTypeFromString; virProcessSchedPolicyTypeToString; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 32c0c3a4a..e5619b168 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -27,7 +27,6 @@ #include <config.h> #include <fcntl.h> -#include <sched.h> #include <limits.h> #include <stdlib.h> #include <stdio.h> @@ -2265,7 +2264,7 @@ static int lxcContainerChild(void *data) static int userns_supported(void) { - return lxcContainerAvailable(LXC_CONTAINER_FEATURE_USER) == 0; + return virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_USER) == 0; } static int userns_required(virDomainDefPtr def) @@ -2399,47 +2398,6 @@ int lxcContainerStart(virDomainDefPtr def, return pid; } -ATTRIBUTE_NORETURN static int -lxcContainerDummyChild(void *argv ATTRIBUTE_UNUSED) -{ - _exit(0); -} - -int lxcContainerAvailable(int features) -{ - int flags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS| - CLONE_NEWIPC|SIGCHLD; - int cpid; - char *childStack; - char *stack; - int stacksize = getpagesize() * 4; - - if (features & LXC_CONTAINER_FEATURE_USER) - flags |= CLONE_NEWUSER; - - if (features & LXC_CONTAINER_FEATURE_NET) - flags |= CLONE_NEWNET; - - if (VIR_ALLOC_N(stack, stacksize) < 0) - return -1; - - childStack = stack + stacksize; - - cpid = clone(lxcContainerDummyChild, childStack, flags, NULL); - VIR_FREE(stack); - if (cpid < 0) { - char ebuf[1024] ATTRIBUTE_UNUSED; - VIR_DEBUG("clone call returned %s, container support is not enabled", - virStrerror(errno, ebuf, sizeof(ebuf))); - return -1; - } else if (virProcessWait(cpid, NULL, false) < 0) { - return -1; - } - - VIR_DEBUG("container support is enabled"); - return 0; -} - int lxcContainerChown(virDomainDefPtr def, const char *path) { uid_t uid; diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index 33eaab49c..fbdfc998a 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -65,8 +65,6 @@ int lxcContainerStart(virDomainDefPtr def, size_t nttyPaths, char **ttyPaths); -int lxcContainerAvailable(int features); - int lxcContainerSetupHostdevCapsMakePath(const char *dev); virArch lxcContainerGetAlt32bitArch(virArch arch); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 04a4b8c2a..a2c1052c6 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1575,7 +1575,7 @@ static int lxcCheckNetNsSupport(void) if (virRun(argv, &ip_rc) < 0 || ip_rc == 255) return 0; - if (lxcContainerAvailable(LXC_CONTAINER_FEATURE_NET) < 0) + if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_NET) < 0) return 0; return 1; @@ -1633,7 +1633,10 @@ static int lxcStateInitialize(bool privileged, } /* Check that this is a container enabled kernel */ - if (lxcContainerAvailable(0) < 0) { + if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_MNT | + VIR_PROCESS_NAMESPACE_PID | + VIR_PROCESS_NAMESPACE_UTS | + VIR_PROCESS_NAMESPACE_IPC) < 0) { VIR_INFO("LXC support not available in this kernel, disabling driver"); return 0; } diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 1ebe863fb..f5c7ebb96 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1183,6 +1183,78 @@ virProcessSetupPrivateMountNS(void) } #endif /* !defined(HAVE_SYS_MOUNT_H) || !defined(HAVE_UNSHARE) */ +#if defined(__linux__) +ATTRIBUTE_NORETURN static int +virProcessDummyChild(void *argv ATTRIBUTE_UNUSED) +{ + _exit(0); +} + +/** + * virProcessNamespaceAvailable: + * @ns: what namespaces to check (bitwise-OR of virProcessNamespaceFlags) + * + * Check if given list of namespaces (@ns) is available. + * If not, appropriate error message is produced. + * + * Returns: 0 on success (all the namespaces from @flags are available), + * -1 on error (with error message reported). + */ +int +virProcessNamespaceAvailable(unsigned int ns) +{ + int flags = 0; + int cpid; + char *childStack; + char *stack; + int stacksize = getpagesize() * 4; + + if (ns & VIR_PROCESS_NAMESPACE_MNT) + flags |= CLONE_NEWNS; + if (ns & VIR_PROCESS_NAMESPACE_IPC) + flags |= CLONE_NEWIPC; + if (ns & VIR_PROCESS_NAMESPACE_NET) + flags |= CLONE_NEWNET; + if (ns & VIR_PROCESS_NAMESPACE_PID) + flags |= CLONE_NEWPID; + if (ns & VIR_PROCESS_NAMESPACE_USER) + flags |= CLONE_NEWUSER; + if (ns & VIR_PROCESS_NAMESPACE_UTS) + flags |= CLONE_NEWUTS; + + /* Signal parent as soon as the child dies. RIP. */ + flags |= SIGCHLD; + + if (VIR_ALLOC_N(stack, stacksize) < 0) + return -1; + + childStack = stack + stacksize; + + cpid = clone(virProcessDummyChild, childStack, flags, NULL); + VIR_FREE(stack); + if (cpid < 0) { + char ebuf[1024] ATTRIBUTE_UNUSED; + VIR_DEBUG("clone call returned %s, container support is not enabled", + virStrerror(errno, ebuf, sizeof(ebuf))); + return -1; + } else if (virProcessWait(cpid, NULL, false) < 0) { + return -1; + } + + VIR_DEBUG("All namespaces (%x) are enabled", ns); + return 0; +} + +#else /* !defined(__linux__) */ + +int +virProcessNamespaceAvailable(unsigned int ns ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("Namespaces are not supported on this platform.")); + return -1; +} +#endif /* !defined(__linux__) */ /** * virProcessExitWithStatus: diff --git a/src/util/virprocess.h b/src/util/virprocess.h index c76a1fbc5..3c5a88277 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -95,5 +95,15 @@ int virProcessSetupPrivateMountNS(void); int virProcessSetScheduler(pid_t pid, virProcessSchedPolicy policy, int priority); +typedef enum { + VIR_PROCESS_NAMESPACE_MNT = (1 << 1), + VIR_PROCESS_NAMESPACE_IPC = (1 << 2), + VIR_PROCESS_NAMESPACE_NET = (1 << 3), + VIR_PROCESS_NAMESPACE_PID = (1 << 4), + VIR_PROCESS_NAMESPACE_USER = (1 << 5), + VIR_PROCESS_NAMESPACE_UTS = (1 << 6), +} virProcessNamespaceFlags; + +int virProcessNamespaceAvailable(unsigned int ns); #endif /* __VIR_PROCESS_H__ */ -- 2.11.0

This is unnecessary wrapper around virProcessNamespaceAvailable(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/lxc/lxc_container.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index e5619b168..601b9b035 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2262,11 +2262,6 @@ static int lxcContainerChild(void *data) return ret; } -static int userns_supported(void) -{ - return virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_USER) == 0; -} - static int userns_required(virDomainDefPtr def) { return def->idmap.uidmap && def->idmap.gidmap; @@ -2346,15 +2341,14 @@ int lxcContainerStart(virDomainDefPtr def, cflags = CLONE_NEWPID|CLONE_NEWNS|SIGCHLD; if (userns_required(def)) { - if (userns_supported()) { - VIR_DEBUG("Enable user namespace"); - cflags |= CLONE_NEWUSER; - } else { + if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_USER) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Kernel doesn't support user namespace")); VIR_FREE(stack); return -1; } + VIR_DEBUG("Enable user namespace"); + cflags |= CLONE_NEWUSER; } if (!nsInheritFDs || nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHARENET] == -1) { if (lxcNeedNetworkNamespace(def)) { -- 2.11.0

This is a simple wrapper over mount(). However, not every system out there is capable of moving a mount point. Therefore, instead of having to deal with this fact in all the places of our code we can have a simple wrapper and deal with this fact at just one place. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 22 +++------------------- src/util/virfile.c | 28 ++++++++++++++++++++++++++++ src/util/virfile.h | 3 +++ 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d02d23b35..70ed87b95 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1605,6 +1605,7 @@ virFileMakeParentPath; virFileMakePath; virFileMakePathWithMode; virFileMatchesNameSuffix; +virFileMoveMount; virFileNBDDeviceAssociate; virFileOpenAs; virFileOpenTty; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 473d0c1a2..8602f01c7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7332,7 +7332,6 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, virDomainObjPtr vm) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - const unsigned long mount_flags = MS_MOVE; char *devPath = NULL; char **devMountsPath = NULL, **devMountsSavePath = NULL; size_t ndevMountsPath = 0, i; @@ -7376,13 +7375,8 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, goto cleanup; } - if (mount(devMountsPath[i], devMountsSavePath[i], - NULL, mount_flags, NULL) < 0) { - virReportSystemError(errno, - _("Unable to move %s mount"), - devMountsPath[i]); + if (virFileMoveMount(devMountsPath[i], devMountsSavePath[i]) < 0) goto cleanup; - } } if (qemuDomainSetupAllDisks(driver, vm, devPath) < 0) @@ -7403,12 +7397,8 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, if (qemuDomainSetupAllRNGs(driver, vm, devPath) < 0) goto cleanup; - if (mount(devPath, "/dev", NULL, mount_flags, NULL) < 0) { - virReportSystemError(errno, - _("Failed to mount %s on /dev"), - devPath); + if (virFileMoveMount(devPath, "/dev") < 0) goto cleanup; - } for (i = 0; i < ndevMountsPath; i++) { if (devMountsSavePath[i] == devPath) @@ -7420,14 +7410,8 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, goto cleanup; } - if (mount(devMountsSavePath[i], devMountsPath[i], - NULL, mount_flags, NULL) < 0) { - virReportSystemError(errno, - _("Failed to mount %s on %s"), - devMountsSavePath[i], - devMountsPath[i]); + if (virFileMoveMount(devMountsSavePath[i], devMountsPath[i]) < 0) goto cleanup; - } } ret = 0; diff --git a/src/util/virfile.c b/src/util/virfile.c index 99157be16..bf8099e34 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3610,6 +3610,24 @@ virFileBindMountDevice(const char *src, return 0; } + +int +virFileMoveMount(const char *src, + const char *dst) +{ + const unsigned long mount_flags = MS_MOVE; + + if (mount(src, dst, NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, + _("Unable to move %s mount to %s"), + src, dst); + return -1; + } + + return 0; +} + + #else /* !defined(__linux__) || !defined(HAVE_SYS_MOUNT_H) */ int @@ -3630,6 +3648,16 @@ virFileBindMountDevice(const char *src ATTRIBUTE_UNUSED, _("mount is not supported on this platform.")); return -1; } + + +int +virFileMoveMount(const char *src ATTRIBUTE_UNUSED, + const char *dst ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("mount move is not supported on this platform.")); + return -1; +} #endif /* !defined(__linux__) || !defined(HAVE_SYS_MOUNT_H) */ diff --git a/src/util/virfile.h b/src/util/virfile.h index 571e5bdc8..0343acd5b 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -318,6 +318,9 @@ int virFileSetupDev(const char *path, int virFileBindMountDevice(const char *src, const char *dst); +int virFileMoveMount(const char *src, + const char *dst); + int virFileGetACLs(const char *file, void **acl); -- 2.11.0