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

Re: [libvirt] [PATCH v4 4/7] Add network events to the remote driver



On 01/09/2014 07:50 PM, Eric Blake wrote:
> On 12/12/2013 07:30 AM, Cedric Bosdonnat wrote:
>> Hi John,
>>
> 
>>>> +    event = virNetworkEventLifecycleNew(net->name, net->uuid, msg->event);
>>>> +    virNetworkFree(net);
>>>> +    remoteDomainEventQueue(priv, event);
> 
>>> Essentially - you need to check for NULL event.
>>
>> Sure, but the weird thing it that it should also complain on the similar
>> functions for domain events, right above this one in the same file.
> 
> Revisiting this.  The problem can only happen on OOM situations, so it's
> not that likely to hit in real life; but we should indeed fix our code.
>  I think the easiest way is to fix remoteDomainEventQueue to gracefully
> do nothing on a NULL event (events are best effort, and if OOM occurs,
> skipping event delivery is an acceptable action, since we have no way of
> reporting the error).  I'll post a patch.

Actually, it turns out we are quite lucky - appending a NULL event to
the queue had no ill effects, because when checking for what callbacks
to dispatch, our first use of events was an early exit if
(!virObjectIsClass(event, cb->klass)).  So the ATTRIBUTE_NONNULL could
actually be deleted - but that in turn implies we have a lot of code
that checks for non-null events before queueing them up which could also
be simplified.  And even if we did that, queueing a NULL event is a
waste of resources compared to just avoiding them altogether.

-- 
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]