[Libvir] PATCH: Allow updating of timer & file handle event watches
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. 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 -=|
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 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. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
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 -=|
On Wed, Sep 19, 2007 at 03:04:12AM +0100, Daniel P. Berrange wrote:
On Tue, Sep 18, 2007 at 03:49:02AM -0400, Daniel Veillard wrote:
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
As long as we don't try to automatically extract the docs for the internals (which may be needed at some point as the internals are growing) I don't think there is a need for a change really, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (2)
-
Daniel P. Berrange -
Daniel Veillard