Currently iohelper's error log is recorded in virFileWrapperFdClose.
In qemuDomainSaveMemory, it usually fails at qemuMigrationSrcToFile,
and then goto cleanup, so the iohelper error log is not recorded,
and so is the another placement. We now record the error log of
iohelper by move it to the virFileWrapperFdFree record.
And for the virCommandWait() - we should call virCommandAbort() in
virFileWrapperFdFree() to make sure no process is left behind. If
virCommandWait() is called then virCommandAbort() is a NO-OP. If it
isn't, then Abort() will kill iohelper. Thanks for Michal for this
advice

Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
---
 src/util/virfile.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 1faeebb..590fc0e 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -330,9 +330,6 @@ virFileWrapperFdClose(virFileWrapperFdPtr wfd)
         return 0;
 
     ret = virCommandWait(wfd->cmd, NULL);
-    if (wfd->err_msg && *wfd->err_msg)
-        VIR_WARN("iohelper reports: %s", wfd->err_msg);
-
     return ret;
 }
 
@@ -351,6 +348,10 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd)
     if (!wfd)
         return;
 
+    if (wfd->err_msg && *wfd->err_msg)
+        VIR_WARN("iohelper reports: %s", wfd->err_msg);
+
+    virCommandAbort(wfd->cmd);
     VIR_FREE(wfd->err_msg);
 
     virCommandFree(wfd->cmd);
-- 
2.8.3


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 07/23/2018 02:22 PM, xinhua.Cao wrote:
> Currently iohelper's error log is recorded in virFileWrapperFdClose.
> In qemuDomainSaveMemory, it usually fails at qemuMigrationSrcToFile,
> and then goto cleanup, so the iohelper error log is not recorded,
> and so is the another placement. We now record the error log of
> iohelper by move it to the virFileWrapperFdFree record.
> And for the virCommandWait() - we should call virCommandAbort() in
> virFileWrapperFdFree() to make sure no process is left behind. If
> virCommandWait() is called then virCommandAbort() is a NO-OP. If it
> isn't, then Abort() will kill iohelper. Thanks for Michal for this
> advice
> 
> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
> ---
>  src/util/virfile.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 1faeebb..590fc0e 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -330,9 +330,6 @@ virFileWrapperFdClose(virFileWrapperFdPtr wfd)
>          return 0;
>  
>      ret = virCommandWait(wfd->cmd, NULL);
> -    if (wfd->err_msg && *wfd->err_msg)
> -        VIR_WARN("iohelper reports: %s", wfd->err_msg);
> -
>      return ret;
>  }
>  
> @@ -351,6 +348,10 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd)
>      if (!wfd)
>          return;
>  
> +    if (wfd->err_msg && *wfd->err_msg)
> +        VIR_WARN("iohelper reports: %s", wfd->err_msg);
> +
> +    virCommandAbort(wfd->cmd);
>      VIR_FREE(wfd->err_msg);
>  
>      virCommandFree(wfd->cmd);
> 

I've reworded the error message a bit, ACKed and pushed. Sorry for the
delay.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list