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

[Daniel P. Berrangé]

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

>  LIBVIRT_REQUIRED=AC_PACKAGE_VERSION
>  SYSTEMD_REQUIRED=211
> +LIBVIRT_GLIB_REQUIRED=1.0.0

Same question here ?

> +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.


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 :|