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

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(a)redhat.com&gt; >> --- >> 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.