[libvirt] [PATCH] Fix serial console telnet protocol support

With e.g.: <serial type='tcp'> <source mode='bind' host='127.0.0.1' service='4444'/> <protocol type='telnet'/> <target port='0'/> </serial> You currently get: Unknown option: listen qemu: could not open serial device 'telnet:127.0.0.1:4444,listen' With the telnet protocol, qemu expects "server" as an option rather than "listen". Signed-off-by: Mark McLoughlin <markmc@redhat.com> Index: libvirt/src/qemu_conf.c =================================================================== --- libvirt.orig/src/qemu_conf.c 2008-08-13 18:14:34.000000000 +0100 +++ libvirt/src/qemu_conf.c 2008-08-13 18:35:20.000000000 +0100 @@ -685,12 +685,19 @@ break; case VIR_DOMAIN_CHR_TYPE_TCP: - if (snprintf(buf, buflen, "%s:%s:%s%s", - dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET ? "telnet" : "tcp", - dev->data.tcp.host, - dev->data.tcp.service, - dev->data.tcp.listen ? ",listen" : "") >= buflen) - return -1; + if (dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET) { + if (snprintf(buf, buflen, "telnet:%s:%s%s", + dev->data.tcp.host, + dev->data.tcp.service, + dev->data.tcp.listen ? ",server" : "") >= buflen) + return -1; + } else { + if (snprintf(buf, buflen, "tcp:%s:%s%s", + dev->data.tcp.host, + dev->data.tcp.service, + dev->data.tcp.listen ? ",listen" : "") >= buflen) + return -1; + } break; case VIR_DOMAIN_CHR_TYPE_UNIX:

Hi Mark, On Wed, Aug 13, 2008 at 06:44:28PM +0100, Mark McLoughlin wrote:
With e.g.:
<serial type='tcp'> <source mode='bind' host='127.0.0.1' service='4444'/> <protocol type='telnet'/> <target port='0'/> </serial>
You currently get:
Unknown option: listen qemu: could not open serial device 'telnet:127.0.0.1:4444,listen'
With the telnet protocol, qemu expects "server" as an option rather than "listen". Not that I'm qualified to comment on this but a testcase in tests/qemuxml2argv* would prevent breaking this in the future. Cheers, -- Guido

On Thu, Aug 14, 2008 at 10:21:25AM +0200, Guido G?nther wrote:
Hi Mark, On Wed, Aug 13, 2008 at 06:44:28PM +0100, Mark McLoughlin wrote:
With e.g.:
<serial type='tcp'> <source mode='bind' host='127.0.0.1' service='4444'/> <protocol type='telnet'/> <target port='0'/> </serial>
You currently get:
Unknown option: listen qemu: could not open serial device 'telnet:127.0.0.1:4444,listen'
With the telnet protocol, qemu expects "server" as an option rather than "listen".
Not that I'm qualified to comment on this but a testcase in tests/qemuxml2argv* would prevent breaking this in the future.
The test case is only as good as the person writing the test. We have an existing test case for Telnet support which is happily validating that we are generating the incorrect syntax. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Wed, Aug 13, 2008 at 06:44:28PM +0100, Mark McLoughlin wrote:
With e.g.:
<serial type='tcp'> <source mode='bind' host='127.0.0.1' service='4444'/> <protocol type='telnet'/> <target port='0'/> </serial>
You currently get:
Unknown option: listen qemu: could not open serial device 'telnet:127.0.0.1:4444,listen'
With the telnet protocol, qemu expects "server" as an option rather than "listen".
ACK, but can you also fix the test case for this to match before committing. We're currently validating that it is 'listen' :-) Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Fri, 2008-08-15 at 10:39 +0100, Daniel P. Berrange wrote:
On Wed, Aug 13, 2008 at 06:44:28PM +0100, Mark McLoughlin wrote:
With the telnet protocol, qemu expects "server" as an option rather than "listen".
ACK, but can you also fix the test case for this to match before committing. We're currently validating that it is 'listen' :-)
Thanks, fixed and committed. Cheers, Mark.
participants (3)
-
Daniel P. Berrange
-
Guido Günther
-
Mark McLoughlin