Re: [libvirt] [PATCH 3/7] qemu: assume -vnc arg always takes a ':'

On 11/05/2015 12:33 PM, Daniel P. Berrange wrote:

As of QEMU 0.9.0 the -vnc option accepts a ':' to separate port from listen address, so the QEMU driver can assume that support for listen addresses is always available.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 4 - src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 141 ++++++++++----------- tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 - tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 - tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 - tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 - tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 - tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 - tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 - tests/qemucaps2xmldata/all_1.6.0-1.caps | 1 - tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 1 - tests/qemuhelptest.c | 8 -- tests/qemuxml2argvtest.c | 1 - tests/qemuxml2xmltest.c | 1 - tests/qemuxmlnstest.c | 1 - 16 files changed, 68 insertions(+), 99 deletions(-)

[...] couple of formatting nits:

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 26dc354..cd79544 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8210,97 +8210,90 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, goto error; }

- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { - switch (virDomainGraphicsListenGetType(graphics, 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) - goto error; + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break;

- listenAddr = netAddr; - /* store the address we found in the <graphics> element so it - * will show up in status. */ - if (virDomainGraphicsListenSetAddress(graphics, 0, - listenAddr, -1, false) < 0) - goto error; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; } + if (ret < 0) + goto error;

- if (!listenAddr) - listenAddr = cfg->vncListen; + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it + * will show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + goto error; + break; + }

- escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); - virBufferAsprintf(&opt, ":%d", - graphics->data.vnc.port - 5900); + if (!listenAddr) + listenAddr = cfg->vncListen;

- VIR_FREE(netAddr); - } else { - virBufferAsprintf(&opt, "%d", - graphics->data.vnc.port - 5900); - } + escapeAddr = strchr(listenAddr, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", listenAddr); + else + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + + VIR_FREE(netAddr); }

- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { - if (!graphics->data.vnc.socket && - graphics->data.vnc.websocket) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VNC WebSockets are not supported " - "with this QEMU binary")); - goto error; - } - virBufferAsprintf(&opt, ",websocket=%d", graphics->data.vnc.websocket); - } + if (!graphics->data.vnc.socket && + graphics->data.vnc.websocket) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VNC WebSockets are not supported " + "with this QEMU binary")); + goto error; + } + virBufferAsprintf(&opt, ",websocket=%d", graphics->data.vnc.websocket); + }

- if (graphics->data.vnc.sharePolicy) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc display sharing policy is not " - "supported with this QEMU")); + if (graphics->data.vnc.sharePolicy) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc display sharing policy is not " + "supported with this QEMU")); goto error;

This goto error needs to be moved left 4 spaces

- } + }

- virBufferAsprintf(&opt, ",share=%s", - virDomainGraphicsVNCSharePolicyTypeToString( + virBufferAsprintf(&opt, ",share=%s", + virDomainGraphicsVNCSharePolicyTypeToString( graphics->data.vnc.sharePolicy));

This line needs to move left 4 spaces

- } + }

- if (graphics->data.vnc.auth.passwd || cfg->vncPassword) - virBufferAddLit(&opt, ",password"); + if (graphics->data.vnc.auth.passwd || cfg->vncPassword) + virBufferAddLit(&opt, ",password");

- if (cfg->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (cfg->vncTLSx509verify) - virBufferAsprintf(&opt, ",x509verify=%s", cfg->vncTLSx509certdir); - else - virBufferAsprintf(&opt, ",x509=%s", cfg->vncTLSx509certdir); - } + if (cfg->vncTLS) { + virBufferAddLit(&opt, ",tls"); + if (cfg->vncTLSx509verify) + virBufferAsprintf(&opt, ",x509verify=%s", cfg->vncTLSx509certdir); + else + virBufferAsprintf(&opt, ",x509=%s", cfg->vncTLSx509certdir); + }

- if (cfg->vncSASL) { - virBufferAddLit(&opt, ",sasl"); + if (cfg->vncSASL) { + virBufferAddLit(&opt, ",sasl");

- if (cfg->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir); + if (cfg->vncSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir);

- /* TODO: Support ACLs later */ - } + /* TODO: Support ACLs later */ }

virCommandAddArg(cmd, "-vnc");

[...]