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

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(a)virtuozzo.com&gt; >>> --- >>> 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.

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.