Re: [libvirt] [PATCH 6/6] qemu: Resolve Coverity RESOURCE_LEAK
On 09/24/2015 03:45 AM, Ján Tomko wrote: > On Wed, Sep 23, 2015 at 07:18:33PM -0400, John Ferlan wrote: >> This seemed to be more of a false positive as for some reason Coverity >> was missing the "ret < 0" goto error condition and somehow believing that >> event could be overwritten. At first I thought it was just the ret != 0 >> condition difference, but it wasn't. >> > > Then those changes are cosmetic and don't need to be in a commit fixing > the RESOURCE_LEAK. > OK - I've split this patch in two - cosmetic and resource_leak and pushed the patches that have been ACK'd... I'll continue to plug through issues noted from patch 1 along with the ones referenced in my cover letter. Tks - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] qemu: Resolve Coverity RESOURCE_LEAK
On Wed, Sep 23, 2015 at 07:18:33PM -0400, John Ferlan wrote: > This seemed to be more of a false positive as for some reason Coverity > was missing the "ret < 0" goto error condition and somehow believing that > event could be overwritten. At first I thought it was just the ret != 0 > condition difference, but it wasn't. > Then those changes are cosmetic and don't need to be in a commit fixing the RESOURCE_LEAK. > In any case, make use of the recent change to qemuDomainEventQueue to > check event == NULL and just pass it as a parameter directly in the > error path. That avoids the error. > > Signed-off-by: John Ferlan > --- > src/qemu/qemu_driver.c | 24 +++- > 1 file changed, 11 insertions(+), 13 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 2387cf3..3a98774 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -3155,7 +3155,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, > virFileWrapperFdFree(wrapperFd); > VIR_FREE(xml); > > -if (ret != 0 && needUnlink) > +if (ret < 0 && needUnlink) > unlink(path); > > return ret; > @@ -3174,7 +3174,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, > virDomainPtr dom, > char *xml = NULL; > bool was_running = false; > int ret = -1; > -int rc; > virObjectEventPtr event = NULL; > qemuDomainObjPrivatePtr priv = vm->privateData; > virCapsPtr caps; > @@ -3249,21 +3248,20 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, > virDomainPtr dom, > /* Shut it down */ > qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED, 0); > virDomainAuditStop(vm, "saved"); > -event = virDomainEventLifecycleNewFromObj(vm, > - VIR_DOMAIN_EVENT_STOPPED, > - VIR_DOMAIN_EVENT_STOPPED_SAVED); > +event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, > + > VIR_DOMAIN_EVENT_STOPPED_SAVED); > endjob: > -if (ret != 0) { > +if (ret < 0) { > if (was_running && virDomainObjIsActive(vm)) { > virErrorPtr save_err = virSaveLastError(); > -rc = qemuProcessStartCPUs(driver, vm, dom->conn, > - VIR_DOMAIN_RUNNING_SAVE_CANCELED, > - QEMU_ASYNC_JOB_SAVE); > -if (rc < 0) { > +if (qemuProcessStartCPUs(driver, vm, dom->conn, > + VIR_DOMAIN_RUNNING_SAVE_CANCELED, > + QEMU_ASYNC_JOB_SAVE) < 0) { > VIR_WARN("Unable to resume guest CPUs after save failure"); > -event = virDomainEventLifecycleNewFromObj(vm, > - > VIR_DOMAIN_EVENT_SUSPENDED, > - > VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR); > +qemuDomainEventQueue(driver, > + virDomainEventLifecycleNewFromObj(vm, > + VIR_DOMAIN_EVENT_SUSPENDED, > + > VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR)); ACK to this hunk as the coverity fix, and the rest pushed separately. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] qemu: Resolve Coverity RESOURCE_LEAK
This seemed to be more of a false positive as for some reason Coverity was missing the "ret < 0" goto error condition and somehow believing that event could be overwritten. At first I thought it was just the ret != 0 condition difference, but it wasn't. In any case, make use of the recent change to qemuDomainEventQueue to check event == NULL and just pass it as a parameter directly in the error path. That avoids the error. Signed-off-by: John Ferlan --- src/qemu/qemu_driver.c | 24 +++- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2387cf3..3a98774 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3155,7 +3155,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, virFileWrapperFdFree(wrapperFd); VIR_FREE(xml); -if (ret != 0 && needUnlink) +if (ret < 0 && needUnlink) unlink(path); return ret; @@ -3174,7 +3174,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, char *xml = NULL; bool was_running = false; int ret = -1; -int rc; virObjectEventPtr event = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virCapsPtr caps; @@ -3249,21 +3248,20 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, /* Shut it down */ qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED, 0); virDomainAuditStop(vm, "saved"); -event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_STOPPED_SAVED); +event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, + VIR_DOMAIN_EVENT_STOPPED_SAVED); endjob: -if (ret != 0) { +if (ret < 0) { if (was_running && virDomainObjIsActive(vm)) { virErrorPtr save_err = virSaveLastError(); -rc = qemuProcessStartCPUs(driver, vm, dom->conn, - VIR_DOMAIN_RUNNING_SAVE_CANCELED, - QEMU_ASYNC_JOB_SAVE); -if (rc < 0) { +if (qemuProcessStartCPUs(driver, vm, dom->conn, + VIR_DOMAIN_RUNNING_SAVE_CANCELED, + QEMU_ASYNC_JOB_SAVE) < 0) { VIR_WARN("Unable to resume guest CPUs after save failure"); -event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR); +qemuDomainEventQueue(driver, + virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR)); } virSetError(save_err); virFreeError(save_err); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list