[libvirt] [PATCHv2 13/14] event: don't turn offline domain into global event

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140107/6f75226f/attachment-0001.sig>