[libvirt] [PATCH 0/4] Small changes to virdbus
Marc Hartmayer (4): virdbus: Grab a ref as long as the while loop is executed virdbus: Unref the D-Bus connection when closing virdbus: Report a debug message that dbus_watch_handle() has failed virdbus: Use the mnemonic macros for dbus_bool_t values src/util/virdbus.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) -- 2.17.0
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Grab a ref for info->bus (a DBus connection) as long as the while loop is running. With the grabbed reference it is ensured that info->bus isn't freed as long as the while loop is executed. This is necessary as it's allowed to drop the last ref for the bus connection in a handler. There was already a bug of this kind in libdbus itself: https://bugs.freedesktop.org/show_bug.cgi?id=15635. Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/util/virdbus.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index ba8b684f17f1..d0e2c76e486f 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -198,8 +198,10 @@ static void virDBusWatchCallback(int fdatch ATTRIBUTE_UNUSED, (void)dbus_watch_handle(watch, dbus_flags); + dbus_connection_ref(info->bus); while (dbus_connection_dispatch(info->bus) == DBUS_DISPATCH_DATA_REMAINS) /* keep dispatching while data remains */; + dbus_connection_unref(info->bus); } -- 2.17.0
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> As documented at https://dbus.freedesktop.org/doc/api/html/group__DBusConnection.html#ga2522a... the creator of a non-shared D-Bus connection has to release the last reference after closing for freeing. Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- src/util/virdbus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index d0e2c76e486f..1d1e39aae728 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -144,6 +144,7 @@ void virDBusCloseSystemBus(void) { if (systembus && !sharedBus) { dbus_connection_close(systembus); + dbus_connection_unref(systembus); systembus = NULL; } } -- 2.17.0
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Report a debug message if dbus_watch_handle() returns FALSE. dbus_watch_handle() returns FALSE if there wasn't enough memory for reading or writing. Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- src/util/virdbus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 1d1e39aae728..01c67fbaf467 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -197,7 +197,8 @@ static void virDBusWatchCallback(int fdatch ATTRIBUTE_UNUSED, if (events & VIR_EVENT_HANDLE_HANGUP) dbus_flags |= DBUS_WATCH_HANGUP; - (void)dbus_watch_handle(watch, dbus_flags); + if (dbus_watch_handle(watch, dbus_flags) == FALSE) + VIR_DEBUG("dbus_watch_handle() returned FALSE"); dbus_connection_ref(info->bus); while (dbus_connection_dispatch(info->bus) == DBUS_DISPATCH_DATA_REMAINS) -- 2.17.0
From: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Use the mnemonic macros of libdbus for 1 (TRUE) and 0 (FALSE). Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- src/util/virdbus.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virdbus.c b/src/util/virdbus.c index 01c67fbaf467..0ace5b250cac 100644 --- a/src/util/virdbus.c +++ b/src/util/virdbus.c @@ -236,7 +236,7 @@ static dbus_bool_t virDBusAddWatch(DBusWatch *watch, struct virDBusWatch *info; if (VIR_ALLOC(info) < 0) - return 0; + return FALSE; if (dbus_watch_get_enabled(watch)) flags = virDBusTranslateWatchFlags(dbus_watch_get_flags(watch)); @@ -253,10 +253,10 @@ static dbus_bool_t virDBusAddWatch(DBusWatch *watch, watch, NULL); if (info->watch < 0) { dbus_watch_set_data(watch, NULL, NULL); - return 0; + return FALSE; } - return 1; + return TRUE; } -- 2.17.0
On 9/21/18 9:02 AM, Marc Hartmayer wrote:
Marc Hartmayer (4): virdbus: Grab a ref as long as the while loop is executed virdbus: Unref the D-Bus connection when closing virdbus: Report a debug message that dbus_watch_handle() has failed virdbus: Use the mnemonic macros for dbus_bool_t values
src/util/virdbus.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> (series) and pushed, John although /me wonders what happens to @info in virDBusAddWatch when the call to virEventAddHandle fails, dbus_watch_set_data(watch, NULL, NULL) is called and the code returns FALSE. I assume it's leaked, different problem though ;-)
On Wed, Sep 26, 2018 at 01:44 AM +0200, John Ferlan <jferlan@redhat.com> wrote:
On 9/21/18 9:02 AM, Marc Hartmayer wrote:
Marc Hartmayer (4): virdbus: Grab a ref as long as the while loop is executed virdbus: Unref the D-Bus connection when closing virdbus: Report a debug message that dbus_watch_handle() has failed virdbus: Use the mnemonic macros for dbus_bool_t values
src/util/virdbus.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com>
(series) and pushed,
John
although /me wonders what happens to @info in virDBusAddWatch when the call to virEventAddHandle fails, dbus_watch_set_data(watch, NULL, NULL) is called and the code returns FALSE. I assume it's leaked, different
dbus_watch_set_data(…) calls the currently assigned 'free_data_function' function for you before setting the new data (https://dbus.freedesktop.org/doc/api/html/dbus-watch_8c_source.html#l00689). So at least this should be fine :)
problem though ;-)
-- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
John Ferlan -
Marc Hartmayer