On 08.12.2016 16:21, 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 */
>
> 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) {
This is better of course )
>
> 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.
>
So I'm on with you change.
Nikolay
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
