[libvirt] [PATCH v2 08/21] qemu: Spawn qemu under mount namespace

[Daniel P. Berrange]

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 at 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/ :|