Re: [libvirt] [PATCH 00/34] network events feature v2
On Fri, Nov 29, 2013 at 04:18:36PM +0100, Cédric Bosdonnat wrote:
This patch serie is replacing the previous one I sent. The
improvements that
were made in between are:
* splitting the 2 huge commits into smaller reviewable ones.
* Get rid of the huge union in virDomainEvent and use virObjects instead.
* No domain events-related code in object_event.c. I left the network events
related code there as it would be pretty small in a separate set of files.
* Rebased on the python split repository
This all looks pretty good to me. The only thing I'm not really liking
is the VIR_EVENT_NAMESPACE stuff exposed in the public header. Since we
have separate public APIs for registering event callbacks for each type
of object, there is no need to have namespaces for event IDs from an
app developers POV. I understand you want to encode things into a single
event ID space internally for ease of handling, but I think we should
not expose this to apps.
I saw something weird in the domain events code when working on this:
VIR_DOMAIN_EVENT_LAST is defined if VIR_ENUM_SENTINELS is defined, but is
generally used independently of it. I did the same for the VIR_NETWORK_EVENT_LAST
thought I'm not sure that's the correct thing to do.
The internal code alwas has VIR_ENUM_SENTINELS defined so can rely
on using any _LAST constant. We simply hide from the client applications
by default.
These changes are all about bringing events for network object like
the
ones existing for domains. This feature is needed for virt-manager to
refresh its UI when networks are started/destroyed/defined/undefined.
The network events are implemented in the test, bridge network and remote
drivers ATM.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|