[libvirt] [PATCH 0/4] Support auditing of guests

This patch series introduces basic support for auditing of guest operations. The auditing hooks are primarily done in the libvirtd dispatch layer, because we want to hook all stateful drivers like QEMU, LXC, UML, etc. We don't want to audit the remote driver, VMWare, XenAPI etc which are just plain RPC drivers. There is an exception for auditing of the SELinux label assignment. That has to be done right inside the sVirt code since the neccessary info isn't available in the libvirtd dispatch layer. This patch series focuses on lifecycle operations, but there are quite alot of other things that are desirable to audit in the future, so further patches will likely follow. The last patch is semi-related, it fixes up a major screwup in the linking of the daemon that caused duplicated copies of the code to be linked. This was exposed by the audit work. The patches are a combination of work by myself and Miloslav NB, it should compile and run fine with any reasonably recent audit package, but if you want correctly identified log messages you need audit 2.0.5 Also, audit logs only appear if running libvirtd as root. Non root users don't have permissions to generate audit logs. Daniel

Integrate with libaudit.so for auditing of important operations. libvirtd gains a couple of config entries for auditing. By default it will enable auditing, if its enabled on the host. It can be configured to force exit if auditing is disabled on the host. It will can also send audit messages via libvirt internal logging API Places requiring audit reporting can use the VIR_AUDIT macro to report data. This is a no-op unless auditing is enabled * autobuild.sh, mingw32-libvirt.spec.in: Disable audit on mingw * configure.ac: Add check for libaudit * daemon/libvirtd.aug, daemon/libvirtd.conf, daemon/test_libvirtd.aug, daemon/libvirtd.c: Add config options to enable auditing * include/libvirt/virterror.h, src/util/virterror.c: Add VIR_FROM_AUDIT source * libvirt.spec.in: Enable audit * src/util/virtaudit.h, src/util/virtaudit.c: Simple internal API for auditing messages --- autobuild.sh | 1 + configure.ac | 51 +++++++++++++++ daemon/libvirtd.aug | 4 + daemon/libvirtd.c | 21 ++++++- daemon/libvirtd.conf | 19 ++++++ daemon/test_libvirtd.aug | 6 ++ include/libvirt/virterror.h | 1 + libvirt.spec.in | 13 ++++ mingw32-libvirt.spec.in | 1 + po/POTFILES.in | 1 + src/Makefile.am | 9 ++- src/util/virtaudit.c | 144 +++++++++++++++++++++++++++++++++++++++++++ src/util/virtaudit.h | 55 ++++++++++++++++ src/util/virterror.c | 3 + 14 files changed, 325 insertions(+), 4 deletions(-) create mode 100644 src/util/virtaudit.c create mode 100644 src/util/virtaudit.h diff --git a/autobuild.sh b/autobuild.sh index c527479..844ce53 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -85,6 +85,7 @@ if [ -x /usr/bin/i686-pc-mingw32-gcc ]; then --without-one \ --without-phyp \ --without-netcf \ + --without-audit \ --without-libvirtd make diff --git a/configure.ac b/configure.ac index bd92b65..5bf31da 100644 --- a/configure.ac +++ b/configure.ac @@ -911,6 +911,52 @@ AM_CONDITIONAL([HAVE_AVAHI], [test "x$with_avahi" = "xyes"]) AC_SUBST([AVAHI_CFLAGS]) AC_SUBST([AVAHI_LIBS]) +dnl Audit library +AC_ARG_WITH([audit], + AC_HELP_STRING([--with-audit], [use audit library @<:@default=check@:>@]), + [], + [with_audit=check]) + +AUDIT_CFLAGS= +AUDIT_LIBS= +if test "$with_audit" != "no" ; then + old_cflags="$CFLAGS" + old_libs="$LIBS" + if test "$with_audit" != "check" && "$with_audit" != "yes" ; then + AUDIT_CFLAGS="-I$with_audit/include" + AUDIT_LIBS="-L$with_audit/lib" + fi + CFLAGS="$CFLAGS $AUDIT_CFLAGS" + LIBS="$LIBS $AUDIT_LIBS" + fail=0 + AC_CHECK_HEADER([libaudit.h], [], [fail=1]) + AC_CHECK_LIB([audit], [audit_is_enabled], [], [fail=1]) + + if test $fail = 1 ; then + if test "$with_audit" = "yes" ; then + AC_MSG_ERROR([You must install the Audit library in order to compile and run libvirt]) + else + with_audit=no + AUDIT_CFLAGS= + AUDIT_LIBS= + fi + else + with_audit=yes + fi + + if test "$with_audit" = "yes" ; then + AUDIT_LIBS="$AUDIT_LIBS -laudit" + AC_DEFINE_UNQUOTED([HAVE_AUDIT], 1, [whether libaudit is available]) + fi + + CFLAGS="$old_cflags" + LIBS="$old_libs" +fi +AM_CONDITIONAL([HAVE_AUDIT], [test "$with_audit" = "yes"]) +AC_SUBST([AUDIT_CFLAGS]) +AC_SUBST([AUDIT_LIBS]) + + dnl SELinux AC_ARG_WITH([selinux], AC_HELP_STRING([--with-selinux], [use SELinux to manage security @<:@default=check@:>@]), @@ -2273,6 +2319,11 @@ fi else AC_MSG_NOTICE([ polkit: no]) fi +if test "$with_audit" = "yes" ; then +AC_MSG_NOTICE([ audit: $AUDIT_CFLAGS $AUDIT_LIBS]) +else +AC_MSG_NOTICE([ audit: no]) +fi if test "$with_selinux" = "yes" ; then AC_MSG_NOTICE([ selinux: $SELINUX_CFLAGS $SELINUX_LIBS]) else diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug index 7406d23..0e06142 100644 --- a/daemon/libvirtd.aug +++ b/daemon/libvirtd.aug @@ -61,6 +61,9 @@ module Libvirtd = | str_entry "log_filters" | str_entry "log_outputs" + let auditing_entry = int_entry "audit_level" + | bool_entry "audit_logging" + (* Each enty in the config is one of the following three ... *) let entry = network_entry | sock_acl_entry @@ -69,6 +72,7 @@ module Libvirtd = | authorization_entry | processing_entry | logging_entry + | auditing_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 1543481..88e85ec 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -64,6 +64,7 @@ #include "memory.h" #include "stream.h" #include "hooks.h" +#include "virtaudit.h" #ifdef HAVE_AVAHI # include "mdns.h" #endif @@ -187,6 +188,9 @@ static int max_requests = 20; /* Total number of 'in-process' RPC calls allowed by a single client*/ static int max_client_requests = 5; +static int audit_level = 1; +static int audit_logging = 0; + #define DH_BITS 1024 static sig_atomic_t sig_errors = 0; @@ -203,6 +207,7 @@ enum { VIR_DAEMON_ERR_NETWORK, VIR_DAEMON_ERR_CONFIG, VIR_DAEMON_ERR_HOOKS, + VIR_DAEMON_ERR_AUDIT, VIR_DAEMON_ERR_LAST }; @@ -217,7 +222,8 @@ VIR_ENUM_IMPL(virDaemonErr, VIR_DAEMON_ERR_LAST, "Unable to drop privileges", "Unable to initialize network sockets", "Unable to load configuration file", - "Unable to look for hook scripts") + "Unable to look for hook scripts", + "Unable to initialize audit system") static void sig_handler(int sig, siginfo_t * siginfo, void* context ATTRIBUTE_UNUSED) { @@ -2854,6 +2860,9 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) GET_CONF_INT (conf, filename, max_requests); GET_CONF_INT (conf, filename, max_client_requests); + GET_CONF_INT (conf, filename, audit_level); + GET_CONF_INT (conf, filename, audit_logging); + GET_CONF_STR (conf, filename, host_uuid); if (virSetHostUUIDStr(host_uuid)) { VIR_ERROR(_("invalid host UUID: %s"), host_uuid); @@ -3194,6 +3203,16 @@ int main(int argc, char **argv) { goto error; } + if (audit_level) { + if (virAuditOpen() < 0) { + if (audit_level > 1) { + ret = VIR_DAEMON_ERR_AUDIT; + goto error; + } + } + } + virAuditLog(audit_logging); + /* setup the hooks if any */ if (virHookInitialize() < 0) { ret = VIR_DAEMON_ERR_HOOKS; diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index d11c0fb..163a80f 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -313,6 +313,25 @@ # log_outputs="3:syslog:libvirtd" # to log all warnings and errors to syslog under the libvirtd ident + +################################################################## +# +# Auditing +# +# This setting allows usage of the auditing subsystem to be altered: +# +# audit_level == 0 -> disable all auditing +# audit_level == 1 -> enable auditing, only if enabled on host (default) +# audit_level == 2 -> enable auditing, and exit if disabled on host +# +#audit_level = 2 +# +# If set to 1, then audit messages will also be sent +# via libvirt logging infrastructure. Defaults to 0 +# +#audit_logging = 1 + +################################################################### # UUID of the host: # Provide the UUID of the host here in case the command # 'dmidecode -s system-uuid' does not provide a valid uuid. In case diff --git a/daemon/test_libvirtd.aug b/daemon/test_libvirtd.aug index b8da28e..5f8b644 100644 --- a/daemon/test_libvirtd.aug +++ b/daemon/test_libvirtd.aug @@ -268,6 +268,9 @@ log_outputs=\"4:stderr\" # Logging filters: log_filters=\"a\" + +# Auditing: +audit_level = 2 " test Libvirtd.lns get conf = @@ -543,3 +546,6 @@ log_filters=\"a\" { "#empty" } { "#comment" = "Logging filters:" } { "log_filters" = "a" } + { "#empty" } + { "#comment" = "Auditing:" } + { "audit_level" = "2" } diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 3bbb293..94d686c 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -73,6 +73,7 @@ typedef enum { VIR_FROM_NWFILTER, /* Error from network filter driver */ VIR_FROM_HOOK, /* Error from Synchronous hooks */ VIR_FROM_DOMAIN_SNAPSHOT, /* Error from domain snapshot */ + VIR_FROM_AUDIT /* Error from auditing subsystem */ } virErrorDomain; diff --git a/libvirt.spec.in b/libvirt.spec.in index a58be54..93ac288 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -65,6 +65,7 @@ %define with_libpcap 0%{!?_without_libpcap:0} %define with_macvtap 0%{!?_without_macvtap:0} %define with_libnl 0%{!?_without_libnl:0} +%define with_audit 0%{!?_without_audit:0} # Non-server/HV driver defaults which are always enabled %define with_python 0%{!?_without_python:1} @@ -162,6 +163,10 @@ %define with_libnl 1 %endif +%if 0%{?fedora} >= 11 || 0%{?rhel} >= 5 +%define with_audit 0%{!?_without_audit:1} +%endif + # Force QEMU to run as non-root %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 %define qemu_user qemu @@ -367,6 +372,9 @@ BuildRequires: netcf-devel >= 0.1.4 %if %{with_esx} BuildRequires: libcurl-devel %endif +%if %{with_audit} +BuildRequires: audit-libs-devel +%endif # Fedora build root suckage BuildRequires: gawk @@ -545,6 +553,10 @@ of recent versions of Linux (and other OSes). %define _without_macvtap --without-macvtap %endif +%if ! %{with_audit} +%define _without_audit --without-audit +%endif + %configure %{?_without_xen} \ %{?_without_qemu} \ %{?_without_openvz} \ @@ -575,6 +587,7 @@ of recent versions of Linux (and other OSes). %{?_without_yajl} \ %{?_without_libpcap} \ %{?_without_macvtap} \ + %{?_without_audit} \ --with-qemu-user=%{qemu_user} \ --with-qemu-group=%{qemu_group} \ --with-init-script=redhat \ diff --git a/mingw32-libvirt.spec.in b/mingw32-libvirt.spec.in index 4bbbc3b..e762c77 100644 --- a/mingw32-libvirt.spec.in +++ b/mingw32-libvirt.spec.in @@ -57,6 +57,7 @@ MinGW Windows libvirt virtualization library. --without-one \ --without-phyp \ --without-netcf \ + --without-audit \ --without-libvirtd make diff --git a/po/POTFILES.in b/po/POTFILES.in index 8a148f3..e30fea0 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -88,6 +88,7 @@ src/util/processinfo.c src/util/stats_linux.c src/util/storage_file.c src/util/util.c +src/util/virtaudit.c src/util/virterror.c src/util/xml.c src/vbox/vbox_driver.c diff --git a/src/Makefile.am b/src/Makefile.am index 9bc4287..8ec8230 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -82,6 +82,7 @@ UTIL_SOURCES = \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ util/xml.c util/xml.h \ + util/virtaudit.c util/virtaudit.h \ util/virterror.c util/virterror_internal.h EXTRA_DIST += util/threads-pthread.c util/threads-win32.c @@ -425,8 +426,9 @@ libvirt_la_BUILT_LIBADD = libvirt_util.la libvirt_util_la_SOURCES = \ $(UTIL_SOURCES) libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ - $(AM_CFLAGS) -libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) $(LIB_PTHREAD) + $(AM_CFLAGS) $(AUDIT_CFLAGS) +libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ + $(LIB_PTHREAD) $(AUDIT_LIBS) noinst_LTLIBRARIES += libvirt_conf.la @@ -1119,12 +1121,13 @@ libvirt_lxc_SOURCES = \ libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS) libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \ $(LIBXML_LIBS) $(NUMACTL_LIBS) $(LIB_PTHREAD) \ - $(LIBNL_LIBS) ../gnulib/lib/libgnu.la + $(LIBNL_LIBS) $(AUDIT_LIBS) ../gnulib/lib/libgnu.la libvirt_lxc_CFLAGS = \ $(LIBPARTED_CFLAGS) \ $(NUMACTL_CFLAGS) \ $(CAPNG_CFLAGS) \ $(YAJL_CFLAGS) \ + $(AUDIT_CFLAGS) \ -I@top_srcdir@/src/conf \ $(AM_CFLAGS) endif diff --git a/src/util/virtaudit.c b/src/util/virtaudit.c new file mode 100644 index 0000000..036a8b9 --- /dev/null +++ b/src/util/virtaudit.c @@ -0,0 +1,144 @@ +/* + * audit.h: auditing support + * + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#ifdef HAVE_AUDIT +# include <libaudit.h> +#endif +#include <stdio.h> +#include <unistd.h> + +#include "virterror_internal.h" +#include "logging.h" +#include "virtaudit.h" + +/* Provide the macros in case the header file is old. + FIXME: should be removed. */ +#ifndef AUDIT_VIRT_CONTROL +# define AUDIT_VIRT_CONTROL 2500 /* Start, Pause, Stop VM */ +#endif +#ifndef AUDIT_VIRT_RESOURCE +# define AUDIT_VIRT_RESOURCE 2501 /* Resource assignment */ +#endif +#ifndef AUDIT_VIRT_MACHINE_ID +# define AUDIT_VIRT_MACHINE_ID 2502 /* Binding of label to VM */ +#endif + +#define VIR_FROM_THIS VIR_FROM_AUDIT + +#if HAVE_AUDIT +static int auditfd = -1; +#endif +static int auditlog = 0; + +int virAuditOpen(void) +{ +#if HAVE_AUDIT + if ((auditfd = audit_open()) < 0) { + virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); + return -1; + } + + return 0; +#else + return -1; +#endif +} + + +void virAuditLog(int logging) +{ + auditlog = logging; +} + + +#if HAVE_AUDIT +void virAuditSend(const char *file ATTRIBUTE_UNUSED, const char *func, size_t linenr, + const char *clienttty, const char *clientaddr, + enum virAuditRecordType type, bool success, + const char *fmt, ...) +#else +void virAuditSend(const char *file ATTRIBUTE_UNUSED, const char *func, size_t linenr, + const char *clienttty ATTRIBUTE_UNUSED, + const char *clientaddr ATTRIBUTE_UNUSED, + enum virAuditRecordType type, bool success, + const char *fmt, ...) +#endif +{ + char *str = NULL; + va_list args; + + /* Duplicate later checks, to short circuit & avoid printf overhead + * when nothing is enabled */ +#if HAVE_AUDIT + if (!auditlog && auditfd < 0) + return; +#else + if (!auditlog) + return; +#endif + + va_start(args, fmt); + if (vasprintf(&str, fmt, args) < 0) { + VIR_WARN0("Out of memory while formatting audit message"); + str = NULL; + } + va_end(args); + + if (auditlog && str) { + if (success) + virLogMessage("audit", VIR_LOG_INFO, func, linenr, 0, + "success=yes %s", str); + else + virLogMessage("audit", VIR_LOG_WARN, func, linenr, 0, + "success=no %s", str); + } + +#if HAVE_AUDIT + if (auditfd < 0) + return; + + if (str) { + static const int record_types[] = { + [VIR_AUDIT_RECORD_MACHINE_CONTROL] = AUDIT_VIRT_CONTROL, + [VIR_AUDIT_RECORD_MACHINE_ID] = AUDIT_VIRT_MACHINE_ID, + [VIR_AUDIT_RECORD_RESOURCE] = AUDIT_VIRT_RESOURCE, + }; + + if (type > ARRAY_CARDINALITY(record_types) || record_types[type] == 0) + VIR_WARN("Unknown audit record type %d", type); + else if (audit_log_user_message(auditfd, record_types[type], str, NULL, + clientaddr, clienttty, success) < 0) { + char ebuf[1024]; + VIR_WARN("Failed to send audit message %s: %s", + NULLSTR(str), virStrerror(errno, ebuf, sizeof ebuf)); + } + } +#endif +} + +void virAuditClose(void) +{ +#if HAVE_AUDIT + close(auditfd); +#endif +} diff --git a/src/util/virtaudit.h b/src/util/virtaudit.h new file mode 100644 index 0000000..b0cb707 --- /dev/null +++ b/src/util/virtaudit.h @@ -0,0 +1,55 @@ +/* + * audit.h: auditing support + * + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + + +#ifndef __LIBVIRT_AUDIT_H__ +# define __LIBVIRT_AUDIT_H__ + +# include "internal.h" +# include <stdbool.h> + +enum virAuditRecordType { + VIR_AUDIT_RECORD_MACHINE_CONTROL, + VIR_AUDIT_RECORD_MACHINE_ID, + VIR_AUDIT_RECORD_RESOURCE, +}; + +int virAuditOpen(void); + +void virAuditLog(int enabled); + +void virAuditSend(const char *file, const char *func, size_t linenr, + const char *clienttty, const char *clientaddr, + enum virAuditRecordType type, bool success, + const char *fmt, ...); + +void virAuditClose(void); + +# define VIR_AUDIT(type, success, ...) \ + virAuditSend(__FILE__, __func__, __LINE__, \ + NULL, NULL, type, success, __VA_ARGS__); + +# define VIR_AUDIT_USER(type, success, clienttty, clientaddr, ...) \ + virAuditSend(__FILE__, __func__, __LINE__, \ + clienttty, clientaddr, type, success, __VA_ARGS__); + + +#endif /* __LIBVIRT_AUDIT_H__ */ diff --git a/src/util/virterror.c b/src/util/virterror.c index 9f632ec..70749a7 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -187,6 +187,9 @@ static const char *virErrorDomainName(virErrorDomain domain) { case VIR_FROM_DOMAIN_SNAPSHOT: dom = "Domain Snapshot "; break; + case VIR_FROM_AUDIT: + dom = "Audit"; + break; } return(dom); } -- 1.7.2.3

From: Miloslav Trmač <mitr@redhat.com> Most operations are audited at the libvirtd level; auditing in src/libvirt.c would result in two audit entries per operation (one in the client, one in libvirtd). The only exception is a domain stopping of its own will (e.g. because the user clicks on "shutdown" inside the interface). There can often be no client connected at the time the domain stops, so libvirtd does not have any virConnectPtr object on which to attach an event watch. This patch therefore adds auditing directly inside the qemu driver (other drivers are not supported). --- daemon/remote.c | 135 ++++++++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_driver.c | 8 +++ 2 files changed, 133 insertions(+), 10 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 6b67678..30c9031 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -57,6 +57,8 @@ #include "memory.h" #include "util.h" #include "stream.h" +#include "uuid.h" +#include "virtaudit.h" #include "libvirt/libvirt-qemu.h" #define VIR_FROM_THIS VIR_FROM_REMOTE @@ -1213,6 +1215,8 @@ remoteDispatchDomainCreate (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -1220,11 +1224,18 @@ remoteDispatchDomainCreate (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainCreate (dom) == -1) { + r = virDomainCreate(dom); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=start name=%s uuid=%s", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; } + virDomainFree(dom); return 0; } @@ -1239,6 +1250,8 @@ remoteDispatchDomainCreateWithFlags (struct qemud_server *server ATTRIBUTE_UNUSE remote_domain_create_with_flags_ret *ret) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -1246,7 +1259,15 @@ remoteDispatchDomainCreateWithFlags (struct qemud_server *server ATTRIBUTE_UNUSE return -1; } - if (virDomainCreateWithFlags (dom, args->flags) == -1) { + r = virDomainCreateWithFlags(dom, args->flags); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=%s name=%s uuid=%s", + (args->flags & VIR_DOMAIN_START_PAUSED) != + 0 ? "start-paused" : "start", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; @@ -1267,13 +1288,20 @@ remoteDispatchDomainCreateXml (struct qemud_server *server ATTRIBUTE_UNUSED, remote_domain_create_xml_ret *ret) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; dom = virDomainCreateXML (conn, args->xml_desc, args->flags); if (dom == NULL) { + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 0, + "op=start name=? uuid=?"); remoteDispatchConnError(rerr, conn); return -1; } + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 1, "op=start name=%s uuid=%s", + dom->name, uuidstr); + make_nonnull_domain (&ret->dom, dom); virDomainFree(dom); @@ -1313,6 +1341,8 @@ remoteDispatchDomainDestroy (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -1320,7 +1350,13 @@ remoteDispatchDomainDestroy (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainDestroy (dom) == -1) { + r = virDomainDestroy(dom); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=stop name=%s uuid=%s", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; @@ -1778,6 +1814,8 @@ remoteDispatchDomainMigratePrepare (struct qemud_server *server ATTRIBUTE_UNUSED r = virDomainMigratePrepare (conn, &cookie, &cookielen, uri_in, uri_out, args->flags, dname, args->resource); + /* This creates a VM, but we don't audit it until the migration succeeds + and the VM actually starts. */ if (r == -1) { VIR_FREE(uri_out); remoteDispatchConnError(rerr, conn); @@ -1810,7 +1848,7 @@ remoteDispatchDomainMigratePerform (struct qemud_server *server ATTRIBUTE_UNUSED { int r; virDomainPtr dom; - char *dname; + char *dname, uuidstr[VIR_UUID_STRING_BUFLEN]; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -1825,6 +1863,11 @@ remoteDispatchDomainMigratePerform (struct qemud_server *server ATTRIBUTE_UNUSED args->cookie.cookie_len, args->uri, args->flags, dname, args->resource); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=migrate-out name=%s uuid=%s", dom->name, uuidstr); + virDomainFree (dom); if (r == -1) { remoteDispatchConnError(rerr, conn); @@ -1844,18 +1887,27 @@ remoteDispatchDomainMigrateFinish (struct qemud_server *server ATTRIBUTE_UNUSED, remote_domain_migrate_finish_ret *ret) { virDomainPtr ddom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; CHECK_CONN (client); + /* Note that we are not able to audit "op=migrate-in" here if + VIR_DRV_FEATURE_MIGRATION_DIRECT is used. */ ddom = virDomainMigrateFinish (conn, args->dname, args->cookie.cookie_val, args->cookie.cookie_len, args->uri, args->flags); if (ddom == NULL) { + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 0, + "op=migrate-in name=%s uuid=?", args->dname); remoteDispatchConnError(rerr, conn); return -1; } + virUUIDFormat(ddom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 1, + "op=migrate-in name=%s uuid=%s", ddom->name, uuidstr); + make_nonnull_domain (&ret->ddom, ddom); virDomainFree (ddom); return 0; @@ -1891,6 +1943,8 @@ remoteDispatchDomainMigratePrepare2 (struct qemud_server *server ATTRIBUTE_UNUSE uri_in, uri_out, args->flags, dname, args->resource, args->dom_xml); + /* This creates a VM, but we don't audit it until the migration succeeds + and the VM actually starts. */ if (r == -1) { remoteDispatchConnError(rerr, conn); return -1; @@ -1916,8 +1970,11 @@ remoteDispatchDomainMigrateFinish2 (struct qemud_server *server ATTRIBUTE_UNUSED remote_domain_migrate_finish2_ret *ret) { virDomainPtr ddom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; CHECK_CONN (client); + /* Note that we are not able to audit "op=migrate-in" here if + VIR_DRV_FEATURE_MIGRATION_DIRECT is used. */ ddom = virDomainMigrateFinish2 (conn, args->dname, args->cookie.cookie_val, args->cookie.cookie_len, @@ -1925,10 +1982,16 @@ remoteDispatchDomainMigrateFinish2 (struct qemud_server *server ATTRIBUTE_UNUSED args->flags, args->retcode); if (ddom == NULL) { + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 0, + "op=migrate-in name=%s uuid=?", args->dname); remoteDispatchConnError(rerr, conn); return -1; } + virUUIDFormat(ddom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 1, + "op=migrate-in name=%s uuid=%s", ddom->name, uuidstr); + make_nonnull_domain (&ret->ddom, ddom); virDomainFree (ddom); @@ -1960,6 +2023,8 @@ remoteDispatchDomainMigratePrepareTunnel(struct qemud_server *server ATTRIBUTE_U r = virDomainMigratePrepareTunnel(conn, stream->st, args->flags, dname, args->resource, args->dom_xml); + /* This creates a VM, but we don't audit it until the migration succeeds + and the VM actually starts. */ if (r == -1) { remoteFreeClientStream(client, stream); remoteDispatchConnError(rerr, conn); @@ -2166,8 +2231,15 @@ remoteDispatchDomainRestore (struct qemud_server *server ATTRIBUTE_UNUSED, remote_domain_restore_args *args, void *ret ATTRIBUTE_UNUSED) { + int r; + + r = virDomainRestore(conn, args->from); - if (virDomainRestore (conn, args->from) == -1) { + /* We don't have enough information! */ + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=start name=? uuid=? file=%s", args->from); + + if (r == -1) { remoteDispatchConnError(rerr, conn); return -1; } @@ -2185,6 +2257,8 @@ remoteDispatchDomainResume (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -2192,7 +2266,13 @@ remoteDispatchDomainResume (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainResume (dom) == -1) { + r = virDomainResume(dom); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 1, + "op=resume name=%s uuid=%s", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; @@ -2211,6 +2291,8 @@ remoteDispatchDomainSave (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -2218,7 +2300,13 @@ remoteDispatchDomainSave (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainSave (dom, args->to) == -1) { + r = virDomainSave(dom, args->to); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=stop name=%s uuid=%s", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; @@ -2237,6 +2325,7 @@ remoteDispatchDomainCoreDump (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -2244,7 +2333,17 @@ remoteDispatchDomainCoreDump (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainCoreDump (dom, args->to, args->flags) == -1) { + r = virDomainCoreDump(dom, args->to, args->flags); + + if ((args->flags & VIR_DUMP_CRASH) != 0) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=stop name=%s uuid=%s", dom->name, uuidstr); + } + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; @@ -2393,6 +2492,8 @@ remoteDispatchDomainSuspend (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -2400,7 +2501,13 @@ remoteDispatchDomainSuspend (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainSuspend (dom) == -1) { + r = virDomainSuspend(dom); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 1, + "op=suspend name=%s uuid=%s", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; @@ -2512,6 +2619,8 @@ remoteDispatchDomainManagedSave (struct qemud_server *server ATTRIBUTE_UNUSED, void *ret ATTRIBUTE_UNUSED) { virDomainPtr dom; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + int r; dom = get_nonnull_domain (conn, args->dom); if (dom == NULL) { @@ -2519,7 +2628,13 @@ remoteDispatchDomainManagedSave (struct qemud_server *server ATTRIBUTE_UNUSED, return -1; } - if (virDomainManagedSave (dom, args->flags) == -1) { + r = virDomainManagedSave(dom, args->flags); + + virUUIDFormat(dom->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, r != -1, + "op=stop name=%s uuid=%s", dom->name, uuidstr); + + if (r == -1) { virDomainFree(dom); remoteDispatchConnError(rerr, conn); return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 25695df..b634347 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -79,6 +79,7 @@ #include "domain_nwfilter.h" #include "hooks.h" #include "storage_file.h" +#include "virtaudit.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -905,8 +906,15 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int hasError) { struct qemud_driver *driver = qemu_driver; virDomainEventPtr event = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); + + /* If the domain stops of its own will, we wouldn't audit it otherwise. */ + virUUIDFormat(vm->def->uuid, uuidstr); + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_CONTROL, 1, + "op=stopped name=%s uuid=%s", vm->def->name, uuidstr); + virDomainObjLock(vm); event = virDomainEventNewFromObj(vm, -- 1.7.2.3

On Tue, Oct 12, 2010 at 06:32:16PM +0100, Daniel P. Berrange wrote:
From: Miloslav Trmač <mitr@redhat.com>
Most operations are audited at the libvirtd level; auditing in src/libvirt.c would result in two audit entries per operation (one in the client, one in libvirtd).
The only exception is a domain stopping of its own will (e.g. because the user clicks on "shutdown" inside the interface). There can often be no client connected at the time the domain stops, so libvirtd does not have any virConnectPtr object on which to attach an event watch. This patch therefore adds auditing directly inside the qemu driver (other drivers are not supported).
Looks fine but using base64 transfer encoding: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com Status: RO RnJvbTogTWlsb3NsYXYgVHJtYcSNIDxtaXRyQHJlZGhhdC5jb20+CgpNb3N0IG9wZXJhdGlvbnMg YXJlIGF1ZGl0ZWQgYXQgdGhlIGxpYnZpcnRkIGxldmVsOyBhdWRpdGluZyBpbgpzcmMvbGlidmly makes applying the patch way harder than it should. I wonder why mails 2, 3 and 4 got the problem nut not 1/4 .... puzzled, could you have a look ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Oct 14, 2010 at 04:09:41PM +0200, Daniel Veillard wrote:
On Tue, Oct 12, 2010 at 06:32:16PM +0100, Daniel P. Berrange wrote:
From: Miloslav Trmač <mitr@redhat.com>
Most operations are audited at the libvirtd level; auditing in src/libvirt.c would result in two audit entries per operation (one in the client, one in libvirtd).
The only exception is a domain stopping of its own will (e.g. because the user clicks on "shutdown" inside the interface). There can often be no client connected at the time the domain stops, so libvirtd does not have any virConnectPtr object on which to attach an event watch. This patch therefore adds auditing directly inside the qemu driver (other drivers are not supported).
Looks fine but using base64 transfer encoding:
Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com Status: RO
RnJvbTogTWlsb3NsYXYgVHJtYcSNIDxtaXRyQHJlZGhhdC5jb20+CgpNb3N0IG9wZXJhdGlvbnMg YXJlIGF1ZGl0ZWQgYXQgdGhlIGxpYnZpcnRkIGxldmVsOyBhdWRpdGluZyBpbgpzcmMvbGlidmly
makes applying the patch way harder than it should. I wonder why mails 2, 3 and 4 got the problem nut not 1/4 .... puzzled, could you have a look ?
I just used git send-email as normal. It is probably the magic characters in Miloslav's name that convinced git to change to a diffrent content encoding Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Oct 14, 2010 at 03:16:42PM +0100, Daniel P. Berrange wrote:
On Thu, Oct 14, 2010 at 04:09:41PM +0200, Daniel Veillard wrote:
On Tue, Oct 12, 2010 at 06:32:16PM +0100, Daniel P. Berrange wrote:
From: Miloslav Trmač <mitr@redhat.com>
Most operations are audited at the libvirtd level; auditing in src/libvirt.c would result in two audit entries per operation (one in the client, one in libvirtd).
The only exception is a domain stopping of its own will (e.g. because the user clicks on "shutdown" inside the interface). There can often be no client connected at the time the domain stops, so libvirtd does not have any virConnectPtr object on which to attach an event watch. This patch therefore adds auditing directly inside the qemu driver (other drivers are not supported).
Looks fine but using base64 transfer encoding:
Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com Status: RO
RnJvbTogTWlsb3NsYXYgVHJtYcSNIDxtaXRyQHJlZGhhdC5jb20+CgpNb3N0IG9wZXJhdGlvbnMg YXJlIGF1ZGl0ZWQgYXQgdGhlIGxpYnZpcnRkIGxldmVsOyBhdWRpdGluZyBpbgpzcmMvbGlidmly
makes applying the patch way harder than it should. I wonder why mails 2, 3 and 4 got the problem nut not 1/4 .... puzzled, could you have a look ?
I just used git send-email as normal. It is probably the magic characters in Miloslav's name that convinced git to change to a diffrent content encoding
Ahhh, well with vim selecting the block and using :'<,'>!base64 -d does the trick, but it slows things down Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Miloslav Trmač <mitr@redhat.com> A more natural auditing point would perhaps be SELinuxSetSecurityProcessLabel, but this happens in the child after root permissions are dropped, so the kernel would refuse the audit record. --- src/security/security_selinux.c | 22 ++++++++++++++++++---- 1 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a9dd836..0995d67 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -28,6 +28,8 @@ #include "pci.h" #include "hostusb.h" #include "storage_file.h" +#include "uuid.h" +#include "virtaudit.h" #define VIR_FROM_THIS VIR_FROM_SECURITY @@ -160,20 +162,22 @@ SELinuxGenSecurityLabel(virSecurityDriverPtr drv ATTRIBUTE_UNUSED, virDomainObjPtr vm) { int rc = -1; - char mcs[1024]; + char mcs[1024], uuidstr[VIR_UUID_STRING_BUFLEN]; char *scontext = NULL; int c1 = 0; int c2 = 0; - if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) - return 0; + if (vm->def->seclabel.type == VIR_DOMAIN_SECLABEL_STATIC) { + rc = 0; + goto done; + } if (vm->def->seclabel.label || vm->def->seclabel.model || vm->def->seclabel.imagelabel) { virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("security label already defined for VM")); - return rc; + goto done; } do { @@ -217,6 +221,16 @@ err: VIR_FREE(vm->def->seclabel.model); done: VIR_FREE(scontext); + + virUUIDFormat(vm->def->uuid, uuidstr); + /* The derived socket context is not audited. */ +#define STR(X) ((X) != NULL ? (X) : "?") + VIR_AUDIT(VIR_AUDIT_RECORD_MACHINE_ID, rc == 0, + "name=%s uuid=%s process-context=%s image-context=%s", + vm->def->name, uuidstr, STR(vm->def->seclabel.label), + STR(vm->def->seclabel.imagelabel)); +#undef STR + return rc; } -- 1.7.2.3

From: Miloslav Trmač <mitr@redhat.com> The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables. Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions & export them, just linkage all loadable modules against libgnu.la statically. * daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers --- daemon/Makefile.am | 6 ++---- src/Makefile.am | 24 +++++++++++++++++++----- src/libvirt.c | 14 ++++++++++++++ src/libvirt_private.syms | 45 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 9 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b020b77..987133c 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -99,11 +99,9 @@ libvirtd_LDADD = \ $(SASL_LIBS) \ $(POLKIT_LIBS) -libvirtd_LDADD += ../src/libvirt_util.la ../src/libvirt-qemu.la +libvirtd_LDADD += ../src/libvirt-qemu.la -if WITH_DRIVER_MODULES - libvirtd_LDADD += ../src/libvirt_driver.la -else +if ! WITH_DRIVER_MODULES if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la endif diff --git a/src/Makefile.am b/src/Makefile.am index 8ec8230..521246c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -82,7 +82,7 @@ UTIL_SOURCES = \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ util/xml.c util/xml.h \ - util/virtaudit.c util/virtaudit.h \ + util/virtaudit.c util/virtaudit.h \ util/virterror.c util/virterror_internal.h EXTRA_DIST += util/threads-pthread.c util/threads-win32.c @@ -566,6 +566,7 @@ libvirt_driver_xen_la_CFLAGS = \ libvirt_driver_xen_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_xen_la_LIBADD = $(XEN_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_xen_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_xen_la_LDFLAGS += -module -avoid-version endif libvirt_driver_xen_la_SOURCES = $(XEN_DRIVER_SOURCES) @@ -578,7 +579,8 @@ else noinst_LTLIBRARIES += libvirt_driver_phyp.la libvirt_la_BUILT_LIBADD += libvirt_driver_phyp.la endif -libvirt_driver_phyp_la_LIBADD = $(LIBSSH2_LIBS) +libvirt_driver_phyp_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_phyp_la_LIBADD += $(LIBSSH2_LIBS) libvirt_driver_phyp_la_CFLAGS = $(LIBSSH2_CFLAGS) \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_phyp_la_SOURCES = $(PHYP_DRIVER_SOURCES) @@ -594,6 +596,7 @@ endif libvirt_driver_openvz_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) if WITH_DRIVER_MODULES +libvirt_driver_openvz_la_LIBADD = ../gnulib/lib/libgnu.la libvirt_driver_openvz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif libvirt_driver_openvz_la_SOURCES = $(OPENVZ_DRIVER_SOURCES) @@ -608,10 +611,11 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_vbox.la endif libvirt_driver_vbox_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) +libvirt_driver_vbox_la_LIBADD = $(DLOPEN_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_vbox_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif -libvirt_driver_vbox_la_LIBADD = $(DLOPEN_LIBS) libvirt_driver_vbox_la_SOURCES = $(VBOX_DRIVER_SOURCES) endif @@ -627,6 +631,7 @@ libvirt_driver_xenapi_la_CFLAGS = $(LIBXENSERVER_CFLAGS) $(LIBCURL_CFLAGS) \ libvirt_driver_xenapi_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_xenapi_la_LIBADD = $(LIBXENSERVER_LIBS) $(LIBCURL_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_xenapi_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_xenapi_la_LDFLAGS += -module -avoid-version endif libvirt_driver_xenapi_la_SOURCES = $(XENAPI_DRIVER_SOURCES) @@ -645,6 +650,7 @@ libvirt_driver_qemu_la_CFLAGS = $(NUMACTL_CFLAGS) \ libvirt_driver_qemu_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_qemu_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_qemu_la_LDFLAGS += -module -avoid-version endif libvirt_driver_qemu_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -670,6 +676,7 @@ endif libvirt_driver_lxc_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) if WITH_DRIVER_MODULES +libvirt_driver_lxc_la_LIBADD = ../gnulib/lib/libgnu.la libvirt_driver_lxc_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif libvirt_driver_lxc_la_SOURCES = $(LXC_DRIVER_SOURCES) @@ -695,6 +702,7 @@ libvirt_driver_uml_la_CFLAGS = $(NUMACTL_CFLAGS) \ libvirt_driver_uml_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_uml_la_LIBADD = $(NUMACTL_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_uml_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_uml_la_LDFLAGS += -module -avoid-version endif libvirt_driver_uml_la_SOURCES = $(UML_DRIVER_SOURCES) @@ -714,6 +722,7 @@ libvirt_driver_one_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_one_la_LIBADD = $(XMLRPC_LIBS) #libvirt_driver_one_la_CFLAGS = "-DWITH_ONE" if WITH_DRIVER_MODULES +libvirt_driver_one_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_one_la_LDFLAGS += -module -avoid-version endif libvirt_driver_one_la_SOURCES = $(ONE_DRIVER_SOURCES) @@ -737,6 +746,7 @@ libvirt_driver_esx_la_CFLAGS = $(LIBCURL_CFLAGS) \ libvirt_driver_esx_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_esx_la_LIBADD = $(LIBCURL_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_esx_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_esx_la_LDFLAGS += -module -avoid-version endif libvirt_driver_esx_la_SOURCES = $(ESX_DRIVER_SOURCES) @@ -754,6 +764,7 @@ endif libvirt_driver_network_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) if WITH_DRIVER_MODULES +libvirt_driver_network_la_LIBADD = ../gnulib/lib/libgnu.la libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif libvirt_driver_network_la_SOURCES = $(NETWORK_DRIVER_SOURCES) @@ -775,6 +786,7 @@ libvirt_driver_interface_la_CFLAGS = $(NETCF_CFLAGS) \ libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_interface_la_LIBADD = $(NETCF_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_interface_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_interface_la_LDFLAGS += -module -avoid-version endif libvirt_driver_interface_la_SOURCES = $(INTERFACE_DRIVER_SOURCES) @@ -791,8 +803,8 @@ endif libvirt_driver_secret_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) if WITH_DRIVER_MODULES -libvirt_driver_secret_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_secret_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_secret_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) endif @@ -812,6 +824,7 @@ noinst_LTLIBRARIES += libvirt_driver_storage.la #libvirt_la_BUILT_LIBADD += libvirt_driver_storage.la endif if WITH_DRIVER_MODULES +libvirt_driver_storage_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_storage_la_LDFLAGS += -module -avoid-version endif libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_SOURCES) @@ -867,6 +880,7 @@ libvirt_driver_nodedev_la_LIBADD += $(UDEV_LIBS) $(PCIACCESS_LIBS) endif if WITH_DRIVER_MODULES +libvirt_driver_nodedev_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_nodedev_la_LDFLAGS += -module -avoid-version endif endif @@ -884,8 +898,8 @@ libvirt_driver_nwfilter_la_CFLAGS = $(LIBPCAP_CFLAGS) \ libvirt_driver_nwfilter_la_LDFLAGS = $(LD_AMFLAGS) libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) if WITH_DRIVER_MODULES -libvirt_driver_nwfilter_la_LDFLAGS += -module -avoid-version libvirt_driver_nwfilter_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_nwfilter_la_LDFLAGS += -module -avoid-version endif libvirt_driver_nwfilter_la_SOURCES = $(NWFILTER_DRIVER_SOURCES) endif diff --git a/src/libvirt.c b/src/libvirt.c index ca383ba..61c47b4 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -355,13 +355,27 @@ virInitialize(void) * If they try to open a connection for a module that * is not loaded they'll get a suitable error at that point */ +# ifdef WITH_TEST virDriverLoadModule("test"); +# endif +# ifdef WITH_XEN virDriverLoadModule("xen"); +# endif +# ifdef WITH_OPENVZ virDriverLoadModule("openvz"); +# endif +# ifdef WITH_VBOX virDriverLoadModule("vbox"); +# endif +# ifdef WITH_ESX virDriverLoadModule("esx"); +# endif +# ifdef WITH_XENAPI virDriverLoadModule("xenapi"); +# endif +# ifdef WITH_REMOTE virDriverLoadModule("remote"); +# endif #else # ifdef WITH_TEST if (testRegister() == -1) return -1; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 301b0ef..73c3492 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3,6 +3,10 @@ # more details. # +# authhelper.h +virRequestUsername; +virRequestPassword; + # bitmap.h virBitmapAlloc; @@ -21,6 +25,8 @@ virBufferContentAndReset; virBufferError; virBufferURIEncodeString; virBufferFreeAndReset; +virBufferUse; +virBufferStrcat; # caps.h @@ -146,8 +152,12 @@ virDomainDiskInsert; virDomainDiskInsertPreAlloced; virDomainDiskRemove; virDomainDiskDefAssignAddress; +virDomainDiskTypeToString; +virDomainDiskTypeFromString; virDomainControllerInsert; virDomainControllerInsertPreAlloced; +virDomainControllerModelTypeFromString; +virDomainControllerModelTypeToString; virDomainFindByID; virDomainFindByName; virDomainFindByUUID; @@ -161,6 +171,8 @@ virDomainHostdevSubsysTypeToString; virDomainInputDefFree; virDomainLifecycleTypeFromString; virDomainLifecycleTypeToString; +virDomainLifecycleCrashTypeFromString; +virDomainLifecycleCrashTypeToString; virDomainLoadAllConfigs; virDomainNetDefFree; virDomainNetTypeToString; @@ -225,11 +237,17 @@ virDomainSnapshotHasChildren; virDomainSnapshotObjUnref; virDomainSnapshotDefParseString; virDomainSnapshotDefFormat; +virDomainSnapshotDefFree; virDomainSnapshotAssignDef; virDomainObjAssignDef; virDomainChrDefForeach; virDomainDiskDefForeachPath; virDomainChrConsoleTargetTypeToString; +virDomainChrConsoleTargetTypeFromString; +virDomainChrTcpProtocolTypeToString; +virDomainChrTcpProtocolTypeFromString; +virDomainDiskCacheTypeToString; +virDomainDiskCacheTypeFromString; # domain_event.h @@ -276,6 +294,11 @@ virDomainConfNWFilterInstantiate; virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; + +# driver.h +virDriverLoadModule; + + # ebtables.h ebtablesAddForwardAllowIn; ebtablesAddForwardPolicyReject; @@ -503,6 +526,8 @@ virNodeDeviceDefFree; virNodeDevCapsDefFree; virNodeDeviceDefFormat; virNodeDeviceDefParseString; +virNodeDeviceDefParseNode; +virNodeDeviceDefParseFile; virNodeDeviceObjLock; virNodeDeviceObjUnlock; virNodeDeviceAssignDef; @@ -644,6 +669,7 @@ virStorageFileIsSharedFS; # threads.h virMutexInit; +virMutexInitRecursive; virMutexDestroy; virMutexLock; virMutexUnlock; @@ -698,6 +724,7 @@ virParseVersionString; virPipeReadUntilEOF; virAsprintf; virRun; +virRunWithHook; virSkipSpaces; virKillProcess; virGetUserDirectory; @@ -708,6 +735,18 @@ virFileFindMountPoint; virFileWaitForDevices; virFileMatchesNameSuffix; virArgvToString; +virStrcpy; +virStrncpy; +virBuildPathInternal; +virFileStripSuffix; +virFileOperation; +virFork; +virRandom; +virRandomInitialize; +virDirCreate; +virIndexToDiskName; +virHexToBin; + # interface.h ifaceCtrl; @@ -732,6 +771,12 @@ virUUIDParse; virSetHostUUIDStr; virGetHostUUID; +# virtaudit.h +virAuditClose; +virAuditLog; +virAuditOpen; +virAuditSend; + # virterror_internal.h virReportErrorHelper; -- 1.7.2.3

On 10/12/2010 11:32 AM, Daniel P. Berrange wrote:
@@ -708,6 +735,18 @@ virFileFindMountPoint; virFileWaitForDevices; virFileMatchesNameSuffix; virArgvToString; +virStrcpy; +virStrncpy; +virBuildPathInternal; +virFileStripSuffix; +virFileOperation; +virFork; +virRandom; +virRandomInitialize; +virDirCreate; +virIndexToDiskName; +virHexToBin;
This is a duplicate with the patch I committed earlier today (I only noticed one function, but you obviously linked shard modules and found a lot more). Should we be running this through sort | uniq -c and detecting any duplicate lines as part of 'make syntax-check'? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Oct 12, 2010 at 06:32:18PM +0100, Daniel P. Berrange wrote:
From: Miloslav Trmač <mitr@redhat.com>
The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables.
Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions & export them, just linkage all loadable modules against libgnu.la statically.
* daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers
Hum, weird, I tried to o a "make rpm" with that patch and got a linking error due to multiple definitions coming from gnulib: CCLD libvirt_lxc CCLD libvirt.la copying selected object files to avoid basename conflicts... ../gnulib/lib/.libs/libgnu.a(areadlink.o): In function `areadlink': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: multiple definition of `areadlink' ./.libs/libvirt_driver_phyp.a(areadlink.o):/u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: first defined here ../gnulib/lib/.libs/libgnu.a(base64.o): In function `base64_encode': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/base64.c:79: multiple definition of `base64_encode' So patches 1-3 look fine to me but that one seems to still have a problem, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Oct 14, 2010 at 04:22:29PM +0200, Daniel Veillard wrote:
On Tue, Oct 12, 2010 at 06:32:18PM +0100, Daniel P. Berrange wrote:
From: Miloslav Trmač <mitr@redhat.com>
The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables.
Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions & export them, just linkage all loadable modules against libgnu.la statically.
* daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers
Hum, weird, I tried to o a "make rpm" with that patch and got a linking error due to multiple definitions coming from gnulib:
CCLD libvirt_lxc CCLD libvirt.la copying selected object files to avoid basename conflicts... ../gnulib/lib/.libs/libgnu.a(areadlink.o): In function `areadlink': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: multiple definition of `areadlink' ./.libs/libvirt_driver_phyp.a(areadlink.o):/u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: first defined here ../gnulib/lib/.libs/libgnu.a(base64.o): In function `base64_encode': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/base64.c:79: multiple definition of `base64_encode'
So patches 1-3 look fine to me but that one seems to still have a problem,
This was a simple mistake. One of my changes to the phyp driver link line was not properly protected by a 'if WITH_DRIVER_MODULES' Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Oct 14, 2010 at 03:45:01PM +0100, Daniel P. Berrange wrote:
On Thu, Oct 14, 2010 at 04:22:29PM +0200, Daniel Veillard wrote:
On Tue, Oct 12, 2010 at 06:32:18PM +0100, Daniel P. Berrange wrote:
From: Miloslav Trmač <mitr@redhat.com>
The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables.
Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions & export them, just linkage all loadable modules against libgnu.la statically.
* daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers
Hum, weird, I tried to o a "make rpm" with that patch and got a linking error due to multiple definitions coming from gnulib:
CCLD libvirt_lxc CCLD libvirt.la copying selected object files to avoid basename conflicts... ../gnulib/lib/.libs/libgnu.a(areadlink.o): In function `areadlink': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: multiple definition of `areadlink' ./.libs/libvirt_driver_phyp.a(areadlink.o):/u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: first defined here ../gnulib/lib/.libs/libgnu.a(base64.o): In function `base64_encode': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/base64.c:79: multiple definition of `base64_encode'
So patches 1-3 look fine to me but that one seems to still have a problem,
This was a simple mistake. One of my changes to the phyp driver link line was not properly protected by a 'if WITH_DRIVER_MODULES'
Okay, ACK to a rebased patch set once this error is fixed, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake