On Wed, Dec 07, 2016 at 09:36:15AM +0100, Michal Privoznik wrote:
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.
>From technical POV, on domain startup process the parent
(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>
---
src/qemu/qemu_domain.c | 377 +++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_domain.h | 20 +++
src/qemu/qemu_process.c | 13 ++
3 files changed, 408 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4aae14d9d..50b401f79 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>
@@ -86,6 +89,10 @@ VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
"start",
);
+VIR_ENUM_IMPL(qemuDomainNamespace, QEMU_DOMAIN_NS_LAST,
+ "mount",
+);
+
struct _qemuDomainLogContext {
int refs;
@@ -146,6 +153,31 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job,
}
+bool
+qemuDomainNamespaceEnabled(virDomainObjPtr vm,
+ qemuDomainNamespace ns)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ return priv->namespaces &&
+ virBitmapIsBitSet(priv->namespaces, ns);
+}
+
+
+static bool
+qemuDomainEnableNamespace(virDomainObjPtr vm,
+ qemuDomainNamespace ns)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (!priv->namespaces &&
+ !(priv->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
+ return -1;
+
+ return virBitmapSetBit(priv->namespaces, ns);
Returning -1 from a method returning 'bool' is not what you want :-)
Also causes a compile warning in the caller about comparing
bool with '< 0'. Looking at the callers, I think you probably
wanted
@@ -169,7 +169,7 @@ qemuDomainNamespaceEnabled(virDomainObjPtr vm,
}
-static bool
+static int
qemuDomainEnableNamespace(virDomainObjPtr vm,
qemuDomainNamespace ns)
{
@@ -179,7 +179,9 @@ qemuDomainEnableNamespace(virDomainObjPtr vm,
!(priv->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
return -1;
- return virBitmapSetBit(priv->namespaces, ns);
+ virBitmapSetBit(priv->namespaces, ns);
+
+ return 0;
}
ACK with such a change made.
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/ :|