[Libvir] PATCH: Make QEMU driver honour 'listen' flag for VNC

In QEMU 0.9.0 or later it is possible to tell QEMU to only listen on a particular IP address. THis patch adapts the code so that it honours the 'listen' attribute on the <graphics> tag if using QEMU >= 0.9.0. It also re-enables the tests for this capability that I temporarily disabled. src/qemu_conf.c | 37 +++++++++++++----- src/qemu_conf.h | 1 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args | 2 tests/qemuxml2argvtest.c | 4 - tests/qemuxml2xmltest.c | 2 5 files changed, 32 insertions(+), 14 deletions(-) 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 -=|

On Mon, Jul 23, 2007 at 07:06:25PM +0100, Daniel P. Berrange wrote:
In QEMU 0.9.0 or later it is possible to tell QEMU to only listen on a particular IP address. THis patch adapts the code so that it honours the 'listen' attribute on the <graphics> tag if using QEMU >= 0.9.0. It also re-enables the tests for this capability that I temporarily disabled.
There was a further complication of this. It turns out our handling of headless VMs was broken -ie, VMs without any <graphics> tag all. We specify -nographic on the command line. Unfortunately when you do this, QEMU splatters any settings for monitor, serial or parallel device you gave on the command line before the -nographic flag. Unfortunately again we order -monitor pty before -nographic, so we fail to get a monitor console setup correctly. This updated patch extends the previous so that we have -nographic before all other args it might affect. src/qemu_conf.c | 52 +++++++++++---- src/qemu_conf.h | 1 tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args | 2 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.args | 2 tests/qemuxml2argvdata/qemuxml2argv-boot-network.args | 2 tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.args | 2 tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args | 2 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.args | 2 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.args | 2 tests/qemuxml2argvdata/qemuxml2argv-disk-many.args | 2 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args | 2 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args | 2 tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args | 2 tests/qemuxml2argvdata/qemuxml2argv-minimal.args | 2 tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.args | 2 tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.args | 2 tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 2 tests/qemuxml2argvtest.c | 4 - tests/qemuxml2xmltest.c | 2 19 files changed, 59 insertions(+), 30 deletions(-) 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 -=|

On Mon, Jul 23, 2007 at 08:51:36PM +0100, Daniel P. Berrange wrote:
On Mon, Jul 23, 2007 at 07:06:25PM +0100, Daniel P. Berrange wrote:
In QEMU 0.9.0 or later it is possible to tell QEMU to only listen on a particular IP address. THis patch adapts the code so that it honours the 'listen' attribute on the <graphics> tag if using QEMU >= 0.9.0. It also re-enables the tests for this capability that I temporarily disabled.
There was a further complication of this. It turns out our handling of headless VMs was broken -ie, VMs without any <graphics> tag all. We specify -nographic on the command line. Unfortunately when you do this, QEMU splatters any settings for monitor, serial or parallel device you gave on the command line before the -nographic flag. Unfortunately again we order -monitor pty before -nographic, so we fail to get a monitor console setup correctly. This updated patch extends the previous so that we have -nographic before all other args it might affect.
Hum, a program whose behaviour depends on the order of the flags, is taht really intended upstream ? [...]
+ /* + * NB, -nographic *MUST* come before any serial, or monitor + * or parallel port flags due to QEMU craziness, where it + * decides to change the serial port & monitor to be on stdout + * if you ask for nographic. So we have to make sure we override + * these stupid defaults ourselves + */
let's remove the s word, it's sure annoying but no need to bury the feeling in the code :-) +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Tue, Jul 24, 2007 at 03:18:23AM -0400, Daniel Veillard wrote:
On Mon, Jul 23, 2007 at 08:51:36PM +0100, Daniel P. Berrange wrote:
On Mon, Jul 23, 2007 at 07:06:25PM +0100, Daniel P. Berrange wrote:
In QEMU 0.9.0 or later it is possible to tell QEMU to only listen on a particular IP address. THis patch adapts the code so that it honours the 'listen' attribute on the <graphics> tag if using QEMU >= 0.9.0. It also re-enables the tests for this capability that I temporarily disabled.
There was a further complication of this. It turns out our handling of headless VMs was broken -ie, VMs without any <graphics> tag all. We specify -nographic on the command line. Unfortunately when you do this, QEMU splatters any settings for monitor, serial or parallel device you gave on the command line before the -nographic flag. Unfortunately again we order -monitor pty before -nographic, so we fail to get a monitor console setup correctly. This updated patch extends the previous so that we have -nographic before all other args it might affect.
Hum, a program whose behaviour depends on the order of the flags, is taht really intended upstream ?
Yeah, the upstream behaviour makes sense in the context in which they've done it & has been like that in QEMU for ages - I just didn't realize the implications of it wrt to way libvirt spawns QEMU. 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 -=|

Daniel P. Berrange wrote:
On Mon, Jul 23, 2007 at 07:06:25PM +0100, Daniel P. Berrange wrote:
In QEMU 0.9.0 or later it is possible to tell QEMU to only listen on a particular IP address. THis patch adapts the code so that it honours the 'listen' attribute on the <graphics> tag if using QEMU >= 0.9.0. It also re-enables the tests for this capability that I temporarily disabled.
There was a further complication of this. It turns out our handling of headless VMs was broken -ie, VMs without any <graphics> tag all. We specify -nographic on the command line. Unfortunately when you do this, QEMU splatters any settings for monitor, serial or parallel device you gave on the command line before the -nographic flag. Unfortunately again we order -monitor pty before -nographic, so we fail to get a monitor console setup correctly. This updated patch extends the previous so that we have -nographic before all other args it might affect.
src/qemu_conf.c | 52 +++++++++++---- src/qemu_conf.h | 1 tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.args | 2 tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.args | 2 tests/qemuxml2argvdata/qemuxml2argv-boot-network.args | 2 tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.args | 2 tests/qemuxml2argvdata/qemuxml2argv-clock-utc.args | 2 tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.args | 2 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.args | 2 tests/qemuxml2argvdata/qemuxml2argv-disk-many.args | 2 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.args | 2 tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.args | 2 tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.args | 2 tests/qemuxml2argvdata/qemuxml2argv-minimal.args | 2 tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.args | 2 tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.args | 2 tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 2 tests/qemuxml2argvtest.c | 4 - tests/qemuxml2xmltest.c | 2 19 files changed, 59 insertions(+), 30 deletions(-)
As far as I understand this patch, it looks good. I'm curious as to why we're using the BR_INET_ADDR_MAXLEN symbol (from our networking code) instead of just INET_ADDRSTRLEN from <netinet/in.h> though. +1. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

On Mon, Jul 23, 2007 at 07:06:25PM +0100, Daniel P. Berrange wrote:
In QEMU 0.9.0 or later it is possible to tell QEMU to only listen on a particular IP address. THis patch adapts the code so that it honours the 'listen' attribute on the <graphics> tag if using QEMU >= 0.9.0. It also re-enables the tests for this capability that I temporarily disabled.
Looks fine to me, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Richard W.M. Jones