On Tue, Dec 11, 2012 at 1:08 PM, Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; wrote: 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(a)gmail.com You can also do the following with git config --edit without --global. [sendemail] to = libvir-list(a)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(a)gmail.com&gt; 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

On Tue, Dec 11, 2012 at 01:33:33PM -0600, Doug Goldstein wrote: No, the XML parser should not apply semantic checks while parsing. We've been down that route before and it wasn't pretty. Using VIR_ERR_CONFIG_UNSUPPORTED is the correct approach here. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|