On 01/07/2014 10:07 AM, John Ferlan wrote:
The intro comments to 'virObjectEventStateRegisterID()' need to be adjusted to remove '@name' and '@id'.
Will do.
Same for 'virObjectEventCallbackLookup()' - sadly missed in patch 7, but I was looking for @name and @id now...
I actually caught that in patch 7 in time (but managed to botch the testsuite in the process of amending that patch).
Leaving only 'virObjectEventNew()' as the lone API that cares about 'id' and 'name', but doesn't utilize. Should that be noted somewhere? Since 'meta.name' and 'meta.id' aren't used anywhere, do they even need to be saved... Would save an alloc/free for name.
virObjectEvent DOES care about id and name - the dispatcher function has to reconstruct the virDomainPtr for handing to the callback function (see virDomainEventDispatchDefaultFunc in domain_event.c). So meta is still important for events, just not for callbacks. But if you found it confusing, then it can't hurt for me to enhance the commit message :)
Should the existing 'testDomainCreateXMLMixed()' be kept as is? And then add a 'testDomainDefineXMLMixed()'? At the very least the name should probably be changed since other test functions distinguish Define & Create in their names.
The test is covering whether an event happens when a domain is created (whether created as transient, or changed from offline to online for persistent); but to register an event at all, the domain already has to exist. When I first wrote the entire function for this patch, I used 'define' to make the domain exist, then register events, then 'create' to see the creation event; but it turned up other bugs, which I then rebased and fixed first, using the portions of the test that worked to expose those problems. But when rebasing, I had to first test create as transient, register, then destroy, then re-create, to avoid tripping the bug fixed in this patch. I added more commentary in my commit message :)
ACK in general though.
Okay, here's what I squashed in when pushing. diff --git i/src/conf/object_event.c w/src/conf/object_event.c index c4aedd9..b01ffe5 100644 --- i/src/conf/object_event.c +++ w/src/conf/object_event.c @@ -774,8 +774,6 @@ virObjectEventStateFlush(virObjectEventStatePtr state) * @conn: connection to associate with callback * @state: domain event state * @uuid: uuid of the object for event filtering - * @name: name of the object for event filtering - * @id: id of the object for event filtering, or 0 * @klass: the base event class * @eventID: ID of the event type to register for * @cb: function to invoke when event occurs diff --git i/src/conf/object_event_private.h w/src/conf/object_event_private.h index 76bd6d1..571fc40 100644 --- i/src/conf/object_event_private.h +++ w/src/conf/object_event_private.h @@ -69,6 +69,8 @@ virObjectEventNew(virClassPtr klass, int eventID, int id, const char *name, - const unsigned char *uuid); + const unsigned char *uuid) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) + ATTRIBUTE_NONNULL(6); #endif -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org