[libvirt] [PATCH v2 04/11] qemu: handle host usb device add/del udev events

[Nikolay Shirokovskiy]

On 16.09.2019 12:03, Peter Krempa wrote:
> On Mon, Sep 16, 2019 at 08:53:10 +0000, Nikolay Shirokovskiy wrote:
>>
>>
>> On 16.09.2019 11:26, Peter Krempa wrote:
>>> On Mon, Sep 09, 2019 at 14:33:07 +0300, Nikolay Shirokovskiy wrote:
>>>> Now when code handling attaching/detaching usb hostdev is appropriately
>>>> changed use it to handle host usb device udev add/del events.
>>>>
>>>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>>>> ---
>>>>  src/qemu/Makefile.inc.am |   2 +
>>>>  src/qemu/qemu_conf.h     |   3 +
>>>>  src/qemu/qemu_domain.c   |   2 +
>>>>  src/qemu/qemu_domain.h   |   2 +
>>>>  src/qemu/qemu_driver.c   | 351 ++++++++++++++++++++++++++++++++++++++-
>>>>  5 files changed, 359 insertions(+), 1 deletion(-)
> 
> [...]
> 
>>>
>>> Is this executed from the event loop thread? If yes we can't do this as 
>>> qemuUdevUSBHandleEvent is taking domain locks and thus potentially
>>> waiting indefinitely for any stuck VM.
>>
>> Yes, this is executed in the event loop thread. But I guess we can take
>> domain lock here as this lock is intended to be grabbed only for short
>> periods of time by design (as stated in THREADS.txt). There are a lot
>> of qemu event handlers that grab domain lock (qemuProcessHandleMonitorEOF
>> for example). AFAIK we use offloading to thread pool if we need to
>> start a job which can take long time because of already running job.
>> And this is offloading is done in qemuUdevUSBHandleEvent.
> 
> The problem isn't the time the lock will be held but the unbounded time
> the VM lock may be held by one of the worker-pool threads executing an
> API.
> 
> This would make the event loop stuck until the API finishes.

API implementation releases domain lock before running long
operation like executing qemu command or executing migration
phase on destination.

> 
> In some cases this same problem exists in our qemu monitor event handler
> code, where we want to do a per-VM event loop to prevent this issue thus
> I don't really want to see another instance of this being added.
> 

Ok. Looks like it does not make sense to grab VM lock in event loop
in this particular case. AFAIU grabbing VM lock makes sense when
we need to notify some API thread waiting for event.

Nikolay