[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




On 01/06/2014 05:27 PM, Eric Blake wrote:
> If a user registers for a domain event filtered to a particular
> domain, but the persistent domain is offline at the time, then
> the code silently failed to set up the filter.  As a result,
> the event fires for all domains, rather than being filtered.
> Network events were immune, since they always passed an id
> 0 argument.
> 
> The key to this patch is realizing that
> virObjectEventDispatchMatchCallback() only cared about uuid;
> so refusing to create a meta for a negative id is pointless,
> and in fact, malloc'ing meta at all was overkill; instead,
> just directly store a uuid and a flag of whether to filter.
> 
> * src/conf/object_event_private.h (_virObjectEventCallback):
> Replace meta with uuid and flag.
> (virObjectEventCallbackListAddID): Update signature.
> * src/conf/object_event.h (virObjectEventStateRegisterID):
> Likewise.
> * src/conf/object_event.c (virObjectEventCallbackListAddID): Drop
> arguments that don't affect filtering.
> (virObjectEventCallbackListRemoveID)
> (virObjectEventDispatchMatchCallback)
> (virObjectEventStateRegisterID): Update clients.
> * src/conf/domain_event.c (virDomainEventCallbackListAdd)
> (virDomainEventStateRegisterID): Likewise.
> * src/conf/network_event.c (virNetworkEventStateRegisterID):
> Likewise.
> 
> Signed-off-by: Eric Blake <eblake redhat com>
> ---
>  src/conf/domain_event.c  | 16 +++++-----------
>  src/conf/network_event.c | 13 +++----------
>  src/conf/object_event.c  | 48 +++++++++++++++++-------------------------------
>  src/conf/object_event.h  |  5 ++---
>  4 files changed, 27 insertions(+), 55 deletions(-)
> 

The intro comments to 'virObjectEventStateRegisterID()' need to be
adjusted to remove '@name' and '@id'.

Same for 'virObjectEventCallbackLookup()' - sadly missed in patch 7, but
I was looking for @name and @id now...

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.

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.


ACK in general though.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]