Il 15/07/2013 22:15, Alex Bligh ha scritto: > Paolo, > > --On 15 July 2013 16:25:01 +0200 Paolo Bonzini <pbonz...@redhat.com> wrote: > > Thanks for the review. > >> Il 06/07/2013 18:24, Alex Bligh ha scritto: >>> Add timed bottom halves. A timed bottom half is a bottom half that >>> will not execute until a given time has passed (qemu_bh_schedule_at) >>> or a given interval has passed (qemu_bh_schedule_in). >> >> ... and may be delayed arbitrarily past that given interval if you are >> running in qemu-img or in other synchronous I/O APIs. > > That's true. However, the problem with timers is worse, in that we > poll for timers even less frequently as far as I can tell.
Right, we poll for bottom halves during qemu_aio_wait(). We don't poll for timers. >> I'm especially >> worried that this will not have any effect if bdrv_aio_cancel is calling >> qemu_aio_wait. bdrv_aio_cancel is presumably one place where you want >> timeout/reconnect functionality to trigger. > > Well, I'm a newbie here, so may well be wrong but I thought qemu_aio_wait > /did/ call bottom halves (but didn't call QemuTimers). Provided time > does actually advance (which inspection suggests it does), then > these bh's should be called just like any other bh's. I may have missed > the point here entirely. So far you are right. But this only happens if qemu_aio_wait() actually returns, so that on the next call we poll for timers. If QEMU is stuck in qemu_aio_wait()'s infinite-timeout poll(), it will never advance and process the timed bottom halves. This goes to the question of having aio_notify() or not. If you have it, you will immediately process timed BHs that are "born expired". For other bottom halves, there will be no difference if you add it or not. >> I would really prefer to have a TimeEventNotifier or something like >> that, which is API-compatibile with EventNotifier (so you can use the >> regular aio-*.c APIs) but triggers when a given time has passed. >> Basically an "heavyweight" QEMUTimer; that would be a timerfd on Linux, >> and a queue timer on Windows. No idea on other POSIX systems, >> unfortunately. > > I was trying to use the bh API because that's what the existing block > drivers use, and what I really wanted was a bh that wouldn't execute > for a while. Do EventNotifiers run whilst AIO is polling for completion? Yes, and they can actually interrupt qemu_aio_wait(). See aio_set_event_notifier. >> Even better would be to remove the whole timer stuff (POSIX timers, >> setitimer, and the Win32 equivalents), and just make the timers use a >> shorter timeout for the main loop. If you do this, I suspect adding >> timer support to AioContext would be much simpler. > > In discussion with Stefan H on IRC, I originally suggested moving the > QemuTimer poll to the AIO loop (or adding another), which is a half > arsed way to do what you are suggesting. He suggested this would be > hairy because the existing users might not be safe to be called there. > This was an attempt at a minimal change to fix that use. > >> BTW, note that qemu-nbd (and qemu-io too) does call timers. > > I'd thought I tested qemu-io. qemu-convert definitely does not. qemu-io is the tool that is used by the unit tests. Conversion is in qemu-img. Paolo > Alex > >> Paolo >> >>> Any qemu >>> clock can be used, and times are specified in nanoseconds. >>> >>> Timed bottom halves can be used where timers cannot. For instance, >>> in block drivers where there is no mainloop that calls timers >>> (qemu-nbd, qemu-img), or where (per stefa...@redhat.com) the >>> aio code loops internally and thus timers never get called. >>> >>> Signed-off-by: Alex Bligh <a...@alex.org.uk> >> >> > > >