[libvirt] [PATCH] qemuBuildCommandLine: Don't add tlsPort if none set
If user hasn't supplied any tlsPort we default to setting it to zero in our internal structure. However, when building command line we test it against -1 which is obviously wrong. --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..ed82cc2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); - if (def->graphics[0]->data.spice.tlsPort != -1) { + if (def->graphics[0]->data.spice.tlsPort) { if (!driver->spiceTLS) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("spice TLS port set in XML configuration," -- 1.7.8.5
On 03/08/2012 06:30 AM, Michal Privoznik wrote:
If user hasn't supplied any tlsPort we default to setting it to zero in our internal structure. However, when building command line we test it against -1 which is obviously wrong. --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..ed82cc2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
- if (def->graphics[0]->data.spice.tlsPort != -1) { + if (def->graphics[0]->data.spice.tlsPort) {
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08.03.2012 23:26, Eric Blake wrote:
On 03/08/2012 06:30 AM, Michal Privoznik wrote:
If user hasn't supplied any tlsPort we default to setting it to zero in our internal structure. However, when building command line we test it against -1 which is obviously wrong. --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..ed82cc2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
- if (def->graphics[0]->data.spice.tlsPort != -1) { + if (def->graphics[0]->data.spice.tlsPort) {
ACK.
Thanks, pushed. However, turned out I've needed to update one test as well: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args index 62ce46f..ebda714 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args @@ -10,6 +10,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ -device rtl8139,vlan=0,id=net0,mac=52:54:00:71:70:89,bus=pci.0,addr=0x7 \ -net tap,script=/etc/qemu-ifup,vlan=0,name=hostnet0 -serial pty \ -usb -device usb-tablet,id=input0 \ --spice port=5900,tls-port=0,x509-dir=/etc/pki/libvirt-spice -vga std \ +-spice port=5900,x509-dir=/etc/pki/libvirt-spice -vga std \ -device AC97,id=sound0,bus=pci.0,addr=0x3 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 That is, don't add wildcard tls-port zero to generated commandline. Sorry for this incompleteness.
On Thu, Mar 08, 2012 at 14:30:05 +0100, Michal Privoznik wrote:
If user hasn't supplied any tlsPort we default to setting it to zero in our internal structure. However, when building command line we test it against -1 which is obviously wrong. --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..ed82cc2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
- if (def->graphics[0]->data.spice.tlsPort != -1) { + if (def->graphics[0]->data.spice.tlsPort) { if (!driver->spiceTLS) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("spice TLS port set in XML configuration,"
Oops, looks like this is a doomed feature. With this change in, a domain configured with <graphics type='spice' autoport='yes'/> fails to start with error: unsupported configuration: spice TLS port set in XML configuration, but TLS is disabled in qemu.conf Apparently tlsPort may be both -1 and 0 depending on autoport value. Jirka
On Mon, Mar 12, 2012 at 02:21:48PM +0100, Jiri Denemark wrote:
On Thu, Mar 08, 2012 at 14:30:05 +0100, Michal Privoznik wrote:
If user hasn't supplied any tlsPort we default to setting it to zero in our internal structure. However, when building command line we test it against -1 which is obviously wrong. --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..ed82cc2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5374,7 +5374,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port);
- if (def->graphics[0]->data.spice.tlsPort != -1) { + if (def->graphics[0]->data.spice.tlsPort) { if (!driver->spiceTLS) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("spice TLS port set in XML configuration,"
Oops, looks like this is a doomed feature. With this change in, a domain configured with
<graphics type='spice' autoport='yes'/>
fails to start with error: unsupported configuration: spice TLS port set in XML configuration, but TLS is disabled in qemu.conf
Apparently tlsPort may be both -1 and 0 depending on autoport value.
Hmm, that smells like a bug - something somewhere is not correctly initializing tlsPort to -1. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Michal Privoznik