From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37119) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhIEE-0004iu-0g for qemu-devel@nongnu.org; Tue, 06 Sep 2016 11:24:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bhIEA-00036I-Od for qemu-devel@nongnu.org; Tue, 06 Sep 2016 11:24:02 -0400 Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]:36540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bhIEA-00036C-DD for qemu-devel@nongnu.org; Tue, 06 Sep 2016 11:23:58 -0400 Received: by mail-wm0-x241.google.com with SMTP id l65so11297397wmf.3 for ; Tue, 06 Sep 2016 08:23:58 -0700 (PDT) Sender: Paolo Bonzini References: <1473170165-540-1-git-send-email-berrange@redhat.com> <1473170165-540-5-git-send-email-berrange@redhat.com> From: Paolo Bonzini Message-ID: <8adbf9ea-1e51-6233-9796-73cd712bef15@redhat.com> Date: Tue, 6 Sep 2016 17:23:55 +0200 MIME-Version: 1.0 In-Reply-To: <1473170165-540-5-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 4/4] char: convert qemu_chr_fe_write to qemu_chr_fe_write_all List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org On 06/09/2016 15:56, Daniel P. Berrange wrote: > The mux chardev was not checking the return value of any > qemu_chr_fe_write() call so would silently loose data > on EAGAIN. > > Similarly the qemu_chr_fe_printf method would not check > errors and was not in a position to retry even if it > could check. > > Signed-off-by: Daniel P. Berrange > --- > qemu-char.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index 5f82ebb..6104f24 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -440,7 +440,9 @@ void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...) > va_list ap; > va_start(ap, fmt); > vsnprintf(buf, sizeof(buf), fmt, ap); > - qemu_chr_fe_write(s, (uint8_t *)buf, strlen(buf)); > + /* XXX this blocks entire thread. Rewrite to use > + * qemu_chr_fe_write and background I/O callbacks */ > + qemu_chr_fe_write_all(s, (uint8_t *)buf, strlen(buf)); > va_end(ap); > } > > @@ -556,7 +558,9 @@ static int mux_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > (secs / 60) % 60, > secs % 60, > (int)(ti % 1000)); > - qemu_chr_fe_write(d->drv, (uint8_t *)buf1, strlen(buf1)); > + /* XXX this blocks entire thread. Rewrite to use > + * qemu_chr_fe_write and background I/O callbacks */ > + qemu_chr_fe_write_all(d->drv, (uint8_t *)buf1, strlen(buf1)); > d->linestart = 0; > } > ret += qemu_chr_fe_write(d->drv, buf+i, 1); > @@ -594,13 +598,15 @@ static void mux_print_help(CharDriverState *chr) > "\n\rEscape-Char set to Ascii: 0x%02x\n\r\n\r", > term_escape_char); > } > - qemu_chr_fe_write(chr, (uint8_t *)cbuf, strlen(cbuf)); > + /* XXX this blocks entire thread. Rewrite to use > + * qemu_chr_fe_write and background I/O callbacks */ > + qemu_chr_fe_write_all(chr, (uint8_t *)cbuf, strlen(cbuf)); > for (i = 0; mux_help[i] != NULL; i++) { > for (j=0; mux_help[i][j] != '\0'; j++) { > if (mux_help[i][j] == '%') > - qemu_chr_fe_write(chr, (uint8_t *)ebuf, strlen(ebuf)); > + qemu_chr_fe_write_all(chr, (uint8_t *)ebuf, strlen(ebuf)); > else > - qemu_chr_fe_write(chr, (uint8_t *)&mux_help[i][j], 1); > + qemu_chr_fe_write_all(chr, (uint8_t *)&mux_help[i][j], 1); > } > } > } > @@ -625,7 +631,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch) > case 'x': > { > const char *term = "QEMU: Terminated\n\r"; > - qemu_chr_fe_write(chr, (uint8_t *)term, strlen(term)); > + qemu_chr_fe_write_all(chr, (uint8_t *)term, strlen(term)); > exit(0); > break; > } > Queued for 2.8, thanks. Paolo