[Ovirt-devel] Re: [PATCH viewer] connect to the ovirt-vnc-proxy server to access a vm's vnc

[Mohammed Morsi]

Pushed, comments inline.

Jason Guiditta wrote:
> ACK that this works for me.  Couple minor notes inline. You might want
> to have someone who is more of a c programmer look at this too, but I
> think it is good to push for now, we can always make it better later if
> needed.
>
>
>   
>> -  const char *hostname;
>> +  //const char *hostname;
>>     
> This ^^ line should just be deleted rather than commented.  We can
> always find in history if needed.
>   
>> -  const char* hostname;
>> +  //const char* hostname;
>>     
> Same as above for ^^
>   
>>    /* This VM isn't in the notebook already, so create a new console. */
>> -  hostname = gtk_entry_get_text (GTK_ENTRY (ca_hostname));
>> -  fd = viewer_open_vnc_socket(hostname, vm->forward_vnc_port);
>> +  //hostname = gtk_entry_get_text (GTK_ENTRY (ca_hostname));
>>     
> One more time ^^
>   
Removed all three of these


>> +  DEBUG ("connecting to local tunnel on port %i", tunnel_port);
>>     
> If this was ruby, I would say this should be a logger call, not sure
> best practice here though
>   
Yes we just have a simpler stdout output interface since it's a
graphical client app, can be changed anytime.


>> +// port to try to listen on, if we can't, increment until we find one we can
>> +const int PORT_RANGE_START = 5600;
>>     
> Since server default is 5900, does this mean we always increment though
> 400 ports?
No, this is the start of the range for the tunnel port which the viewer
opens on your local machine. When the user clicks a vm name,
ovirt-viewer connects to this local port, which accepts vnc data,
prepending the target vm name, and sends it onto port 5900 on the
server. Since this port is going to be opened on the client's machine,
we don't know which ports are available ahead of time, so I start trying
to open ports at 5600 and increment until we found one that's available.


   -Mo