Re: [libvirt] [PATCH 1/2] virCommandDoAsyncIO: Resolve race

On Thu, Feb 07, 2013 at 12:36:36PM +0100, Michal Privoznik wrote: > With current implementation, virCommandWait unregister the stdio > callback and finish reading itself. However, the eventloop may > already have been in process of executing the callback in which > case one obtain these obscure error messages, like: > > 2013-02-06 11:41:43.766+0000: 19078: warning : virEventPollRemoveHandle:183 : Ignoring invalid remove watch -1 > 2013-02-06 11:41:43.766+0000: 19078: warning : virFileClose:65 : Tried to close invalid fd 25 > > The solution is to introduce a mutex and condition to virCommand, > and wait in virCommandWait for the eventloop to process all IOs. > This, however, requires all callees to unlock all mutexes held, > otherwise the eventloop may try to lock one of them and experience > deadlock. If that's the case, we will never be woken up to unlock > the problematic mutex. > --- > src/qemu/qemu_driver.c | 58 +++++++++++++++++++++++++--- > src/qemu/qemu_migration.c | 15 +++++++- > src/util/vircommand.c | 98 ++++++++++++++++++++++++++++++++++++----------- > src/util/virfile.c | 4 ++ > 4 files changed, 146 insertions(+), 29 deletions(-) ACK, reluctantly - I feel this async I/O code has grown rather more complex than it ought to have been. The async I/O code in virCommand was supposed to be about simplifying life - but the requiring the callers to do all this mutex locking/unlocking seems to have made things worse than they were before we had async I/O code IMHO. I'm half inclined to say we should just revert the whole lot. Daniel