On Tue, Feb 19, 2019 at 05:52:29PM +0100, Andrea Bolognani wrote:
Right now we're reporting errors in virFileWrapperFdFree(), but that's hardly the appropriate place to do so, as free functions are supposed to do nothing more than release allocated resources.
We want to move that code back into virFileWrapperFdClose(), but before we can do that we need to make sure the function is actually called every time we're done processing the wrapped file. The cleanup path is the obvious candidate.
For two of the users we can just move the call, but for the other two we need to duplicate it instead in order not to alter the existing behavior.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5118f4ad42..30f69b339b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3231,6 +3231,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
cleanup: VIR_FORCE_CLOSE(fd); + qemuFileWrapperFDClose(vm, wrapperFd);
Don't we need to check & propagate the return status of this, otherwise callers would mistakenly think qemuDomainSaveMemory has succeeeded, despite qemuFileWrapperFDClose having raised an error. Likewise all the other places below.
virFileWrapperFdFree(wrapperFd); virObjectUnref(cfg);
@@ -3834,9 +3835,10 @@ doCoreDump(virQEMUDriverPtr driver,
cleanup: VIR_FORCE_CLOSE(fd); + qemuFileWrapperFDClose(vm, wrapperFd); + virFileWrapperFdFree(wrapperFd); if (ret != 0) unlink(path); - virFileWrapperFdFree(wrapperFd); VIR_FREE(compressedpath); virObjectUnref(cfg); return ret; @@ -7043,17 +7045,16 @@ qemuDomainRestoreFlags(virConnectPtr conn,
ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path, false, QEMU_ASYNC_JOB_START); - if (virFileWrapperFdClose(wrapperFd) < 0) - VIR_WARN("Failed to close %s", path);
qemuProcessEndJob(driver, vm);
cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(fd); + virFileWrapperFdClose(wrapperFd); + virFileWrapperFdFree(wrapperFd); virQEMUSaveDataFree(data); VIR_FREE(xmlout); - virFileWrapperFdFree(wrapperFd); if (vm && ret < 0) qemuDomainRemoveInactiveJob(driver, vm); virDomainObjEndAPI(&vm); @@ -7318,14 +7319,13 @@ qemuDomainObjRestore(virConnectPtr conn,
ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, data, path, start_paused, asyncJob); - if (virFileWrapperFdClose(wrapperFd) < 0) - VIR_WARN("Failed to close %s", path);
cleanup: virQEMUSaveDataFree(data); VIR_FREE(xmlout); virDomainDefFree(def); VIR_FORCE_CLOSE(fd); + virFileWrapperFdClose(wrapperFd); virFileWrapperFdFree(wrapperFd); return ret; } -- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|