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&gt; --- 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)); } } 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) { + 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 (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) - goto error; + if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) + goto error; - graphics->data.spice.tlsPort = tlsPort; + graphics->data.spice.tlsPort = tlsPort; + } else { + graphics->data.spice.tlsPort = 5902; + } } return 0; error: - virPortAllocatorRelease(driver->remotePorts, port); + if (allocate) + virPortAllocatorRelease(driver->remotePorts, port); return -1; } @@ -4573,7 +4583,7 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessVNCAllocatePorts(driver, graphics) < 0) goto cleanup; } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics) < 0) + if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics, true) < 0) goto cleanup; } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 2e1d393..5c70803 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -107,4 +107,9 @@ int qemuProcessReadLog(int fd, char *buf, int buflen, int off, bool skipchar); int qemuProcessSetSchedParams(int id, pid_t pid, size_t nsp, virDomainThreadSchedParamPtr sp); +int qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, + virDomainGraphicsDefPtr graphics, + bool allocate); + #endif /* __QEMU_PROCESS_H__ */ -- 2.0.5

Instead of checking defaultMode for every channel that has no mode configured, test it only once outside of channel loop. This fixes a bug that in case all possible channels are fore example set to insecure, but defaultMode is set to secure, we wouldn't auto-generate TLS port. This results in failure while starting a guest. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1143832 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/qemu/qemu_process.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aeb479c..bd63fd7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4046,24 +4046,25 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, 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; - } + /* default mode will be used */ break; } } + 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; + } } if (needPort || graphics->data.spice.port == -1) { -- 2.0.5