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@xxxxxxxxxx> > --- > > 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@xxxxxxxxxx> 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list