From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=48046 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PqT1F-0008Nx-Qe
	for qemu-devel@nongnu.org; Fri, 18 Feb 2011 11:17:22 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1PqT1E-0006PZ-Hc
	for qemu-devel@nongnu.org; Fri, 18 Feb 2011 11:17:21 -0500
Received: from thoth.sbs.de ([192.35.17.2]:17390)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jan.kiszka@siemens.com>) id 1PqT1E-0006Oz-5r
	for qemu-devel@nongnu.org; Fri, 18 Feb 2011 11:17:20 -0500
Message-ID: <4D5E9B78.3030109@siemens.com>
Date: Fri, 18 Feb 2011 17:16:56 +0100
From: Jan Kiszka <jan.kiszka@siemens.com>
MIME-Version: 1.0
References: <1297959841-41235-1-git-send-email-gingold@adacore.com>
	<1298035036-51807-1-git-send-email-gingold@adacore.com>
	<4D5E9170.3080609@siemens.com>
	<ACCD9893-5AF4-4BF2-A501-448C54D7552C@adacore.com>
In-Reply-To: <ACCD9893-5AF4-4BF2-A501-448C54D7552C@adacore.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH] Use sigwait instead of sigwaitinfo.
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Tristan Gingold <gingold@adacore.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel <qemu-devel@nongnu.org>

On 2011-02-18 16:50, Tristan Gingold wrote:
> 
> On Feb 18, 2011, at 4:34 PM, Jan Kiszka wrote:
>>
>> This and the above handling of sigwait return codes changes the error
>> handling strategy.
> 
> Did it ?  I don't think so.
> 
>> So far we silently skipped errors, now we silently
>> terminate the compatfd thread. I think none of both approaches is good.
> 
> I think that both silently terminate the compatfd.  The previous code is:
> 
>   do {
>         siginfo_t siginfo;
> 
>         err = sigwaitinfo(&info->mask, &siginfo);
>         if (err == -1 && errno == EINTR) {
>             err = 0;
>             continue;
>         }
> 
>         if (err > 0) {
>             char buffer[128];
>             size_t offset = 0;
> 
>             memcpy(buffer, &err, sizeof(err));
>             while (offset < sizeof(buffer)) {
>                 ssize_t len;
> 
>                 len = write(info->fd, buffer + offset,
>                             sizeof(buffer) - offset);
>                 if (len == -1 && errno == EINTR)
>                     continue;
> 
>                 if (len <= 0) {
>                     err = -1;
>                     break;
>                 }
> 
>                 offset += len;
>             }
>         }
>     } while (err >= 0);
> 
> So in case of any error, err is set to a negative value which exits the thread.

Ah, sorry, oversaw that. In that case the patch is fine as it does not
change the existing behavior.

> 
>> Failing sigwait is likely a reason to bail out, but loudly, writing some
>> error message to the console and triggering a shutdown of qemu.
> 
> I agree with that.
> 
>> 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).
> 
> What do you mean by overflow ?  Unless I am wrong, the fd is not non-blocking, write will block.

True. So any error returned here is a real one.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux