Re: [libvirt] [PATCH 1/2] cgroup: fix including sys/mount.h for OpenBSD

Martin Kletzander wrote: > On Fri, Jan 08, 2016 at 10:36:07PM +0300, Roman Bogorodskiy wrote: > >From: Jasper Lievisse Adriaanse <jasper(a)openbsd.org&gt; > > > >Include sys/param.h along with sys/mount.h to fix compilation on > >OpenBSD. > > > >Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; > >--- > > src/util/vircgroup.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c > >index e39c4d1..e8e0875 100644 > >--- a/src/util/vircgroup.c > >+++ b/src/util/vircgroup.c > >@@ -27,7 +27,8 @@ > > #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R > > # include <mntent.h> > > #endif > >-#if defined HAVE_SYS_MOUNT_H > >+#if defined HAVE_SYS_PARAM_H && defined HAVE_SYS_MOUNT_H > >+# include <sys/param.h> > > # include <sys/mount.h> > > Wouldn't this cause an error for systems that have sys/mount.h and need > it for the mount() call, but don't have sys/param.h ? That's an interesting question. A check for sys/mount.h was added with the initial implementation of the mount() using code in commit 1da631e. The mount() call currently is used only on platforms with cgroups available (i.e. Linux). Moreover, HAVE_SYS_MOUNT_H does not play any role in cgroups detection: #if defined(__linux__) && defined(HAVE_GETMNTENT_R) && \ defined(_DIRENT_HAVE_D_TYPE) && defined(_SC_CLK_TCK) # define VIR_CGROUP_SUPPORTED #endif So, sys/mount.h and mount() are only needed on Linux anyway, and I'm wondering if we actually can just do something like this: #if defined HAVE_SYS_MOUNT_H && defined HAVE_GETMNTENT_R # include <sys/mount.h> #endif as if we don't have HAVE_GETMNTENT_R define, we will not have VIR_CGROUP_SUPPORTED, and therefore we'll not need the mount() call.

I've tested it on FreeBSD and OpenBSD and it builds fine. Unfortunately, cannot test it on Linux at this moment.

Roman Bogorodskiy