On Wed, Mar 21, 2018 at 11:02:43AM +0100, Pavel Hrdina wrote:
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@redhat.com> ---
Changes in v3: - fixed a bug while loading XML interface files, now it fails gracefully with error message
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 | 398 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/gdbus.h | 108 ++++++++++++++ test/Makefile.am | 3 +- test/travis-run | 2 +- 9 files changed, 547 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])
FYI, you can actually get the default value for this from pkg-config $ pkgconf --variable interfaces_dir dbus-1 /usr/share/dbus-1/interfaces And arguably don't need to make that configurable install dir at all with args. Not a blocker for this though, so fine if you patch that separately. You would need a BuildRequires on dbus-devel to get the pkgconfig file though.
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)
+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) {
Nitpick g_str_equal() - no need to repost for that though. Sevaral similar cases below that I won't repeat.
+ getFunc = data->properties[i].getFunc; + break; + } + }
+void +virtDBusGDBusRegisterSubtree(GDBusConnection *bus, + gchar const *objectPath, + GDBusInterfaceInfo *interface, + virtDBusGDBusEnumerateFunc enumerate, + virtDBusGDBusMethodTable *methods, + virtDBusGDBusPropertyTable *properties, + gpointer userData);
Not sure how much you care, but if you want libvirt-dbus APIs to match glib conventions, then method names would be all lowercsae and underscore separated Regardless of comments, Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|