[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libvir] Re: [PATCH 2/7] Fix issues with QEMU monitor interface.



Daniel Veillard wrote:
> > +            /* Copy data, skipping 3-byte escape sequences */
> > +            for (i = 0; i < got; i++) {
> > +                if (data[i] == '\033')
> > +                    skip = 3;
> > +                if (skip)
> > +                    skip--;
> > +                else
> > +                    buf[size++] = data[i];
> > +            }
> > +            buf[size] = '\0';
> >          }
> 
>   It seems that if for some reason you do a partial read on the QEmu
> console descriptor ending in the middle of the escape command you may
> have a problem.

It should be OK.  Partial reads are why I'm setting using the "skip"
variable which is persistent across read() calls.  Any time we see
'\033' we'll always skip three bytes from qemu.  Note that partial
reads across qemuMonitorCommand calls doesn't really matter, because
we really just care about finding the next prompt anyway, and there
shouldn't be any data received between the prompt and the execution of
the next command.


Daniel P. Berrange wrote:
> We're reading from a Psuedo-TTY which is line buffered, so I think the OS 
> should guarentee that we can read a whole lines worth of data without 
> getting EAGAIN.

QEMU disables line buffering when it initializes the pty.


> It looks sane to me - I had no idea QEMU was sending this escape
> sequences.

It comes from qemu's readline.c:term_update and is a bit of a pain.

... Actually, on closer inspection, I think this patch might be
misguided.  The only case where you should get escape sequences after
the "\n" but before the "(qemu)" is when you are sending CRLF to a
version of KVM that's supposed to be better at handling it -- it turns
out my kvm patch was incomplete and didn't reset all of the input
state.

When monitor commands are terminated with "\r" rather than "\n",
this should never occur.  And so filtering escape sequences should be
unnecessary, as they should only show up on the echoed command line.

There were also some bugs in my libvirt patches (a merge error left
two commands terminated with "\n", and I left some debug output).
I'll fix things up and send an updated series.

-jim


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]