On 07.02.2013 16:27, Daniel P. Berrange wrote: > 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 > I agree. The other solution that has came up to my mind is just to spawn virCommandProcessIO (which is doing its own poll()) in a separate thread and hence we don't need to require the unlock. virCommandWait would just join the thread then. However, I am not so comfortable with spawning threads around with no real reason. Having said that, I will push 2/2 as it is unrelated, and postpone pushing 1/2 for a while to let others express themselves. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list