Ján Tomko <jtomko@xxxxxxxxxx> writes: > On 06/23/2014 08:15 PM, Giuseppe Scrivano wrote: >> When looking for a port to allocate, the port allocator didn't take in >> consideration ports that are statically set by the user. Defining >> these two graphics elements in the XML would cause an error, as the >> port allocator would try to use the same port for the spice graphics >> element: >> >> <graphics type='spice' autoport='yes'/> >> <graphics type='vnc' port='5900' autoport='no'/> >> >> The new *[pP]ortAllocated variables keep track of the ports that were >> successfully registered (either bound or simply tracked as used) by >> the port allocator to allow a clean rollback on errors. >> >> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1081881 >> >> Signed-off-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx> >> --- >> src/conf/domain_conf.h | 3 ++ >> src/qemu/qemu_process.c | 79 +++++++++++++++++++++++++++++++++++++++++++------ >> 2 files changed, 73 insertions(+), 9 deletions(-) >> > >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index 0b8155b..06f1e54 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -3487,6 +3487,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, >> if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) >> return -1; >> graphics->data.vnc.port = port; >> + graphics->data.vnc.portAllocated = true; >> } >> >> if (graphics->data.vnc.websocket == -1) { >> @@ -3548,6 +3549,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, >> goto error; >> >> graphics->data.spice.port = port; >> + graphics->data.spice.portAllocated = true; >> } >> >> if (needTLSPort || graphics->data.spice.tlsPort == -1) { >> @@ -3573,6 +3575,7 @@ qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver, >> goto error; >> >> graphics->data.spice.tlsPort = tlsPort; >> + graphics->data.spice.tlsPortAllocated = true; >> } >> } >> >> @@ -3803,6 +3806,36 @@ int qemuProcessStart(virConnectPtr conn, >> >> for (i = 0; i < vm->def->ngraphics; ++i) { >> virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; >> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && >> + !graphics->data.vnc.autoport) { >> + if (virPortAllocatorSetUsed(driver->remotePorts, >> + graphics->data.vnc.port, >> + true) < 0) { > > virPortAllocatorSetUsed doesn't error out if the static port is already marked > as used. If it's already used by another domain, this one fails to start and > releases the port in qemuProcessStop. > > If you change it to fail on occupied ports, it will also catch duplicit static > ports, but only in the port allocator range. I expect someone will file a bug > about conflicting ports out of that range as well :) sorry, it had to be failing on occupied ports, I'll change it in v2. > >> + goto cleanup; >> + } >> + >> + graphics->data.vnc.portAllocated = true; >> + >> + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && >> + !graphics->data.spice.autoport) { > > If either of the ports is -1, you shouldn't be reserving them. (For VNC, we > set autoport to true if port is -1 when parsing the XML. For SPICE, this only > happens if both ports are -1). I will modify it to reserve a port only if !autoport && port != -1 and fix the release part too. Thanks, Giuseppe -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list