Re: [libvirt] PATCH: 2/4: Interrupt main thread if in poll()

12 May 2009


      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@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

    