Re: [libvirt] [PATCH v3 2/4] qemu: Always call virFileWrapperFdClose()
On Wed, Feb 20, 2019 at 01:16:34PM +0100, Andrea Bolognani wrote:
On Tue, 2019-02-19 at 17:01 +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 19, 2019 at 05:52:29PM +0100, Andrea Bolognani wrote:
[...]
> > @@ -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.
In cases where qemuFileWrapperFDClose() returning an error was not
considered an overall failure in the existing code, I have preserved
that behavior.
FWIW, I consider that existing code to be buggy. It is akin to ignoring
the return status of close(). Data you think you've written can in fact
be lost.
We usually only ignore close() return value if we're already in an
error cleanup scenario. Any success codepaths should always honour
the close() status.
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 :|