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

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