[libvirt] [PATCH 01/19] libxl_migration: Resolve Coverity NULL_RETURNS

[John Ferlan]

On 08/27/2014 05:29 PM, Jim Fehlig wrote:
> 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 prior to
>> generating an event for the dom == NULL condition, the resume/suspend
>> event should be queue'd after the virDomainSaveStatus() call which will
>> goto cleanup and queue the saved event anyway.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/libxl/libxl_migration.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> index dbb5a8f..53ae63a 100644
>> --- a/src/libxl/libxl_migration.c
>> +++ b/src/libxl/libxl_migration.c
>> @@ -512,6 +512,11 @@ libxlDomainMigrationFinish(virConnectPtr dconn,
>>      if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>>          goto cleanup;
>>  
>> +    if (event) {
>> +        libxlDomainEventQueue(driver, event);
>> +        event = NULL;
>> +    }
>> +
>>      dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
>>  
>>      if (dom == NULL) {
>> @@ -519,7 +524,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn,
>>          libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED);
>>          event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
>>                                           VIR_DOMAIN_EVENT_STOPPED_FAILED);
>> -        libxlDomainEventQueue(driver, event);
>>      }
>>   
> 
> See my question in your first series about whether the dom == NULL check
> is even needed.  If not, I can send a patch to remove the check, in
> which case this patch wouldn't be needed.
> 
> https://www.redhat.com/archives/libvir-list/2014-August/msg01399.html
> 
> 

I am going to remove this one from my series and let you handle it.

I would seem perhaps that the code was there to ensure that if either of
the calls to virDomainObjSetState() ended up resulting in 'dom' not
returning something from the virGetDomain(), then like perhaps the qemu
migration code, the "best choice" was to remove it. In particular I
looked at the second VIR_DOMAIN_SHUTOFF_FAILED in qemuMigrationFinish()
for a "comparison".  I'm by far no libvirt migration processing expert
though...

Tks -

John