[libvirt] [PATCH 09/10] libxl_migration: Resolve Coverity NULL_RETURNS

[John Ferlan]

On 08/27/2014 10:42 AM, Ján Tomko wrote:
> On 08/27/2014 03:51 PM, John Ferlan wrote:
>> Coverity noted that all callers to libxlDomainEventQueue() could ensure
>> the second parameter (event) was true before calling except this case.
>> As I look at the code and how events are used - it seems that two events
>> would have been sent which more than likely causes some bad stuff for
>> the second one.  So just remove one call and let the cleanup: handle
>> the event.  In the future if there's code between getting the event
>> and cleanup that needs to send the event, this will have to change in
>> order to send the event and set event = NULL (although it seems unlikely
>> to happen).
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/libxl/libxl_migration.c | 1 -
>>  1 file changed, 1 deletion(-)
> 
> 
>>
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> index dbb5a8f..eb65536 100644
>> --- a/src/libxl/libxl_migration.c
>> +++ b/src/libxl/libxl_migration.c
>> @@ -519,7 +519,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn,
>>          libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
>>          event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
>>                                           VIR_DOMAIN_EVENT_STOPPED_FAILED);
> 
> This (possibly) overwrites the event created earlier.
> But I'm not sure if the right solution is to send both events, or clear the
> original one.
> 
> Jan
> 

Hmmm.. right

Maybe a:

    if (event) {
        libxlDomainEventQueue(driver, event);
        event = NULL;
    }

before the:

    dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);

will at least cause the RESUME/SUSPEND events to be sent and then if the
dom == NULL the STOPPED event would be sent as well

I've copied Jim Fehlig for his opinion - since he wrote the code...

John

>> -        libxlDomainEventQueue(driver, event);
>>      }
>>  
>>   cleanup:
>>
> 
>