[libvirt] PATCH: Fix DBus thread safety

[Jim Meyering]

"Daniel P. Berrange" <berrange at 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__);