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@xxxxxxxxxxxxx> >>>> --- >>>> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
