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@xxxxxxxxxx>
> ---
> 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/ :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
