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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> 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 :|
