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

Re: [libvirt] [PATCH] events: Don't fail on registering events for two different domains



On 06/27/2012 06:12 AM, Michal Privoznik wrote:
> virConnectDomainEventRegisterAny() takes a domain as an argument.
> So it should be possible to register the same event (be it
> VIR_DOMAIN_EVENT_ID_LIFECYCLE for example) for two different domains.
> That is, we need to take domain into account when searching for
> duplicate event being already registered.
> ---
>  src/conf/domain_event.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
> index 4ecc413..3cfd940 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -363,7 +363,11 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
>      for (i = 0 ; i < cbList->count ; i++) {
>          if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) &&
>              cbList->callbacks[i]->eventID == eventID &&
> -            cbList->callbacks[i]->conn == conn) {
> +            cbList->callbacks[i]->conn == conn &&
> +            ((dom && cbList->callbacks[i]->dom &&
> +              memcmp(cbList->callbacks[i]->dom->uuid,
> +                     dom->uuid, VIR_UUID_BUFLEN) == 0) ||
> +             (!dom && !cbList->callbacks[i]->dom))) {

This misses the case of registering a catchall against NULL domain then
attempting to re-register the same event against a specific domain
(which one of the two would fire?).  It also misses the case of
registering a domain-specific handler, then attempting to broaden things
into a global handler.

I think the idea of double registration makes sense (in particular, if I
have a per-domain callback, but then want to switch to global, I would
rather have a window with both handlers at once than a window with no
handler at all), but I have not tested whether we actually handle it by
firing both the global and domain-specific callback.

If we are happy with double registration, then your patch looks correct
on the registration side (although it may be incomplete, if we mishandle
double firing).  On the other hand, if we want to prevent double
registration, then you need to further forbid registration when ((dom &&
!cbList->callbacks[i]->dom) || (!dom && cblist->callbacks[i]->dom)).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



Attachment: signature.asc
Description: OpenPGP digital signature


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