[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] PATCH: Allow updating of timer & file handle event watches



On Tue, Sep 18, 2007 at 03:49:02AM -0400, Daniel Veillard wrote:
> On Tue, Sep 18, 2007 at 02:56:35AM +0100, Daniel P. Berrange wrote:
> > The forthcoming patch to add Avahi support requires a couple of enhancements
> > to our event loop to support integration with Avahi's event callback needs
> > 
> >  - Ability to update the event mask for a pre-existing file handle watch
> >  - Ability to change the frequency of a timer watch
> >  - Ability to have a timer which fires immediately
> >  - Ability to disable events on a timer watch
> > 
> > The first point was trivial - just change the 'events' flag we have recorded
> > in the virEventHandle struct.
> > 
> > The second point was also trivial - just change the period of the timer as
> > recorded in virEventTimer struct.
> > 
> > The last two points were more tricky, but doable. To fire a timer immediately
> > I tweaked the logic so that it allows a frequency of '0'. Obviously to avoid
> > spinning 100%, the app should unregister the timer once it has fired the
> > desired number of times (usually 1). To disable a timer temporarily I allow
> > the use of '-1' in the frequency.
> > 
> > THe code had a bad mix of terminology using a 'timer' and 'timeout' field
> > in the virEventTimer struct which confused me no end. So I renamed the
> > 'timeout' field to 'frequency' which reflects its actual purpose.
> > 
> > Finally, when building with debugging, the event loop can generate alot of
> > data, so I added a EVENT_DEBUG macro to the file which prefixes EVENT: on
> > all debug output so allow easy filtering.
> 
>   Looks fine to me +1

Ok I comitted that.

>   From a stylistic POV I notice the function comment description is in the
> header files, not in the C file like others parts of libvirt, as long as 
> one doesn't forget to update them when updating the code, that's fine.

Not sure why I put them in the header files originally - I guess since its
internal API its not in the docs, so its easy to browse the docs directly
in the header file without being cluttered by the code. If you think they're
better off in the C file for consistency, I'll move it

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]