8:02 a.m.
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
8:02 a.m.
New subject: [libvirt] [PATCH 1/4] virdbus: Grab a ref as long as the while loop is executed
From: Marc Hartmayer <mhartmay(a)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(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)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
8:02 a.m.
New subject: [libvirt] [PATCH 2/4] virdbus: Unref the D-Bus connection when closing
From: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
As documented at
https://dbus.freedesktop.org/doc/api/html/group__DBusConnection.html#ga25...
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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)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
8:02 a.m.
New subject: [libvirt] [PATCH 3/4] virdbus: Report a debug message that dbus_watch_handle() has failed
From: Marc Hartmayer <mhartmay(a)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(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)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
8:02 a.m.
New subject: [libvirt] [PATCH 4/4] virdbus: Use the mnemonic macros for dbus_bool_t values
From: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Use the mnemonic macros of libdbus for 1 (TRUE) and 0 (FALSE).
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi(a)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
6:44 p.m.
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(a)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 ;-)
3:30 a.m.
On Wed, Sep 26, 2018 at 01:44 AM +0200, John Ferlan <jferlan(a)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(a)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
2280
days inactive
2285
days old
6 comments
2 participants
participants (2)
-
John Ferlan -
Marc Hartmayer