From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40492 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PqSLo-0002eH-Vr for qemu-devel@nongnu.org; Fri, 18 Feb 2011 10:34:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PqSLk-0000FP-4k for qemu-devel@nongnu.org; Fri, 18 Feb 2011 10:34:32 -0500 Received: from david.siemens.de ([192.35.17.14]:22657) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PqSLj-0000El-Q2 for qemu-devel@nongnu.org; Fri, 18 Feb 2011 10:34:28 -0500 Message-ID: <4D5E9170.3080609@siemens.com> Date: Fri, 18 Feb 2011 16:34:08 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1297959841-41235-1-git-send-email-gingold@adacore.com> <1298035036-51807-1-git-send-email-gingold@adacore.com> In-Reply-To: <1298035036-51807-1-git-send-email-gingold@adacore.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] Use sigwait instead of sigwaitinfo. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tristan Gingold Cc: Paolo Bonzini , qemu-devel On 2011-02-18 14:17, Tristan Gingold wrote: > Fix compilation failure on Darwin. > > Signed-off-by: Tristan Gingold > --- > compatfd.c | 36 ++++++++++++++++++------------------ > 1 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/compatfd.c b/compatfd.c > index a7cebc4..bd377c4 100644 > --- a/compatfd.c > +++ b/compatfd.c > @@ -26,45 +26,45 @@ struct sigfd_compat_info > static void *sigwait_compat(void *opaque) > { > struct sigfd_compat_info *info = opaque; > - int err; > sigset_t all; > > sigfillset(&all); > sigprocmask(SIG_BLOCK, &all, NULL); > > - do { > - siginfo_t siginfo; > + while (1) { > + int sig; > + int err; > > - err = sigwaitinfo(&info->mask, &siginfo); > - if (err == -1 && errno == EINTR) { > - err = 0; > - continue; > - } > - > - if (err > 0) { > - char buffer[128]; > + err = sigwait(&info->mask, &sig); > + if (err != 0) { > + if (errno == EINTR) { > + continue; > + } else { > + return NULL; > + } > + } else { > + struct qemu_signalfd_siginfo buffer; > size_t offset = 0; > > - memcpy(buffer, &err, sizeof(err)); > + memset(&buffer, 0, sizeof(buffer)); > + buffer.ssi_signo = sig; > + > while (offset < sizeof(buffer)) { > ssize_t len; > > - len = write(info->fd, buffer + offset, > + len = write(info->fd, (char *)&buffer + offset, > sizeof(buffer) - offset); > if (len == -1 && errno == EINTR) > continue; > > if (len <= 0) { > - err = -1; > - break; > + return NULL; This and the above handling of sigwait return codes changes the error handling strategy. So far we silently skipped errors, now we silently terminate the compatfd thread. I think none of both approaches is good. Failing sigwait is likely a reason to bail out, but loudly, writing some error message to the console and triggering a shutdown of qemu. An overflow of the compatfd pipe to the main thread may be due to some very unfortunate overload scenario. Not sure if that qualifies for a thread termination (definitely not for a silent one). Error handling should probably be adjusted independently of this darwin build fix. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux