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

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 */ Some would prefer no comments because the logic should be self explanatory. IDC, but would rather see the comment before rather than within the "if" statement. In any case, why isn't this just: if (graphics->data.vnc.websocket > 0) {

note: no comments. IOW: If a user defined a specific port, set that in the remotePorts. ACK in general - I can adjust the check before pushing based on your feedback. I could also wait for a v2 if you want to create an Unreserve helper with switch/case too.