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