From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42846) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULZtp-00086m-Nz for qemu-devel@nongnu.org; Fri, 29 Mar 2013 10:03:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ULZtj-0000WT-PY for qemu-devel@nongnu.org; Fri, 29 Mar 2013 10:03:21 -0400 Received: from mail-gg0-x231.google.com ([2607:f8b0:4002:c02::231]:34091) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ULZtj-0000W7-LM for qemu-devel@nongnu.org; Fri, 29 Mar 2013 10:03:15 -0400 Received: by mail-gg0-f177.google.com with SMTP id q1so26756gge.22 for ; Fri, 29 Mar 2013 07:03:15 -0700 (PDT) From: Anthony Liguori In-Reply-To: <20130329124243.GF7778@amit.redhat.com> References: <9b59ac17b9d0bb3972a73fed04d415f07b391936.1362505276.git.amit.shah@redhat.com> <20130329095311.GB14019@amit.redhat.com> <87fvzefl7c.fsf@codemonkey.ws> <20130329124243.GF7778@amit.redhat.com> Date: Fri, 29 Mar 2013 09:03:10 -0500 Message-ID: <87mwtm482p.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: qemu list Amit Shah writes: > On (Fri) 29 Mar 2013 [07:24:07], Anthony Liguori wrote: >> Amit Shah writes: >> >> > On (Tue) 05 Mar 2013 [23:21:18], Amit Shah wrote: >> >> From: Anthony Liguori >> >> >> >> This is a special GSource that supports CharDriverState style >> >> poll callbacks. >> >> >> >> For reviewability and bisectability, this code is #if 0'd out in this >> >> patch to avoid unused warnings since all of the functions are static. >> >> >> >> Signed-off-by: Anthony Liguori >> >> Signed-off-by: Amit Shah >> > >> > >> >> +static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1) >> >> +{ >> >> + GIOStatus status; >> >> + gsize bytes_written; >> >> + int len; >> >> + const uint8_t *buf = _buf; >> >> + >> >> + len = len1; >> >> + while (len > 0) { >> >> + status = g_io_channel_write_chars(fd, (const gchar *)buf, len, >> >> + &bytes_written, NULL); >> >> + if (status != G_IO_STATUS_NORMAL) { >> >> + if (status != G_IO_STATUS_AGAIN) { >> >> + return -1; >> >> + } >> > >> > It's not quite right to return -1 here; previous iterations of the >> > while loop could have successfully written data, and (len1 - len) >> > could be +ve. >> >> Once -1 is returned, it's a terminal error. It doesn't matter that we >> may have written some data. > > Why do you say that? Because you're quoting the wrong patch :-) This bit is rewritten by a later patch which is the source of your problem below. In the patch you quote, we busy spin until all data is written. However, with: commit 23673ca740e0eda66901ca801a5a901df378b063 Author: Anthony Liguori Date: Tue Mar 5 23:21:23 2013 +0530 qemu-char: add watch support We started to return EAGAIN even if we have a partially successful write. I'm running a patch through testing right now that rewrites this function to have sane semantics (return bytes written on partial write). I'll post as soon as testing completes. Regards, Anthony Liguori