[libvirt] [dbus PATCH 6/8] introduce support for GDBus implementation

[Pavel Hrdina]

On Mon, Mar 19, 2018 at 01:47:20PM +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 19, 2018 at 10:30:50AM +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 at redhat.com>
> > ---
> >  README               |   1 +
> >  configure.ac         |  12 ++
> >  data/Makefile.am     |   5 +
> >  libvirt-dbus.spec.in |   6 +
> >  src/Makefile.am      |  17 ++-
> >  src/gdbus.c          | 393 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/gdbus.h          |  95 +++++++++++++
> >  test/Makefile.am     |   3 +-
> >  test/travis-run      |   2 +-
> >  9 files changed, 529 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 ba397ca..d6ed0ef 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -11,10 +11,14 @@ AC_USE_SYSTEM_EXTENSIONS
> >  
> >  AM_SILENT_RULES([yes])
> >  
> > +GLIB2_REQUIRED=2.50.0
> 
> Is there a rational for that version, or is just what's on your OS install ?
> 
> The GBus APIs have been around alot longer than this - mostly 2.30.0 iiuc

This is version taken from CentOS 7.  I can check the minimal version
based on the used APIs.

> >  LIBVIRT_REQUIRED=AC_PACKAGE_VERSION
> >  SYSTEMD_REQUIRED=211
> > +LIBVIRT_GLIB_REQUIRED=1.0.0
> 
> Same question here ?

For some reason I though that this is the first release.
Form libvirt-glib we need only the gvir_event_register which
means that we don't have to depend on any specific version.

I will fix that.

> > +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)
> 
> 
> > diff --git a/src/gdbus.c b/src/gdbus.c
> > new file mode 100644
> > index 0000000..7dd5766
> > --- /dev/null
> > +++ b/src/gdbus.c
> > @@ -0,0 +1,393 @@
> > +#include "gdbus.h"
> > +
> > +#include <glib/gprintf.h>
> > +
> > +struct _virtDBusGDBusMethodData {
> > +    virtDBusGDBusMethodTable *methods;
> > +    virtDBusGDBusPropertyTable *properties;
> > +    gpointer *userData;
> > +};
> > +typedef struct _virtDBusGDBusMethodData virtDBusGDBusMethodData;
> > +
> > +struct _virtDBusGDBusSubtreeData {
> > +    GDBusInterfaceInfo *interface;
> > +    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/{@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);
> > +}
> 
> My main comment would be that this is all using the low level dbus APIs
> in glib, rather than the high level APIs. I can understand if that is
> easier to adapt to given the existing code though.

This was based on the gio/tests/gdbus-example-server.c since we don't
need to use GObjects.  From what I understand the high level API is
supposed to be used together with GObjects.  And it looks like that
even in that case, you need to also use the low level APIs anyway.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180319/bebad013/attachment-0001.sig>