On Thu, Nov 10, 2016 at 10:06:37AM +0100, Thomas Huth wrote: > When using the serial console in the GTK interface of QEMU (and > QEMU has been compiled with CONFIG_VTE), it is possible to trigger > the assert() statement in vty_receive() in spapr_vty.c by pasting > a chunk of text with length > 16 into the QEMU window. > Most of the other serial backends seem to simply drop characters > that they can not handle, so I think we should also do the same in > spapr-vty to fix this issue. And since it is quite ugly when pasted > text is chopped after 16 bytes, we also increase the size of the > input buffer here so that we can at least handle a couple of text > lines.
Adding Paolo to CC, as qemu-char maintainer. So, vastly increasing the buffer like this doesn't seem right - the plain 16550 serial driver doesn't maintain a buffer bigger than its small internal FIFO (32 characters IIRC) - or a single byte if the FIFO is disabled. I thought the other side of the char driver was supposed to call can_receive() first and not deliver more bytes than we can handle - hence the assert. That said, I think vty_can_receive() has a bug - looking at other drivers, I think it's supposed to return the amount of buffer space available, but we're just returning 0 or 1. Although AFAICT that should still work, just inefficiently. If you use a serial port with the same GTK interface, do you get the same problem with pastes of >16 characters being truncated? > > Buglink: https://bugs.launchpad.net/qemu/+bug/1639322 > Signed-off-by: Thomas Huth <th...@redhat.com> > --- > hw/char/spapr_vty.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c > index 31822fe..bee6c34 100644 > --- a/hw/char/spapr_vty.c > +++ b/hw/char/spapr_vty.c > @@ -1,4 +1,5 @@ > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "qapi/error.h" > #include "qemu-common.h" > #include "cpu.h" > @@ -7,7 +8,7 @@ > #include "hw/ppc/spapr.h" > #include "hw/ppc/spapr_vio.h" > > -#define VTERM_BUFSIZE 16 > +#define VTERM_BUFSIZE 2048 > > typedef struct VIOsPAPRVTYDevice { > VIOsPAPRDevice sdev; > @@ -37,7 +38,15 @@ static void vty_receive(void *opaque, const uint8_t *buf, > int size) > qemu_irq_pulse(spapr_vio_qirq(&dev->sdev)); > } > for (i = 0; i < size; i++) { > - assert((dev->in - dev->out) < VTERM_BUFSIZE); > + if (dev->in - dev->out >= VTERM_BUFSIZE) { > + static bool reported; > + if (!reported) { > + error_report("VTY input buffer exhausted - characters > dropped." > + " (input size = %i)", size); > + reported = true; > + } > + break; > + } > dev->buf[dev->in++ % VTERM_BUFSIZE] = buf[i]; > } > } -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
