On Fri, Feb 27, 2015 at 03:35:26PM +0100, Pavel Hrdina wrote:
We have two different places that needs to be updated while touching code for allocation spice ports. Add a bool option to 'qemuProcessSPICEAllocatePorts' function to switch between true and fake allocation so we can use this function also in qemu_driver to generate native domain definition.Signed-off-by: Pavel Hrdina <[email protected]> --- src/qemu/qemu_driver.c | 45 ++------------------------------------------- src/qemu/qemu_process.c | 42 ++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 5 +++++ 3 files changed, 33 insertions(+), 59 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e282464..46439cb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6579,49 +6579,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, !graphics->data.vnc.socket && graphics->data.vnc.autoport) { graphics->data.vnc.port = 5900; } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - size_t j; - bool needTLSPort = false; - bool needPort = false; - int defaultMode = graphics->data.spice.defaultMode; - - if (graphics->data.spice.autoport) { - /* check if tlsPort or port need allocation */ - for (j = 0; j < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST; j++) { - switch (graphics->data.spice.channels[j]) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - needTLSPort = true; - break; - - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - needPort = true; - break; - - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - needTLSPort = true; - break; - - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - needPort = true; - break; - - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - if (cfg->spiceTLS) - needTLSPort = true; - needPort = true; - break; - } - break; - } - } - } - - if (needPort || graphics->data.spice.port == -1) - graphics->data.spice.port = 5901; - - if (needTLSPort || graphics->data.spice.tlsPort == -1) - graphics->data.spice.tlsPort = 5902; + ignore_value(qemuProcessSPICEAllocatePorts(driver, cfg, + graphics, false));
Don't ignore any errors (even though currently there won't be any), just jump to clean-up for return value < 0.
}
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 515402e..aeb479c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4019,10 +4019,11 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
return 0;
}
-static int
+int
qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg,
- virDomainGraphicsDefPtr graphics)
+ virDomainGraphicsDefPtr graphics,
+ bool allocate)
{
unsigned short port = 0;
unsigned short tlsPort;
@@ -4066,30 +4067,39 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
}
if (needPort || graphics->data.spice.port == -1) {
- if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
- goto error;
+ if (allocate) {
+ if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
+ goto error;
- graphics->data.spice.port = port;
+ graphics->data.spice.port = port;
+ } else {
+ graphics->data.spice.port = 5901;
+ }
}
if (needTLSPort || graphics->data.spice.tlsPort == -1) {
- if (!cfg->spiceTLS) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Auto allocation of spice TLS port requested "
- "but spice TLS is disabled in qemu.conf"));
- goto error;
- }
+ if (allocate) {
I'd rather use:
if (!allocate) {
if (needPort || graphics->data.spice.port == -1)
graphics->data.spice.port = 5901;
if (needTlsPort || graphics->data.spice.tlsPort == -1)
graphics->data.spice.tlsPort = 5902;
return 0;
}
/* and here deal with the allocations */
but what you've done is fine, too.
ACK with or without that changed.
pgp2FA80kDhjQ.pgp
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
