[libvirt] [PATCH 2/4] configure: Make ACL mandatory when building the QEMU driver

[Daniel P. Berrange]

On Tue, Feb 14, 2017 at 04:55:05PM +0100, Michal Privoznik wrote:
> On 02/14/2017 04:40 PM, Daniel P. Berrange wrote:
> > On Tue, Feb 14, 2017 at 04:33:09PM +0100, Andrea Bolognani wrote:
> >> When we're building a private /dev for the isolated QEMU
> >> process, we want to be able to replicate the contents of
> >> the original /dev as closely as possible, including ACLs.
> >>
> >> To ensure that's always possible, make ACL support mandatory
> >> when the QEMU driver is enabled.
> >>
> >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1421036
> >> ---
> >>  m4/virt-acl.m4 | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/m4/virt-acl.m4 b/m4/virt-acl.m4
> >> index f7d1c6d..7a8b8e5 100644
> >> --- a/m4/virt-acl.m4
> >> +++ b/m4/virt-acl.m4
> >> @@ -21,6 +21,10 @@ AC_DEFUN([LIBVIRT_CHECK_ACL], [
> >>  
> >>    AC_CHECK_HEADERS([sys/acl.h])
> >>  
> >> +  if test "x$ac_cv_header_sys_acl_h:x$with_qemu" = "xno:xyes"; then
> >> +    AC_MSG_ERROR([Unable to find <sys/acl.h>, required by qemu driver])
> >> +  fi
> > 
> > I understand the desire to simplify the code by assuming the libacl
> > APIs always exist, but we shouldn't do it this way. Instead we should
> > follow our normal practice of creating a src/util/viracl.{c,h} file
> > which wrap the APIs and providing no-op stubs which just raise an
> > error when the library is missing at build time.
> > 
> 
> Unfortunately, it's more complicated than that. What if your distro has
> getfacl/setfacl but missing the header files (libacl-devel)? In that
> case libvirt is built without ACL support. Now imagine you have a
> device, say /dev/kvm with 0777 and ACL entry that denies specific
> group/user. If we build without ACL support, and use namespaces we will
> in fact allow that group/user to start a domain because we just create a
> device with 0777 perms and do not copy ACL entries.
> 
> Or vice versa - I explicitly allow user under which I'm running in ACLs
> to /dev/kvm while it's perms are 0660. Lacking libacl headers would
> render me unable to use namespaces.

Your platform has libacl available so it is not difficult to fix that
by building with libacl support. We print out the configure summary
precisely so users can see if there's any libraries they forgot to
install which might be useful.

Mandating libacl will prevent use of the QEMU driver on platforms lacking
the libacl library.  I see various bug reports indicating portability
problems for libacl wrt other platforms, in particular OS-X. So IMHO it
is not acceptable to make it a mandatory requirement, when there's no
good reason for that aside beyond helping people who forget to install
-devel library packages

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