| Message ID | 1473170165-540-5-git-send-email-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <berrange@redhat.com> > --- > 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
On 09/06/2016 08:56 AM, 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 s/loose/lose/ > 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 <berrange@redhat.com> > --- > 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; }
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 <berrange@redhat.com> --- qemu-char.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)