"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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__);
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list