[Ovirt-devel] [PATCH server] added ovirt vnc proxy server, to proxy vnc request to managed vms

[Jason Guiditta]

Not quite ACK, though overall, it mostly work for me.  Main issue is
service will not start for me (permission denied), have to run .rb file
manually.  Other suggestions inline below

On Thu, 2009-05-07 at 08:49 -0400, Mohammed Morsi wrote:
> run on startup by default like the other ovirt services
> ---
>  conf/ovirt-vnc-proxy                       |   49 ++++++++++++++
>  installer/modules/ovirt/manifests/ovirt.pp |    1 +
>  ovirt-server.spec.in                       |    5 ++
>  src/vnc-proxy/vnc-proxy.rb                 |   94 ++++++++++++++++++++++++++++
>  4 files changed, 149 insertions(+), 0 deletions(-)
>  create mode 100755 conf/ovirt-vnc-proxy
>  create mode 100644 src/vnc-proxy/vnc-proxy.rb
> 
<snip>
> diff --git a/src/vnc-proxy/vnc-proxy.rb b/src/vnc-proxy/vnc-proxy.rb
> new file mode 100644
> index 0000000..b20bb1f
> --- /dev/null
> +++ b/src/vnc-proxy/vnc-proxy.rb
> @@ -0,0 +1,94 @@
> +#!/usr/bin/ruby
> +#
> +# vnc-proxy.rb
> +# ovirt vnc proxy server, relays ovirt encoded
> +#   vnc requests to correct node
> +# Copyright (C) 2008 Red Hat, Inc.
You know it is 2009, right?  :)

> +# Written by Mohammed Morsi <mmorsi at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +$: << File.join(File.dirname(__FILE__), "../dutils")
> +
> +require 'dutils'
> +require 'daemons'
> +include Daemonize
> +
> +###########
> +OVIRT_SERVER_PORT = 5500
> +VM_NAME_MAX_LEN   = 250
> +VNC_DATA_MAX_LEN = 800000
> +
> +###########
> +
I think we should send errors into a log file a la db-o-matic. In fact,
some of the rest of the structure would be worth emulating, I think.
Specifically, it would be good to make this a class rather than a
script, adding an initialize method, and cleaning up the logging
statements a bit, so DEBUG becomes @logger.debug 'msg'.  Iain had
duplicated the Logger class you will see in db-o-matic, but I think that
was due more to time constraints than anything else (I'd check, but he
is off today).  So it would make sense to pull that out into its own
file, probably put it in /dutils, then include in your class.  Writing
this, I see it kind of looks like a lot, but I don't think it really
would be, just use one of those existing files as a template, should
just need a couple wrappers and substitutions here. 
> +$debug = true
> +
> +def DEBUG(msg)
> +   puts msg if $debug
> +end
> +
> +$verbose = true
> +def VERBOSE(msg)
> +   puts msg if $verbose
> +end
> +
> +###########
> +
> +# TODO catch errors
> +
> +daemonize
> +
> +server = TCPServer.open(OVIRT_SERVER_PORT)
> +
> +while(true) do
> +  Thread.start(server.accept) do |client|
> +    DEBUG "client accepted"
> +
> +    # first msg will be the vm description
> +    vm_description = client.recv(VM_NAME_MAX_LEN).to_s
> +    DEBUG "vm received: " + vm_description + ";"
> +
> +    # lookup vm
> +    vm = Vm.find(:first, :conditions => [ "description = ?", vm_description ])
> +    unless vm.nil? || vm.host.nil? || vm.state != "running"
I think this would be more simply stated as:
if (vm && vm.state == "running")
> +      # connect to node
> +      DEBUG "connecting to node " + vm.host.hostname + ":" + vm.vnc_port.to_s
> +      node_socket = TCPSocket.open(vm.host.hostname, vm.vnc_port)
> +
> +      # begin new thread to process server->client messages
> +      Thread.start do
> +        DEBUG "listening for server->client data"
> +        while(true)do
> +          node_data = node_socket.recv VNC_DATA_MAX_LEN
> +          client.write node_data
> +          VERBOSE "server -> client data " + node_data.size.to_s
> +        end
> +      end
> +
> +      # process client -> server messages
> +      DEBUG "listening for client->server data"
> +      while(true) do
> +        client_data = client.recv VNC_DATA_MAX_LEN
> +        node_socket.write client_data
> +        VERBOSE "client -> server data " + client_data.size.to_s
> +      end
> +
> +      node_socket.close
> +    end
> +
> +    client.close
> + end
> +end