If you've ever tried running a huge page backed guest under different user than root, you probably failed. Problem is even though we have corresponding APIs in the security drivers, there's no implementation and thus we don't relabel the huge page path. But even if we did, so far all of the domains share the same path: /hugepageMount/libvirt/qemu Our only option there would be to set 0777 mode on the qemu dir which is totally unsafe. Therefore, we can create dir on per-domain basis, i.e.: /hugepageMount/libvirt/qemu/domainName and chown domainName dir to the user that domain is configured to run under. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_command.c | 4 +- src/qemu/qemu_conf.c | 45 ++++++++++++++++------ src/qemu/qemu_conf.h | 16 +++++--- src/qemu/qemu_driver.c | 19 +++------ src/qemu/qemu_process.c | 25 +++++++++++- .../qemuxml2argv-hugepages-numa.args | 4 +- .../qemuxml2argv-hugepages-pages.args | 14 +++---- .../qemuxml2argv-hugepages-pages2.args | 2 +- .../qemuxml2argv-hugepages-pages3.args | 2 +- .../qemuxml2argv-hugepages-pages5.args | 2 +- .../qemuxml2argv-hugepages-shared.args | 12 +++--- tests/qemuxml2argvdata/qemuxml2argv-hugepages.args | 2 +- .../qemuxml2argv-memory-hotplug-dimm-addr.args | 4 +- .../qemuxml2argv-memory-hotplug-dimm.args | 4 +- 14 files changed, 97 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 51b5aaf..becdce4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3303,7 +3303,7 @@ qemuBuildMemoryBackendStr(unsigned long long size, return -1; if (pagesize) { - if (qemuGetHupageMemPath(cfg, pagesize, &mem_path) < 0) + if (qemuGetDomainHupageMemPath(def, cfg, pagesize, &mem_path) < 0) goto cleanup; *backendType = "memory-backend-file"; @@ -7177,7 +7177,7 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg, return -1; } - if (qemuGetHupageMemPath(cfg, def->mem.hugepages[0].size, &mem_path) < 0) + if (qemuGetDomainHupageMemPath(def, cfg, def->mem.hugepages[0].size, &mem_path) < 0) return -1; virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path", mem_path, NULL); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fbac3ea..9be5b60 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1456,7 +1456,7 @@ qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED, } char * -qemuGetHugepagePath(virHugeTLBFSPtr hugepage) +qemuGetBaseHugepagePath(virHugeTLBFSPtr hugepage) { char *ret; @@ -1467,8 +1467,26 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage) } +char * +qemuGetDomainHugepagePath(const virDomainDef *def, + virHugeTLBFSPtr hugepage) +{ + char *base = qemuGetBaseHugepagePath(hugepage); + char *ret; + + if (!base || + virAsprintf(&ret, "%s/%s", base, def->name) < 0) { + VIR_FREE(base); + return NULL; + } + + return ret; +} + + /** - * qemuGetDefaultHugepath: + * qemuGetDomainDefaultHugepath: + * @def: domain definition * @hugetlbfs: array of configured hugepages * @nhugetlbfs: number of item in the array * @@ -1477,8 +1495,9 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage) * Returns 0 on success, -1 otherwise. * */ char * -qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, - size_t nhugetlbfs) +qemuGetDomainDefaultHugepath(const virDomainDef *def, + virHugeTLBFSPtr hugetlbfs, + size_t nhugetlbfs) { size_t i; @@ -1489,12 +1508,12 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, if (i == nhugetlbfs) i = 0; - return qemuGetHugepagePath(&hugetlbfs[i]); + return qemuGetDomainHugepagePath(def, &hugetlbfs[i]); } /** - * qemuGetHupageMemPath: Construct HP enabled memory backend path + * qemuGetDomainHupageMemPath: Construct HP enabled memory backend path * * If no specific hugepage size is requested (@pagesize is zero) * the default hugepage size is used). @@ -1504,9 +1523,10 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, * -1 otherwise. */ int -qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg, - unsigned long long pagesize, - char **memPath) +qemuGetDomainHupageMemPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + unsigned long long pagesize, + char **memPath) { size_t i = 0; @@ -1518,8 +1538,9 @@ qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg, } if (!pagesize) { - if (!(*memPath = qemuGetDefaultHugepath(cfg->hugetlbfs, - cfg->nhugetlbfs))) + if (!(*memPath = qemuGetDomainDefaultHugepath(def, + cfg->hugetlbfs, + cfg->nhugetlbfs))) return -1; } else { for (i = 0; i < cfg->nhugetlbfs; i++) { @@ -1535,7 +1556,7 @@ qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg, return -1; } - if (!(*memPath = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) + if (!(*memPath = qemuGetDomainHugepagePath(def, &cfg->hugetlbfs[i]))) return -1; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index f6e3257..d191e10 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -335,11 +335,15 @@ virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn, virDomainSnapshotDiskDefPtr def); -char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage); -char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, - size_t nhugetlbfs); +char * qemuGetBaseHugepagePath(virHugeTLBFSPtr hugepage); +char * qemuGetDomainHugepagePath(const virDomainDef *def, + virHugeTLBFSPtr hugepage); +char * qemuGetDomainDefaultHugepath(const virDomainDef *def, + virHugeTLBFSPtr hugetlbfs, + size_t nhugetlbfs); -int qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg, - unsigned long long pagesize, - char **memPath); +int qemuGetDomainHupageMemPath(const virDomainDef *def, + virQEMUDriverConfigPtr cfg, + unsigned long long pagesize, + char **memPath); #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fdfe912..8c8f969 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -852,7 +852,7 @@ qemuStateInitialize(bool privileged, * it, since we can't assume the root mount point has permissions that * will let our spawned QEMU instances use it. */ for (i = 0; i < cfg->nhugetlbfs; i++) { - hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]); + hugepagePath = qemuGetBaseHugepagePath(&cfg->hugetlbfs[i]); if (!hugepagePath) goto error; @@ -863,19 +863,10 @@ qemuStateInitialize(bool privileged, hugepagePath); goto error; } - if (privileged) { - if (virFileUpdatePerm(cfg->hugetlbfs[i].mnt_dir, - 0, S_IXGRP | S_IXOTH) < 0) - goto error; - if (chown(hugepagePath, cfg->user, cfg->group) < 0) { - virReportSystemError(errno, - _("unable to set ownership on %s to %d:%d"), - hugepagePath, - (int) cfg->user, - (int) cfg->group); - goto error; - } - } + if (privileged && + virFileUpdatePerm(cfg->hugetlbfs[i].mnt_dir, + 0, S_IXGRP | S_IXOTH) < 0) + goto error; VIR_FREE(hugepagePath); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8f379a..93a38e1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5264,11 +5264,19 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, if (vm->def->mem.nhugepages) { for (i = 0; i < cfg->nhugetlbfs; i++) { - char *hugepagePath = qemuGetHugepagePath(&cfg->hugetlbfs[i]); + char *hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); if (!hugepagePath) goto cleanup; + if (virFileMakePathWithMode(hugepagePath, 0700) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), + hugepagePath); + VIR_FREE(hugepagePath); + goto cleanup; + } + if (virSecurityManagerSetHugepages(driver->securityManager, vm->def, hugepagePath) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -6129,6 +6137,21 @@ void qemuProcessStop(virQEMUDriverPtr driver, networkReleaseActualDevice(vm->def, net); } + if (vm->def->mem.nhugepages) { + for (i = 0; i < cfg->nhugetlbfs; i++) { + char *hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); + + if (!hugepagePath) + continue; + + if (rmdir(hugepagePath) < 0) + VIR_WARN("Unable to remove hugepage path: %s (errno=%d)", + hugepagePath, errno); + + VIR_FREE(hugepagePath); + } + } + retry: if ((ret = qemuRemoveCgroup(vm)) < 0) { if (ret == -EBUSY && (retries++ < 5)) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args index 7b90784..bd562a8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args @@ -11,10 +11,10 @@ QEMU_AUDIO_DRV=spice \ -m size=1048576k,slots=16,maxmem=1099511627776k \ -smp 2,sockets=2,cores=1,threads=1 \ -mem-prealloc \ --mem-path /dev/hugepages2M/libvirt/qemu \ +-mem-path /dev/hugepages2M/libvirt/qemu/fedora \ -numa node,nodeid=0,cpus=0-1,mem=1024 \ -object memory-backend-file,id=memdimm0,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=1-3,\ +mem-path=/dev/hugepages1G/libvirt/qemu/fedora,size=1073741824,host-nodes=1-3,\ policy=bind \ -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \ -uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args index 9f0e696..e0aa151 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.args @@ -11,19 +11,19 @@ QEMU_AUDIO_DRV=none \ -m 4096 \ -smp 4,sockets=4,cores=1,threads=1 \ -object memory-backend-file,id=ram-node0,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu/QEMUGuest1,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=1073741824,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages2M/libvirt/qemu/QEMUGuest1,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -object memory-backend-file,id=ram-node2,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu/QEMUGuest1,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ -object memory-backend-file,id=ram-node3,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=3,\ +mem-path=/dev/hugepages1G/libvirt/qemu/QEMUGuest1,size=1073741824,host-nodes=3,\ policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args index a37f03d..b4cd088 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args @@ -11,7 +11,7 @@ QEMU_AUDIO_DRV=none \ -m 1024 \ -smp 2,sockets=2,cores=1,threads=1 \ -mem-prealloc \ --mem-path /dev/hugepages2M/libvirt/qemu \ +-mem-path /dev/hugepages2M/libvirt/qemu/SomeDummyHugepagesGuest \ -numa node,nodeid=0,cpus=0,mem=256 \ -numa node,nodeid=1,cpus=1,mem=768 \ -uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args index 57dd3fa..bf28d74 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args @@ -13,7 +13,7 @@ QEMU_AUDIO_DRV=none \ -object memory-backend-ram,id=ram-node0,size=268435456 \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=805306368 \ +mem-path=/dev/hugepages1G/libvirt/qemu/SomeDummyHugepagesGuest,size=805306368 \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \ -nographic \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args index a826149..a0a6506 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args @@ -10,7 +10,7 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m 1024 \ -mem-prealloc \ --mem-path /dev/hugepages2M/libvirt/qemu \ +-mem-path /dev/hugepages2M/libvirt/qemu/SomeDummyHugepagesGuest \ -smp 2,sockets=2,cores=1,threads=1 \ -uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \ -nographic \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args index f9fc218..03d736a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-shared.args @@ -11,19 +11,19 @@ QEMU_AUDIO_DRV=none \ -m 4096 \ -smp 4,sockets=4,cores=1,threads=1 \ -object memory-backend-file,id=ram-node0,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu/QEMUGuest1,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -object memory-backend-file,id=ram-node1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,share=yes,size=1073741824,\ +mem-path=/dev/hugepages2M/libvirt/qemu/QEMUGuest1,share=yes,size=1073741824,\ host-nodes=0-3,policy=bind \ -numa node,nodeid=1,cpus=1,memdev=ram-node1 \ -object memory-backend-file,id=ram-node2,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,share=no,size=1073741824,host-nodes=0-3,\ -policy=bind \ +mem-path=/dev/hugepages1G/libvirt/qemu/QEMUGuest1,share=no,size=1073741824,\ +host-nodes=0-3,policy=bind \ -numa node,nodeid=2,cpus=2,memdev=ram-node2 \ -object memory-backend-file,id=ram-node3,prealloc=yes,\ -mem-path=/dev/hugepages1G/libvirt/qemu,size=1073741824,host-nodes=3,\ +mem-path=/dev/hugepages1G/libvirt/qemu/QEMUGuest1,size=1073741824,host-nodes=3,\ policy=bind \ -numa node,nodeid=3,cpus=3,memdev=ram-node3 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args index 361c8e5..2d437da 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages.args @@ -10,7 +10,7 @@ QEMU_AUDIO_DRV=none \ -M pc \ -m 214 \ -mem-prealloc \ --mem-path /dev/hugepages2M/libvirt/qemu \ +-mem-path /dev/hugepages2M/libvirt/qemu/QEMUGuest1 \ -smp 1,sockets=1,cores=1,threads=1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ -nographic \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args index fdbb4c3..b635327 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args @@ -12,8 +12,8 @@ QEMU_AUDIO_DRV=none \ -smp 2,sockets=2,cores=1,threads=1 \ -numa node,nodeid=0,cpus=0-1,mem=214 \ -object memory-backend-file,id=memdimm0,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=536870912,host-nodes=1-3,\ -policy=bind \ +mem-path=/dev/hugepages2M/libvirt/qemu/QEMUGuest1,size=536870912,\ +host-nodes=1-3,policy=bind \ -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0,addr=4294967296 \ -object memory-backend-ram,id=memdimm2,size=536870912 \ -device pc-dimm,node=0,memdev=memdimm2,id=dimm2,slot=2 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args index 1587aba..10d8593 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args @@ -14,8 +14,8 @@ QEMU_AUDIO_DRV=none \ -object memory-backend-ram,id=memdimm0,size=536870912 \ -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \ -object memory-backend-file,id=memdimm1,prealloc=yes,\ -mem-path=/dev/hugepages2M/libvirt/qemu,size=536870912,host-nodes=1-3,\ -policy=bind \ +mem-path=/dev/hugepages2M/libvirt/qemu/QEMUGuest1,size=536870912,\ +host-nodes=1-3,policy=bind \ -device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ -nographic \ -- 2.8.4

This part of code that LXC currently uses will be reused so move to a generic function. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- configure.ac | 2 +- src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 18 +----------------- src/util/virprocess.c | 38 ++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 2 ++ 5 files changed, 43 insertions(+), 18 deletions(-) diff --git a/configure.ac b/configure.ac index fd50ff8..5661752 100644 --- a/configure.ac +++ b/configure.ac @@ -291,7 +291,7 @@ dnl and various less common threadsafe functions AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \ getmntent_r getpwuid_r getrlimit getuid kill mmap newlocale posix_fallocate \ posix_memalign prlimit regexec sched_getaffinity setgroups setns \ - setrlimit symlink sysctlbyname getifaddrs sched_setscheduler]) + setrlimit symlink sysctlbyname getifaddrs sched_setscheduler unshare]) dnl Availability of pthread functions. Because of $LIB_PTHREAD, we dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b7d26fd..316d1e0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2247,6 +2247,7 @@ virProcessSetMaxMemLock; virProcessSetMaxProcesses; virProcessSetNamespaces; virProcessSetScheduler; +virProcessSetupPrivateMountNS; virProcessTranslateStatus; virProcessWait; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 508bc3e..29f1179 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2092,8 +2092,6 @@ lxcCreateTty(virLXCControllerPtr ctrl, int *ttymaster, static int virLXCControllerSetupPrivateNS(void) { - int ret = -1; - /* * If doing a chroot style setup, we need to prepare * a private /dev/pts for the child now, which they @@ -2115,21 +2113,7 @@ virLXCControllerSetupPrivateNS(void) * marked as shared */ - if (unshare(CLONE_NEWNS) < 0) { - virReportSystemError(errno, "%s", - _("Cannot unshare mount namespace")); - goto cleanup; - } - - if (mount("", "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) { - virReportSystemError(errno, "%s", - _("Failed to switch root mount into slave mode")); - goto cleanup; - } - - ret = 0; - cleanup: - return ret; + return virProcessSetupPrivateMountNS(); } diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 3cacc89..d85407a 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -28,6 +28,9 @@ #include <stdlib.h> #include <sys/wait.h> #include <unistd.h> +#if HAVE_SYS_MOUNT_H +# include <sys/mount.h> +#endif #if HAVE_SETRLIMIT # include <sys/time.h> # include <sys/resource.h> @@ -1146,6 +1149,41 @@ virProcessRunInMountNamespace(pid_t pid, } +#if defined(HAVE_SYS_MOUNT_H) && defined(HAVE_UNSHARE) +int +virProcessSetupPrivateMountNS(void) +{ + int ret = -1; + + if (unshare(CLONE_NEWNS) < 0) { + virReportSystemError(errno, "%s", + _("Cannot unshare mount namespace")); + goto cleanup; + } + + if (mount("", "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Failed to switch root mount into slave mode")); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + +#else /* !defined(HAVE_SYS_MOUNT_H) || !defined(HAVE_UNSHARE */ + +int +virProcessSetupPrivateMountNS(void) +{ + virReportSystemError(ENOSYS, "%s", + _("Namespaces are not supported on this platform.")); + return -1; +} +#endif /* !defined(HAVE_SYS_MOUNT_H) || !defined(HAVE_UNSHARE */ + + /** * virProcessExitWithStatus: * @status: raw status to be reproduced when this process dies diff --git a/src/util/virprocess.h b/src/util/virprocess.h index 04e9802..c76a1fb 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -90,6 +90,8 @@ int virProcessRunInMountNamespace(pid_t pid, virProcessNamespaceCallback cb, void *opaque); +int virProcessSetupPrivateMountNS(void); + int virProcessSetScheduler(pid_t pid, virProcessSchedPolicy policy, int priority); -- 2.8.4

This part of code that LXC currently uses will be reused so move to a generic function. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 2 ++ src/lxc/lxc_container.c | 20 ++------------ src/lxc/lxc_controller.c | 14 +--------- src/util/virfile.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 6 ++++ 5 files changed, 84 insertions(+), 30 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 316d1e0..ae563eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1556,6 +1556,7 @@ virDirRead; virFileAbsPath; virFileAccessibleAs; virFileActivateDirOverride; +virFileBindMountDevice; virFileBuildPath; virFileClose; virFileDeleteTree; @@ -1601,6 +1602,7 @@ virFileResolveAllLinks; virFileResolveLink; virFileRewrite; virFileSanitizePath; +virFileSetupDev; virFileSkipRoot; virFileStripSuffix; virFileTouch; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 5357df4..5d7da8f 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1112,20 +1112,6 @@ static int lxcContainerMountFSDevPTS(virDomainDefPtr def, return ret; } -static int lxcContainerBindMountDevice(const char *src, const char *dst) -{ - if (virFileTouch(dst, 0666) < 0) - return -1; - - if (mount(src, dst, "none", MS_BIND, NULL) < 0) { - virReportSystemError(errno, _("Failed to bind %s on to %s"), src, - dst); - return -1; - } - - return 0; -} - static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths) { size_t i; @@ -1149,7 +1135,7 @@ static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths) } /* We have private devpts capability, so bind that */ - if (lxcContainerBindMountDevice("/dev/pts/ptmx", "/dev/ptmx") < 0) + if (virFileBindMountDevice("/dev/pts/ptmx", "/dev/ptmx") < 0) return -1; for (i = 0; i < nttyPaths; i++) { @@ -1157,7 +1143,7 @@ static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths) if (virAsprintf(&tty, "/dev/tty%zu", i+1) < 0) return -1; - if (lxcContainerBindMountDevice(ttyPaths[i], tty) < 0) { + if (virFileBindMountDevice(ttyPaths[i], tty) < 0) { return -1; VIR_FREE(tty); } @@ -1165,7 +1151,7 @@ static int lxcContainerSetupDevices(char **ttyPaths, size_t nttyPaths) VIR_FREE(tty); if (i == 0 && - lxcContainerBindMountDevice(ttyPaths[i], "/dev/console") < 0) + virFileBindMountDevice(ttyPaths[i], "/dev/console") < 0) return -1; } return 0; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 29f1179..2170b0a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1457,12 +1457,6 @@ static int virLXCControllerSetupDev(virLXCControllerPtr ctrl) LXC_STATE_DIR, ctrl->def->name) < 0) goto cleanup; - if (virFileMakePath(dev) < 0) { - virReportSystemError(errno, - _("Failed to make path %s"), dev); - goto cleanup; - } - /* * tmpfs is limited to 64kb, since we only have device nodes in there * and don't want to DOS the entire OS RAM usage @@ -1472,14 +1466,8 @@ static int virLXCControllerSetupDev(virLXCControllerPtr ctrl) "mode=755,size=65536%s", mount_options) < 0) goto cleanup; - VIR_DEBUG("Mount devfs on %s type=tmpfs flags=%x, opts=%s", - dev, MS_NOSUID, opts); - if (mount("devfs", dev, "tmpfs", MS_NOSUID, opts) < 0) { - virReportSystemError(errno, - _("Failed to mount devfs on %s type %s (%s)"), - dev, "tmpfs", opts); + if (virFileSetupDev(dev, opts) < 0) goto cleanup; - } if (lxcContainerChown(ctrl->def, dev) < 0) goto cleanup; diff --git a/src/util/virfile.c b/src/util/virfile.c index f006abf..6200004 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -32,6 +32,9 @@ #include <sys/types.h> #include <sys/socket.h> #include <sys/wait.h> +#if defined(HAVE_SYS_MOUNT_H) +# include <sys/mount.h> +#endif #include <unistd.h> #include <dirent.h> #include <dirname.h> @@ -3501,3 +3504,72 @@ int virFileIsSharedFS(const char *path) VIR_FILE_SHFS_SMB | VIR_FILE_SHFS_CIFS); } + + +#if defined(HAVE_SYS_MOUNT_H) +int +virFileSetupDev(const char *path, + const char *mount_options) +{ + const unsigned long mount_flags = MS_NOSUID; + const char *mount_fs = "tmpfs"; + int ret = -1; + + if (virFileMakePath(path) < 0) { + virReportSystemError(errno, + _("Failed to make path %s"), path); + goto cleanup; + } + + VIR_DEBUG("Mount devfs on %s type=tmpfs flags=%lx, opts=%s", + path, mount_flags, mount_options); + if (mount("devfs", path, mount_fs, mount_flags, mount_options) < 0) { + virReportSystemError(errno, + _("Failed to mount devfs on %s type %s (%s)"), + path, mount_fs, mount_options); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + + +int +virFileBindMountDevice(const char *src, + const char *dst) +{ + if (virFileTouch(dst, 0666) < 0) + return -1; + + if (mount(src, dst, "none", MS_BIND, NULL) < 0) { + virReportSystemError(errno, _("Failed to bind %s on to %s"), src, + dst); + return -1; + } + + return 0; +} + +#else /* !defined(HAVE_SYS_MOUNT_H) */ + +int +virFileSetupDev(const char *path ATTRIBUTE_UNUSED, + const char *mount_options ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("mount is not supported on this platform.")); + return -1; +} + + +int +virFileBindMountDevice(const char *src ATTRIBUTE_UNUSED, + const char *dst ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", + _("mount is not supported on this platform.")); + return -1; +} +#endif /* !defined(HAVE_SYS_MOUNT_H) */ diff --git a/src/util/virfile.h b/src/util/virfile.h index b4ae6ea..957fd91 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -307,4 +307,10 @@ int virFileGetHugepageSize(const char *path, unsigned long long *size); int virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs, size_t *ret_nfs); + +int virFileSetupDev(const char *path, + const char *mount_options); + +int virFileBindMountDevice(const char *src, + const char *dst); #endif /* __VIR_FILE_H */ -- 2.8.4

Namely, virFileGetACLs, virFileSetACLs, virFileFreeACLs and virFileCopyACLs. These functions are going to be required when we are creating /dev for qemu. We have copy anything that's in host's /dev exactly as is. Including ACLs. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- configure.ac | 10 +++++- src/Makefile.am | 4 +-- src/libvirt_private.syms | 4 +++ src/util/virfile.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 11 +++++++ 5 files changed, 107 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 5661752..972b399 100644 --- a/configure.ac +++ b/configure.ac @@ -332,11 +332,19 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \ - libtasn1.h sys/ucred.h sys/mount.h]) + libtasn1.h sys/ucred.h sys/mount.h sys/acl.h]) dnl Check whether endian provides handy macros. AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]]) AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64]) +ACL_CFLAGS="" +ACL_LIBS="" +if test "x$ac_cv_header_sys_acl_h" = "xyes" ; then + ACL_LIBS="-lacl" +fi +AC_SUBST([ACL_CFLAGS]) +AC_SUBST([ACL_LIBS]) + dnl We need to decide at configure time if libvirt will use real atomic dnl operations ("lock free") or emulated ones with a mutex. diff --git a/src/Makefile.am b/src/Makefile.am index b358e0e..c08a350 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1125,12 +1125,12 @@ libvirt_util_la_SOURCES = \ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ - $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) \ + $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) $(ACL_CFLAGS) \ -I$(srcdir)/conf libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ - $(SECDRIVER_LIBS) $(NUMACTL_LIBS) \ + $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(ACL_LIBS) \ $(POLKIT_LIBS) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ae563eb..5f560bf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1559,6 +1559,7 @@ virFileActivateDirOverride; virFileBindMountDevice; virFileBuildPath; virFileClose; +virFileCopyACLs; virFileDeleteTree; virFileDirectFdFlag; virFileExists; @@ -1568,6 +1569,8 @@ virFileFindHugeTLBFS; virFileFindMountPoint; virFileFindResource; virFileFindResourceFull; +virFileFreeACLs; +virFileGetACLs; virFileGetHugepageSize; virFileGetMountReverseSubtree; virFileGetMountSubtree; @@ -1602,6 +1605,7 @@ virFileResolveAllLinks; virFileResolveLink; virFileRewrite; virFileSanitizePath; +virFileSetACLs; virFileSetupDev; virFileSkipRoot; virFileStripSuffix; diff --git a/src/util/virfile.c b/src/util/virfile.c index 6200004..c777025 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -48,6 +48,9 @@ #if HAVE_SYS_SYSCALL_H # include <sys/syscall.h> #endif +#if HAVE_SYS_ACL_H +# include <sys/acl.h> +#endif #ifdef __linux__ # if HAVE_LINUX_MAGIC_H @@ -3573,3 +3576,81 @@ virFileBindMountDevice(const char *src ATTRIBUTE_UNUSED, return -1; } #endif /* !defined(HAVE_SYS_MOUNT_H) */ + + +#if defined(HAVE_SYS_ACL_H) && !defined(LIBVIRT_NSS) && !defined(LIBVIRT_SETUID_RPC_CLIENT) +int +virFileGetACLs(const char *file, + void **acl) +{ + if (!(*acl = acl_get_file(file, ACL_TYPE_ACCESS))) + return -1; + + return 0; +} + + +int +virFileSetACLs(const char *file, + void *acl) +{ + if (acl_set_file(file, ACL_TYPE_ACCESS, acl) < 0) + return -1; + + return 0; +} + + +void +virFileFreeACLs(void **acl) +{ + acl_free(*acl); + *acl = NULL; +} + +#else /* !defined(HAVE_SYS_ACL_H) || defined(LIBVIRT_NSS) || defined(LIBVIRT_SETUID_RPC_CLIENT) */ + +int +virFileGetACLs(const char *file ATTRIBUTE_UNUSED, + void **acl ATTRIBUTE_UNUSED) +{ + errno = ENOTSUP; + return -1; +} + + +int +virFileSetACLs(const char *file ATTRIBUTE_UNUSED, + void *acl ATTRIBUTE_UNUSED) +{ + errno = ENOTSUP; + return -1; +} + + +void +virFileFreeACLs(void **acl) +{ + *acl = NULL; +} + +#endif /* !defined(HAVE_SYS_ACL_H) || defined(LIBVIRT_NSS) || defined(LIBVIRT_SETUID_RPC_CLIENT) */ + +int +virFileCopyACLs(const char *src, + const char *dst) +{ + void *acl = NULL; + int ret = -1; + + if (virFileGetACLs(src, &acl) < 0) + return ret; + + if (virFileSetACLs(dst, acl) < 0) + goto cleanup; + + ret = 0; + cleanup: + virFileFreeACLs(&acl); + return ret; +} diff --git a/src/util/virfile.h b/src/util/virfile.h index 957fd91..28fed23 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -313,4 +313,15 @@ int virFileSetupDev(const char *path, int virFileBindMountDevice(const char *src, const char *dst); + +int virFileGetACLs(const char *file, + void **acl); + +int virFileSetACLs(const char *file, + void *acl); + +void virFileFreeACLs(void **acl); + +int virFileCopyACLs(const char *src, + const char *dst); #endif /* __VIR_FILE_H */ -- 2.8.4

On Thu, Nov 24, 2016 at 03:47:54PM +0100, Michal Privoznik wrote: You can simplify this by just adding #undef HAVE_SYS_ACL_H to config-post.h for both the NSS + SETUID blocks Should we consider doing virReportSystemError in these methods rather than letting the caller report the error, or is there a need for the caller to ignore errors in some cases ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|

On Mon, Dec 05, 2016 at 03:18:29PM +0100, Michal Privoznik wrote: I was thinking that if you get ENOTSUPP with GetACLs() we'd just return success, and a NULL acl. The SetACLs method would obviously have to treat a NULL acl as a no-op. IOW, it could "just work" even when ACLs are disabled. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|

We will need this function in near future so that we know what /dev device corresponds to the USB device. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virusb.c | 5 +++++ src/util/virusb.h | 1 + 3 files changed, 7 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5f560bf..8a172c3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2602,6 +2602,7 @@ virUSBDeviceFree; virUSBDeviceGetBus; virUSBDeviceGetDevno; virUSBDeviceGetName; +virUSBDeviceGetPath; virUSBDeviceGetUsedBy; virUSBDeviceListAdd; virUSBDeviceListCount; diff --git a/src/util/virusb.c b/src/util/virusb.c index 6a001a7..8cd2f57 100644 --- a/src/util/virusb.c +++ b/src/util/virusb.c @@ -406,6 +406,11 @@ const char *virUSBDeviceGetName(virUSBDevicePtr dev) return dev->name; } +const char *virUSBDeviceGetPath(virUSBDevicePtr dev) +{ + return dev->path; +} + unsigned int virUSBDeviceGetBus(virUSBDevicePtr dev) { return dev->bus; diff --git a/src/util/virusb.h b/src/util/virusb.h index f98ea21..716e8c6 100644 --- a/src/util/virusb.h +++ b/src/util/virusb.h @@ -67,6 +67,7 @@ void virUSBDeviceGetUsedBy(virUSBDevicePtr dev, const char **drv_name, const char **dom_name); const char *virUSBDeviceGetName(virUSBDevicePtr dev); +const char *virUSBDeviceGetPath(virUSBDevicePtr usb); unsigned int virUSBDeviceGetBus(virUSBDevicePtr dev); unsigned int virUSBDeviceGetDevno(virUSBDevicePtr dev); -- 2.8.4

We will need this function in near future so that we know what /dev device corresponds to the SCSI device. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virscsi.c | 6 ++++++ src/util/virscsi.h | 1 + 3 files changed, 8 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8a172c3..a5eb852 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2296,6 +2296,7 @@ virSCSIDeviceGetAdapter; virSCSIDeviceGetBus; virSCSIDeviceGetDevName; virSCSIDeviceGetName; +virSCSIDeviceGetPath; virSCSIDeviceGetReadonly; virSCSIDeviceGetSgName; virSCSIDeviceGetShareable; diff --git a/src/util/virscsi.c b/src/util/virscsi.c index 4843367..4fd8838 100644 --- a/src/util/virscsi.c +++ b/src/util/virscsi.c @@ -315,6 +315,12 @@ virSCSIDeviceGetName(virSCSIDevicePtr dev) return dev->name; } +const char * +virSCSIDeviceGetPath(virSCSIDevicePtr dev) +{ + return dev->sg_path; +} + unsigned int virSCSIDeviceGetAdapter(virSCSIDevicePtr dev) { diff --git a/src/util/virscsi.h b/src/util/virscsi.h index df40d7f..7d88d4e 100644 --- a/src/util/virscsi.h +++ b/src/util/virscsi.h @@ -58,6 +58,7 @@ int virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, const char *domname); bool virSCSIDeviceIsAvailable(virSCSIDevicePtr dev); const char *virSCSIDeviceGetName(virSCSIDevicePtr dev); +const char *virSCSIDeviceGetPath(virSCSIDevicePtr dev); unsigned int virSCSIDeviceGetAdapter(virSCSIDevicePtr dev); unsigned int virSCSIDeviceGetBus(virSCSIDevicePtr dev); unsigned int virSCSIDeviceGetTarget(virSCSIDevicePtr dev); -- 2.8.4

This is a list of devices that qemu needs for its run (apart from what's configured for domain). The devices on the list are enabled in the CGroups by default so they will be good candidates for initial /dev for new qemu. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_cgroup.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 211d0b5..68c019e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -42,7 +42,7 @@ VIR_LOG_INIT("qemu.qemu_cgroup"); -static const char *const defaultDeviceACL[] = { +const char *const defaultDeviceACL[] = { "/dev/null", "/dev/full", "/dev/zero", "/dev/random", "/dev/urandom", "/dev/ptmx", "/dev/kvm", "/dev/kqemu", diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 6e2c742..8ae4a72 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -76,4 +76,5 @@ int qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, qemuCgroupEmulatorAllNodesDataPtr *data); void qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data); +extern const char *const defaultDeviceACL[]; #endif /* __QEMU_CGROUP_H__ */ -- 2.8.4

Prime time. When it comes to spawning qemu process and relabelling all the devices it's going to touch, there's inherent race with other applications in the system (e.g. udev). Instead of trying convincing udev to not touch libvirt managed devices, we can create a separate mount namespace for the qemu, and mount our own /dev there. Of course this puts more work onto us as we have to maintain /dev files on each domain start and device hot(un-)plug. On the other hand, this enhances security also. (libvirtd) creates: /var/lib/libvirt/qemu/$domain.dev /var/lib/libvirt/qemu/$domain.devpts The child (which is going to be qemu eventually) calls unshare() to create new mount namespace. From now on anything that child does is invisible to the parent. Child then mounts tmpfs on $domain.dev (so that it still sees original /dev from the host) and creates some devices (as explained in one of the previous patches). The devices have to be created exactly as they are in the host (including perms, seclabels, ACLs, ...). After that it moves $domain.dev mount to /dev. What's the $domain.devpts mount there for then you ask? QEMU can create PTYs for some chardevs. And historically we exposed the host ends in our domain XML allowing users to connect to them. Therefore we must preserve devpts mount to be shared with the host's one. To make this patch as small as possible, creating of devices configured for domain in question is implemented in next patches. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 10 ++ src/qemu/qemu_process.c | 13 +++ 3 files changed, 312 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 137d4d5..d6a1c29 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -55,6 +55,9 @@ #include <sys/time.h> #include <fcntl.h> +#if defined(HAVE_SYS_MOUNT_H) +# include <sys/mount.h> +#endif #include <libxml/xpathInternals.h> @@ -1627,6 +1630,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainChrTypeToString(priv->monConfig->type)); } + if (priv->containerized) + virBufferAddLit(buf, "<containerized/>\n"); + qemuDomainObjPrivateXMLFormatVcpus(buf, vm->def); if (priv->qemuCaps) { @@ -1809,6 +1815,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, goto error; } + priv->containerized = virXPathBoolean("count(./containerized) > 0", ctxt) > 0; + if ((n = virXPathNodeSet("./vcpus/vcpu", ctxt, &nodes)) < 0) goto error; @@ -6653,3 +6661,284 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, return true; } + + +static int +qemuDomainCreateDevice(const char *device, + const char *path, + bool allow_noent) +{ + char *devicePath = NULL; + struct stat sb; + int ret = -1; + + if (!STRPREFIX(device, "/dev")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid device: %s"), + device); + goto cleanup; + } + + if (virAsprintf(&devicePath, "%s/%s", + path, device + 4) < 0) + goto cleanup; + + if (stat(device, &sb) < 0) { + if (errno == ENOENT && allow_noent) { + /* Ignore non-existent device. */ + ret = 0; + goto cleanup; + } + + virReportSystemError(errno, _("Unable to stat %s"), device); + goto cleanup; + } + + if (virFileMakeParentPath(devicePath) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), + devicePath); + goto cleanup; + } + + if (mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) { + virReportSystemError(errno, + _("Failed to make device %s"), + devicePath); + goto cleanup; + } + + if (chown(devicePath, sb.st_uid, sb.st_gid) < 0) { + virReportSystemError(errno, + _("Failed to chown device %s"), + devicePath); + goto cleanup; + } + + if (virFileCopyACLs(device, devicePath) < 0 && + errno != ENOTSUP) { + virReportSystemError(errno, + _("Failed to copy ACLs on device %s"), + devicePath); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(devicePath); + return ret; +} + + + +static int +qemuDomainPopulateDevices(virQEMUDriverPtr driver, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + const char *path) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *const *devices = (const char *const *) cfg->cgroupDeviceACL; + size_t i; + int ret = -1; + + if (!devices) + devices = defaultDeviceACL; + + for (i = 0; devices[i]; i++) { + if (qemuDomainCreateDevice(devices[i], path, true) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnref(cfg); + return ret; +} + + +static int +qemuDomainSetupDev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *path) +{ + char *mount_options = NULL; + char *opts = NULL; + int ret = -1; + + VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name); + + mount_options = virSecurityManagerGetMountOptions(driver->securityManager, + vm->def); + + if (!mount_options && + VIR_STRDUP(mount_options, "") < 0) + goto cleanup; + + /* + * tmpfs is limited to 64kb, since we only have device nodes in there + * and don't want to DOS the entire OS RAM usage + */ + if (virAsprintf(&opts, + "mode=755,size=65536%s", mount_options) < 0) + goto cleanup; + + if (virFileSetupDev(path, opts) < 0) + goto cleanup; + + if (qemuDomainPopulateDevices(driver, vm, path) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(opts); + VIR_FREE(mount_options); + return ret; +} + + +int +qemuDomainBuildNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + const unsigned long mount_flags = MS_MOVE; + char *devPath = NULL; + char *devptsPath = NULL; + int ret = -1; + + if (!priv->containerized) { + ret = 0; + goto cleanup; + } + + if (virAsprintf(&devPath, "%s/%s.dev", + cfg->stateDir, vm->def->name) < 0 || + virAsprintf(&devptsPath, "%s/%s.devpts", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + if (qemuDomainSetupDev(driver, vm, devPath) < 0) + goto cleanup; + + /* Save /dev/pts mount point because /dev/pts/NNN is exposed in our live + * XML and other applications are supposed to be able to use it. */ + if (mount("/dev/pts", devptsPath, NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Unable to move /dev/pts mount")); + goto cleanup; + } + + if (mount(devPath, "/dev", NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on /dev"), + devPath); + goto cleanup; + } + + if (virFileMakePath("/dev/pts") < 0) { + virReportSystemError(errno, "%s", + _("Cannot create /dev/pts")); + goto cleanup; + } + + if (mount(devptsPath, "/dev/pts", NULL, mount_flags, NULL) < 0) { + virReportSystemError(errno, + _("Failed to mount %s on /dev/pts"), + devptsPath); + goto cleanup; + } + + if (virFileBindMountDevice("/dev/pts/ptmx", "/dev/ptmx") < 0) + goto cleanup; + + VIR_DEBUG("blaaah: %d", system("find /dev/ -ls > /tmp/blaaah")); + VIR_DEBUG("blaaah: %d", system("echo >> /tmp/blaaah")); + VIR_DEBUG("blaaah: %d", system("mount >> /tmp/blaaah")); + ret = 0; + cleanup: + virObjectUnref(cfg); + VIR_FREE(devPath); + return ret; +} + + +int +qemuDomainCreateNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret = -1; + char *path = NULL; + +#if !defined(__linux__) + /* Namespaces are Linux specific. On other platforms just + * carry on with the old behaviour. */ + return 0; +#endif + + if (!virQEMUDriverIsPrivileged(driver)) { + ret = 0; + goto cleanup; + } + + if (virAsprintf(&path, "%s/%s.dev", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePath(path) < 0) { + virReportSystemError(errno, + _("Failed to create %s"), + path); + goto cleanup; + } + + VIR_FREE(path); + if (virAsprintf(&path, "%s/%s.devpts", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + if (virFileMakePath(path) < 0) { + virReportSystemError(errno, + _("Failed to create %s"), + path); + goto cleanup; + } + + priv->containerized = true; + ret = 0; + cleanup: + VIR_FREE(path); + virObjectUnref(cfg); + return ret; +} + + +void +qemuDomainDeleteNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; + char *path; + + if (!priv->containerized) + return; + + if (virAsprintf(&path, "%s/%s.dev", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + virFileDeleteTree(path); + + VIR_FREE(path); + if (virAsprintf(&path, "%s/%s.devpts", + cfg->stateDir, vm->def->name) < 0) + goto cleanup; + + virFileDeleteTree(path); + cleanup: + virObjectUnref(cfg); + VIR_FREE(path); +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7650ff3..9c34476 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -170,6 +170,8 @@ typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr; struct _qemuDomainObjPrivate { struct qemuDomainJobObj job; + bool containerized; + qemuMonitorPtr mon; virDomainChrSourceDefPtr monConfig; bool monJSON; @@ -785,4 +787,12 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver, bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, virQEMUCapsPtr qemuCaps); +int qemuDomainBuildNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +int qemuDomainCreateNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); + +void qemuDomainDeleteNamespace(virQEMUDriverPtr driver, + virDomainObjPtr vm); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 93a38e1..4f64dd1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2661,6 +2661,12 @@ static int qemuProcessHook(void *data) if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0) goto cleanup; + if (virProcessSetupPrivateMountNS() < 0) + goto cleanup; + + if (qemuDomainBuildNamespace(h->driver, h->vm) < 0) + goto cleanup; + if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) { if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) && @@ -5434,6 +5440,11 @@ qemuProcessLaunch(virConnectPtr conn, qemuDomainLogContextMarkPosition(logCtxt); + VIR_DEBUG("Building mount namespace"); + + if (qemuDomainCreateNamespace(driver, vm) < 0) + goto cleanup; + VIR_DEBUG("Clear emulator capabilities: %d", cfg->clearEmulatorCapabilities); if (cfg->clearEmulatorCapabilities) @@ -6210,6 +6221,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, } } + qemuDomainDeleteNamespace(driver, vm); + vm->taint = 0; vm->pid = -1; virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); -- 2.8.4

On 05.12.2016 14:26, Daniel P. Berrange wrote: Yep. Ah, okay. Right. I was thinking about this but was unable to come with a reason for other namespaces. But this doesn't hurt and looks like to be more future-proof, so why not, right? Meanwhile, we discussed this in other thread and found out we really ned copy as-is. D'oh! I remembered I needed to undo some debug lines somewhere... Ah, good point. Again, very good point. I'll fix it in v2. Michal

When starting a domain and separate mount namespace is used, we have to create all the /dev entries that are configured for the domain. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2d1c02b..17fb3d0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6843,6 +6843,149 @@ qemuDomainSetupAllDisks(virQEMUDriverPtr driver, } +static int +qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev, + char **path) +{ + int ret = -1; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + virPCIDevicePtr pci = NULL; + virUSBDevicePtr usb = NULL; + virSCSIDevicePtr scsi = NULL; + char *tmpPath = NULL; + bool freeTmpPath = false; + + *path = NULL; + + switch ((virDomainHostdevMode) dev->mode) { + case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: + switch ((virDomainHostdevSubsysType) dev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + pci = virPCIDeviceNew(pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function); + if (!pci) + goto cleanup; + + if (!(tmpPath = virPCIDeviceGetIOMMUGroupDev(pci))) + goto cleanup; + freeTmpPath = true; + } + break; + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + if (dev->missing) + break; + usb = virUSBDeviceNew(usbsrc->bus, + usbsrc->device, + NULL); + if (!usb) + goto cleanup; + + if (!(tmpPath = (char *) virUSBDeviceGetPath(usb))) + goto cleanup; + break; + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + /* Follow qemuSetupDiskCgroup() and qemuSetImageCgroupInternal() + * which does nothing for non local storage + */ + VIR_DEBUG("Not updating /dev for hostdev iSCSI path '%s'", iscsisrc->path); + } else { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + scsi = virSCSIDeviceNew(NULL, + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit, + dev->readonly, + dev->shareable); + + if (!scsi) + goto cleanup; + + if (!(tmpPath = (char *) virSCSIDeviceGetPath(scsi))) + goto cleanup; + } + break; + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: + break; + } + break; + + case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: + case VIR_DOMAIN_HOSTDEV_MODE_LAST: + /* nada */ + break; + } + + if (VIR_STRDUP(*path, tmpPath) < 0) + goto cleanup; + + ret = 0; + cleanup: + virPCIDeviceFree(pci); + virUSBDeviceFree(usb); + virSCSIDeviceFree(scsi); + if (freeTmpPath) + VIR_FREE(tmpPath); + return ret; +} + + +static int +qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev, + const char *devPath) +{ + int ret = -1; + char *path = NULL; + + if (qemuDomainGetHostdevPath(dev, &path) < 0) + goto cleanup; + + if (!path) { + /* There's no /dev device that we need to create. Claim success. */ + ret = 0; + goto cleanup; + } + + if (qemuDomainCreateDevice(path, devPath, false) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(path); + return ret; +} + + +static int +qemuDomainSetupAllHostdevs(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *devPath) +{ + size_t i; + + VIR_DEBUG("Setting up hostdevs"); + for (i = 0; i < vm->def->nhostdevs; i++) { + if (qemuDomainSetupHostdev(driver, + vm->def->hostdevs[i], + devPath) < 0) + return -1; + } + VIR_DEBUG("Setup all hostdevs"); + return 0; +} + + int qemuDomainBuildNamespace(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -6879,6 +7022,9 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, if (qemuDomainSetupAllDisks(driver, vm, devPath) < 0) goto cleanup; + if (qemuDomainSetupAllHostdevs(driver, vm, devPath) < 0) + goto cleanup; + if (mount(devPath, "/dev", NULL, mount_flags, NULL) < 0) { virReportSystemError(errno, _("Failed to mount %s on /dev"), -- 2.8.4

When starting a domain and separate mount namespace is used, we have to create all the /dev entries that are configured for the domain. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9871f4d..07fa900 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7018,6 +7018,35 @@ qemuDomainSetupAllChardevs(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, } +static int +qemuDomainSetupTPM(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *devPath) +{ + virDomainTPMDefPtr dev = vm->def->tpm; + + if (!dev) + return 0; + + VIR_DEBUG("Setting up TPM"); + + switch (dev->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + if (qemuDomainCreateDevice(dev->data.passthrough.source.data.file.path, + devPath, false) < 0) + return -1; + break; + + case VIR_DOMAIN_TPM_TYPE_LAST: + /* nada */ + break; + } + + VIR_DEBUG("Setup TPM"); + return 0; +} + + int qemuDomainBuildNamespace(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -7060,6 +7089,9 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, if (qemuDomainSetupAllChardevs(driver, vm, devPath) < 0) goto cleanup; + if (qemuDomainSetupTPM(driver, vm, devPath) < 0) + goto cleanup; + if (mount(devPath, "/dev", NULL, mount_flags, NULL) < 0) { virReportSystemError(errno, _("Failed to mount %s on /dev"), -- 2.8.4

When starting a domain and separate mount namespace is used, we have to create all the /dev entries that are configured for the domain. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4afa15e..b9492a9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7093,6 +7093,46 @@ qemuDomainSetupAllInputs(virQEMUDriverPtr driver, } +static int +qemuDomainSetupRNG(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainRNGDefPtr rng, + const char *devPath) +{ + switch ((virDomainRNGBackend) rng->backend) { + case VIR_DOMAIN_RNG_BACKEND_RANDOM: + if (qemuDomainCreateDevice(rng->source.file, devPath, false) < 0) + return -1; + + case VIR_DOMAIN_RNG_BACKEND_EGD: + case VIR_DOMAIN_RNG_BACKEND_LAST: + /* nada */ + break; + } + + return 0; +} + + +static int +qemuDomainSetupAllRNGs(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *devPath) +{ + size_t i; + + VIR_DEBUG("Setting up RNGs"); + for (i = 0; i < vm->def->nrngs; i++) { + if (qemuDomainSetupRNG(driver, + vm->def->rngs[i], + devPath) < 0) + return -1; + } + + VIR_DEBUG("Setup all RNGs"); + return 0; +} + + int qemuDomainBuildNamespace(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -7141,6 +7181,9 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, if (qemuDomainSetupAllInputs(driver, vm, devPath) < 0) goto cleanup; + 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"), -- 2.8.4

When attaching a device to a domain that's using separate mount namespace we must maintain /dev entries in order for qemu process to see them. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 8 +++ src/qemu/qemu_hotplug.c | 48 +++++++++----- src/qemu/qemu_security.c | 38 +++++++++++ src/qemu/qemu_security.h | 8 +++ 5 files changed, 246 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a1edde9..ac09b2c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7364,6 +7364,13 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, def->src->path = tmpsrc; } break; + case VIR_DOMAIN_DEVICE_HOSTDEV: { + virDomainHostdevDefPtr def = data->devDef->data.hostdev; + if (virSecurityManagerSetHostdevLabel(data->driver->securityManager, + data->vm->def, def, NULL) < 0) + goto cleanup; + } break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: @@ -7371,7 +7378,6 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_HOSTDEV: case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_GRAPHICS: @@ -7451,6 +7457,91 @@ qemuDomainAttachDeviceMknod(virQEMUDriverPtr driver, } +static int +qemuDomainDetachDeviceUnlinkHelper(pid_t pid ATTRIBUTE_UNUSED, + void *opaque) +{ + const char *path = opaque; + + VIR_DEBUG("Unlinking %s", path); + if (unlink(path) < 0 && errno != ENOENT) { + virReportSystemError(errno, + _("Unable to remove device %s"), path); + return -1; + } + + return 0; +} + + +static int +qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + const char *file) +{ + /* Technically, this is not needed. Yet. But in the future + * security managers might do some reference counting over + * Set/Restore label and thus for every SetLabel() there + * should be corresponding RestoreLabel(). */ + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: { + virDomainDiskDefPtr def = dev->data.disk; + char *tmpsrc = def->src->path; + def->src->path = (char *) file; + if (virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, def) < 0) { + def->src->path = tmpsrc; + return -1; + } + def->src->path = tmpsrc; + } break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: { + virDomainHostdevDefPtr def = dev->data.hostdev; + if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, def, NULL) < 0) + return -1; + } break; + + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected device type %d"), + dev->type); + return -1; + } + + if (virProcessRunInMountNamespace(vm->pid, + qemuDomainDetachDeviceUnlinkHelper, + (void *)file) < 0) + return -1; + + return 0; +} + + int qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -7512,3 +7603,69 @@ qemuDomainNamespaceTeardownDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, * I don't, therefore: */ return 0; } + + +int +qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = {.type = VIR_DOMAIN_DEVICE_HOSTDEV, .data.hostdev = hostdev}; + int ret = -1; + char *path = NULL; + + if (!priv->containerized) + return 0; + + if (qemuDomainGetHostdevPath(hostdev, &path) < 0) + goto cleanup; + + if (!path) { + /* There's no /dev device that we need to create. Claim success. */ + ret = 0; + goto cleanup; + } + + if (qemuDomainAttachDeviceMknod(driver, + vm, + &dev, + path) < 0) + goto cleanup; + ret = 0; + cleanup: + VIR_FREE(path); + return ret; +} + + +int +qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = {.type = VIR_DOMAIN_DEVICE_HOSTDEV, .data.hostdev = hostdev}; + int ret = -1; + char *path = NULL; + + if (!priv->containerized) + return 0; + + if (qemuDomainGetHostdevPath(hostdev, &path) < 0) + goto cleanup; + + if (!path) { + /* There's no /dev device that we need to create. Claim success. */ + ret = 0; + goto cleanup; + } + + if (qemuDomainDetachDeviceUnlink(driver, vm, &dev, path) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(path); + return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 045d1ba..ef85ae4 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -803,4 +803,12 @@ int qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver, int qemuDomainNamespaceTeardownDisk(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk); + +int qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev); + +int qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9b47b5f..58b2439 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1384,6 +1384,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, bool releaseaddr = false; bool teardowncgroup = false; bool teardownlabel = false; + bool teardowndevice = false; int backend; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; @@ -1435,12 +1436,15 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, } vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL; + if (qemuDomainNamespaceSetupHostdev(driver, vm, hostdev) < 0) + goto error; + teardowndevice = true; + if (qemuSetupHostdevCgroup(vm, hostdev) < 0) goto error; teardowncgroup = true; - if (virSecurityManagerSetHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + if (qemuSecuritySetHostdevLabel(driver, vm, hostdev) < 0) goto error; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) teardownlabel = true; @@ -1493,9 +1497,11 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0) VIR_WARN("Unable to restore host device labelling on hotplug fail"); + if (teardowndevice && + qemuDomainNamespaceTeardownHostdev(driver, vm, hostdev) < 0) + VIR_WARN("Unable to remove host device from /dev"); if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); @@ -2275,6 +2281,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, bool added = false; bool teardowncgroup = false; bool teardownlabel = false; + bool teardowndevice = false; int ret = -1; if (priv->usbaddrs) { @@ -2288,12 +2295,15 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, added = true; + if (qemuDomainNamespaceSetupHostdev(driver, vm, hostdev) < 0) + goto cleanup; + teardowndevice = true; + if (qemuSetupHostdevCgroup(vm, hostdev) < 0) goto cleanup; teardowncgroup = true; - if (virSecurityManagerSetHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + if (qemuSecuritySetHostdevLabel(driver, vm, hostdev) < 0) goto cleanup; teardownlabel = true; @@ -2323,9 +2333,11 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0) VIR_WARN("Unable to restore host device labelling on hotplug fail"); + if (teardowndevice && + qemuDomainNamespaceTeardownHostdev(driver, vm, hostdev) < 0) + VIR_WARN("Unable to remove host device from /dev"); if (added) qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); if (releaseaddr) @@ -2351,6 +2363,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, char *drivealias = NULL; bool teardowncgroup = false; bool teardownlabel = false; + bool teardowndevice = false; bool driveAdded = false; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { @@ -2389,12 +2402,15 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, return -1; } + if (qemuDomainNamespaceSetupHostdev(driver, vm, hostdev) < 0) + goto cleanup; + teardowndevice = true; + if (qemuSetupHostdevCgroup(vm, hostdev) < 0) goto cleanup; teardowncgroup = true; - if (virSecurityManagerSetHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + if (qemuSecuritySetHostdevLabel(driver, vm, hostdev) < 0) goto cleanup; teardownlabel = true; @@ -2441,9 +2457,11 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail"); if (teardownlabel && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0) VIR_WARN("Unable to restore host device labelling on hotplug fail"); + if (teardowndevice && + qemuDomainNamespaceTeardownHostdev(driver, vm, hostdev) < 0) + VIR_WARN("Unable to remove host device from /dev"); } VIR_FREE(drivealias); VIR_FREE(drvstr); @@ -3651,13 +3669,15 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainAuditHostdev(vm, hostdev, "detach", true); if (!is_vfio && - virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, hostdev, NULL) < 0) + qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0) VIR_WARN("Failed to restore host device labelling"); if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) VIR_WARN("Failed to remove host device cgroup ACL"); + if (qemuDomainNamespaceTeardownHostdev(driver, vm, hostdev) < 0) + VIR_WARN("Unable to remove host device from /dev"); + switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: qemuDomainRemovePCIHostDevice(driver, vm, hostdev); diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index f800d9b..de616e8 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -168,3 +168,41 @@ qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver, vm->def, disk); } + + +int +qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->containerized) { + /* Already handled by namespace code. */ + return 0; + } + + return virSecurityManagerSetHostdevLabel(driver->securityManager, + vm->def, + hostdev, + NULL); +} + + +int +qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (priv->containerized) { + /* Already handled by namespace code. */ + return 0; + } + + return virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, + hostdev, + NULL); +} diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index e3324ca..cc373b3 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -44,4 +44,12 @@ int qemuSecuritySetDiskLabel(virQEMUDriverPtr driver, int qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk); + +int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev); + +int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev); #endif /* __QEMU_SECURITY_H__ */ -- 2.8.4

When attaching a device to a domain that's using separate mount namespace we must maintain /dev entries in order for qemu process to see them. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_domain.h | 8 ++++++ src/qemu/qemu_hotplug.c | 10 +++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc7ce2d..0b46336 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7372,6 +7372,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, } break; case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_RNG: /* No labelling. */ break; @@ -7390,7 +7391,6 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_RNG: case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: @@ -7508,6 +7508,7 @@ qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver, } break; case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_RNG: /* No labelling. */ break; @@ -7526,7 +7527,6 @@ qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver, case VIR_DOMAIN_DEVICE_SMARTCARD: case VIR_DOMAIN_DEVICE_MEMBALLOON: case VIR_DOMAIN_DEVICE_NVRAM: - case VIR_DOMAIN_DEVICE_RNG: case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: @@ -7731,3 +7731,71 @@ qemuDomainNamespaceTeardownChardev(virQEMUDriverPtr driver, cleanup: return ret; } + + +int +qemuDomainNamespaceSetupRNG(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainRNGDefPtr rng) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = {.type = VIR_DOMAIN_DEVICE_RNG, .data.rng = rng}; + const char *path = NULL; + int ret = -1; + + if (!priv->containerized) + return 0; + + switch ((virDomainRNGBackend) rng->backend) { + case VIR_DOMAIN_RNG_BACKEND_RANDOM: + path = rng->source.file; + break; + + case VIR_DOMAIN_RNG_BACKEND_EGD: + case VIR_DOMAIN_RNG_BACKEND_LAST: + ret = 0; + goto cleanup; + } + + if (qemuDomainAttachDeviceMknod(driver, + vm, + &dev, + path) < 0) + goto cleanup; + ret = 0; + cleanup: + return ret; +} + + +int +qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainRNGDefPtr rng) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = {.type = VIR_DOMAIN_DEVICE_RNG, .data.rng = rng}; + int ret = -1; + const char *path = NULL; + + if (!priv->containerized) + return 0; + + switch ((virDomainRNGBackend) rng->backend) { + case VIR_DOMAIN_RNG_BACKEND_RANDOM: + path = rng->source.file; + break; + + case VIR_DOMAIN_RNG_BACKEND_EGD: + case VIR_DOMAIN_RNG_BACKEND_LAST: + ret = 0; + goto cleanup; + } + + if (qemuDomainDetachDeviceUnlink(driver, vm, &dev, path) < 0) + goto cleanup; + + ret = 0; + cleanup: + return ret; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 37b1e41..78d00de 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -819,4 +819,12 @@ int qemuDomainNamespaceSetupChardev(virQEMUDriverPtr driver, int qemuDomainNamespaceTeardownChardev(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainChrDefPtr chr); + +int qemuDomainNamespaceSetupRNG(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainRNGDefPtr rng); + +int qemuDomainNamespaceTeardownRNG(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainRNGDefPtr rng); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4f012d0..61f7c5a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1980,6 +1980,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, char *secAlias = NULL; bool releaseaddr = false; bool teardowncgroup = false; + bool teardowndevice = false; bool chardevAdded = false; bool objAdded = false; bool tlsobjAdded = false; @@ -2025,6 +2026,10 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, goto cleanup; } + if (qemuDomainNamespaceSetupRNG(driver, vm, rng) < 0) + goto cleanup; + teardowndevice = true; + if (qemuSetupRNGCgroup(vm, rng) < 0) goto cleanup; teardowncgroup = true; @@ -2111,6 +2116,8 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, qemuDomainReleaseDeviceAddress(vm, &rng->info, NULL); if (teardowncgroup && qemuTeardownRNGCgroup(vm, rng) < 0) VIR_WARN("Unable to remove RNG device cgroup ACL on hotplug fail"); + if (teardowndevice && qemuDomainNamespaceTeardownRNG(driver, vm, rng) < 0) + VIR_WARN("Unable to remove chr device from /dev"); } VIR_FREE(tlsAlias); @@ -3975,6 +3982,9 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver, if (qemuTeardownRNGCgroup(vm, rng) < 0) VIR_WARN("Failed to remove RNG device cgroup ACL"); + if (qemuDomainNamespaceTeardownRNG(driver, vm, rng) < 0) + VIR_WARN("Unable to remove RNG device from /dev"); + event = virDomainEventDeviceRemovedNewFromObj(vm, rng->info.alias); qemuDomainEventQueue(driver, event); -- 2.8.4

On Thu, Nov 24, 2016 at 03:48:10PM +0100, Michal Privoznik wrote: Similarly to my earlier question, I wonder if we're better off explicitly referring to the namespace we're actually using to make future enhancements simpler. eg allow either namespaces = [ "mount" ] or namespaces = [ ] so we can extend this to non-mount namespaces later if desired. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|