Re: [libvirt] [PATCH v3 3/4] util: Move error reporting back to virFileWrapperFdClose()

On Tue, Feb 19, 2019 at 05:52:30PM +0100, Andrea Bolognani wrote: > virFileWrapperFdFree(), like all free functions, is supposed > to only release allocated resources, so error reporting is > better suited for virFileWrapperFdClose(). > > This reverts most of commit b0c3e931804a. > > Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; > --- > src/util/virfile.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index 42add5a2cd..8e045279f0 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -337,6 +337,9 @@ virFileWrapperFdClose(virFileWrapperFdPtr wfd) > > ret = virCommandWait(wfd->cmd, NULL); > > + if (wfd->err_msg && *wfd->err_msg) > + VIR_WARN("iohelper reports: %s", wfd->err_msg); > + > wfd->closed = true; > > return ret; > @@ -357,9 +360,6 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd) > if (!wfd) > return; > > - if (wfd->err_msg && *wfd->err_msg) > - VIR_WARN("iohelper reports: %s", wfd->err_msg); > - > virCommandAbort(wfd->cmd); It feels like this could be reverted too. We already call virCommandFree which I think should call Abort for us - though not 100% confident since the scenario in which "reap = true" is set is quite complex.