Patchwork disable sigcld handling before calling pclose()

login
register
mail settings
Submitter Wen Congyang
Date Dec. 9, 2010, 3:41 a.m.
Message ID <4D005006.9080307@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/74885/
State New
Headers show

Comments

Wen Congyang - Dec. 9, 2010, 3:41 a.m.
When I use the command 'virsh save' to save the domain state,
I receive the following error message:
operation failed: Migration unexpectedly failed.

I debug the qemu by adding some printf(), and find the function
pclose() returns -1.

I use strace to trace qemu, the log is as the following:
======
close(17)                               = 0
--- SIGCHLD (Child exited) @ 0 (0) ---
wait4(-1, NULL, WNOHANG, NULL)          = 22016
rt_sigreturn(0)                         = 0
wait4(22016, 0x7fff7f1034fc, 0, NULL)   = -1 ECHILD (No child processes)
Wen Congyang - Dec. 14, 2010, 9:23 a.m.
At 2010-12-09 11:41, Wen Congyang Write:
> When I use the command 'virsh save' to save the domain state,
> I receive the following error message:
> operation failed: Migration unexpectedly failed.
> 
> I debug the qemu by adding some printf(), and find the function
> pclose() returns -1.
> 
> I use strace to trace qemu, the log is as the following:
> ======
> close(17)                               = 0
> --- SIGCHLD (Child exited) @ 0 (0) ---
> wait4(-1, NULL, WNOHANG, NULL)          = 22016
> rt_sigreturn(0)                         = 0
> wait4(22016, 0x7fff7f1034fc, 0, NULL)   = -1 ECHILD (No child processes)
> ======
> 
> We wait the child twice: one is in signal SIGCHLD handling and the other
> one is in pclose().
> 
> We should disable sigcld handling before calling pclose().
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> 
Ping :)
Wen Congyang - Dec. 20, 2010, 1:25 a.m.
At 12/14/2010 05:23 PM, Wen Congyang Write:
> At 2010-12-09 11:41, Wen Congyang Write:
>> When I use the command 'virsh save' to save the domain state,
>> I receive the following error message:
>> operation failed: Migration unexpectedly failed.
>>
>> I debug the qemu by adding some printf(), and find the function
>> pclose() returns -1.
>>
>> I use strace to trace qemu, the log is as the following:
>> ======
>> close(17)                               = 0
>> --- SIGCHLD (Child exited) @ 0 (0) ---
>> wait4(-1, NULL, WNOHANG, NULL)          = 22016
>> rt_sigreturn(0)                         = 0
>> wait4(22016, 0x7fff7f1034fc, 0, NULL)   = -1 ECHILD (No child processes)
>> ======
>>
>> We wait the child twice: one is in signal SIGCHLD handling and the other
>> one is in pclose().
>>
>> We should disable sigcld handling before calling pclose().
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>
> Ping :)
> 
> 

Ping Again... :)
Andreas Färber - Dec. 20, 2010, 6:33 a.m.
Am 20.12.2010 um 02:25 schrieb Wen Congyang:

> At 12/14/2010 05:23 PM, Wen Congyang Write:
>> At 2010-12-09 11:41, Wen Congyang Write:
>>> When I use the command 'virsh save' to save the domain state,
>>> I receive the following error message:
>>> operation failed: Migration unexpectedly failed.
>>>
>>> I debug the qemu by adding some printf(), and find the function
>>> pclose() returns -1.
>>>
>>> I use strace to trace qemu, the log is as the following:
>>> ======
>>> close(17)                               = 0
>>> --- SIGCHLD (Child exited) @ 0 (0) ---
>>> wait4(-1, NULL, WNOHANG, NULL)          = 22016
>>> rt_sigreturn(0)                         = 0
>>> wait4(22016, 0x7fff7f1034fc, 0, NULL)   = -1 ECHILD (No child  
>>> processes)
>>> ======
>>>
>>> We wait the child twice: one is in signal SIGCHLD handling and the  
>>> other
>>> one is in pclose().
>>>
>>> We should disable sigcld handling before calling pclose().
>>>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>
>> Ping :)
>>
>>
>
> Ping Again... :)

os-posix.c part looks sane to me, but what about Win32? Wouldn't it  
need stub functions?

Andreas
Wen Congyang - Dec. 21, 2010, 2:44 a.m.
At 12/20/2010 02:33 PM, Andreas Färber Write:
> Am 20.12.2010 um 02:25 schrieb Wen Congyang:
> 
>> At 12/14/2010 05:23 PM, Wen Congyang Write:
>>> At 2010-12-09 11:41, Wen Congyang Write:
>>>> When I use the command 'virsh save' to save the domain state,
>>>> I receive the following error message:
>>>> operation failed: Migration unexpectedly failed.
>>>>
>>>> I debug the qemu by adding some printf(), and find the function
>>>> pclose() returns -1.
>>>>
>>>> I use strace to trace qemu, the log is as the following:
>>>> ======
>>>> close(17)                               = 0
>>>> --- SIGCHLD (Child exited) @ 0 (0) ---
>>>> wait4(-1, NULL, WNOHANG, NULL)          = 22016
>>>> rt_sigreturn(0)                         = 0
>>>> wait4(22016, 0x7fff7f1034fc, 0, NULL)   = -1 ECHILD (No child
>>>> processes)
>>>> ======
>>>>
>>>> We wait the child twice: one is in signal SIGCHLD handling and the
>>>> other
>>>> one is in pclose().
>>>>
>>>> We should disable sigcld handling before calling pclose().
>>>>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>
>>> Ping :)
>>>
>>>
>>
>> Ping Again... :)
> 
> os-posix.c part looks sane to me, but what about Win32? Wouldn't it need
> stub functions?
I do not know whether there is same BUG on Windows.

I will add stub functions for Win32.
> 
> Andreas
>

Patch

======

We wait the child twice: one is in signal SIGCHLD handling and the other
one is in pclose().

We should disable sigcld handling before calling pclose().

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>

---
 os-posix.c      |   19 +++++++++++++++++++
 qemu-os-posix.h |    2 ++
 savevm.c        |    2 ++
 3 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 38c29d1..b163995 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -86,6 +86,25 @@  void os_setup_signal_handling(void)
     sigaction(SIGCHLD, &act, NULL);
 }
 
+void os_stop_sigchld_handling(void)
+{
+    struct sigaction act;
+
+    memset(&act, 0, sizeof(act));
+    act.sa_handler = SIG_DFL;
+    sigaction(SIGCHLD, &act, NULL);
+}
+
+void os_resume_sigchld_handling(void)
+{
+    struct sigaction act;
+
+    memset(&act, 0, sizeof(act));
+    act.sa_handler = sigchld_handler;
+    act.sa_flags = SA_NOCLDSTOP;
+    sigaction(SIGCHLD, &act, NULL);
+}
+
 /* Find a likely location for support files using the location of the binary.
    For installed binaries this will be "$bindir/../share/qemu".  When
    running from the build tree this will be "$bindir/../pc-bios".  */
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 353f878..e819295 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -33,6 +33,8 @@  static inline void os_host_main_loop_wait(int *timeout)
 void os_set_line_buffering(void);
 void os_set_proc_name(const char *s);
 void os_setup_signal_handling(void);
+void os_stop_sigchld_handling(void);
+void os_resume_sigchld_handling(void);
 void os_daemonize(void);
 void os_setup_post(void);
 
diff --git a/savevm.c b/savevm.c
index d38f79e..08a5f88 100644
--- a/savevm.c
+++ b/savevm.c
@@ -234,7 +234,9 @@  static int stdio_pclose(void *opaque)
 {
     QEMUFileStdio *s = opaque;
     int ret;
+    os_stop_sigchld_handling();
     ret = pclose(s->stdio_file);
+    os_resume_sigchld_handling();
     qemu_free(s);
     return ret;
 }