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

2007-08-13 Thread Daniel Veillard
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:

 @@ -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 :-)

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2007-08-13 Thread 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  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2007-08-13 Thread Jim Paris
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

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list