On Mon, May 11, 2009 at 12:17:23PM +0100, Daniel P. Berrange wrote:
> Several methods forgot to call virEventInterruptLocked() in critical
> times, or called it at the wrong point. This fixes that problem.
>
> It also changes watch and timer numbers to start from 1, instead of 0.
> This is because we've had a number of bugs where code has passed an
> uninitialized timer/watch ID to an update/delete method, and this has
> resulted in deleting a real important timer/watch. Changing to start
> from 1, and logging invalid watches will protect us from uninitialized
> watches/timers.
Sounds good !
> virEventLock();
> for (i = 0 ; i < eventLoop.handlesCount ; i++) {
> if (eventLoop.handles[i].watch == watch) {
> eventLoop.handles[i].events =
> virEventHandleTypeToPollEvent(events);
> + virEventInterruptLocked();
> break;
> }
> }
> - virEventInterruptLocked();
> virEventUnlock();
> }
Okay so basically the core of the patch is to lock as soon as we match
and not once out of the loop
> @@ -172,11 +185,11 @@ int virEventRemoveHandleImpl(int watch)
> if (eventLoop.handles[i].watch == watch) {
> EVENT_DEBUG("mark delete %d %d", i, eventLoop.handles[i].fd);
> eventLoop.handles[i].deleted = 1;
> + virEventInterruptLocked();
> virEventUnlock();
> return 0;
> }
> }
> - virEventInterruptLocked();
> virEventUnlock();
> return -1;
> }
same here
> @@ -244,10 +263,10 @@ void virEventUpdateTimeoutImpl(int timer
> frequency >= 0 ? frequency +
> (((unsigned long long)tv.tv_sec)*1000) +
> (((unsigned long long)tv.tv_usec)/1000) : 0;
> + virEventInterruptLocked();
> break;
> }
> }
> - virEventInterruptLocked();
> virEventUnlock();
> }
and here
> @@ -267,11 +292,11 @@ int virEventRemoveTimeoutImpl(int timer)
>
> if (eventLoop.timeouts[i].timer == timer) {
> eventLoop.timeouts[i].deleted = 1;
> + virEventInterruptLocked();
> virEventUnlock();
> return 0;
> }
> }
> - virEventInterruptLocked();
> virEventUnlock();
> return -1;
> }
and here. I assume the problem were on imbricated loops.
Patch looks fine, ACK, and an important fix !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list