[libvirt] [PATCH] qemu_conf: Check for namespaces availability more wisely

[Daniel P. Berrange]

On Wed, Feb 15, 2017 at 10:20:27AM +0100, Michal Privoznik wrote:
> The bare fact that mnt namespace is available is not enough for
> us to allow/enable qemu namespaces feature. There are other
> requirements: we must copy all the ACL & SELinux labels otherwise
> we might grant access that is administratively forbidden or vice
> versa.
> At the same time, the check for namespace prerequisites is moved
> from domain startup time to qemu.conf parser as it doesn't make
> much sense to allow users to start misconfigured libvirt just to
> find out they can't start a single domain.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_conf.c   | 20 ++++++++++++++++----
>  src/qemu/qemu_conf.h   |  3 ++-
>  src/qemu/qemu_domain.c | 43 ++++++++++++++++++++++++++++---------------
>  src/qemu/qemu_domain.h |  2 ++
>  src/qemu/qemu_driver.c |  2 +-
>  5 files changed, 49 insertions(+), 21 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 0223a95d2..ad482d0ee 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -321,12 +321,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>      if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
>          goto error;
>  
> -#if defined(__linux__)
>      if (privileged &&
> -        virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_MNT) == 0 &&
> +        qemuDomainNamespaceAvailable(QEMU_DOMAIN_NS_MOUNT) &&
>          virBitmapSetBit(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) < 0)
>          goto error;
> -#endif /* defined(__linux__) */
>  
>  #ifdef DEFAULT_LOADER_NVRAM
>      if (virFirmwareParseList(DEFAULT_LOADER_NVRAM,
> @@ -438,7 +436,8 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
>  
>  
>  int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> -                                const char *filename)
> +                                const char *filename,
> +                                bool privileged)
>  {
>      virConfPtr conf = NULL;
>      int ret = -1;
> @@ -832,6 +831,19 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>                  goto cleanup;
>              }
>  
> +            if (!privileged) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("cannot use namespaces in session mode"));
> +                goto cleanup;
> +            }
> +
> +            if (qemuDomainNamespaceAvailable(ns) < 0) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("%s namespace is not available"),
> +                               namespaces[i]);
> +                goto cleanup;
> +            }
> +
>              if (virBitmapSetBit(cfg->namespaces, ns) < 0) {
>                  virReportError(VIR_ERR_INTERNAL_ERROR,
>                                 _("Unable to enable namespace: %s"),
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 91904ed4f..e585f81af 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -294,7 +294,8 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def);
>  virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged);
>  
>  int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> -                                const char *filename);
> +                                const char *filename,
> +                                bool privileged);
>  
>  virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver);
>  bool virQEMUDriverIsPrivileged(virQEMUDriverPtr driver);
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3adec5c14..c3dcea0c4 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7643,21 +7643,8 @@ qemuDomainCreateNamespace(virQEMUDriverPtr driver,
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      int ret = -1;
>  
> -    if (!virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT)) {
> -        ret = 0;
> -        goto cleanup;
> -    }
> -
> -    if (!virQEMUDriverIsPrivileged(driver)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("cannot use namespaces in session mode"));
> -        goto cleanup;
> -    }
> -
> -    if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_MNT) < 0)
> -        goto cleanup;
> -
> -    if (qemuDomainEnableNamespace(vm, QEMU_DOMAIN_NS_MOUNT) < 0)
> +    if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) &&
> +        qemuDomainEnableNamespace(vm, QEMU_DOMAIN_NS_MOUNT) < 0)
>          goto cleanup;
>  
>      ret = 0;
> @@ -7667,6 +7654,32 @@ qemuDomainCreateNamespace(virQEMUDriverPtr driver,
>  }
>  
>  
> +bool
> +qemuDomainNamespaceAvailable(qemuDomainNamespace ns)
> +{
> +
> +    switch (ns) {
> +    case QEMU_DOMAIN_NS_MOUNT:
> +#if !defined(__linux__)
> +        /* Namespaces are Linux specific. */
> +        return false;
> +#endif
> +#if !defined(HAVE_SYS_ACL_H) || !defined(WITH_SELINUX)
> +        /* We can't create the exact copy of paths if either of
> +         * these is not available. */
> +        return false;
> +#endif

Pretty sure this will cause the compiler to complain about
unreachable code paths because you'll get

    return false;
    return false;
    if (virProcessNamespaceAvailable(....)

You need to use #elif for the acl/selnux check, and then
use an #else for this last bit:

> +        if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_MNT) < 0)
> +            return false;
> +        break;
> +    case QEMU_DOMAIN_NS_LAST:
> +        break;
> +    }

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