[libvirt] [PATCH 1/N] qemu driver FreeBSD support: fix compilation

[Doug Goldstein]

On Tue, Dec 11, 2012 at 1:08 PM, Roman Bogorodskiy
<bogorodskiy at gmail.com> wrote:
> Another round of the FreeBSD patches. Here's the first chunk to fix
> compilation.
>
> Roman Bogorodskiy
>

Not to nitpick, but if possible in the future send the patches inline
to the mailing list. You can accomplish this by doing the following:

$ git config --edit (optionally add --global for your whole user
account to change)

[sendemail]
  smtpserver = smtp.gmail.com
  smtpserverport = 587
  smtpencryption = tls
  smtpuser = bogorodskiy at gmail.com

You can also do the following with git config --edit without --global.

[sendemail]
  to = libvir-list at redhat.com

Now you can just run the following in your branch:
$ git send-email master

And your patches will go to the ML.

Now I'll paste your patch inline with some notes.

commit 4205ba22dcef12ac41cf11cf8fe2a84f42612154
Author: Roman Bogorodskiy <bogorodskiy at gmail.com>
Date:   Tue Dec 11 22:31:39 2012 +0400

    Qemu FreeBSD: fix compilation

    * Autotools changes:
       - Don't assume Qemu is Linux-only
       - Check Linux headers only on Linux
       - Disable firewalld on FreeBSD
    * Initctl:
       Initctl seem to present only on Linux, so
       stub it on other platforms
    * Raw I/O: Linux-only as well
    * Headers cleanup

diff --git a/configure.ac b/configure.ac
index a695e52..b29b65d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -360,10 +360,11 @@ dnl are also linux specific.  The "network" and
storage_fs drivers are known
 dnl to not work on MacOS X presently, so we also make a note if compiling
 dnl for that

-with_linux=no with_osx=no
+with_linux=no with_osx=no with_freebsd=no
 case $host in
   *-*-linux*) with_linux=yes ;;
   *-*-darwin*) with_osx=yes ;;
+  *-*-freebsd*) with_freebsd=yes ;;
 esac

 if test $with_linux = no; then
@@ -371,14 +372,15 @@ if test $with_linux = no; then
     then
         with_lxc=no
     fi
-    if test "x$with_qemu" != xyes
-    then
-        with_qemu=no
-    fi

This would appear to cause undesired results on Mac OS X.

     with_dtrace=no
 fi

+if test $with_freebsd = yes; then
+   with_firewalld=no
+fi
+

This should be unnecessary. We should be checking whether we want to
enable this or not and opting not to if the check fails.

 AM_CONDITIONAL([WITH_LINUX], [test "$with_linux" = "yes"])
+AM_CONDITIONAL([WITH_FREEBSD], [test "$with_freebsd" = "yes"])

 dnl Allow to build without Xen, QEMU/KVM, test or remote driver
 AC_ARG_WITH([xen],
@@ -949,9 +951,11 @@ fi
 dnl
 dnl check for kernel headers required by src/bridge.c
 dnl
-if test "$with_qemu" = "yes" || test "$with_lxc" = "yes" ; then
-  AC_CHECK_HEADERS([linux/param.h linux/sockios.h linux/if_bridge.h
linux/if_tun.h],,
-                   AC_MSG_ERROR([You must install kernel-headers in
order to compile libvirt with QEMU or LXC support]))
+if test "$with_linux" = "yes"; then
+  if test "$with_qemu" = "yes" || test "$with_lxc" = "yes" ; then
+    AC_CHECK_HEADERS([linux/param.h linux/sockios.h linux/if_bridge.h
linux/if_tun.h],,
+                     AC_MSG_ERROR([You must install kernel-headers in
order to compile libvirt with QEMU or LXC support]))
+  fi
 fi


@@ -2880,14 +2884,24 @@ if test "x$with_libblkid" = "xyes"; then
 fi
 AM_CONDITIONAL([HAVE_LIBBLKID], [test "x$with_libblkid" = "xyes"])

+if test $with_freebsd = yes; then
+  default_qemu_user=root
+  default_qemu_group=wheel
+else
+  default_qemu_user=root
+  default_qemu_group=root
+fi
+
 AC_ARG_WITH([qemu-user],
-  AC_HELP_STRING([--with-qemu-user], [username to run QEMU system
instance as @<:@default=root@:>@]),
+  AC_HELP_STRING([--with-qemu-user],
+  [username to run QEMU system instance as @<:@default=platform
dependent@:>@]),
   [QEMU_USER=${withval}],
-  [QEMU_USER=root])
+  [QEMU_USER=${default_qemu_user}])
 AC_ARG_WITH([qemu-group],
-  AC_HELP_STRING([--with-qemu-group], [groupname to run QEMU system
instance as @<:@default=root@:>@]),
+  AC_HELP_STRING([--with-qemu-group],
+  [groupname to run QEMU system instance as @<:@default=platform
dependent@:>@]),
   [QEMU_GROUP=${withval}],
-  [QEMU_GROUP=root])
+  [QEMU_GROUP=${default_qemu_group}])
 AC_DEFINE_UNQUOTED([QEMU_USER], ["$QEMU_USER"], [QEMU user account])
 AC_DEFINE_UNQUOTED([QEMU_GROUP], ["$QEMU_GROUP"], [QEMU group account])

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8d380a1..e95609c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -34,7 +34,6 @@
 #include <sys/wait.h>
 #include <arpa/inet.h>
 #include <sys/utsname.h>
-#include <mntent.h>

 #include "virterror_internal.h"
 #include "qemu_conf.h"

This probably belongs in its own commit.

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab04599..5d355eb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -27,7 +27,12 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/resource.h>
+#if defined(__linux__)
 #include <linux/capability.h>
+#elif defined(__FreeBSD__)
+#include <sys/param.h>
+#include <sys/cpuset.h>
+#endif

 #include "qemu_process.h"
 #include "qemu_domain.h"
@@ -3707,7 +3712,12 @@ int qemuProcessStart(virConnectPtr conn,
     /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */
     for (i = 0; i < vm->def->ndisks; i++) {
         if (vm->def->disks[i]->rawio == 1)
+#ifdef CAP_SYS_RAWIO
             virCommandAllowCap(cmd, CAP_SYS_RAWIO);
+#else
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("Raw I/O is not supported on this platform"));
+#endif
     }

It probably would be better to not allow VMs to be defined with rawio
enabled if this the case. You'll want to make these changes in
src/conf/domain_conf.c


     virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData);
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index cdd3dc0..ae2f525 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -35,6 +35,7 @@

 #define VIR_FROM_THIS VIR_FROM_INITCTL

+#if defined(__linux__) || (defined(__FreeBSD_kernel__) &&
!(defined(__FreeBSD__)))
 /* These constants & struct definitions are taken from
  * systemd, under terms of LGPLv2+
  *
@@ -161,3 +162,11 @@ cleanup:
     VIR_FORCE_CLOSE(fd);
     return ret;
 }
+#else
+int virInitctlSetRunLevel(virInitctlRunLevel level ATTRIBUTE_UNUSED,
+                          const char *vroot ATTRIBUTE_UNUSED)
+{
+   virReportError(VIR_ERR_NO_SUPPORT, "%s", __FUNCTION__);
+   return -1;
+}
+#endif

Are these changes really necessary? This is from SystemD and it talks
about BSD in the source.

-- 
Doug Goldstein