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

[Daniel P. Berrange]

On Mon, Aug 13, 2007 at 05:59:15AM -0400, Daniel Veillard wrote:
> On Sun, Aug 12, 2007 at 07:11:34PM -0400, Jim Paris wrote:
> > Due to the TTY layer, sending "\n" to the qemu monitor translates
> > into "\r\n" when received.  This triggers a bug in older versions of
> > QEMU (KVM <= 33) because the same command is executed twice, and
> > still has problems with fixed QEMU because the "(qemu)" prompt is
> > printed twice.  Switch all monitor commands to end with "\r" which
> > avoids both issues.
> > 
> > The QEMU monitor sends frequent terminal escape sequences, typically
> > \033[D and \033[K.  At times, these interfere with the prompt
> > detection when they get sent between "\n" and "(qemu) ".  Fix the
> > issue by filtering out these sequences when they are received.
> 
>   I think DanP can better comment on the QEmu interaction than me,
> but the patch looks simple and clean except:

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

> 
> > @@ -1333,14 +1335,23 @@ static int qemudMonitorCommand(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> >                  return -1;
> >              }
> >              buf = b;
> > -            memmove(buf+size, data, got);
> > -            buf[size+got] = '\0';
> > -            size += got;
> > +
> > +            /* 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. But it's possible that the way reads are done, and input
> is chunked garantees that this won't happen, DanP can probably confirm 
> it's just fine :-)

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.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|