[libvirt] [RFC] making (newly public) EventImpl interface moreconsistent

David Lively dlively at virtualiron.com
Wed Nov 5 16:26:07 UTC 2008


On Wed, 2008-11-05 at 11:51 +0000, Daniel P. Berrange wrote:
> DevKit & HAL are just APIs built ontop of DBus, so the key here
> is integration with DBus watch APIs. AFAIK, those only require
> that the event loop impl have one callback per unique FD. 

Here's what I'm seeing when registering for dbus watch callbacks.  In
halNodeDriverStartup (in node_device_hal.c in the submitted host dev
enum patch), I register for dbus watch callbacks:

    /* Register dbus watch callbacks */
    if (!dbus_connection_set_watch_functions(dbus_conn,
                                             add_dbus_watch,
                                             remove_dbus_watch,
                                             toggle_dbus_watch,
                                             NULL, NULL)) {
        fprintf(stderr, "%s: dbus_connection_set_watch_functions failed\n",
                __FUNCTION__);
        goto failure;
    }

And then I instrumented add/remove/toggle_dbus_watch.  add_dbus_watch is
called twice as soon as we register the watch functions:
  add_dbus_watch 0xae4200  fd 6  flags 0x2  enabled 0
  add_dbus_watch 0xaeb950  fd 6  flags 0x1  enabled 1
  *** DUPLICATE HANDLE 6 at [0] ***
So here we have two different DBusWatches sharing the same unix fd.  In
this case, the first one (POLLOUT flags) is disabled, and never toggled,
so things happen to work just fine.  The current qemud AddHandleImpl
will in fact overwrite the first entry with the second, so it has
totally lost the first watch.

But this is just lucky.  Because the behavior of adding a duplicate
handle is undefined, the implementation could just as well have ignored
the second entry, in which case DBus events would never be received.

I'll look into the glib handle-watching code (which I'm not familiar
with currently) and see how it behaves, but I can't imagine it doesn't
support this when DBus watches clearly require it.

Dave





More information about the libvir-list mailing list