10:22 a.m.
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 <phrdina(a)redhat.com>
---
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.