Re: [libvirt] PATCH: Fix DBus thread safety
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
We are using DBus from multiple threads in libvirtd. We are not
calling
dbus_threads_init_default(). Very bad things result. Happy-crashy-daemon
This patch fixes that, and also stops DBus messing around with SIGPIPE and
also stops it calling exit() when the bus disconnects, so we can actually
see the error & continue with life.
These all look fine.
I can attest to having stared hard at dbus' SIGPIPE-handling code
(and asking why, oh why, a *library* handles a signal, by default)
so am glad to see it turned off here.
However, now there are 3 dbus-init-related lines in each of those files,
not counting the slightly-separated *_set_exit_on_disconnect call.
It'd be nice to encapsulate those, or at least to add a comment
before each saying to keep them in sync.
Hmm. actually there are four calls in each place:
dbus_connection_set_change_sigpipe(FALSE);
dbus_threads_init_default();
dbus_error_init
... = dbus_bus_get(...
Only the last one can fail.
For the record, are these crashes easy to reproduce?
diff -rup libvirt-0.6.0.orig/qemud/qemud.c
libvirt-0.6.0.new/qemud/qemud.c
--- libvirt-0.6.0.orig/qemud/qemud.c 2009-02-18 10:56:34.000000000 +0000
+++ libvirt-0.6.0.new/qemud/qemud.c 2009-02-18 12:52:18.000000000 +0000
@@ -860,6 +860,10 @@ static struct qemud_server *qemudNetwork
if (auth_unix_rw == REMOTE_AUTH_POLKIT ||
auth_unix_ro == REMOTE_AUTH_POLKIT) {
DBusError derr;
+
+ dbus_connection_set_change_sigpipe(FALSE);
+ dbus_threads_init_default();
+
dbus_error_init(&derr);
server->sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr);
if (!(server->sysbus)) {
@@ -868,6 +872,7 @@ static struct qemud_server *qemudNetwork
dbus_error_free(&derr);
goto cleanup;
}
+ dbus_connection_set_exit_on_disconnect(server->sysbus, FALSE);
}
#endif
diff -rup libvirt-0.6.0.orig/src/node_device_hal.c
libvirt-0.6.0.new/src/node_device_hal.c
--- libvirt-0.6.0.orig/src/node_device_hal.c 2009-01-16 12:44:22.000000000 +0000
+++ libvirt-0.6.0.new/src/node_device_hal.c 2009-02-18 12:52:48.000000000 +0000
@@ -685,6 +685,9 @@ static int halDeviceMonitorStartup(void)
nodeDeviceLock(driverState);
/* Allocate and initialize a new HAL context */
+ dbus_connection_set_change_sigpipe(FALSE);
+ dbus_threads_init_default();
+
dbus_error_init(&err);
hal_ctx = libhal_ctx_new();
if (hal_ctx == NULL) {
@@ -696,6 +699,8 @@ static int halDeviceMonitorStartup(void)
fprintf(stderr, "%s: dbus_bus_get failed\n", __FUNCTION__);
goto failure;
}
+ dbus_connection_set_exit_on_disconnect(dbus_conn, FALSE);
+
if (!libhal_ctx_set_dbus_connection(hal_ctx, dbus_conn)) {
fprintf(stderr, "%s: libhal_ctx_set_dbus_connection failed\n",
__FUNCTION__);