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

Daniel P. Berrangé berrange at redhat.com
Tue Feb 19 16:59:40 UTC 2019


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 at redhat.com>
> ---
>  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.

>  
>      VIR_FREE(wfd->err_msg);

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




More information about the libvir-list mailing list