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

[Daniel P. Berrange]

On Wed, Feb 15, 2017 at 11:19:24AM +0100, Michal Privoznik wrote:
> On 02/15/2017 10:43 AM, Daniel P. Berrange wrote:
> > 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(-)
> >>
> 
> 
> >> +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(....)
> 
> Ah. Obviously. What about this?
> 
> +bool
> +qemuDomainNamespaceAvailable(qemuDomainNamespace ns ATTRIBUTE_UNUSED)
> +{
> +#if !defined(__linux__)
> +    /* Namespaces are Linux specific. */
> +    return false;
> +
> +#else /* defined(__linux__) */
> +
> +    switch (ns) {
> +    case QEMU_DOMAIN_NS_MOUNT:
> +# 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;
> +# else
> +        if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_MNT) < 0)
> +            return false;
> +# endif
> +        break;
> +    case QEMU_DOMAIN_NS_LAST:
> +        break;
> +    }
> +
> +    return true;
> +#endif /* defined(__linux__) */
> +}
> +

ACK that looks fine.

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