[libvirt] [Xen-devel] [PATCH RFC] libxl: use libxl_event_wait to process libxl events

[Jim Fehlig]

On 11/20/2015 08:31 AM, Ian Campbell wrote:
> On Fri, 2015-11-13 at 14:36 -0700, Jim Fehlig wrote:
>> Prior to this patch, libxl events were delivered to libvirt via
>> the libxlDomainEventHandler callback registered with libxl.
>> Documenation in $xensrc/tools/libxl/libxl_event.h states that the
>> callback "may occur on any thread in which the application calls
>> libxl". This can result in deadlock since many of the libvirt
>> callees of libxl hold a lock on the virDomainObj they are working
>> on. When the callback is invoked, it attempts to find a virDomainObj
>> corresponding to the domain ID provided by libxl. Searching the
>> domain obj list results in locking each obj before checking if it is
>> active, and its ID equals the requested ID. Deadlock is possible
>> when attempting to lock an obj that is already locked further up
>> the call stack. Indeed, Max Ustermann recently reported an instance
>> of this deadlock
>>
>> https://www.redhat.com/archives/libvir-list/2015-November/msg00130.html
>>
>> This patch moves processing of libxl events to a thread, where
>> libxl_event_wait() is used to collect events. This allows processing
>> libxl events asynchronously in libvirt, avoiding the deadlock.
>>
>> Reported-by: max ustermann <ustermann78 at web.de>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>>
>> The only reservations I have about this patch come from comments
>> about libxl_event_wait in libxl_event.h
>>
>>   Like libxl_event_check but blocks if no suitable events are
>>   available, until some are.  Uses libxl_osevent_beforepoll/
>>   _afterpoll so may be inefficient if very many domains are being
>>   handled by a single program.
>>
>> libvirt does handle "very many domains" :-). But thus far, I haven't
>> noticed any problems in my testing. The reporter expressed interest
>> in testing the patch. Positive results from that testing would improve
>> my confidence, as would an ACK from Ian Jackson.
> Would it be possible/allowable to have your cake and eat it by using the
> callbacks but having them simply enqueue the events on the libvirt side and
> kick the dedicated thread for further processing?

Sure. But it is not clear how "externally triggered" events are handled. E.g.
guest admin performs shutdown or reboot. I was under the impression
libxl_event_wait() or registering an event_occurs handler was the only way to
receive those events.

Regards,
Jim