On 05/19/2016 07:35 AM, Pavel Hrdina wrote:
> This prepares the code for other listen types.
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 57 ++++++++++++++++++++++---------------------------
> 1 file changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ee43e21..e401ac5 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7500,10 +7500,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> {
> virBuffer opt = VIR_BUFFER_INITIALIZER;
> virDomainGraphicsListenDefPtr glisten = NULL;
> - int defaultMode = graphics->data.spice.defaultMode;
> int port = graphics->data.spice.port;
> int tlsPort = graphics->data.spice.tlsPort;
> size_t i;
> + bool hasSecure = false;
> + bool hasInsecure = false;
>
> if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -7513,8 +7514,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>
> glisten = virDomainGraphicsGetListen(graphics, 0);
>
> - if (port > 0)
> + if (port > 0) {
> virBufferAsprintf(&opt, "port=%u,", port);
> + hasInsecure = true;
> + }
>
> if (tlsPort > 0) {
> if (!cfg->spiceTLS) {
> @@ -7524,6 +7527,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> goto error;
> }
> virBufferAsprintf(&opt, "tls-port=%u,", tlsPort);
> + hasSecure = true;
> }
>
> if (port > 0 || tlsPort > 0) {
> @@ -7561,17 +7565,30 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> !cfg->spicePassword)
> virBufferAddLit(&opt, "disable-ticketing,");
>
> - if (tlsPort > 0)
> + if (hasSecure)
> virBufferAsprintf(&opt, "x509-dir=%s,", cfg->spiceTLSx509certdir);
>
> - switch (defaultMode) {
> + switch (graphics->data.spice.defaultMode) {
> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> + if (!hasSecure) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("spice defaultMode secure requested in XML "
> + "configuration, but TLS is not available"));
> + goto error;
> + }
> virBufferAddLit(&opt, "tls-channel=default,");
> break;
> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE:
> + if (!hasInsecure) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("spice defaultMode insecure requested in XML "
> + "configuration, but plaintext is not available"));
> + goto error;
> + }
> virBufferAddLit(&opt, "plaintext-channel=default,");
> break;
> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY:
> + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST:
> /* nothing */
> break;
> }
> @@ -7579,10 +7596,10 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> for (i = 0; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; i++) {
> switch (graphics->data.spice.channels[i]) {
> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> - if (tlsPort <= 0) {
> + if (!hasSecure) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("spice secure channels set in XML configuration, "
> - "but TLS port is not provided"));
> + _("spice secure channels set in XML "
> + "configuration, but TLS is not available"));
> goto error;
> }
I kinda prefer the original messages mentioning the lack of port as the
culprit. So maybe plaintext port and TLS port? If I saw the 'plaintext' error
as is I know I'd be confused and go digging into the code to figure out what
was triggering it.
ACK otherwise. IMO these first 8 patches are worth pushing and we work out any
additional stuff like the type='network' weirdness, and additional test cases,
on top of these cleanups
- Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list