[libvirt] [dbus PATCH v2 1/3] introduce support for GDBus implementation

We will switch to GDBus implementation of D-Bus protocol because sd-bus implementation is not thread safe. Processing messages in threads is essential since Libvirt API can take some significant amount of time to return and that would block the whole libvirt-dbus daemon. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Changes in v2: - changed glib2 required version to 2.44.0, required for the auto-cleanup macros - changed libvirt-glib required version to 0.0.7 README | 1 + configure.ac | 12 ++ data/Makefile.am | 5 + libvirt-dbus.spec.in | 6 + src/Makefile.am | 17 ++- src/gdbus.c | 401 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/gdbus.h | 107 ++++++++++++++ test/Makefile.am | 3 +- test/travis-run | 2 +- 9 files changed, 549 insertions(+), 5 deletions(-) create mode 100644 src/gdbus.c create mode 100644 src/gdbus.h diff --git a/README b/README index 754d957..a85114e 100644 --- a/README +++ b/README @@ -58,6 +58,7 @@ The packages required to build libvirt-dbus are - systemd-211 - libvirt + - glib2 Patches submissions =================== diff --git a/configure.ac b/configure.ac index df1a375..0e3bc01 100644 --- a/configure.ac +++ b/configure.ac @@ -11,10 +11,14 @@ AC_USE_SYSTEM_EXTENSIONS AM_SILENT_RULES([yes]) +GLIB2_REQUIRED=2.44.0 LIBVIRT_REQUIRED=1.2.8 SYSTEMD_REQUIRED=211 +LIBVIRT_GLIB_REQUIRED=0.0.7 +AC_SUBST([GLIB2_REQUIRED]) dnl used in the .spec file AC_SUBST([LIBVIRT_REQUIRED]) dnl used in the .spec file AC_SUBST([SYSTEMD_REQUIRED]) dnl used in the .spec file +AC_SUBST([LIBVIRT_GLIB_REQUIRED]) dnl used in the .spec file LIBVIRT_DBUS_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` LIBVIRT_DBUS_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'` @@ -34,8 +38,11 @@ AC_PROG_MKDIR_P AM_PROG_CC_C_O AC_PROG_CC_STDC +PKG_CHECK_MODULES(GIO2, gio-unix-2.0 >= GLIB2_REQUIRED) +PKG_CHECK_MODULES(GLIB2, glib-2.0 >= GLIB2_REQUIRED) PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED) PKG_CHECK_MODULES(SYSTEMD, libsystemd >= $SYSTEMD_REQUIRED) +PKG_CHECK_MODULES(LIBVIRT_GLIB, libvirt-glib-1.0 >= LIBVIRT_GLIB_REQUIRED) LIBVIRT_COMPILE_WARNINGS LIBVIRT_LINKER_RELRO @@ -56,6 +63,11 @@ LIBVIRT_ARG_WITH([DBUS_SYSTEM_POLICIES], [where D-Bus system policies directory DBUS_SYSTEM_POLICIES_DIR="$with_dbus_system_policies" AC_SUBST(DBUS_SYSTEM_POLICIES_DIR) +LIBVIRT_ARG_WITH([DBUS_INTERFACES], [where D-Bus interfaces directory is], + ['$(datadir)/dbus-1/interfaces']) +DBUS_INTERFACES_DIR="$with_dbus_interfaces" +AC_SUBST([DBUS_INTERFACES_DIR]) + LIBVIRT_ARG_WITH([SYSTEM_USER], [username to run system instance as], ['libvirtdbus']) SYSTEM_USER=$with_system_user diff --git a/data/Makefile.am b/data/Makefile.am index 9a53305..a886687 100644 --- a/data/Makefile.am +++ b/data/Makefile.am @@ -18,11 +18,16 @@ polkit_files = \ polkitdir = $(sysconfdir)/polkit-1/rules.d polkit_DATA = $(polkit_files:.rules.in=.rules) +interfaces_files = +interfacesdir = $(DBUS_INTERFACES_DIR) +interfaces_DATA = $(interfaces_files) + EXTRA_DIST = \ $(service_in_files) \ $(system_service_in_files) \ $(system_policy_files) \ $(polkit_files) \ + $(interfaces_files) \ $(NULL) CLEANFILES = \ diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index ce5cecf..662ece1 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -1,7 +1,9 @@ # -*- rpm-spec -*- +%define glib2_version @GLIB2_REQUIRED@ %define libvirt_version @LIBVIRT_REQUIRED@ %define systemd_version @SYSTEMD_REQUIRED@ +%define libvirt_glib_version @LIBVIRT_GLIB_REQUIRED@ %define system_user @SYSTEM_USER@ Name: @PACKAGE@ @@ -15,11 +17,15 @@ Source0: ftp://libvirt.org/libvirt/dbus/%{name}-%{version}.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRequires: gcc +BuildRequires: glib2-devel >= %{glib2_version} BuildRequires: libvirt-devel >= %{libvirt_version} BuildRequires: systemd-devel >= %{systemd_version} +BuildRequires: libvirt-glib-devel >= %{libvirt_glib_version} +Requires: glib2 >= %{glib2_version} Requires: libvirt-libs >= %{libvirt_version} Requires: systemd-libs >= %{systemd_version} +Requires: libvirt-glib >= %{libvirt_glib_version} Requires(pre): shadow-utils diff --git a/src/Makefile.am b/src/Makefile.am index 1a5b50b..e7bba9d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,12 +1,14 @@ AM_CPPFLAGS = \ - -I$(top_srcdir)/src + -I$(top_srcdir)/src \ + -DVIRT_DBUS_INTERFACES_DIR=\"$(DBUS_INTERFACES_DIR)\" DAEMON_SOURCES = \ main.c \ connect.c connect.h \ util.c util.h \ domain.c domain.h \ - events.c events.h + events.c events.h \ + gdbus.c gdbus.h EXTRA_DIST = \ $(DAEMON_SOURCES) @@ -18,18 +20,27 @@ libvirt_dbus_SOURCES = $(DAEMON_SOURCES) libvirt_dbus_CFLAGS = \ $(SYSTEMD_CFLAGS) \ + $(GIO2_CFLAGS) \ + $(GLIB2_CFLAGS) \ $(LIBVIRT_CFLAGS) \ + $(LIBVIRT_GLIB_CFLAGS) \ $(WARN_CFLAGS) \ $(PIE_CFLAGS) \ $(NULL) libvirt_dbus_LDFLAGS = \ $(SYSTEMD_LDFLAGS) \ + $(GIO2_LDFLAGS) \ + $(GLIB2_LDFLAGS) \ $(LIBVIRT_LDFLAGS) \ + $(LIBVIRT_GLIB_LDFLAGS) \ $(RELRO_LDFLAGS) \ $(PID_LDFLAGS) \ $(NULL) libvirt_dbus_LDADD = \ $(SYSTEMD_LIBS) \ - $(LIBVIRT_LIBS) + $(GIO2_LIBS) \ + $(GLIB2_LIBS) \ + $(LIBVIRT_LIBS) \ + $(LIBVIRT_GLIB_LIBS) diff --git a/src/gdbus.c b/src/gdbus.c new file mode 100644 index 0000000..de02415 --- /dev/null +++ b/src/gdbus.c @@ -0,0 +1,401 @@ +#include "gdbus.h" + +#include <glib/gprintf.h> + +struct _virtDBusGDBusMethodData { + virtDBusGDBusMethodTable *methods; + virtDBusGDBusPropertyTable *properties; + gpointer *userData; +}; +typedef struct _virtDBusGDBusMethodData virtDBusGDBusMethodData; + +struct _virtDBusGDBusSubtreeData { + GDBusInterfaceInfo *interface; + virtDBusGDBusEnumerateFunc enumerate; + virtDBusGDBusMethodData *methodData; +}; +typedef struct _virtDBusGDBusSubtreeData virtDBusGDBusSubtreeData; + +static const gchar *dbusInterfacePrefix = NULL; + +/** + * virtDBusGDBusLoadIntrospectData: + * @interface: name of the interface + * + * Reads an interface XML description from file and returns new + * interface info. The caller owns an reference to the returned info. + * + * The file path is constructed as: + * + * VIRT_DBUS_INTERFACES_DIR/{(a)interface}.xml + * + * Returns interface info on success, NULL on failure. + */ +GDBusInterfaceInfo * +virtDBusGDBusLoadIntrospectData(gchar const *interface) +{ + g_autofree gchar *introspectFile = NULL; + g_autofree gchar *introspectXML = NULL; + g_autoptr(GDBusNodeInfo) nodeInfo = NULL; + GDBusInterfaceInfo *ret; + + if (!dbusInterfacePrefix) { + dbusInterfacePrefix = g_getenv("VIRT_DBUS_INTERFACES_DIR"); + if (!dbusInterfacePrefix) + dbusInterfacePrefix = VIRT_DBUS_INTERFACES_DIR; + } + + introspectFile = g_strdup_printf("%s/%s.xml", dbusInterfacePrefix, interface); + g_assert(introspectFile != NULL); + + g_file_get_contents(introspectFile, &introspectXML, NULL, NULL); + g_assert(introspectXML != NULL); + + nodeInfo = g_dbus_node_info_new_for_xml(introspectXML, NULL); + if (!nodeInfo) + return NULL; + + ret = nodeInfo->interfaces[0]; + if (!ret) + return NULL; + + return g_dbus_interface_info_ref(ret); +} + +static void +virtDBusGDBusHandlePropertyGet(GVariant *parameters, + GDBusMethodInvocation *invocation, + const gchar *objectPath, + virtDBusGDBusMethodData *data) +{ + virtDBusGDBusPropertyGetFunc getFunc = NULL; + const gchar *interface; + const gchar *name; + GVariant *value = NULL; + GError *error = NULL; + + g_variant_get(parameters, "(&s&s)", &interface, &name); + + for (gint i = 0; data->properties[i].name; i++) { + if (g_strcmp0(name, data->properties[i].name) == 0) { + getFunc = data->properties[i].getFunc; + break; + } + } + + if (!getFunc) { + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_UNKNOWN_PROPERTY, + "unknown property '%s'", name); + return; + } + + getFunc(objectPath, data->userData, &value, &error); + + if (error) { + g_dbus_method_invocation_return_gerror(invocation, error); + return; + } + + g_return_if_fail(value); + + g_dbus_method_invocation_return_value(invocation, + g_variant_new("(v)", value)); +} + +static void +virtDBusGDBusHandlePropertySet(GVariant *parameters, + GDBusMethodInvocation *invocation, + const gchar *objectPath, + virtDBusGDBusMethodData *data) +{ + virtDBusGDBusPropertySetFunc setFunc = NULL; + const gchar *interface; + const gchar *name; + g_autoptr(GVariant) value = NULL; + GError *error = NULL; + + g_variant_get(parameters, "(&s&sv)", &interface, &name, &value); + + for (gint i = 0; data->properties[i].name; i++) { + if (g_strcmp0(name, data->properties[i].name) == 0) { + setFunc = data->properties[i].setFunc; + break; + } + } + + if (!setFunc) { + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_UNKNOWN_PROPERTY, + "unknown property '%s'", name); + return; + } + + setFunc(value, objectPath, data->userData, &error); + + if (error) + g_dbus_method_invocation_return_gerror(invocation, error); + else + g_dbus_method_invocation_return_value(invocation, NULL); +} + +static void +virtDBusGDBusHandlePropertyGetAll(GDBusMethodInvocation *invocation, + const gchar *objectPath, + virtDBusGDBusMethodData *data) +{ + GVariant *value; + g_auto(GVariantBuilder) builder; + GError *error = NULL; + + g_variant_builder_init(&builder, G_VARIANT_TYPE("(a{sv})")); + + g_variant_builder_open(&builder, G_VARIANT_TYPE("a{sv}")); + + for (gint i = 0; data->properties[i].name; i++) { + data->properties[i].getFunc(objectPath, data->userData, + &value, &error); + + if (error) { + g_dbus_method_invocation_return_gerror(invocation, error); + return; + } + + g_return_if_fail(value); + + g_variant_builder_add(&builder, "{sv}", + data->properties[i].name, + g_variant_new_variant(value)); + } + + g_variant_builder_close(&builder); + + g_dbus_method_invocation_return_value(invocation, + g_variant_builder_end(&builder)); +} + +static void +virtDBusGDBusHandleMethod(GVariant *parameters, + GDBusMethodInvocation *invocation, + const gchar *objectPath, + const gchar *methodName, + virtDBusGDBusMethodData *data) +{ + virtDBusGDBusMethodFunc methodFunc = NULL; + GDBusMessage *msg = g_dbus_method_invocation_get_message(invocation); + GUnixFDList *inFDs = NULL; + GVariant *outArgs = NULL; + GUnixFDList *outFDs = NULL; + GError *error = NULL; + + for (gint i = 0; data->methods[i].name; i++) { + if (g_strcmp0(methodName, data->methods[i].name) == 0) { + methodFunc = data->methods[i].methodFunc; + break; + } + } + + if (!methodFunc) { + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_UNKNOWN_METHOD, + "unknown method '%s'", methodName); + return; + } + + inFDs = g_dbus_message_get_unix_fd_list(msg); + + methodFunc(parameters, inFDs, objectPath, data->userData, + &outArgs, &outFDs, &error); + + if (error) { + g_dbus_method_invocation_return_gerror(invocation, error); + return; + } + + g_return_if_fail(outArgs || !outFDs); + + g_dbus_method_invocation_return_value_with_unix_fd_list(invocation, + outArgs, + outFDs); +} + +static void +virtDBusGDBusHandleMethodCall(GDBusConnection *connection G_GNUC_UNUSED, + const gchar *sender G_GNUC_UNUSED, + const gchar *objectPath, + const gchar *interfaceName, + const gchar *methodName, + GVariant *parameters, + GDBusMethodInvocation *invocation, + gpointer userData) +{ + virtDBusGDBusMethodData *data = userData; + + if (g_strcmp0(interfaceName, "org.freedesktop.DBus.Properties") == 0) { + if (g_strcmp0(methodName, "Get") == 0) { + virtDBusGDBusHandlePropertyGet(parameters, invocation, + objectPath, data); + } else if (g_strcmp0(methodName, "Set") == 0) { + virtDBusGDBusHandlePropertySet(parameters, invocation, + objectPath, data); + } else if (g_strcmp0(methodName, "GetAll") == 0) { + virtDBusGDBusHandlePropertyGetAll(invocation, objectPath, data); + } else { + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_UNKNOWN_METHOD, + "unknown method '%s'", methodName); + } + } else { + virtDBusGDBusHandleMethod(parameters, invocation, objectPath, + methodName, data); + } +} + +static const GDBusInterfaceVTable virtDBusGDBusVtable = { + virtDBusGDBusHandleMethodCall, + NULL, + NULL, + { NULL } +}; + +/** + * virtDBusGDBusRegisterObject: + * @bus: GDBus connection + * @objectPath: object path + * @interface: interface info of the object + * @methods: table of method handlers + * @properties: table of property handlers + * @userData: data that are passed to method and property handlers + * + * Registers a new D-Bus object that we would like to handle. + */ +void +virtDBusGDBusRegisterObject(GDBusConnection *bus, + gchar const *objectPath, + GDBusInterfaceInfo *interface, + virtDBusGDBusMethodTable *methods, + virtDBusGDBusPropertyTable *properties, + gpointer userData) +{ + virtDBusGDBusMethodData *data = g_new0(virtDBusGDBusMethodData, 1); + + g_assert(data); + + data->methods = methods; + data->properties = properties; + data->userData = userData; + + g_dbus_connection_register_object(bus, + objectPath, + interface, + &virtDBusGDBusVtable, + data, g_free, + NULL); +} + +static gchar ** +virtDBusGDBusEnumerate(GDBusConnection *connection G_GNUC_UNUSED, + const gchar *sender G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData) +{ + virtDBusGDBusSubtreeData *data = userData; + + if (data->enumerate) + return data->enumerate(data->methodData->userData); + + return NULL; +} + +static GDBusInterfaceInfo ** +virtDBusGDBusIntrospect(GDBusConnection *bus G_GNUC_UNUSED, + const gchar *sender G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + const gchar *node G_GNUC_UNUSED, + gpointer userData) +{ + virtDBusGDBusSubtreeData *data = userData; + GDBusInterfaceInfo **ret = g_new0(GDBusInterfaceInfo *, 2); + + g_assert(ret); + ret[0] = g_dbus_interface_info_ref(data->interface); + + return ret; +} + +static const GDBusInterfaceVTable * +virtDBusGDBusDispatch(GDBusConnection *bus G_GNUC_UNUSED, + const gchar *sender G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + const gchar *interfaceName G_GNUC_UNUSED, + const gchar *node G_GNUC_UNUSED, + gpointer *outUserData, + gpointer userData) +{ + virtDBusGDBusSubtreeData *data = userData; + + *outUserData = data->methodData; + return &virtDBusGDBusVtable; +} + +static const GDBusSubtreeVTable virtDBusGDBusSubreeVtable = { + virtDBusGDBusEnumerate, + virtDBusGDBusIntrospect, + virtDBusGDBusDispatch, + { NULL } +}; + +static void +virtDBusGDBusSubtreeDataFree(gpointer opaque) +{ + virtDBusGDBusSubtreeData *data = opaque; + g_free(data->methodData); + g_free(data); +} + +/** + * virtDBusGDBusRegisterSubtree: + * @bus: GDBus connection + * @objectPath: object prefix path + * @interface: interface info of the object + * @methods: table of method handlers + * @properties: table of property handlers + * @userData: data that are passed to method and property handlers + * + * Registers a new D-Bus object prefix that we would like to handle. + */ +void +virtDBusGDBusRegisterSubtree(GDBusConnection *bus, + gchar const *objectPath, + GDBusInterfaceInfo *interface, + virtDBusGDBusEnumerateFunc enumerate, + virtDBusGDBusMethodTable *methods, + virtDBusGDBusPropertyTable *properties, + gpointer userData) +{ + virtDBusGDBusSubtreeData *data = g_new0(virtDBusGDBusSubtreeData, 1); + + g_assert(data); + + data->methodData = g_new0(virtDBusGDBusMethodData, 1); + + g_assert(data->methodData); + + data->interface = interface; + data->enumerate = enumerate; + data->methodData->methods = methods; + data->methodData->properties = properties; + data->methodData->userData = userData; + + g_dbus_connection_register_subtree(bus, + objectPath, + &virtDBusGDBusSubreeVtable, + G_DBUS_SUBTREE_FLAGS_DISPATCH_TO_UNENUMERATED_NODES, + data, + virtDBusGDBusSubtreeDataFree, + NULL); +} diff --git a/src/gdbus.h b/src/gdbus.h new file mode 100644 index 0000000..b9c71cc --- /dev/null +++ b/src/gdbus.h @@ -0,0 +1,107 @@ +#pragma once + +#include <gio/gio.h> + +/** + * virtDBusGDBusMethodFunc: + * @inArgs: input arguments of the method call + * @inFDs: list of input file descriptors + * @objectPath: the object path the method was called on + * @userData: user data passed when registering new object or subtree + * @outArgs: return location of output arguments + * @outFDs: return location of output file descriptors + * @error: return location for error + * + * Handles D-Bus method call. In case of error the handler has + * to set an @error. + */ +typedef void +(*virtDBusGDBusMethodFunc)(GVariant *inArgs, + GUnixFDList *inFDs, + const gchar *objectPath, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs, + GError **error); + +/** + * virtDBusGDBusPropertyGetFunc: + * @objectPath: the object path the method was called on + * @userData: user data passed when registering new object or subtree + * @value: return location for property value + * @error: return location for error + * + * Handles D-Bus Get action on a property. In case of error the handler + * has to set an @error, otherwise @value has to be set. + */ +typedef void +(*virtDBusGDBusPropertyGetFunc)(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error); + +/** + * virtDBusGDBusPropertySetFunc: + * @objectPath: the object path the method was called on + * @value: new value that should be set to the property + * @userData: user data passed when registering new object or subtree + * @error: return location for error + * + * Handles D-Bus Set action on a property. In case of error the handler + * has to set an @error. + */ +typedef void +(*virtDBusGDBusPropertySetFunc)(GVariant *value, + const gchar *objectPath, + gpointer userData, + GError **error); + +/** + * virtDBusGDBusEnumerateFunc: + * @userData: user data passed when registering new subtree + * + * Handles D-Bus introspection for subtree of objects. + * + * Returns a list of objects or NULL. + */ +typedef gchar ** +(*virtDBusGDBusEnumerateFunc)(gpointer userData); + +struct _virtDBusGDBusMethodTable { + const gchar *name; + virtDBusGDBusMethodFunc methodFunc; +}; +typedef struct _virtDBusGDBusMethodTable virtDBusGDBusMethodTable; + +struct _virtDBusGDBusPropertyTable { + const gchar *name; + virtDBusGDBusPropertyGetFunc getFunc; + virtDBusGDBusPropertySetFunc setFunc; +}; +typedef struct _virtDBusGDBusPropertyTable virtDBusGDBusPropertyTable; + +typedef guint virtDBusGDBusSource; +typedef guint virtDBusGDBusOwner; + +GDBusInterfaceInfo * +virtDBusGDBusLoadIntrospectData(gchar const *interface); + +void +virtDBusGDBusRegisterObject(GDBusConnection *bus, + gchar const *objectPath, + GDBusInterfaceInfo *interface, + virtDBusGDBusMethodTable *methods, + virtDBusGDBusPropertyTable *properties, + gpointer userData); + +void +virtDBusGDBusRegisterSubtree(GDBusConnection *bus, + gchar const *objectPath, + GDBusInterfaceInfo *interface, + virtDBusGDBusEnumerateFunc enumerate, + virtDBusGDBusMethodTable *methods, + virtDBusGDBusPropertyTable *properties, + gpointer userData); + +G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virtDBusGDBusSource, g_source_remove, 0); +G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virtDBusGDBusOwner, g_bus_unown_name, 0); diff --git a/test/Makefile.am b/test/Makefile.am index 4651d08..d3997f3 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -13,4 +13,5 @@ EXTRA_DIST = \ travis-run TESTS_ENVIRONMENT = \ - abs_top_builddir=$(abs_top_builddir) + abs_top_builddir=$(abs_top_builddir) \ + VIRT_DBUS_INTERFACES_DIR=$(abs_top_srcdir)/data diff --git a/test/travis-run b/test/travis-run index 482b286..28260f4 100755 --- a/test/travis-run +++ b/test/travis-run @@ -22,7 +22,7 @@ sudo chroot "$CHROOT" << EOF set -ex # install build deps apt-get update -apt-get install -y dh-autoreconf pkg-config libvirt-dev libsystemd-dev libtool python3-gi python3-dbus dbus +apt-get install -y dh-autoreconf pkg-config libvirt-dev libsystemd-dev libglib2.0-dev libtool python3-gi python3-dbus dbus # run build and tests as user chown -R buildd:buildd /build -- 2.14.3