Re: [libvirt] [PATCH 2/3] qemu: mark user defined websocket as used

On 08.12.2016 23:07, John Ferlan wrote: > > > On 11/22/2016 06:09 AM, Nikolay Shirokovskiy wrote: >> We need extra state variable to distinguish between autogenerated >> and user defined cases after auto generation is done. >> --- >> src/conf/domain_conf.h | 1 + >> src/qemu/qemu_process.c | 19 +++++++++++++++++-- >> 2 files changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h >> index 541b600..9bc4522 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -1468,6 +1468,7 @@ struct _virDomainGraphicsDef { >> int port; >> bool portReserved; >> int websocket; >> + bool websocketGenerated; >> bool autoport; >> char *keymap; >> virDomainGraphicsAuthDef auth; >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index 6aaaa10..1799f33 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -3574,6 +3574,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver, >> if (virPortAllocatorAcquire(driver->webSocketPorts, &port) < 0) >> return -1; >> graphics->data.vnc.websocket = port; >> + graphics->data.vnc.websocketGenerated = true; >> } >> >> return 0; >> @@ -4065,6 +4066,12 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver, >> return -1; >> graphics->data.vnc.portReserved = true; >> } >> + if (graphics->data.vnc.websocket != -1 && /* auto websocket */ >> + graphics->data.vnc.websocket && /* no websocket */ >> + virPortAllocatorSetUsed(driver->remotePorts, >> + graphics->data.vnc.websocket, >> + true) < 0) >> + return -1; >> break; >> >> case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: >> @@ -6189,8 +6196,16 @@ void qemuProcessStop(virQEMUDriverPtr driver, >> false); >> graphics->data.vnc.portReserved = false; >> } >> - virPortAllocatorRelease(driver->webSocketPorts, >> - graphics->data.vnc.websocket); >> + if (graphics->data.vnc.websocketGenerated) { >> + virPortAllocatorRelease(driver->webSocketPorts, >> + graphics->data.vnc.websocket); >> + graphics->data.vnc.websocketGenerated = false; >> + graphics->data.vnc.websocket = -1; > > One more question... Should this be 0 instead of -1? > > We set to -1 during Reserve and set the Generated flag indicating that > the user didn't set to -1, but we did. Not quite. -1 is valid user input. Reserve does not change websocket value.

> > So when we Release the autogenerated port that we created because -1 was > set, shouldn't we set it back to 0 just like it would have been before > we decided to set to -1 and set the Generated flag? We autogenerate only because initial value is -1 which means 'autogenerate' by convention. So if flag is set we should revert back that is set websocket to -1. > > Avoids other code that then may *print* the websocket as -1... websocket -1 is valid xml telling websocket should be autogenerated. Nikolay