8:18 a.m.
On 05.12.2016 15:00, Daniel P. Berrange wrote:
On Thu, Nov 24, 2016 at 03:47:54PM +0100, Michal Privoznik wrote:
> Namely, virFileGetACLs, virFileSetACLs, virFileFreeACLs and
> virFileCopyACLs. These functions are going to be required when we
> are creating /dev for qemu. We have copy anything that's in
> host's /dev exactly as is. Including ACLs.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> configure.ac | 10 +++++-
> src/Makefile.am | 4 +--
> src/libvirt_private.syms | 4 +++
> src/util/virfile.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virfile.h | 11 +++++++
> 5 files changed, 107 insertions(+), 3 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 5661752..972b399 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -332,11 +332,19 @@ dnl Availability of various common headers (non-fatal if
missing).
> AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
> sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
> sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
> - libtasn1.h sys/ucred.h sys/mount.h])
> + libtasn1.h sys/ucred.h sys/mount.h sys/acl.h])
> dnl Check whether endian provides handy macros.
> AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]])
> AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
>
> +ACL_CFLAGS=""
> +ACL_LIBS=""
> +if test "x$ac_cv_header_sys_acl_h" = "xyes" ; then
> + ACL_LIBS="-lacl"
> +fi
> +AC_SUBST([ACL_CFLAGS])
> +AC_SUBST([ACL_LIBS])
> +
> dnl We need to decide at configure time if libvirt will use real atomic
> dnl operations ("lock free") or emulated ones with a mutex.
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index b358e0e..c08a350 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1125,12 +1125,12 @@ libvirt_util_la_SOURCES = \
> libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \
> $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \
> $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \
> - $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) \
> + $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) $(ACL_CFLAGS) \
> -I$(srcdir)/conf
> libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \
> $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
> $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \
> - $(SECDRIVER_LIBS) $(NUMACTL_LIBS) \
> + $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(ACL_LIBS) \
> $(POLKIT_LIBS)
>
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ae563eb..5f560bf 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1559,6 +1559,7 @@ virFileActivateDirOverride;
> virFileBindMountDevice;
> virFileBuildPath;
> virFileClose;
> +virFileCopyACLs;
> virFileDeleteTree;
> virFileDirectFdFlag;
> virFileExists;
> @@ -1568,6 +1569,8 @@ virFileFindHugeTLBFS;
> virFileFindMountPoint;
> virFileFindResource;
> virFileFindResourceFull;
> +virFileFreeACLs;
> +virFileGetACLs;
> virFileGetHugepageSize;
> virFileGetMountReverseSubtree;
> virFileGetMountSubtree;
> @@ -1602,6 +1605,7 @@ virFileResolveAllLinks;
> virFileResolveLink;
> virFileRewrite;
> virFileSanitizePath;
> +virFileSetACLs;
> virFileSetupDev;
> virFileSkipRoot;
> virFileStripSuffix;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 6200004..c777025 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -48,6 +48,9 @@
> #if HAVE_SYS_SYSCALL_H
> # include <sys/syscall.h>
> #endif
> +#if HAVE_SYS_ACL_H
> +# include <sys/acl.h>
> +#endif
>
> #ifdef __linux__
> # if HAVE_LINUX_MAGIC_H
> @@ -3573,3 +3576,81 @@ virFileBindMountDevice(const char *src ATTRIBUTE_UNUSED,
> return -1;
> }
> #endif /* !defined(HAVE_SYS_MOUNT_H) */
> +
> +
> +#if defined(HAVE_SYS_ACL_H) && !defined(LIBVIRT_NSS) &&
!defined(LIBVIRT_SETUID_RPC_CLIENT)
You can simplify this by just adding #undef HAVE_SYS_ACL_H to config-post.h
for both the NSS + SETUID blocks
Ah, will do.
> +int
> +virFileGetACLs(const char *file,
> + void **acl)
> +{
> + if (!(*acl = acl_get_file(file, ACL_TYPE_ACCESS)))
> + return -1;
> +
> + return 0;
> +}
> +
> +
> +int
> +virFileSetACLs(const char *file,
> + void *acl)
> +{
> + if (acl_set_file(file, ACL_TYPE_ACCESS, acl) < 0)
> + return -1;
> +
> + return 0;
> +}
Should we consider doing virReportSystemError in these methods
rather than letting the caller report the error, or is there
a need for the caller to ignore errors in some cases ?
I'd rather not. If a filesystem is mounted with ACLs disabled, we can
avoid spurious errors in the logs ("Hey, I couldn't copy ACLs on
/dev/blaah") even if the error is properly ignored later in the code.
And then if GetACLs() doesn't report the error, no other *ACL() function
should.
Michal