From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60956 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Puywh-0000q0-El for qemu-devel@nongnu.org; Wed, 02 Mar 2011 22:11:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Puywg-0006Hj-43 for qemu-devel@nongnu.org; Wed, 02 Mar 2011 22:11:19 -0500 Received: from [222.73.24.84] (port=65020 helo=song.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Puywf-0006HM-Hs for qemu-devel@nongnu.org; Wed, 02 Mar 2011 22:11:18 -0500 Message-ID: <4D6F0688.10402@cn.fujitsu.com> Date: Thu, 03 Mar 2011 11:10:00 +0800 From: Wen Congyang MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2] disable sigcld handling before calling pclose() References: <4D005006.9080307@cn.fujitsu.com> <4D07378B.3050901@cn.fujitsu.com> <4D0EB06D.6060001@cn.fujitsu.com> <29897010-A575-4C94-8016-C0FC03652B64@web.de> <4D102795.9040107@cn.fujitsu.com> In-Reply-To: <4D102795.9040107@cn.fujitsu.com> Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=ISO-8859-1 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Andreas_F=E4rber?= , qemu-devel , Luiz Capitulino , Anthony Liguori , Paolo Bonzini At 12/21/2010 12:05 PM, 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(). > > v2: > - Add stub functions for Win32 > > Signed-off-by: Wen Congyang > > --- > os-posix.c | 19 +++++++++++++++++++ > qemu-os-posix.h | 2 ++ > qemu-os-win32.h | 2 ++ > savevm.c | 2 ++ > 4 files changed, 25 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 81fd9ab..1c317f1 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/qemu-os-win32.h b/qemu-os-win32.h > index 1a07e5e..f31c5ef 100644 > --- a/qemu-os-win32.h > +++ b/qemu-os-win32.h > @@ -43,6 +43,8 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque); > void os_host_main_loop_wait(int *timeout); > > static inline void os_setup_signal_handling(void) {} > +static inline void os_stop_sigchld_handling(void) {} > +static inline void os_resume_sigchld_handling(void) {} > static inline void os_daemonize(void) {} > static inline void os_setup_post(void) {} > void os_set_line_buffering(void); > diff --git a/savevm.c b/savevm.c > index 90aa237..387b70b 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; > } Ping Again... :) This is a bug fix. 2 months has gone, but I do not receive any comment. Should we remove SIGCHLD handling as Paolo said?