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

[John Ferlan]

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.

John

> +            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;
> +            } else if (graphics->data.vnc.websocket) {
> +                virPortAllocatorSetUsed(driver->remotePorts,
> +                                        graphics->data.vnc.websocket,
> +                                        false);
> +            }
>          }
>          if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
>              if (graphics->data.spice.autoport) {
>