[libvirt] [dbus PATCH 0/8] some improvements and rewrite to GDBus
Pavel Hrdina (8): configure: call AC_PROG_CC_STDC to enable C99 if necessary configure: use LIBVIRT_ARG_WITH macro spec: explicitly require gcc m4: remove gnulib compile warning exceptions maint: set the first minimal version that will be released introduce support for GDBus implementation switch from sd-bus to GDBus implementation main: introduce thread pool to process D-Bus messages README | 2 +- configure.ac | 53 ++-- data/Makefile.am | 7 + data/org.libvirt.Connect.xml | 56 ++++ data/org.libvirt.Domain.xml | 51 +++ libvirt-dbus.spec.in | 11 +- m4/virt-compile-warnings.m4 | 37 --- src/Makefile.am | 20 +- src/connect.c | 320 ++++++++----------- src/connect.h | 36 +-- src/domain.c | 729 +++++++++++++++++++------------------------ src/domain.h | 7 +- src/events.c | 177 +++++------ src/gdbus.c | 448 ++++++++++++++++++++++++++ src/gdbus.h | 99 ++++++ src/main.c | 307 ++++++++---------- src/util.c | 170 +++++----- src/util.h | 53 ++-- test/Makefile.am | 3 +- test/travis-run | 2 +- 20 files changed, 1468 insertions(+), 1120 deletions(-) create mode 100644 data/org.libvirt.Connect.xml create mode 100644 data/org.libvirt.Domain.xml create mode 100644 src/gdbus.c create mode 100644 src/gdbus.h -- 2.14.3
This is required to enable C99 on older systems, CentOS 7 for example. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index aef3d37..6715960 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,7 @@ AC_SUBST([LIBVIRT_DBUS_VERSION_NUMBER]) AC_PROG_CC AC_PROG_MKDIR_P AM_PROG_CC_C_O +AC_PROG_CC_STDC PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED) PKG_CHECK_MODULES(SYSTEMD, libsystemd >= $SYSTEMD_REQUIRED) -- 2.14.3
On Mon, Mar 19, 2018 at 10:30:45AM +0100, Pavel Hrdina wrote:
This is required to enable C99 on older systems, CentOS 7 for example.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 1 + 1 file changed, 1 insertion(+)
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 :|
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/configure.ac b/configure.ac index 6715960..df1a375 100644 --- a/configure.ac +++ b/configure.ac @@ -41,34 +41,19 @@ LIBVIRT_COMPILE_WARNINGS LIBVIRT_LINKER_RELRO LIBVIRT_COMPILE_PIE -AC_ARG_WITH(dbus-services, - [AC_HELP_STRING([--with-dbus-services=<dir>], - [where D-BUS session services directory is])]) -if ! test -z "$with_dbus_services" ; then - DBUS_SERVICES_DIR="$with_dbus_services" -else - DBUS_SERVICES_DIR="$datadir/dbus-1/services" -fi +LIBVIRT_ARG_WITH([DBUS_SERVICES], [where D-Bus session services direcotry is], + ['$(datadir)/dbus-1/services']) +DBUS_SERVICES_DIR="$with_dbus_services" AC_SUBST(DBUS_SERVICES_DIR) -AC_ARG_WITH(dbus-system-services, - [AC_HELP_STRING([--with-dbus-system-services=<dir>], - [where D-BUS system services directory is])]) -if ! test -z "$with_dbus_system_services" ; then - DBUS_SYSTEM_SERVICES_DIR="$with_dbus_system_services" -else - DBUS_SYSTEM_SERVICES_DIR="$datadir/dbus-1/system-services" -fi +LIBVIRT_ARG_WITH([DBUS_SYSTEM_SERVICES], [where D-Bus system services directory is], + ['$(datadir)/dbus-1/system-services']) +DBUS_SYSTEM_SERVICES_DIR="$with_dbus_system_services" AC_SUBST(DBUS_SYSTEM_SERVICES_DIR) -AC_ARG_WITH(dbus-system-policies, - [AC_HELP_STRING([--with-dbus-system-policies=<dir>], - [where D-BUS system policies directory is])]) -if ! test -z "$with_dbus_system_policies" ; then - DBUS_SYSTEM_POLICIES_DIR="$with_dbus_system_policies" -else - DBUS_SYSTEM_POLICIES_DIR="$datadir/dbus-1/system.d" -fi +LIBVIRT_ARG_WITH([DBUS_SYSTEM_POLICIES], [where D-Bus system policies directory is], + ['$(datadir)/dbus-1/system.d']) +DBUS_SYSTEM_POLICIES_DIR="$with_dbus_system_policies" AC_SUBST(DBUS_SYSTEM_POLICIES_DIR) LIBVIRT_ARG_WITH([SYSTEM_USER], [username to run system instance as], -- 2.14.3
On Mon, Mar 19, 2018 at 10:30:46AM +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-)
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 :|
Based on discussion on fedora devel list gcc will be removed from default buildroot. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-dbus.spec.in | 1 + 1 file changed, 1 insertion(+) diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 87f20d4..ce5cecf 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -14,6 +14,7 @@ URL: http://libvirt.org/ Source0: ftp://libvirt.org/libvirt/dbus/%{name}-%{version}.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) +BuildRequires: gcc BuildRequires: libvirt-devel >= %{libvirt_version} BuildRequires: systemd-devel >= %{systemd_version} -- 2.14.3
On Mon, Mar 19, 2018 at 10:30:47AM +0100, Pavel Hrdina wrote:
Based on discussion on fedora devel list gcc will be removed from default buildroot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-dbus.spec.in | 1 + 1 file changed, 1 insertion(+)
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 :|
We don't use gnulib so there is no need to have them. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- m4/virt-compile-warnings.m4 | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 4d96a0e..644c321 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -37,12 +37,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wconversion" # Too many to deal with dontwarn="$dontwarn -Wsign-conversion" - # GNULIB gettext.h violates - dontwarn="$dontwarn -Wvla" - # Many GNULIB header violations - dontwarn="$dontwarn -Wundef" - # Need to allow bad cast for execve() - dontwarn="$dontwarn -Wcast-qual" # We need to use long long in many places dontwarn="$dontwarn -Wlong-long" # We allow manual list of all enum cases without default: @@ -53,8 +47,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -funsafe-loop-optimizations dontwarn="$dontwarn -Wunsafe-loop-optimizations" - # Gnulib's stat-time.h violates this - dontwarn="$dontwarn -Waggregate-return" # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall dontwarn="$dontwarn -Wenum-compare" # gcc 5.1 -Wformat-signedness mishandles enums, not ready for prime time @@ -123,26 +115,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # Remove the ones we don't want, blacklisted earlier gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) - # GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff. - # Unfortunately, this means you can't simply use '-Wsign-compare' - # with gl_MANYWARN_COMPLEMENT - # So we have -W enabled, and then have to explicitly turn off... - wantwarn="$wantwarn -Wno-sign-compare" - - # GNULIB expects this to be part of -Wc++-compat, but we turn - # that one off, so we need to manually enable this again - wantwarn="$wantwarn -Wjump-misses-init" - - # GNULIB turns on -Wformat=2 which implies -Wformat-nonliteral, - # so we need to manually re-exclude it. Also, older gcc 4.2 - # added an implied ATTRIBUTE_NONNULL on any parameter marked - # ATTRIBUTE_FMT_PRINT, which causes -Wformat failure on our - # intentional use of virReportError(code, NULL). - wantwarn="$wantwarn -Wno-format-nonliteral" - if test $lv_cv_gcc_wformat_null_works = no; then - wantwarn="$wantwarn -Wno-format" - fi - # -Wformat enables this by default, and we should keep it, # but need to rewrite various areas of code first wantwarn="$wantwarn -Wno-format-truncation" @@ -213,13 +185,4 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_DEFINE([HAVE_SUGGEST_ATTRIBUTE_FORMAT], [1], [Whether -Wsuggest-attribute=format works]) ;; esac - - # Silence certain warnings in gnulib, and use improved glibc headers - AH_VERBATIM([FORTIFY_SOURCE], - [/* Enable compile-time and run-time bounds-checking, and some warnings, - without upsetting newer glibc. */ - #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ - # define _FORTIFY_SOURCE 2 - #endif - ]) ]) -- 2.14.3
On Mon, Mar 19, 2018 at 10:30:48AM +0100, Pavel Hrdina wrote:
We don't use gnulib so there is no need to have them.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- m4/virt-compile-warnings.m4 | 37 ------------------------------------- 1 file changed, 37 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 4d96a0e..644c321 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -37,12 +37,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wconversion" # Too many to deal with dontwarn="$dontwarn -Wsign-conversion" - # GNULIB gettext.h violates - dontwarn="$dontwarn -Wvla" - # Many GNULIB header violations - dontwarn="$dontwarn -Wundef" - # Need to allow bad cast for execve() - dontwarn="$dontwarn -Wcast-qual" # We need to use long long in many places dontwarn="$dontwarn -Wlong-long" # We allow manual list of all enum cases without default: @@ -53,8 +47,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -funsafe-loop-optimizations dontwarn="$dontwarn -Wunsafe-loop-optimizations" - # Gnulib's stat-time.h violates this - dontwarn="$dontwarn -Waggregate-return" # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall dontwarn="$dontwarn -Wenum-compare" # gcc 5.1 -Wformat-signedness mishandles enums, not ready for prime time @@ -123,26 +115,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # Remove the ones we don't want, blacklisted earlier gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn])
- # GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff. - # Unfortunately, this means you can't simply use '-Wsign-compare' - # with gl_MANYWARN_COMPLEMENT - # So we have -W enabled, and then have to explicitly turn off... - wantwarn="$wantwarn -Wno-sign-compare" - - # GNULIB expects this to be part of -Wc++-compat, but we turn - # that one off, so we need to manually enable this again - wantwarn="$wantwarn -Wjump-misses-init"
We still have this at the top of the file: dontwarn="$dontwarn -Wc++-compat" so this -Wjump-misses-init should be kept
- - # Silence certain warnings in gnulib, and use improved glibc headers - AH_VERBATIM([FORTIFY_SOURCE], - [/* Enable compile-time and run-time bounds-checking, and some warnings, - without upsetting newer glibc. */ - #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ - # define _FORTIFY_SOURCE 2 - #endif - ])
Why are you disabling _FORTIFY_SOURCE - this is an important security check. 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 :|
On Mon, Mar 19, 2018 at 01:39:38PM +0000, Daniel P. Berrangé wrote:
On Mon, Mar 19, 2018 at 10:30:48AM +0100, Pavel Hrdina wrote:
We don't use gnulib so there is no need to have them.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- m4/virt-compile-warnings.m4 | 37 ------------------------------------- 1 file changed, 37 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 4d96a0e..644c321 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -37,12 +37,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wconversion" # Too many to deal with dontwarn="$dontwarn -Wsign-conversion" - # GNULIB gettext.h violates - dontwarn="$dontwarn -Wvla" - # Many GNULIB header violations - dontwarn="$dontwarn -Wundef" - # Need to allow bad cast for execve() - dontwarn="$dontwarn -Wcast-qual" # We need to use long long in many places dontwarn="$dontwarn -Wlong-long" # We allow manual list of all enum cases without default: @@ -53,8 +47,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wstrict-overflow" # Not a problem since we don't use -funsafe-loop-optimizations dontwarn="$dontwarn -Wunsafe-loop-optimizations" - # Gnulib's stat-time.h violates this - dontwarn="$dontwarn -Waggregate-return" # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall dontwarn="$dontwarn -Wenum-compare" # gcc 5.1 -Wformat-signedness mishandles enums, not ready for prime time @@ -123,26 +115,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # Remove the ones we don't want, blacklisted earlier gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn])
- # GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff. - # Unfortunately, this means you can't simply use '-Wsign-compare' - # with gl_MANYWARN_COMPLEMENT - # So we have -W enabled, and then have to explicitly turn off... - wantwarn="$wantwarn -Wno-sign-compare" - - # GNULIB expects this to be part of -Wc++-compat, but we turn - # that one off, so we need to manually enable this again - wantwarn="$wantwarn -Wjump-misses-init"
We still have this at the top of the file:
dontwarn="$dontwarn -Wc++-compat"
so this -Wjump-misses-init should be kept
OK, didn't realize that.
- - # Silence certain warnings in gnulib, and use improved glibc headers - AH_VERBATIM([FORTIFY_SOURCE], - [/* Enable compile-time and run-time bounds-checking, and some warnings, - without upsetting newer glibc. */ - #if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ - # define _FORTIFY_SOURCE 2 - #endif - ])
Why are you disabling _FORTIFY_SOURCE - this is an important security check.
That's a good question :) I saw gnulib in the comment and didn't look what it actually does. I'll keep it here. Thanks, Pavel
We will require libvirt to have at least the same version as libvirt-dbus. The 1.2.12 version is the lowest one which we will support since it introduced virDomainDefineXMLFlags() and we don't have to support the non-flags APIs. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index df1a375..ba397ca 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([libvirt-dbus], [0.0.1], [libvir-list@redhat.com], [], [http://libvirt.org]) +AC_INIT([libvirt-dbus], [1.2.12], [libvir-list@redhat.com], [], [http://libvirt.org]) AC_CONFIG_SRCDIR(src/main.c) AC_CONFIG_AUX_DIR([build-aux]) @@ -11,7 +11,7 @@ AC_USE_SYSTEM_EXTENSIONS AM_SILENT_RULES([yes]) -LIBVIRT_REQUIRED=1.2.8 +LIBVIRT_REQUIRED=AC_PACKAGE_VERSION SYSTEMD_REQUIRED=211 AC_SUBST([LIBVIRT_REQUIRED]) dnl used in the .spec file AC_SUBST([SYSTEMD_REQUIRED]) dnl used in the .spec file -- 2.14.3
On Mon, Mar 19, 2018 at 10:30:49AM +0100, Pavel Hrdina wrote:
We will require libvirt to have at least the same version as libvirt-dbus. The 1.2.12 version is the lowest one which we will support since it introduced virDomainDefineXMLFlags() and we don't have to support the non-flags APIs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index df1a375..ba397ca 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([libvirt-dbus], [0.0.1], [libvir-list@redhat.com], [], [http://libvirt.org]) +AC_INIT([libvirt-dbus], [1.2.12], [libvir-list@redhat.com], [], [http://libvirt.org])
AC_CONFIG_SRCDIR(src/main.c) AC_CONFIG_AUX_DIR([build-aux]) @@ -11,7 +11,7 @@ AC_USE_SYSTEM_EXTENSIONS
AM_SILENT_RULES([yes])
-LIBVIRT_REQUIRED=1.2.8 +LIBVIRT_REQUIRED=AC_PACKAGE_VERSION
This feels a bit odd to me - what will you set the AC_INIT version to if you want todo 5 releases of libvirt-dbus, without bumping the min required version of libvirt. I can understand having the libvirt-dbus versions be set in lockstep to libvirt versions in AC_INIT, if you plan to always do a libvirt-dbus release on the 1st of the month at same time as libvirt, but even then I doubt you'd want to set min required libvirt to match. 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 :|
On Mon, Mar 19, 2018 at 01:42:11PM +0000, Daniel P. Berrangé wrote:
On Mon, Mar 19, 2018 at 10:30:49AM +0100, Pavel Hrdina wrote:
We will require libvirt to have at least the same version as libvirt-dbus. The 1.2.12 version is the lowest one which we will support since it introduced virDomainDefineXMLFlags() and we don't have to support the non-flags APIs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index df1a375..ba397ca 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([libvirt-dbus], [0.0.1], [libvir-list@redhat.com], [], [http://libvirt.org]) +AC_INIT([libvirt-dbus], [1.2.12], [libvir-list@redhat.com], [], [http://libvirt.org])
AC_CONFIG_SRCDIR(src/main.c) AC_CONFIG_AUX_DIR([build-aux]) @@ -11,7 +11,7 @@ AC_USE_SYSTEM_EXTENSIONS
AM_SILENT_RULES([yes])
-LIBVIRT_REQUIRED=1.2.8 +LIBVIRT_REQUIRED=AC_PACKAGE_VERSION
This feels a bit odd to me - what will you set the AC_INIT version to if you want todo 5 releases of libvirt-dbus, without bumping the min required version of libvirt.
I can understand having the libvirt-dbus versions be set in lockstep to libvirt versions in AC_INIT, if you plan to always do a libvirt-dbus release on the 1st of the month at same time as libvirt, but even then I doubt you'd want to set min required libvirt to match.
The reasoning behind it is that I would like to avoid using #if LIBVIR_CHECK_VERSION() like we do in libvirt-python and strictly depend on libvirt version that libvirt-dbus will be written for. The other solution is to not use XML files for interface but have the interface specified directly in the code in order to be able to generate correct introspect data depending on what was compiled in. Since this is more or less a binding, I think it would be better to follow libvirt versions like we do with other bindings. This was the first idea that I had and now I realize that it's not good one. I can change it back, however we need to make sure to properly update the libvirt required version once we implement new libvirt APIs. Pavel
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 :|
On Mon, Mar 19, 2018 at 02:56:49PM +0100, Pavel Hrdina wrote:
On Mon, Mar 19, 2018 at 01:42:11PM +0000, Daniel P. Berrangé wrote:
On Mon, Mar 19, 2018 at 10:30:49AM +0100, Pavel Hrdina wrote:
We will require libvirt to have at least the same version as libvirt-dbus. The 1.2.12 version is the lowest one which we will support since it introduced virDomainDefineXMLFlags() and we don't have to support the non-flags APIs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index df1a375..ba397ca 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([libvirt-dbus], [0.0.1], [libvir-list@redhat.com], [], [http://libvirt.org]) +AC_INIT([libvirt-dbus], [1.2.12], [libvir-list@redhat.com], [], [http://libvirt.org])
AC_CONFIG_SRCDIR(src/main.c) AC_CONFIG_AUX_DIR([build-aux]) @@ -11,7 +11,7 @@ AC_USE_SYSTEM_EXTENSIONS
AM_SILENT_RULES([yes])
-LIBVIRT_REQUIRED=1.2.8 +LIBVIRT_REQUIRED=AC_PACKAGE_VERSION
This feels a bit odd to me - what will you set the AC_INIT version to if you want todo 5 releases of libvirt-dbus, without bumping the min required version of libvirt.
I can understand having the libvirt-dbus versions be set in lockstep to libvirt versions in AC_INIT, if you plan to always do a libvirt-dbus release on the 1st of the month at same time as libvirt, but even then I doubt you'd want to set min required libvirt to match.
The reasoning behind it is that I would like to avoid using #if LIBVIR_CHECK_VERSION() like we do in libvirt-python and strictly depend on libvirt version that libvirt-dbus will be written for.
Wanting to avoid conditional version checks is a totally reasonable thing todo, so no objection to that. I'm just not sure it makes sense to tie the version numbers together unless you jump straight to the very latest version 4.2.0 right now and bump it again on each release.
The other solution is to not use XML files for interface but have the interface specified directly in the code in order to be able to generate correct introspect data depending on what was compiled in.
Since this is more or less a binding, I think it would be better to follow libvirt versions like we do with other bindings.
This was the first idea that I had and now I realize that it's not good one. I can change it back, however we need to make sure to properly update the libvirt required version once we implement new libvirt APIs.
You could write a 'make check' rule to validate this. eg run eu-readelf -a libvirt-dbus | grep LIBVIRT this will give you all the ELF versions from libvirt that are used by libvirt-dbus. You can thus check if you've used a version that is newer than the value listed in configure.ac 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 :|
On Mon, Mar 19, 2018 at 02:16:15PM +0000, Daniel P. Berrangé wrote:
On Mon, Mar 19, 2018 at 02:56:49PM +0100, Pavel Hrdina wrote:
On Mon, Mar 19, 2018 at 01:42:11PM +0000, Daniel P. Berrangé wrote:
On Mon, Mar 19, 2018 at 10:30:49AM +0100, Pavel Hrdina wrote:
We will require libvirt to have at least the same version as libvirt-dbus. The 1.2.12 version is the lowest one which we will support since it introduced virDomainDefineXMLFlags() and we don't have to support the non-flags APIs.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index df1a375..ba397ca 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([libvirt-dbus], [0.0.1], [libvir-list@redhat.com], [], [http://libvirt.org]) +AC_INIT([libvirt-dbus], [1.2.12], [libvir-list@redhat.com], [], [http://libvirt.org])
AC_CONFIG_SRCDIR(src/main.c) AC_CONFIG_AUX_DIR([build-aux]) @@ -11,7 +11,7 @@ AC_USE_SYSTEM_EXTENSIONS
AM_SILENT_RULES([yes])
-LIBVIRT_REQUIRED=1.2.8 +LIBVIRT_REQUIRED=AC_PACKAGE_VERSION
This feels a bit odd to me - what will you set the AC_INIT version to if you want todo 5 releases of libvirt-dbus, without bumping the min required version of libvirt.
I can understand having the libvirt-dbus versions be set in lockstep to libvirt versions in AC_INIT, if you plan to always do a libvirt-dbus release on the 1st of the month at same time as libvirt, but even then I doubt you'd want to set min required libvirt to match.
The reasoning behind it is that I would like to avoid using #if LIBVIR_CHECK_VERSION() like we do in libvirt-python and strictly depend on libvirt version that libvirt-dbus will be written for.
Wanting to avoid conditional version checks is a totally reasonable thing todo, so no objection to that. I'm just not sure it makes sense to tie the version numbers together unless you jump straight to the very latest version 4.2.0 right now and bump it again on each release.
That was my original idea to not tie the libvirt-dbus version numbers together with libvirt. Jumping straight to the latest version would mean more conservative distribution would not be able to introduce libvirt-dbus unless they bump libvirt itself or they adds downstream only patches to remove non-existent APIs in the old libvirt. That's why I changed my mind to follow libvirt versions and start with some reasonably old version, implement all the libvirt APIs till that version and make a release. After that we would implement remaining APIs based on the libvirt releases where new APIs was introduced and match libvirt-dbus releases accordingly.
The other solution is to not use XML files for interface but have the interface specified directly in the code in order to be able to generate correct introspect data depending on what was compiled in.
Since this is more or less a binding, I think it would be better to follow libvirt versions like we do with other bindings.
This was the first idea that I had and now I realize that it's not good one. I can change it back, however we need to make sure to properly update the libvirt required version once we implement new libvirt APIs.
You could write a 'make check' rule to validate this. eg run
eu-readelf -a libvirt-dbus | grep LIBVIRT
this will give you all the ELF versions from libvirt that are used by libvirt-dbus. You can thus check if you've used a version that is newer than the value listed in configure.ac
Nice, TIL :). Pavel
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> --- 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 LIBVIRT_REQUIRED=AC_PACKAGE_VERSION SYSTEMD_REQUIRED=211 +LIBVIRT_GLIB_REQUIRED=1.0.0 +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..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); +} + +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 G_GNUC_UNUSED) +{ + 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, + 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->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..21ee6fc --- /dev/null +++ b/src/gdbus.h @@ -0,0 +1,95 @@ +#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); + +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, + 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
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@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 :|
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@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
This removes all the sd-bus code and uses GDBus instead. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- README | 1 - configure.ac | 3 - data/Makefile.am | 4 +- data/org.libvirt.Connect.xml | 56 ++++ data/org.libvirt.Domain.xml | 51 +++ libvirt-dbus.spec.in | 4 +- src/Makefile.am | 3 - src/connect.c | 316 ++++++++----------- src/connect.h | 35 +-- src/domain.c | 729 +++++++++++++++++++------------------------ src/domain.h | 7 +- src/events.c | 177 +++++------ src/main.c | 299 +++++++----------- src/util.c | 170 +++++----- src/util.h | 53 ++-- test/travis-run | 2 +- 16 files changed, 855 insertions(+), 1055 deletions(-) create mode 100644 data/org.libvirt.Connect.xml create mode 100644 data/org.libvirt.Domain.xml diff --git a/README b/README index a85114e..b95b09b 100644 --- a/README +++ b/README @@ -56,7 +56,6 @@ raised in future releases based on this distro build target policy. The packages required to build libvirt-dbus are - - systemd-211 - libvirt - glib2 diff --git a/configure.ac b/configure.ac index d6ed0ef..6db5a59 100644 --- a/configure.ac +++ b/configure.ac @@ -13,11 +13,9 @@ AM_SILENT_RULES([yes]) GLIB2_REQUIRED=2.50.0 LIBVIRT_REQUIRED=AC_PACKAGE_VERSION -SYSTEMD_REQUIRED=211 LIBVIRT_GLIB_REQUIRED=1.0.0 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}'` @@ -41,7 +39,6 @@ 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 diff --git a/data/Makefile.am b/data/Makefile.am index a886687..dd60713 100644 --- a/data/Makefile.am +++ b/data/Makefile.am @@ -18,7 +18,9 @@ polkit_files = \ polkitdir = $(sysconfdir)/polkit-1/rules.d polkit_DATA = $(polkit_files:.rules.in=.rules) -interfaces_files = +interfaces_files = \ + org.libvirt.Connect.xml \ + org.libvirt.Domain.xml interfacesdir = $(DBUS_INTERFACES_DIR) interfaces_DATA = $(interfaces_files) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml new file mode 100644 index 0000000..a40ce1d --- /dev/null +++ b/data/org.libvirt.Connect.xml @@ -0,0 +1,56 @@ +<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" +"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd"> + +<node name="/org/libvirt/connect"> + <interface name="org.libvirt.Connect"> + <method name="CreateXML"> + <arg name="xml" type="s" direction="in"/> + <arg name="flags" type="u" direction="in"/> + <arg name="domain" type="o" direction="out"/> + </method> + <method name="DefineXML"> + <arg name="xml" type="s" direction="in"/> + <arg name="domain" type="o" direction="out"/> + </method> + <method name="ListDomains"> + <arg name="flags" type="u" direction="in"/> + <arg name="domains" type="ao" direction="out"/> + </method> + <signal name="DomainCrashed"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainDefined"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainPMSuspended"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainResumed"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainShutdown"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainStarted"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainStopped"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainSuspended"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + <signal name="DomainUndefined"> + <arg name="reason" type="s"/> + <arg name="domain" type="o"/> + </signal> + </interface> +</node> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml new file mode 100644 index 0000000..48bf40f --- /dev/null +++ b/data/org.libvirt.Domain.xml @@ -0,0 +1,51 @@ +<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" +"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd"> + +<node name="/org/libvirt/domain"> + <interface name="org.libvirt.Domain"> + <property name="Name" type="s" access="read"/> + <property name="UUID" type="s" access="read"/> + <property name="Id" type="u" access="read"/> + <property name="Vcpus" type="u" access="read"/> + <property name="OSType" type="s" access="read"/> + <property name="Active" type="b" access="read"/> + <property name="Persistent" type="b" access="read"/> + <property name="State" type="s" access="read"/> + <property name="Autostart" type="b" access="read"/> + <method name="GetXMLDesc"> + <arg name="flags" type="u" direction="in"/> + <arg name="xml" type="s" direction="out"/> + </method> + <method name="GetStats"> + <arg name="stats" type="u" direction="in"/> + <arg name="flags" type="u" direction="in"/> + <arg name="records" type="a{sv}" direction="out"/> + </method> + <method name="Shutdown"/> + <method name="Destroy"/> + <method name="Reboot"> + <arg name="flags" type="u" direction="in"/> + </method> + <method name="Reset"> + <arg name="flags" type="u" direction="in"/> + </method> + <method name="Create"/> + <method name="Undefine"/> + <signal name="DeviceAdded"> + <arg name="device" type="s"/> + </signal> + <signal name="DeviceRemoved"> + <arg name="device" type="s"/> + </signal> + <signal name="DiskChange"> + <arg name="oldSrcPath" type="s"/> + <arg name="newSrcPath" type="s"/> + <arg name="device" type="s"/> + <arg name="reason" type="s"/> + </signal> + <signal name="TrayChange"> + <arg name="device" type="s"/> + <arg name="reason" type="s"/> + </signal> + </interface> +</node> diff --git a/libvirt-dbus.spec.in b/libvirt-dbus.spec.in index 662ece1..eb531bb 100644 --- a/libvirt-dbus.spec.in +++ b/libvirt-dbus.spec.in @@ -2,7 +2,6 @@ %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@ @@ -19,12 +18,10 @@ 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 @@ -61,5 +58,6 @@ exit 0 %{_datadir}/dbus-1/services/org.libvirt.service %{_datadir}/dbus-1/system-services/org.libvirt.service %{_datadir}/dbus-1/system.d/org.libvirt.conf +%{_datadir}/dbus-1/interfaces/org.libvirt.*.xml %changelog diff --git a/src/Makefile.am b/src/Makefile.am index e7bba9d..7248561 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -19,7 +19,6 @@ bin_PROGRAMS = libvirt-dbus libvirt_dbus_SOURCES = $(DAEMON_SOURCES) libvirt_dbus_CFLAGS = \ - $(SYSTEMD_CFLAGS) \ $(GIO2_CFLAGS) \ $(GLIB2_CFLAGS) \ $(LIBVIRT_CFLAGS) \ @@ -29,7 +28,6 @@ libvirt_dbus_CFLAGS = \ $(NULL) libvirt_dbus_LDFLAGS = \ - $(SYSTEMD_LDFLAGS) \ $(GIO2_LDFLAGS) \ $(GLIB2_LDFLAGS) \ $(LIBVIRT_LDFLAGS) \ @@ -39,7 +37,6 @@ libvirt_dbus_LDFLAGS = \ $(NULL) libvirt_dbus_LDADD = \ - $(SYSTEMD_LIBS) \ $(GIO2_LIBS) \ $(GLIB2_LIBS) \ $(LIBVIRT_LIBS) \ diff --git a/src/connect.c b/src/connect.c index 695fc0d..a3b1cf2 100644 --- a/src/connect.c +++ b/src/connect.c @@ -1,12 +1,11 @@ +#include "connect.h" #include "domain.h" #include "events.h" -#include "connect.h" #include "util.h" -#include <stdbool.h> -#include <stdlib.h> +#include <glib/gprintf.h> -static int virtDBusConnectCredType[] = { +static gint virtDBusConnectCredType[] = { VIR_CRED_AUTHNAME, VIR_CRED_ECHOPROMPT, VIR_CRED_REALM, @@ -15,31 +14,31 @@ static int virtDBusConnectCredType[] = { VIR_CRED_EXTERNAL, }; -static int -virtDBusConnectAuthCallback(virConnectCredentialPtr cred VIRT_ATTR_UNUSED, - unsigned int ncred VIRT_ATTR_UNUSED, - void *cbdata) +static gint +virtDBusConnectAuthCallback(virConnectCredentialPtr cred G_GNUC_UNUSED, + guint ncred G_GNUC_UNUSED, + gpointer cbdata) { - sd_bus_error *error = cbdata; - - return virtDBusUtilSetError(error, - "Interactive authentication is not supported. " - "Use client configuration file for libvirt."); + GError **error = cbdata; + g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, + "Interactive authentication is not supported. " + "Use client configuration file for libvirt."); + return -1; } static virConnectAuth virtDBusConnectAuth = { virtDBusConnectCredType, - VIRT_N_ELEMENTS(virtDBusConnectCredType), + G_N_ELEMENTS(virtDBusConnectCredType), virtDBusConnectAuthCallback, NULL, }; static void virtDBusConnectClose(virtDBusConnect *connect, - bool deregisterEvents) + gboolean deregisterEvents) { - for (int i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) { + for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) { if (connect->callback_ids[i] >= 0) { if (deregisterEvents) { virConnectDomainEventDeregisterAny(connect->connection, @@ -53,130 +52,91 @@ virtDBusConnectClose(virtDBusConnect *connect, connect->connection = NULL; } -int +gboolean virtDBusConnectOpen(virtDBusConnect *connect, - sd_bus_error *error) + GError **error) { if (connect->connection) { if (virConnectIsAlive(connect->connection)) - return 0; + return TRUE; else - virtDBusConnectClose(connect, false); + virtDBusConnectClose(connect, FALSE); } virtDBusConnectAuth.cbdata = error; connect->connection = virConnectOpenAuth(connect->uri, &virtDBusConnectAuth, 0); - if (!connect->connection) - return virtDBusUtilSetLastVirtError(error); + if (!connect->connection) { + if (!*error) + virtDBusUtilSetLastVirtError(error); + return FALSE; + } virtDBusEventsRegister(connect); - return 0; + return TRUE; } -static int -virtDBusConnectEnumarateDomains(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path VIRT_ATTR_UNUSED, - void *userdata, - char ***nodes, - sd_bus_error *error) +static void +virtDBusConnectListDomains(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainListFreep) virDomainPtr *domains = NULL; - _cleanup_(virtDBusUtilStrvFreep) char **paths = NULL; - int n_domains; - int r; - - r = virtDBusConnectOpen(connect, error); - if (r < 0) - return r; - - n_domains = virConnectListAllDomains(connect->connection, &domains, 0); - if (n_domains < 0) - return virtDBusUtilSetLastVirtError(error); - - paths = calloc(n_domains + 1, sizeof(char *)); - - for (int i = 0; i < n_domains; i += 1) - paths[i] = virtDBusUtilBusPathForVirDomain(domains[i], - connect->domainPath); + virtDBusConnect *connect = userData; + g_autoptr(virDomainPtr) domains = NULL; + guint flags; + GVariantBuilder builder; + GVariant *gdomains; - *nodes = paths; - paths = NULL; + g_variant_get(inArgs, "(u)", &flags); - return 0; -} + if (!virtDBusConnectOpen(connect, error)) + return; -static int -virtDBusConnectListDomains(sd_bus_message *message, - void *userdata, - sd_bus_error *error) -{ - virtDBusConnect *connect = userdata; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_(virtDBusUtilVirDomainListFreep) virDomainPtr *domains = NULL; - uint32_t flags; - int r; - - r = sd_bus_message_read(message, "u", &flags); - if (r < 0) - return r; - - r = virtDBusConnectOpen(connect, error); - if (r < 0) - return r; - - r = virConnectListAllDomains(connect->connection, &domains, flags); - if (r < 0) + if (virConnectListAllDomains(connect->connection, &domains, flags) < 0) return virtDBusUtilSetLastVirtError(error); - r = sd_bus_message_new_method_return(message, &reply); - if (r < 0) - return r; - - r = sd_bus_message_open_container(reply, 'a', "o"); - if (r < 0) - return r; + if (!*domains) + return; - for (int i = 0; domains[i] != NULL; i += 1) { - _cleanup_(virtDBusUtilFreep) char *path = NULL; + g_variant_builder_init(&builder, G_VARIANT_TYPE("ao")); + for (gint i = 0; domains[i]; i++) { + g_autofree gchar *path = NULL; path = virtDBusUtilBusPathForVirDomain(domains[i], connect->domainPath); - r = sd_bus_message_append(reply, "o", path); - if (r < 0) - return r; + g_variant_builder_add(&builder, "o", path); } - r = sd_bus_message_close_container(reply); - if (r < 0) - return r; - - return sd_bus_send(NULL, reply, NULL); + gdomains = g_variant_builder_end(&builder); + *outArgs = g_variant_new_tuple(&gdomains, 1); } -static int -virtDBusConnectCreateXML(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusConnectCreateXML(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - const char *xml; - uint32_t flags; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - int r; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + g_autofree gchar *path = NULL; + gchar *xml; + guint flags; - r = sd_bus_message_read(message, "su", &xml, &flags); - if (r < 0) - return r; + g_variant_get(inArgs, "(&su)", &xml, &flags); - r = virtDBusConnectOpen(connect, error); - if (r < 0) - return r; + if (!virtDBusConnectOpen(connect, error)) + return; domain = virDomainCreateXML(connect->connection, xml, flags); if (!domain) @@ -184,27 +144,27 @@ virtDBusConnectCreateXML(sd_bus_message *message, path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - return sd_bus_reply_method_return(message, "o", path); + *outArgs = g_variant_new("(o)", path); } -static int -virtDBusConnectDefineXML(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusConnectDefineXML(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - const char *xml; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - int r; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + g_autofree gchar *path = NULL; + gchar *xml; - r = sd_bus_message_read(message, "s", &xml); - if (r < 0) - return r; + g_variant_get(inArgs, "(&s)", &xml); - r = virtDBusConnectOpen(connect, error); - if (r < 0) - return r; + if (!virtDBusConnectOpen(connect, error)) + return; domain = virDomainDefineXML(connect->connection, xml); if (!domain) @@ -212,97 +172,73 @@ virtDBusConnectDefineXML(sd_bus_message *message, path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - return sd_bus_reply_method_return(message, "o", path); + *outArgs = g_variant_new("(o)", path); } -static const sd_bus_vtable virt_connect_vtable[] = { - SD_BUS_VTABLE_START(0), +static virtDBusGDBusMethodTable virtDBusConnectMethodTable[] = { + { "ListDomains", virtDBusConnectListDomains }, + { "CreateXML", virtDBusConnectCreateXML }, + { "DefineXML", virtDBusConnectDefineXML }, + { NULL, NULL } +}; - SD_BUS_METHOD("ListDomains", "u", "ao", virtDBusConnectListDomains, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("CreateXML", "su", "o", virtDBusConnectCreateXML, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("DefineXML", "s", "o", virtDBusConnectDefineXML, SD_BUS_VTABLE_UNPRIVILEGED), +static GDBusInterfaceInfo *interfaceInfo = NULL; - SD_BUS_SIGNAL("DomainDefined", "so", 0), - SD_BUS_SIGNAL("DomainUndefined", "so", 0), - SD_BUS_SIGNAL("DomainStarted", "so", 0), - SD_BUS_SIGNAL("DomainSuspended", "so", 0), - SD_BUS_SIGNAL("DomainResumed", "so", 0), - SD_BUS_SIGNAL("DomainStopped", "so", 0), - SD_BUS_SIGNAL("DomainShutdown", "so", 0), - SD_BUS_SIGNAL("DomainPMSuspended", "so", 0), - SD_BUS_SIGNAL("DomainCrashed", "so", 0), +static void +virtDBusConnectFree(virtDBusConnect *connect) +{ + if (connect->connection) + virtDBusConnectClose(connect, TRUE); - SD_BUS_VTABLE_END -}; + g_free(connect->domainPath); + g_free(connect); +} +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virtDBusConnect, virtDBusConnectFree); -int +void virtDBusConnectNew(virtDBusConnect **connectp, - sd_bus *bus, - const char *uri, - const char *connectPath) + GDBusConnection *bus, + const gchar *uri, + const gchar *connectPath) { - _cleanup_(virtDBusConnectFreep) virtDBusConnect *connect = NULL; - int r; + g_autoptr(virtDBusConnect) connect = NULL; + + if (!interfaceInfo) { + interfaceInfo = virtDBusGDBusLoadIntrospectData(VIRT_DBUS_CONNECT_INTERFACE); + g_assert(interfaceInfo != NULL); + } - connect = calloc(1, sizeof(virtDBusConnect)); - for (int i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) + connect = g_new0(virtDBusConnect, 1); + g_assert(connect != NULL); + + for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) connect->callback_ids[i] = -1; - connect->bus = sd_bus_ref(bus); + connect->bus = bus; connect->uri = uri; connect->connectPath = connectPath; - connect->enumerateDomains = virtDBusConnectEnumarateDomains; - - r = sd_bus_add_object_vtable(connect->bus, - NULL, - connect->connectPath, - VIRT_DBUS_CONNECT_INTERFACE, - virt_connect_vtable, - connect); - if (r < 0) - return r; + virtDBusGDBusRegisterObject(bus, + connect->connectPath, + interfaceInfo, + virtDBusConnectMethodTable, + NULL, + connect); - if ((r = virtDBusDomainRegister(connect, bus) < 0)) - return r; + virtDBusDomainRegister(connect); *connectp = connect; connect = NULL; - - return 0; -} - -virtDBusConnect * -virtDBusConnectFree(virtDBusConnect *connect) -{ - if (connect->bus) - sd_bus_unref(connect->bus); - - if (connect->connection) - virtDBusConnectClose(connect, true); - - free(connect->domainPath); - - free(connect); - - return NULL; -} - -void -virtDBusConnectFreep(virtDBusConnect **connectp) -{ - if (*connectp) - virtDBusConnectFree(*connectp); } void -virtDBusConnectListFree(virtDBusConnect ***connectList) +virtDBusConnectListFree(virtDBusConnect **connectList) { - if (!*connectList) + if (!connectList) return; - for (int i = 0; (*connectList)[i]; i += 1) - virtDBusConnectFree((*connectList)[i]); + for (gint i = 0; connectList[i]; i += 1) + virtDBusConnectFree(connectList[i]); - free(*connectList); + g_free(connectList); } diff --git a/src/connect.h b/src/connect.h index 9e5f533..f8b1d06 100644 --- a/src/connect.h +++ b/src/connect.h @@ -2,39 +2,32 @@ #define VIR_ENUM_SENTINELS +#include "util.h" + #include <libvirt/libvirt.h> -#include <systemd/sd-bus.h> #define VIRT_DBUS_CONNECT_INTERFACE "org.libvirt.Connect" struct virtDBusConnect { - sd_bus *bus; - const char *uri; - const char *connectPath; - char *domainPath; + GDBusConnection *bus; + const gchar *uri; + const gchar *connectPath; + gchar *domainPath; virConnectPtr connection; - sd_bus_node_enumerator_t enumerateDomains; - - int callback_ids[VIR_DOMAIN_EVENT_ID_LAST]; + gint callback_ids[VIR_DOMAIN_EVENT_ID_LAST]; }; typedef struct virtDBusConnect virtDBusConnect; -int +void virtDBusConnectNew(virtDBusConnect **connectp, - sd_bus *bus, - const char *uri, - const char *connectPath); + GDBusConnection *bus, + const gchar *uri, + const gchar *connectPath); -int +gboolean virtDBusConnectOpen(virtDBusConnect *connect, - sd_bus_error *error); - -virtDBusConnect * -virtDBusConnectFree(virtDBusConnect *connect); - -void -virtDBusConnectFreep(virtDBusConnect **connectp); + GError **error); void -virtDBusConnectListFree(virtDBusConnect ***connectList); +virtDBusConnectListFree(virtDBusConnect **connectList); diff --git a/src/domain.c b/src/domain.c index f68a3a0..8403ea6 100644 --- a/src/domain.c +++ b/src/domain.c @@ -1,208 +1,192 @@ -#define _GNU_SOURCE - #include "domain.h" #include "util.h" #include <libvirt/libvirt.h> -#include <stdio.h> static virDomainPtr virtDBusDomainGetVirDomain(virtDBusConnect *connect, - const char *path, - sd_bus_error *error) + const gchar *objectPath, + GError **error) { virDomainPtr domain; if (virtDBusConnectOpen(connect, error) < 0) return NULL; - domain = virtDBusUtilVirDomainFromBusPath(connect->connection, path, + domain = virtDBusUtilVirDomainFromBusPath(connect->connection, + objectPath, connect->domainPath); - if (domain == NULL) { - sd_bus_error_setf(error, - SD_BUS_ERROR_UNKNOWN_OBJECT, - "Unknown object '%s'.", path); + if (!domain) { + virtDBusUtilSetLastVirtError(error); return NULL; } return domain; } -static int -virtDBusDomainGetName(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetName(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - const char *name = ""; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + const gchar *name; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; name = virDomainGetName(domain); - if (name == NULL) - return sd_bus_message_append(reply, "s", ""); + if (!name) + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "s", name); + *value = g_variant_new("s", name); } -static int -virtDBusDomainGetUUID(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetUUID(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - char uuid[VIR_UUID_STRING_BUFLEN] = ""; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gchar uuid[VIR_UUID_STRING_BUFLEN] = ""; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - virDomainGetUUIDString(domain, uuid); + if (virDomainGetUUIDString(domain, uuid) < 0) + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "s", uuid); + *value = g_variant_new("s", uuid); } -static int -virtDBusDomainGetId(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetId(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + guint id; + + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + id = virDomainGetID(domain); + if (id == (guint)-1) + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "u", virDomainGetID(domain)); + *value = g_variant_new("u", id); } -static int -virtDBusDomainGetVcpus(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetVcpus(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gint vcpus; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + + vcpus = virDomainGetVcpusFlags(domain, VIR_DOMAIN_VCPU_CURRENT); + if (vcpus < 0) + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "u", virDomainGetVcpusFlags(domain, VIR_DOMAIN_VCPU_CURRENT)); + *value = g_variant_new("u", vcpus); } -static int -virtDBusDomainGetOsType(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetOsType(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - _cleanup_(virtDBusUtilFreep) char *os_type = NULL; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + g_autofree gchar *osType = NULL; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - os_type = virDomainGetOSType(domain); - if (os_type == NULL) - return sd_bus_message_append(reply, "s", ""); + osType = virDomainGetOSType(domain); + if (!osType) + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "s", os_type); + *value = g_variant_new("s", osType); } -static int -virtDBusDomainGetActive(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetActive(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int active; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gint active; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; active = virDomainIsActive(domain); if (active < 0) - return sd_bus_message_append(reply, "b", 0); + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "b", active); + *value = g_variant_new("b", !!active); } -static int -virtDBusDomainGetPersistent(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetPersistent(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int persistent; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gint persistent; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; persistent = virDomainIsPersistent(domain); if (persistent < 0) - return sd_bus_message_append(reply, "b", 0); + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "b", persistent); + *value = g_variant_new("b", !!persistent); } -static int -virtDBusDomainGetState(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetState(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int state = 0; - const char *string; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gint state = 0; + const gchar *string; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - virDomainGetState(domain, &state, NULL, 0); + if (virDomainGetState(domain, &state, NULL, 0) < 0) + return virtDBusUtilSetLastVirtError(error); switch (state) { case VIR_DOMAIN_NOSTATE: @@ -232,88 +216,80 @@ virtDBusDomainGetState(sd_bus *bus VIRT_ATTR_UNUSED, break; } - return sd_bus_message_append(reply, "s", string); + *value = g_variant_new("s", string); } -static int -virtDBusDomainGetAutostart(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - const char *property VIRT_ATTR_UNUSED, - sd_bus_message *reply, - void *userdata, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainGetAutostart(const gchar *objectPath, + gpointer userData, + GVariant **value, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int autostart = 0; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gint autostart = 0; - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - virDomainGetAutostart(domain, &autostart); + if (virDomainGetAutostart(domain, &autostart) < 0) + return virtDBusUtilSetLastVirtError(error); - return sd_bus_message_append(reply, "b", autostart); + *value = g_variant_new("b", !!autostart); } -static int -virtDBusDomainGetXMLDesc(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainGetXMLDesc(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - _cleanup_(virtDBusUtilFreep) char *description = NULL; - uint32_t flags; - int r; - - r = sd_bus_message_read(message, "u", &flags); - if (r < 0) - return r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - description = virDomainGetXMLDesc(domain, flags); - if (!description) + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + g_autofree gchar *xml = NULL; + guint flags; + + g_variant_get(inArgs, "(u)", &flags); + + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + + xml = virDomainGetXMLDesc(domain, flags); + if (!xml) return virtDBusUtilSetLastVirtError(error); - return sd_bus_reply_method_return(message, "s", description); + *outArgs = g_variant_new("(s)", xml); } -static void -virtDBusDomainStatsRecordListFreep(virDomainStatsRecordPtr **statsp) -{ - if (*statsp) - virDomainStatsRecordListFree(*statsp); -} +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainStatsRecordPtr, virDomainStatsRecordListFree); -static int -virtDBusDomainGetStats(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainGetStats(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; virDomainPtr domains[2]; - _cleanup_(virtDBusDomainStatsRecordListFreep) virDomainStatsRecordPtr *records = NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - uint32_t flags, stats; - int r; + g_autoptr(virDomainStatsRecordPtr) records = NULL; + guint stats; + guint flags; + GVariant *grecords; - r = sd_bus_message_read(message, "uu", &stats, &flags); - if (r < 0) - return r; + g_variant_get(inArgs, "(uu)", &stats, &flags); - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; domains[0] = domain; domains[1] = NULL; @@ -321,243 +297,182 @@ virtDBusDomainGetStats(sd_bus_message *message, if (virDomainListGetStats(domains, stats, &records, flags) != 1) return virtDBusUtilSetLastVirtError(error); - r = sd_bus_message_new_method_return(message, &reply); - if (r < 0) - return r; - - r = virtDBusUtilMessageAppendTypedParameters(reply, records[0]->params, records[0]->nparams); - if (r < 0) - return r; + grecords = virtDBusUtilTypedParamsToGVariant(records[0]->params, + records[0]->nparams); - return sd_bus_send(NULL, reply, NULL); + *outArgs = g_variant_new_tuple(&grecords, 1); } -static int -virtDBusDomainShutdown(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainShutdown(GVariant *inArgs G_GNUC_UNUSED, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - r = virDomainShutdown(domain); - if (r < 0) - return virtDBusUtilSetLastVirtError(error); + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - return sd_bus_reply_method_return(message, ""); + if (virDomainShutdown(domain) < 0) + virtDBusUtilSetLastVirtError(error); } -static int -virtDBusDomainDestroy(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainDestroy(GVariant *inArgs G_GNUC_UNUSED, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - r = virDomainDestroy(domain); - if (r < 0) - return virtDBusUtilSetLastVirtError(error); + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - return sd_bus_reply_method_return(message, ""); + if (virDomainDestroy(domain) < 0) + virtDBusUtilSetLastVirtError(error); } -static int -virtDBusDomainReboot(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainReboot(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) + { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - uint32_t flags; - int r; - - r = sd_bus_message_read(message, "u", &flags); - if (r < 0) - return r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - r = virDomainReboot(domain, flags); - if (r < 0) - return virtDBusUtilSetLastVirtError(error); + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + guint flags; - return sd_bus_reply_method_return(message, ""); -} + g_variant_get(inArgs, "(u)", &flags); -static int -virtDBusDomainReset(sd_bus_message *message, - void *userdata, - sd_bus_error *error) -{ - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - uint32_t flags; - int r; - - r = sd_bus_message_read(message, "u", &flags); - if (r < 0) - return r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - r = virDomainReset(domain, flags); - if (r < 0) - return virtDBusUtilSetLastVirtError(error); + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - return sd_bus_reply_method_return(message, ""); + if (virDomainReboot(domain, flags) < 0) + virtDBusUtilSetLastVirtError(error); } -static int -virtDBusDomainCreate(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainReset(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) + { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - r = virDomainCreate(domain); - if (r < 0) - return virtDBusUtilSetLastVirtError(error); + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + guint flags; - return sd_bus_reply_method_return(message, ""); + g_variant_get(inArgs, "(u)", &flags); + + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + + if (virDomainReset(domain, flags) < 0) + virtDBusUtilSetLastVirtError(error); } -static int -virtDBusDomainUndefine(sd_bus_message *message, - void *userdata, - sd_bus_error *error) +static void +virtDBusDomainCreate(GVariant *inArgs G_GNUC_UNUSED, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int r; - - domain = virtDBusDomainGetVirDomain(connect, - sd_bus_message_get_path(message), - error); - if (domain == NULL) - return -1; - - r = virDomainUndefine(domain); - if (r < 0) - return virtDBusUtilSetLastVirtError(error); + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; - return sd_bus_reply_method_return(message, ""); -} + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; -static const sd_bus_vtable virt_domain_vtable[] = { - SD_BUS_VTABLE_START(0), - - SD_BUS_PROPERTY("Name", "s", virtDBusDomainGetName, 0, 0), - SD_BUS_PROPERTY("UUID", "s", virtDBusDomainGetUUID, 0, 0), - SD_BUS_PROPERTY("Id", "u", virtDBusDomainGetId, 0, 0), - SD_BUS_PROPERTY("Vcpus", "u", virtDBusDomainGetVcpus, 0, 0), - SD_BUS_PROPERTY("OSType", "s", virtDBusDomainGetOsType, 0, 0), - SD_BUS_PROPERTY("Active", "b", virtDBusDomainGetActive, 0, 0), - SD_BUS_PROPERTY("Persistent", "b", virtDBusDomainGetPersistent, 0, 0), - SD_BUS_PROPERTY("State", "s", virtDBusDomainGetState, 0, 0), - SD_BUS_PROPERTY("Autostart", "b", virtDBusDomainGetAutostart, 0, 0), - - SD_BUS_METHOD("GetXMLDesc", "u", "s", virtDBusDomainGetXMLDesc, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("GetStats", "uu", "a{sv}", virtDBusDomainGetStats, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("Shutdown", "", "", virtDBusDomainShutdown, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("Destroy", "", "", virtDBusDomainDestroy, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("Reboot", "u", "", virtDBusDomainReboot, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("Reset", "u", "", virtDBusDomainReset, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("Create", "", "", virtDBusDomainCreate, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD("Undefine", "", "", virtDBusDomainUndefine, SD_BUS_VTABLE_UNPRIVILEGED), - - SD_BUS_SIGNAL("DeviceAdded", "s", 0), - SD_BUS_SIGNAL("DeviceRemoved", "s", 0), - SD_BUS_SIGNAL("DiskChange", "ssss", 0), - SD_BUS_SIGNAL("TrayChange", "ss", 0), - - SD_BUS_VTABLE_END -}; + if (virDomainCreate(domain) < 0) + virtDBusUtilSetLastVirtError(error); +} -static int -virtDBusDomainLookup(sd_bus *bus VIRT_ATTR_UNUSED, - const char *path, - const char *interface VIRT_ATTR_UNUSED, - void *userdata, - void **found, - sd_bus_error *error VIRT_ATTR_UNUSED) +static void +virtDBusDomainUndefine(GVariant *inArgs G_GNUC_UNUSED, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath G_GNUC_UNUSED, + gpointer userData, + GVariant **outArgs G_GNUC_UNUSED, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) { - virtDBusConnect *connect = userdata; - _cleanup_(virtDBusUtilFreep) char *name = NULL; - _cleanup_(virtDBusUtilVirDomainFreep) virDomainPtr domain = NULL; - int r; + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; - r = sd_bus_path_decode(path, connect->domainPath, &name); - if (r < 0) - return r; + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; - if (*name == '\0') - return 0; + if (virDomainUndefine(domain) < 0) + virtDBusUtilSetLastVirtError(error); +} - domain = virtDBusDomainGetVirDomain(connect, path, error); - if (!domain) - return 0; +static virtDBusGDBusPropertyTable virtDBusDomainPropertyTable[] = { + { "Name", virtDBusDomainGetName, NULL }, + { "UUID", virtDBusDomainGetUUID, NULL }, + { "Id", virtDBusDomainGetId, NULL }, + { "Vcpus", virtDBusDomainGetVcpus, NULL }, + { "OSType", virtDBusDomainGetOsType, NULL }, + { "Active", virtDBusDomainGetActive, NULL }, + { "Persistent", virtDBusDomainGetPersistent, NULL }, + { "State", virtDBusDomainGetState, NULL }, + { "Autostart", virtDBusDomainGetAutostart, NULL }, + { NULL, NULL, NULL } +}; - /* - * There's no way to unref the pointer we're returning here. So, - * return the connect object and look up the domain again in the - * domain_* callbacks. - */ - *found = connect; +static virtDBusGDBusMethodTable virtDBusDomainMethodTable[] = { + { "GetXMLDesc", virtDBusDomainGetXMLDesc }, + { "GetStats", virtDBusDomainGetStats }, + { "Shutdown", virtDBusDomainShutdown }, + { "Destroy", virtDBusDomainDestroy }, + { "Reboot", virtDBusDomainReboot }, + { "Reset", virtDBusDomainReset }, + { "Create", virtDBusDomainCreate }, + { "Undefine", virtDBusDomainUndefine }, + { NULL, NULL } +}; - return 1; -} +static GDBusInterfaceInfo *interfaceInfo = NULL; -int -virtDBusDomainRegister(virtDBusConnect *connect, - sd_bus *bus) +void +virtDBusDomainRegister(virtDBusConnect *connect) { - int r; - - r = asprintf(&connect->domainPath, "%s/domain", connect->connectPath); - if (r < 0) - return r; - - r = sd_bus_add_node_enumerator(bus, NULL, connect->domainPath, - connect->enumerateDomains, connect); - if (r < 0) - return r; - - return sd_bus_add_fallback_vtable(bus, - NULL, - connect->domainPath, - VIRT_DBUS_DOMAIN_INTERFACE, - virt_domain_vtable, - virtDBusDomainLookup, - connect); + connect->domainPath = g_strdup_printf("%s/domain", connect->connectPath); + g_assert(connect->domainPath); + + if (!interfaceInfo) { + interfaceInfo = virtDBusGDBusLoadIntrospectData(VIRT_DBUS_DOMAIN_INTERFACE); + g_assert(interfaceInfo != NULL); + } + + virtDBusGDBusRegisterSubtree(connect->bus, + connect->domainPath, + interfaceInfo, + virtDBusDomainMethodTable, + virtDBusDomainPropertyTable, + connect); } diff --git a/src/domain.h b/src/domain.h index 03a29ed..d56d703 100644 --- a/src/domain.h +++ b/src/domain.h @@ -2,10 +2,7 @@ #include "connect.h" -#include <systemd/sd-bus.h> - #define VIRT_DBUS_DOMAIN_INTERFACE "org.libvirt.Domain" -int -virtDBusDomainRegister(virtDBusConnect *connect, - sd_bus *bus); +void +virtDBusDomainRegister(virtDBusConnect *connect); diff --git a/src/events.c b/src/events.c index c45acd7..dada55f 100644 --- a/src/events.c +++ b/src/events.c @@ -2,23 +2,19 @@ #include "events.h" #include "util.h" -#include <assert.h> #include <libvirt/libvirt.h> -#include <systemd/sd-bus.h> -static int -virtDBusEventsDomainLifecycle(virConnectPtr connection VIRT_ATTR_UNUSED, +static gint +virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED, virDomainPtr domain, - int event, - int detail VIRT_ATTR_UNUSED, - void *opaque) + gint event, + gint detail G_GNUC_UNUSED, + gpointer opaque) { virtDBusConnect *connect = opaque; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL; - const char *signal = NULL; - const char *name; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - int r; + const gchar *signal = NULL; + const gchar *name; + g_autofree gchar *path = NULL; switch (event) { case VIR_DOMAIN_EVENT_DEFINED: @@ -52,103 +48,77 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection VIRT_ATTR_UNUSED, return 0; } - r = sd_bus_message_new_signal(connect->bus, - &message, - connect->connectPath, - VIRT_DBUS_CONNECT_INTERFACE, - signal); - if (r < 0) - return r; - name = virDomainGetName(domain); path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - r = sd_bus_message_append(message, "so", name ? : "", path); - if (r < 0) - return r; + g_dbus_connection_emit_signal(connect->bus, + NULL, + connect->connectPath, + VIRT_DBUS_CONNECT_INTERFACE, + signal, + g_variant_new("(so)", name, path), + NULL); - return sd_bus_send(connect->bus, message, NULL); + return 0; } -static int -virtDBusEventsDomainDeviceAdded(virConnectPtr connection VIRT_ATTR_UNUSED, +static gint +virtDBusEventsDomainDeviceAdded(virConnectPtr connection G_GNUC_UNUSED, virDomainPtr domain, - const char *device, - void *opaque) + const gchar *device, + gpointer opaque) { virtDBusConnect *connect = opaque; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - int r; + g_autofree gchar *path = NULL; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - r = sd_bus_message_new_signal(connect->bus, - &message, + g_dbus_connection_emit_signal(connect->bus, + NULL, path, VIRT_DBUS_DOMAIN_INTERFACE, - "DeviceAdded"); - if (r < 0) - return r; + "DeviceAdded", + g_variant_new("(s)", device), + NULL); - r = sd_bus_message_append(message, "s", device); - if (r < 0) - return r; - - return sd_bus_send(connect->bus, message, NULL); + return 0; } -static int -virtDBusEventsDomainDeviceRemoved(virConnectPtr connection VIRT_ATTR_UNUSED, +static gint +virtDBusEventsDomainDeviceRemoved(virConnectPtr connection G_GNUC_UNUSED, virDomainPtr domain, - const char *device, - void *opaque) + const gchar *device, + gpointer opaque) { virtDBusConnect *connect = opaque; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - int r; + g_autofree gchar *path = NULL; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - r = sd_bus_message_new_signal(connect->bus, - &message, + g_dbus_connection_emit_signal(connect->bus, + NULL, path, VIRT_DBUS_DOMAIN_INTERFACE, - "DeviceRemoved"); - if (r < 0) - return r; - - r = sd_bus_message_append(message, "s", device); - if (r < 0) - return r; + "DeviceRemoved", + g_variant_new("(s)", device), + NULL); - return sd_bus_send(connect->bus, message, NULL); + return 0; } -static int -virtDBusEventsDomainTrayChange(virConnectPtr connection VIRT_ATTR_UNUSED, +static gint +virtDBusEventsDomainTrayChange(virConnectPtr connection G_GNUC_UNUSED, virDomainPtr domain, - const char *device, - int reason, - void *opaque) + const gchar *device, + gint reason, + gpointer opaque) { virtDBusConnect *connect = opaque; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - const char *reasonstr; - int r; + g_autofree gchar *path = NULL; + const gchar *reasonstr; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - r = sd_bus_message_new_signal(connect->bus, - &message, - path, - VIRT_DBUS_DOMAIN_INTERFACE, - "TrayChange"); - if (r < 0) - return r; - switch (reason) { case VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN: reasonstr = "open"; @@ -161,38 +131,32 @@ virtDBusEventsDomainTrayChange(virConnectPtr connection VIRT_ATTR_UNUSED, break; } - r = sd_bus_message_append(message, "ss", device, reasonstr); - if (r < 0) - return r; + g_dbus_connection_emit_signal(connect->bus, + NULL, + path, + VIRT_DBUS_DOMAIN_INTERFACE, + "TrayChange", + g_variant_new("(ss)", device, reasonstr), + NULL); - return sd_bus_send(connect->bus, message, NULL); + return 0; } -static int -virtDBusEventsDomainDiskChange(virConnectPtr connection VIRT_ATTR_UNUSED, +static gint +virtDBusEventsDomainDiskChange(virConnectPtr connection G_GNUC_UNUSED, virDomainPtr domain, - const char *old_src_path, - const char *new_src_path, - const char *device, - int reason, - void *opaque) + const gchar *old_src_path, + const gchar *new_src_path, + const gchar *device, + gint reason, + gpointer opaque) { virtDBusConnect *connect = opaque; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *message = NULL; - _cleanup_(virtDBusUtilFreep) char *path = NULL; - const char *reasonstr; - int r; + g_autofree gchar *path = NULL; + const gchar *reasonstr; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - r = sd_bus_message_new_signal(connect->bus, - &message, - path, - VIRT_DBUS_DOMAIN_INTERFACE, - "DiskChange"); - if (r < 0) - return r; - switch (reason) { case VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START: reasonstr = "missing-on-start"; @@ -205,19 +169,24 @@ virtDBusEventsDomainDiskChange(virConnectPtr connection VIRT_ATTR_UNUSED, break; } - r = sd_bus_message_append(message, "ssss", old_src_path, new_src_path, device, reasonstr); - if (r < 0) - return r; + g_dbus_connection_emit_signal(connect->bus, + NULL, + path, + VIRT_DBUS_DOMAIN_INTERFACE, + "DiskChange", + g_variant_new("(ssss)", old_src_path, + new_src_path, device, reasonstr), + NULL); - return sd_bus_send(connect->bus, message, NULL); + return 0; } static void virtDBusEventsRegisterEvent(virtDBusConnect *connect, - int id, + gint id, virConnectDomainEventGenericCallback callback) { - assert(connect->callback_ids[id] == -1); + g_assert(connect->callback_ids[id] == -1); connect->callback_ids[id] = virConnectDomainEventRegisterAny(connect->connection, NULL, diff --git a/src/main.c b/src/main.c index 6f11aab..2991075 100644 --- a/src/main.c +++ b/src/main.c @@ -1,92 +1,23 @@ -#include "config.h" - #include "connect.h" #include "util.h" -#include <errno.h> -#include <getopt.h> -#include <poll.h> -#include <stdbool.h> -#include <stdio.h> -#include <stdlib.h> -#include <sys/signalfd.h> -#include <systemd/sd-bus.h> -#include <unistd.h> - -static int loop_status; - -static int -virtDBusGetLibvirtEvents(sd_bus *bus) -{ - int events; - int virt_events = 0; - - events = sd_bus_get_events(bus); - - if (events & POLLIN) - virt_events |= VIR_EVENT_HANDLE_READABLE; - - if (events & POLLOUT) - virt_events |= VIR_EVENT_HANDLE_WRITABLE; - - return virt_events; -} - -static int -virtDBusProcessEvents(sd_bus *bus) -{ - for (;;) { - int r; - - r = sd_bus_process(bus, NULL); - if (r < 0) - return r; - - if (r == 0) - break; - } - - return 0; -} - -static void -virtDBusVirEventRemoveHandlep(int *watchp) -{ - if (*watchp >= 0) - virEventRemoveHandle(*watchp); -} - -static void -virtDBusHandleSignal(int watch VIRT_ATTR_UNUSED, - int fd VIRT_ATTR_UNUSED, - int events VIRT_ATTR_UNUSED, - void *opaque VIRT_ATTR_UNUSED) -{ - loop_status = -ECANCELED; -} - -static void -virtDBusHandleBusEvent(int watch, - int fd VIRT_ATTR_UNUSED, - int events VIRT_ATTR_UNUSED, - void *opaque) -{ - sd_bus *bus = opaque; - - loop_status = virtDBusProcessEvents(bus); +#include <glib-unix.h> +#include <libvirt-glib/libvirt-glib.h> - if (loop_status < 0) - return; - - virEventUpdateHandle(watch, virtDBusGetLibvirtEvents(bus)); -} +struct _virtDBusDriver { + const gchar *uri; + const gchar *object; +}; +typedef struct _virtDBusDriver virtDBusDriver; -struct virtDBusDriver { - const char *uri; - const char *object; +struct _virtDBusRegisterData { + virtDBusConnect **connectList; + const virtDBusDriver *drivers; + gsize ndrivers; }; +typedef struct _virtDBusRegisterData virtDBusRegisterData; -static const struct virtDBusDriver sessionDrivers[] = { +static const virtDBusDriver sessionDrivers[] = { { "qemu:///session", "/org/libvirt/QEMU" }, { "test:///default", "/org/libvirt/Test" }, { "uml:///session", "/org/libvirt/UML" }, @@ -96,7 +27,7 @@ static const struct virtDBusDriver sessionDrivers[] = { { "vmwarews:///session", "/org/libvirt/VMwareWS" }, }; -static const struct virtDBusDriver systemDrivers[] = { +static const virtDBusDriver systemDrivers[] = { { "bhyve:///system", "/org/libvirt/BHyve" }, { "lxc:///", "/org/libvirt/LXC" }, { "openvz:///system", "/org/libvirt/OpenVZ" }, @@ -108,131 +39,127 @@ static const struct virtDBusDriver systemDrivers[] = { { "xen:///", "/org/libvirt/Xen" }, }; -int -main(int argc, char *argv[]) +static gboolean +virtDBusHandleSignal(gpointer data) { - enum { - ARG_SYSTEM = 255, - ARG_SESSION - }; + g_main_loop_quit(data); + return TRUE; +} - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "system", no_argument, NULL, ARG_SYSTEM }, - { "session", no_argument, NULL, ARG_SESSION }, - {} - }; +static void +virtDBusAcquired(GDBusConnection *connection, + const gchar *name G_GNUC_UNUSED, + gpointer opaque) +{ + virtDBusRegisterData *data = opaque; - bool system_bus; - const struct virtDBusDriver *drivers = NULL; - int ndrivers = 0; - - _cleanup_(virtDBusConnectListFree) virtDBusConnect **connect = NULL; - _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; - _cleanup_(virtDBusUtilClosep) int signal_fd = -1; - _cleanup_(virtDBusVirEventRemoveHandlep) int bus_watch = -1; - _cleanup_(virtDBusVirEventRemoveHandlep) int signal_watch = -1; - sigset_t mask; - int c; - int r; - - if (geteuid() == 0) { - system_bus = true; - } else { - system_bus = false; + for (gsize i = 0; i < data->ndrivers; i += 1) { + virtDBusConnectNew(&data->connectList[i], connection, + data->drivers[i].uri, data->drivers[i].object); } - while ((c = getopt_long(argc, argv, "hc:", options, NULL)) >= 0) { - switch (c) { - case 'h': - printf("Usage: %s [OPTIONS]\n", program_invocation_short_name); - printf("\n"); - printf("Provide a D-Bus interface to a libvirtd.\n"); - printf("\n"); - printf(" -h, --help Display this help text and exit\n"); - printf(" --session Connect to the session bus\n"); - printf(" --system Connect to the system bus\n"); - return 0; - - case ARG_SYSTEM: - system_bus = true; - break; - - case ARG_SESSION: - system_bus = false; - break; - - default: - return EXIT_FAILURE; - } - } +} - sigemptyset(&mask); - sigaddset(&mask, SIGTERM); - sigaddset(&mask, SIGINT); - sigprocmask(SIG_BLOCK, &mask, NULL); +static void +virtDBusNameAcquired(GDBusConnection *connection G_GNUC_UNUSED, + const gchar *name G_GNUC_UNUSED, + gpointer data G_GNUC_UNUSED) +{ +} - virEventRegisterDefaultImpl(); +static void +virtDBusNameLost(GDBusConnection *connection G_GNUC_UNUSED, + const gchar *name G_GNUC_UNUSED, + gpointer data G_GNUC_UNUSED) +{ + g_printerr("Disconnected from D-Bus.\n"); + exit(EXIT_FAILURE); +} - r = system_bus ? sd_bus_open_system(&bus) : sd_bus_open_user(&bus); - if (r < 0) { - fprintf(stderr, "Failed to connect to session bus: %s\n", strerror(-r)); - return EXIT_FAILURE; +static void +virtDBusRegisterDataFree(virtDBusRegisterData *data) +{ + virtDBusConnectListFree(data->connectList); +} +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virtDBusRegisterData, virtDBusRegisterDataFree); + +int +main(gint argc, gchar *argv[]) +{ + static gboolean systemOpt = FALSE; + static gboolean sessionOpt = FALSE; + GBusType busType; + g_auto(virtDBusGDBusSource) sigintSource = 0; + g_auto(virtDBusGDBusSource) sigtermSource = 0; + g_auto(virtDBusGDBusOwner) busOwner = 0; + g_autoptr(GOptionContext) context = NULL; + g_autoptr(GError) error = NULL; + g_autoptr(GMainLoop) loop = NULL; + g_auto(virtDBusRegisterData) data = { 0 }; + + static GOptionEntry options[] = { + { "system", 0, 0, G_OPTION_ARG_NONE, &systemOpt, + "Connect to the system bus", NULL }, + { "session", 0, 0, G_OPTION_ARG_NONE, &sessionOpt, + "Connect to the session bus", NULL }, + { NULL } + }; + + context = g_option_context_new("Provide a D-Bus interface to a libvirtd."); + g_option_context_add_main_entries(context, options, NULL); + + if (!g_option_context_parse(context, &argc, &argv, &error)) { + g_printerr("%s\n", error->message); + exit(EXIT_FAILURE); } - r = sd_bus_request_name(bus, "org.libvirt", 0); - if (r < 0) { - fprintf(stderr, "Failed to acquire service name: %s\n", strerror(-r)); - return EXIT_FAILURE; + if (sessionOpt && systemOpt) { + g_printerr("Only one of --session or --system can be used.\n"); + exit(EXIT_FAILURE); } - if (system_bus) { - drivers = systemDrivers; - ndrivers = VIRT_N_ELEMENTS(systemDrivers); + if (sessionOpt) { + busType = G_BUS_TYPE_SESSION; + } else if (systemOpt) { + busType = G_BUS_TYPE_SYSTEM; } else { - drivers = sessionDrivers; - ndrivers = VIRT_N_ELEMENTS(sessionDrivers); + if (geteuid() == 0) { + busType = G_BUS_TYPE_SYSTEM; + } else { + busType = G_BUS_TYPE_SESSION; + } } - connect = calloc(ndrivers + 1, sizeof(virtDBusConnect *)); - - for (int i = 0; i < ndrivers; i += 1) { - r = virtDBusConnectNew(&connect[i], bus, - drivers[i].uri, drivers[i].object); - if (r < 0) { - fprintf(stderr, "Failed to register libvirt connection.\n"); - return EXIT_FAILURE; - } + if (busType == G_BUS_TYPE_SYSTEM) { + data.drivers = systemDrivers; + data.ndrivers = G_N_ELEMENTS(systemDrivers); + } else { + data.drivers = sessionDrivers; + data.ndrivers = G_N_ELEMENTS(sessionDrivers); } + data.connectList = g_new0(virtDBusConnect *, data.ndrivers + 1); + g_assert(data.connectList); - r = virtDBusProcessEvents(bus); - if (r < 0) - return EXIT_FAILURE; + loop = g_main_loop_new(NULL, FALSE); - bus_watch = virEventAddHandle(sd_bus_get_fd(bus), - virtDBusGetLibvirtEvents(bus), - virtDBusHandleBusEvent, - bus, - NULL); + sigtermSource = g_unix_signal_add(SIGTERM, + virtDBusHandleSignal, + loop); - signal_fd = signalfd(-1, &mask, SFD_NONBLOCK | SFD_CLOEXEC); - signal_watch = virEventAddHandle(signal_fd, - VIR_EVENT_HANDLE_READABLE, + sigintSource = g_unix_signal_add(SIGINT, virtDBusHandleSignal, - NULL, - NULL); - if (signal_watch < 0) { - fprintf(stderr, "Failed to register signal handler.\n"); - return EXIT_FAILURE; - } + loop); - while (loop_status >= 0) - virEventRunDefaultImpl(); + gvir_event_register(); - if (loop_status < 0 && loop_status != -ECANCELED) { - fprintf(stderr, "Error: %s\n", strerror(-loop_status)); - return EXIT_FAILURE; - } + busOwner = g_bus_own_name(busType, "org.libvirt", + G_BUS_NAME_OWNER_FLAGS_NONE, + virtDBusAcquired, + virtDBusNameAcquired, + virtDBusNameLost, + &data, NULL); + + g_main_loop_run(loop); return EXIT_SUCCESS; } diff --git a/src/util.c b/src/util.c index 9042a0f..c0b5bbe 100644 --- a/src/util.c +++ b/src/util.c @@ -1,155 +1,129 @@ #include "util.h" #include <libvirt/virterror.h> -#include <stdlib.h> -#include <unistd.h> +#include <string.h> -int -virtDBusUtilMessageAppendTypedParameters(sd_bus_message *message, - virTypedParameterPtr parameters, - int n_parameters) +static const GDBusErrorEntry virtDBusUtilErrorEntries[] = { + { VIRT_DBUS_ERROR_LIBVIRT, "org.libvirt.Error" }, +}; + +G_STATIC_ASSERT(G_N_ELEMENTS(virtDBusUtilErrorEntries) == VIRT_DBUS_N_ERRORS); + +GQuark +virtDBusErrorQuark(void) { - int r; + static volatile gsize quarkVolatile = 0; + g_dbus_error_register_error_domain("virt-dbus-error-quark", + &quarkVolatile, + virtDBusUtilErrorEntries, + G_N_ELEMENTS(virtDBusUtilErrorEntries)); + return (GQuark) quarkVolatile; +} - r = sd_bus_message_open_container(message, 'a', "{sv}"); - if (r < 0) - return r; +GVariant * +virtDBusUtilTypedParamsToGVariant(virTypedParameterPtr params, + gint nparams) +{ + GVariantBuilder builder; - for (int i = 0; i < n_parameters; i += 1) { - r = sd_bus_message_open_container(message, SD_BUS_TYPE_DICT_ENTRY, "sv"); - if (r < 0) - return r; + g_variant_builder_init(&builder, G_VARIANT_TYPE("a{sv}")); - r = sd_bus_message_append(message, "s", parameters[i].field); - if (r < 0) - return r; + for (gint i = 0; i < nparams; i++) { + GVariant *value = NULL; - switch (parameters[i].type) { + switch (params[i].type) { case VIR_TYPED_PARAM_INT: - r = sd_bus_message_append(message, "v", "i", parameters[i].value.i); + value = g_variant_new("i", params[i].value.i); break; case VIR_TYPED_PARAM_UINT: - r = sd_bus_message_append(message, "v", "u", parameters[i].value.ui); + value = g_variant_new("u", params[i].value.ui); break; case VIR_TYPED_PARAM_LLONG: - r = sd_bus_message_append(message, "v", "x", parameters[i].value.l); + value = g_variant_new("x", params[i].value.l); break; case VIR_TYPED_PARAM_ULLONG: - r = sd_bus_message_append(message, "v", "t", parameters[i].value.ul); + value = g_variant_new("t", params[i].value.ul); break; case VIR_TYPED_PARAM_DOUBLE: - r = sd_bus_message_append(message, "v", "d", parameters[i].value.d); + value = g_variant_new("d", params[i].value.d); break; case VIR_TYPED_PARAM_BOOLEAN: - r = sd_bus_message_append(message, "v", "b", parameters[i].value.b); + value = g_variant_new("b", params[i].value.b); break; case VIR_TYPED_PARAM_STRING: - r = sd_bus_message_append(message, "v", "s", parameters[i].value.s); + value = g_variant_new("s", params[i].value.s); break; } - if (r < 0) - return r; - - r = sd_bus_message_close_container(message); - if (r < 0) - return r; + g_variant_builder_add(&builder, "{sv}", + params[i].field, + g_variant_new_variant(value)); } - return sd_bus_message_close_container(message); + return g_variant_builder_end(&builder); } -int -virtDBusUtilSetLastVirtError(sd_bus_error *error) +void +virtDBusUtilSetLastVirtError(GError **error) { virErrorPtr vir_error; vir_error = virGetLastError(); - if (!vir_error) - return 0; + if (!vir_error) { + g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, + "unknown error"); + } else { + g_set_error_literal(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, + vir_error->message); + } +} - return sd_bus_error_set(error, VIRT_DBUS_ERROR_INTERFACE, vir_error->message); +static gchar * +virtDBusUtilEncodeUUID(const gchar *uuid) +{ + gchar *ret = g_strdup_printf("_%s", uuid); + g_assert(ret); + return g_strdelimit(ret, "-", '_'); } -int -virtDBusUtilSetError(sd_bus_error *error, - const char *message) +static gchar * +virtDBusUtilDecodeUUID(const gchar *uuid) { - return sd_bus_error_set(error, VIRT_DBUS_ERROR_INTERFACE, message); + gchar *ret = g_strdup(uuid+1); + g_assert(ret); + return g_strdelimit(ret, "_", '-'); } -char * +gchar * virtDBusUtilBusPathForVirDomain(virDomainPtr domain, - const char *domainPath) + const gchar *domainPath) { - char *path = NULL; - char uuid[VIR_UUID_STRING_BUFLEN] = ""; - + gchar uuid[VIR_UUID_STRING_BUFLEN] = ""; + g_autofree gchar *newUuid = NULL; virDomainGetUUIDString(domain, uuid); - sd_bus_path_encode(domainPath, uuid, &path); - - return path; + newUuid = virtDBusUtilEncodeUUID(uuid); + return g_strdup_printf("%s/%s", domainPath, newUuid); } virDomainPtr virtDBusUtilVirDomainFromBusPath(virConnectPtr connection, - const char *path, - const char *domainPath) + const gchar *path, + const gchar *domainPath) { - _cleanup_(virtDBusUtilFreep) char *name = NULL; - int r; + g_autofree gchar *name = NULL; + gsize prefixLen = strlen(domainPath) + 1; - r = sd_bus_path_decode(path, domainPath, &name); - if (r < 0) - return NULL; + name = virtDBusUtilDecodeUUID(path+prefixLen); + g_assert(name); return virDomainLookupByUUIDString(connection, name); } void -virtDBusUtilFreep(void *p) -{ - free(*(void **)p); -} - -void -virtDBusUtilClosep(int *fdp) -{ - if (*fdp >= 0) - close(*fdp); -} - -void -virtDBusUtilStrvFreep(void *p) -{ - char **strv = *(char ***)p; - - if (strv == NULL) - return; - - for (unsigned i = 0; strv[i] != NULL; i++) - free(strv[i]); - - free(strv); -} - -void -virtDBusUtilVirDomainFreep(virDomainPtr *domainp) -{ - if (*domainp) - virDomainFree(*domainp); -} - -void -virtDBusUtilVirDomainListFreep(virDomainPtr **domainsp) +virtDBusUtilVirDomainListFree(virDomainPtr *domains) { - virDomainPtr *domains = *domainsp; - - if (!domains) - return; - - for (int i = 0; domains[i] != NULL; i += 1) + for (gint i = 0; domains[i] != NULL; i += 1) virDomainFree(domains[i]); - free(domains); + g_free(domains); } diff --git a/src/util.h b/src/util.h index 2a6f7e2..8cafafa 100644 --- a/src/util.h +++ b/src/util.h @@ -1,49 +1,38 @@ #pragma once -#include <libvirt/libvirt.h> -#include <systemd/sd-bus.h> - -#define VIRT_DBUS_ERROR_INTERFACE "org.libvirt.Error" - -#define _cleanup_(_x) __attribute__((__cleanup__(_x))) +#include "gdbus.h" -#define VIRT_ATTR_UNUSED __attribute__((__unused__)) +#include <libvirt/libvirt.h> -#define VIRT_N_ELEMENTS(array) (sizeof(array) / sizeof(*(array))) +#define VIRT_DBUS_ERROR virtDBusErrorQuark() +typedef enum { + VIRT_DBUS_ERROR_LIBVIRT, + VIRT_DBUS_N_ERRORS /*< skip >*/ +} VirtDBusError; -int -virtDBusUtilMessageAppendTypedParameters(sd_bus_message *message, - virTypedParameterPtr parameters, - int n_parameters); +GQuark +virtDBusErrorQuark(void); -int -virtDBusUtilSetLastVirtError(sd_bus_error *error); +GVariant * +virtDBusUtilTypedParamsToGVariant(virTypedParameterPtr params, + gint nparams); -int -virtDBusUtilSetError(sd_bus_error *error, - const char *message); +void +virtDBusUtilSetLastVirtError(GError **error); -char * +gchar * virtDBusUtilBusPathForVirDomain(virDomainPtr domain, - const char *domainPath); + const gchar *domainPath); virDomainPtr virtDBusUtilVirDomainFromBusPath(virConnectPtr connection, - const char *path, - const char *domainPath); + const gchar *path, + const gchar *domainPath); void -virtDBusUtilFreep(void *p); +virtDBusUtilVirDomainListFree(virDomainPtr *domains); -void -virtDBusUtilClosep(int *fdp); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomain, virDomainFree); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainPtr, virtDBusUtilVirDomainListFree); -void -virtDBusUtilStrvFreep(void *p); - -void -virtDBusUtilVirDomainFreep(virDomainPtr *domainp); - -void -virtDBusUtilVirDomainListFreep(virDomainPtr **domainsp); diff --git a/test/travis-run b/test/travis-run index 28260f4..7577253 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 libglib2.0-dev libtool python3-gi python3-dbus dbus +apt-get install -y dh-autoreconf pkg-config libvirt-dev libglib2.0-dev libtool python3-gi python3-dbus dbus # run build and tests as user chown -R buildd:buildd /build -- 2.14.3
The default thread count is currently 4 and it is also configurable via --threads/-t paramter for the libvirt-dbus daemon. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/connect.c | 4 +++ src/connect.h | 1 + src/gdbus.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++------------- src/gdbus.h | 4 +++ src/main.c | 10 ++++++ 5 files changed, 95 insertions(+), 21 deletions(-) diff --git a/src/connect.c b/src/connect.c index a3b1cf2..8363a4d 100644 --- a/src/connect.c +++ b/src/connect.c @@ -56,6 +56,8 @@ gboolean virtDBusConnectOpen(virtDBusConnect *connect, GError **error) { + g_autoptr(GMutexLocker) lock = g_mutex_locker_new(&connect->lock); + if (connect->connection) { if (virConnectIsAlive(connect->connection)) return TRUE; @@ -211,6 +213,8 @@ virtDBusConnectNew(virtDBusConnect **connectp, connect = g_new0(virtDBusConnect, 1); g_assert(connect != NULL); + g_mutex_init(&connect->lock); + for (gint i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i += 1) connect->callback_ids[i] = -1; diff --git a/src/connect.h b/src/connect.h index f8b1d06..17792cd 100644 --- a/src/connect.h +++ b/src/connect.h @@ -14,6 +14,7 @@ struct virtDBusConnect { const gchar *connectPath; gchar *domainPath; virConnectPtr connection; + GMutex lock; gint callback_ids[VIR_DOMAIN_EVENT_ID_LAST]; }; diff --git a/src/gdbus.c b/src/gdbus.c index 7dd5766..76f0df6 100644 --- a/src/gdbus.c +++ b/src/gdbus.c @@ -15,6 +15,16 @@ struct _virtDBusGDBusSubtreeData { }; typedef struct _virtDBusGDBusSubtreeData virtDBusGDBusSubtreeData; +struct _virtDBusGDBusThreadData { + const gchar *objectPath; + const gchar *interfaceName; + const gchar *methodName; + GVariant *parameters; + GDBusMethodInvocation *invocation; + virtDBusGDBusMethodData *methodData; +}; +typedef struct _virtDBusGDBusThreadData virtDBusGDBusThreadData; + static const gchar *dbusInterfacePrefix = NULL; /** @@ -221,6 +231,38 @@ virtDBusGDBusHandleMethod(GVariant *parameters, outFDs); } +static void +virtDBusGDBusMethodCallThread(gpointer threadData, + gpointer userData G_GNUC_UNUSED) +{ + g_autofree virtDBusGDBusThreadData *data = threadData; + + if (g_strcmp0(data->interfaceName, "org.freedesktop.DBus.Properties") == 0) { + if (g_strcmp0(data->methodName, "Get") == 0) { + virtDBusGDBusHandlePropertyGet(data->parameters, data->invocation, + data->objectPath, data->methodData); + } else if (g_strcmp0(data->methodName, "Set") == 0) { + virtDBusGDBusHandlePropertySet(data->parameters, data->invocation, + data->objectPath, data->methodData); + } else if (g_strcmp0(data->methodName, "GetAll") == 0) { + virtDBusGDBusHandlePropertyGetAll(data->invocation, data->objectPath, + data->methodData); + } else { + g_dbus_method_invocation_return_error(data->invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_UNKNOWN_METHOD, + "unknown method '%s'", + data->methodName); + } + } else { + virtDBusGDBusHandleMethod(data->parameters, data->invocation, + data->objectPath, data->methodName, + data->methodData); + } +} + +GThreadPool *threadPool; + static void virtDBusGDBusHandleMethodCall(GDBusConnection *connection G_GNUC_UNUSED, const gchar *sender G_GNUC_UNUSED, @@ -231,27 +273,18 @@ virtDBusGDBusHandleMethodCall(GDBusConnection *connection G_GNUC_UNUSED, 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); - } + virtDBusGDBusThreadData *data = g_new0(virtDBusGDBusThreadData, 1); + + g_assert(data); + + data->objectPath = objectPath; + data->interfaceName = interfaceName; + data->methodName = methodName; + data->parameters = parameters; + data->invocation = invocation; + data->methodData = userData; + + g_thread_pool_push(threadPool, data, NULL); } static const GDBusInterfaceVTable virtDBusGDBusVtable = { @@ -391,3 +424,25 @@ virtDBusGDBusRegisterSubtree(GDBusConnection *bus, virtDBusGDBusSubtreeDataFree, NULL); } + +/** + * virtDBusGDBusPrepareThreadPool: + * @maxThreads: the number of maximum threads in thread pool + * @error: return location for error or NULL + * + * Initializes thread pool to be used to process D-Bus messages. + * + * Returns TRUE on success, FALSE on error and sets @error. + */ +gboolean +virtDBusGDBusPrepareThreadPool(gint maxThreads, + GError **error) +{ + threadPool = g_thread_pool_new(virtDBusGDBusMethodCallThread, + NULL, + maxThreads, + FALSE, + error); + + return !!threadPool; +} diff --git a/src/gdbus.h b/src/gdbus.h index 21ee6fc..05e6527 100644 --- a/src/gdbus.h +++ b/src/gdbus.h @@ -91,5 +91,9 @@ virtDBusGDBusRegisterSubtree(GDBusConnection *bus, virtDBusGDBusPropertyTable *properties, gpointer userData); +gboolean +virtDBusGDBusPrepareThreadPool(gint maxThreads, + GError **error); + 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/src/main.c b/src/main.c index 2991075..70e5902 100644 --- a/src/main.c +++ b/src/main.c @@ -83,11 +83,14 @@ virtDBusRegisterDataFree(virtDBusRegisterData *data) } G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virtDBusRegisterData, virtDBusRegisterDataFree); +#define VIRT_DBUS_MAX_THREADS 4 + int main(gint argc, gchar *argv[]) { static gboolean systemOpt = FALSE; static gboolean sessionOpt = FALSE; + static gint maxThreads = VIRT_DBUS_MAX_THREADS; GBusType busType; g_auto(virtDBusGDBusSource) sigintSource = 0; g_auto(virtDBusGDBusSource) sigtermSource = 0; @@ -102,6 +105,8 @@ main(gint argc, gchar *argv[]) "Connect to the system bus", NULL }, { "session", 0, 0, G_OPTION_ARG_NONE, &sessionOpt, "Connect to the session bus", NULL }, + { "threads", 't', 0, G_OPTION_ARG_INT, &maxThreads, + "Configure maximal number of worker threads", "N" }, { NULL } }; @@ -140,6 +145,11 @@ main(gint argc, gchar *argv[]) data.connectList = g_new0(virtDBusConnect *, data.ndrivers + 1); g_assert(data.connectList); + if (!virtDBusGDBusPrepareThreadPool(maxThreads, &error)) { + g_printerr("%s\n", error->message); + exit(EXIT_FAILURE); + } + loop = g_main_loop_new(NULL, FALSE); sigtermSource = g_unix_signal_add(SIGTERM, -- 2.14.3
participants (2)
-
Daniel P. Berrangé -
Pavel Hrdina