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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list