Ovirt devel - Jan 2009 - server and viewer changes to forward a vm's vnc port

This patch set contains two patches. The first updates the ovirt
server to allow vm administrator to forward a vm's vnc port on
a specific port on the server. Upon starting the vm, taskomatic
will open and forward these ports, and will close them upon
vm shutdown. The second patch updates the viewer to use the forward
vnc port and to connect to the server when establishing a vnc
connection with a vm

---
 src/app/controllers/vm_controller.rb |    2 +
 src/app/models/vm.rb                 |    4 +
 src/app/views/vm/_form.rhtml         |   15 ++++
 src/app/views/vm/show.rhtml          |    4 +
 src/db/migrate/034_add_vm_vnc.rb     |   30 +++++++
 src/task-omatic/taskomatic.rb        |    4 +
 src/task-omatic/vnc.rb               |  140 ++++++++++++++++++++++++++++++++++
 7 files changed, 199 insertions(+), 0 deletions(-)
 create mode 100644 src/db/migrate/034_add_vm_vnc.rb
 create mode 100644 src/task-omatic/vnc.rb

diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index 56501fd..8e1cbee 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -116,6 +116,7 @@ class VmController < ApplicationController
         new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
         needs_restart = true unless current_storage_ids == new_storage_ids
       end
+      params[:vm][:forward_vnc] = params[:forward_vnc]
       params[:vm][:needs_restart] = 1 if needs_restart
       @vm.update_attributes!(params[:vm])
       _setup_vm_provision(params)
@@ -345,6 +346,7 @@ class VmController < ApplicationController
       vm_resource_pool.create_with_parent(hardware_pool)
       params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
     end
+    params[:vm][:forward_vnc] = params[:forward_vnc]
     @vm = Vm.new(params[:vm])
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index bf99e2d..15af463 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -40,6 +40,10 @@ class Vm < ActiveRecord::Base
   validates_format_of :uuid,
      :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
 
+  validates_numericality_of :forward_vnc_port,
+     :greater_than => 0,
+     :if => Proc.new { |vm| vm.forward_vnc }
+
   validates_numericality_of :needs_restart,
      :greater_than_or_equal_to => 0,
      :less_than_or_equal_to => 1,
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 523e81e..486798f 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,6 +51,21 @@
     <div class="clear_row"></div>
     <div class="clear_row"></div>
 
+    <div style="width: 50%; float: left;">
+    <%= check_box_tag_with_label "Forward vm's vnc
<b>port</b> locally", "forward_vnc", 1,
@vm.forward_vnc %>
+    </div>
+    <div style="width: 40%; float: left;">
+    <%= text_field_with_label "", "vm",
"forward_vnc_port", { :style=>"width: 80px;", :size =>
7, :disabled => ! @vm.forward_vnc } %>
+    </div>
+    <div style="clear:both;"></div>
+    <div class="clear_row"></div>
+    <script type="text/javascript">
+      $("#forward_vnc").click(function(){
+        $("#vm_forward_vnc_port").attr("disabled",
$("#forward_vnc").is(":checked") ? "" :
"disabled");
+      });
+    </script>
+
+
    <%= check_box_tag_with_label "Start VM Now? (pending current
resource availability)", "start_now", nil if create or @vm.state
== Vm::STATE_STOPPED %>
    <%= check_box_tag_with_label "Restart VM Now? (pending current
resource availability)", "restart_now", nil if @vm.state ==
Vm::STATE_RUNNING %>
 
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index f361131..add29b4 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -88,6 +88,7 @@
     <div id="vms_selection_id"
style="display:none"><%= @vm.id %></div>
     <div class="selection_key">
         Uuid:<br/>
+        <%= @vm.forward_vnc ? "VNC uri:<br/>" : ""
%>
 	Num vcpus allocated:<br/>
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
@@ -100,6 +101,9 @@
     </div>
     <div class="selection_value">
        <%=h @vm.uuid %><br/>
+       <%= url = request.url
+           url = request.url[0..(url.index('/', 8) - 1)] +
":" + @vm.forward_vnc_port.to_s
+           @vm.forward_vnc ? (url + "<br/>") : ""
%>
        <%=h @vm.num_vcpus_allocated %><br/>
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
diff --git a/src/db/migrate/034_add_vm_vnc.rb b/src/db/migrate/034_add_vm_vnc.rb
new file mode 100644
index 0000000..a93e457
--- /dev/null
+++ b/src/db/migrate/034_add_vm_vnc.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi
+#
+# 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.
+
+class AddVmVnc < ActiveRecord::Migration
+  def self.up
+   add_column :vms, :forward_vnc, :bool, :default => false
+   add_column :vms, :forward_vnc_port, :int, :default => 0
+  end
+
+  def self.down
+   drop_column :vms, :forward_vnc
+   drop_column :vms, :forward_vnc_port
+  end
+end
+
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index bcb9bd3..dca6fb7 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -32,6 +32,7 @@ include Daemonize
 
 require 'task_vm'
 require 'task_storage'
+require 'vnc'
 
 class TaskOmatic
 
@@ -232,6 +233,8 @@ class TaskOmatic
       raise "Error destroying VM: #{result.text}" unless
result.status == 0
     end
 
+    closeVmVncPort(vm)
+
     # undefine can fail, for instance, if we live migrated from A -> B, and
     # then we are shutting down the VM on B (because it only has
"transient"
     # XML).  Therefore, just ignore undefine errors so we do the rest
@@ -303,6 +306,7 @@ class TaskOmatic
     # of places so you'll see a lot of .reloads.
     db_vm.reload
     set_vm_vnc_port(db_vm, result.description) unless result.status != 0
+    forwardVmVncPort(db_vm)
 
     # This information is not available via the libvirt interface.
     db_vm.memory_used = db_vm.memory_allocated
diff --git a/src/task-omatic/vnc.rb b/src/task-omatic/vnc.rb
new file mode 100644
index 0000000..3eb0ca6
--- /dev/null
+++ b/src/task-omatic/vnc.rb
@@ -0,0 +1,140 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# 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.
+
+# TODO no ruby/libiptc wrapper exists, when
+# it does replace iptables command w/ calls to it
+ at iptables_cmd='/sbin/iptables '
+
+# TODO replace this w/ dnsruby inclusion / call
+ at dns_lookup_cmd='/usr/bin/dig'
+
+ at ip_forward_command='echo 1 > /proc/sys/net/ipv4/ip_forward'
+
+ at vnc_debug = false
+
+# TODO can this be retreived in any way
+# since machine will have both external
+# and internal network interface
+ at local_ip = '192.168.50.2'
+
+############################## 'private' methods
+
+def _debug(msg)
+  puts "\n" + msg + "\n" if @vnc_debug
+end
+
+def _findVmHostIp(vm)
+  cmdout='/tmp/ovirtvnc' + vm.forward_vnc_port.to_s
+  cmd=@dns_lookup_cmd + ' ' + vm.host.hostname + 
+      ' +noall +answer +short > ' + cmdout
+
+  system(cmd)
+
+  result = File.read(cmdout).rstrip
+  _debug( "vm host hostname  resolved to " + result.to_s )
+  return result
+end
+
+def _vncPortOpen?(port)
+  cmdout='/tmp/ovirtvnc' + port.to_s
+  cmd=@iptables_cmd + ' -t nat -nL | grep ' + port.to_s  +
+       ' > ' + cmdout
+  _debug("vncPortOpen? iptables command: " + cmd +
+         " cmdout " + cmdout)
+
+  system(cmd)
+  
+   return File.size(cmdout) != 0
+end
+
+def _forwardRules(vm)
+  ip = _findVmHostIp(vm) 
+  return " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j ACCEPT",
+         " -s " + ip + " -p tcp --sport " +
vm.vnc_port.to_s + " -j ACCEPT"
+end
+
+def _natRules(vm)
+  ip = _findVmHostIp(vm) 
+
+  # TODO should a "-d external_server_ip" be added to DNAT?
+  return " -p tcp --dport " + vm.forward_vnc_port.to_s + " -j
DNAT --to " + ip + ":" + vm.vnc_port.to_s,
+         " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j SNAT --to " + @local_ip
+end
+
+############################## 'public' methods
+
+
+def forwardVmVncPort(vm)
+  return unless vm.forward_vnc
+  unless vm.forward_vnc_port > 0 
+    raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+  end
+  
+   if _vncPortOpen?(vm.forward_vnc_port)
+     raise "Port already open " + vm.forward_vnc_port.to_s
+   end
+
+   forward_rule1, forward_rule2 = _forwardRules(vm)
+   forward_rule1 = @iptables_cmd + " -A FORWARD " + forward_rule1
+   forward_rule2 = @iptables_cmd + " -A FORWARD " + forward_rule2
+
+   prerouting_rule, postrouting_rule = _natRules(vm)
+   prerouting_rule = @iptables_cmd + " -t nat -A PREROUTING " +
prerouting_rule
+   postrouting_rule = @iptables_cmd + " -t nat -A POSTROUTING " +
postrouting_rule
+
+   _debug(" open\n forward rule 1: "     + forward_rule1 +
+          "\n forward_rule 2: "   + forward_rule2 +
+          "\n prerouting rule: "  + prerouting_rule +
+          "\n postrouting rule: " + postrouting_rule)
+
+   system(forward_rule1)
+   system(forward_rule2)
+   system(prerouting_rule)
+   system(postrouting_rule)
+   system(@ip_forward_command)
+end
+
+def closeVmVncPort(vm)
+  # FIXME forward_vnc may have been changed while the vm is running
+  return unless vm.forward_vnc
+  unless vm.forward_vnc_port > 0 
+    raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+  end
+
+   unless _vncPortOpen?(vm.forward_vnc_port)
+     raise "Port not open " + vm.forward_vnc_port.to_s
+   end
+
+   forward_rule1, forward_rule2 = _forwardRules(vm)
+   forward_rule1 = @iptables_cmd + " -D FORWARD " + forward_rule1
+   forward_rule2 = @iptables_cmd + " -D FORWARD " + forward_rule2
+
+   prerouting_rule, postrouting_rule = _natRules(vm)
+   prerouting_rule = @iptables_cmd + " -t nat -D PREROUTING " +
prerouting_rule
+   postrouting_rule = @iptables_cmd + " -t nat -D POSTROUTING " +
postrouting_rule
+
+   _debug(" close\n forward rule 1: "     + forward_rule1 +
+          "\n forward_rule 2: "   + forward_rule2 +
+          "\n prerouting rule: "  + prerouting_rule +
+          "\n postrouting rule: " + postrouting_rule)
+
+   system(forward_rule1)
+   system(forward_rule2)
+   system(prerouting_rule)
+   system(postrouting_rule)
+end
-- 
1.6.0.6

---
 src/app/controllers/vm_controller.rb |    8 ++-
 src/app/models/vm.rb                 |   17 ++++
 src/app/views/vm/_form.rhtml         |   15 ++++
 src/app/views/vm/show.rhtml          |    4 +
 src/db/migrate/034_add_vm_vnc.rb     |   30 ++++++++
 src/task-omatic/taskomatic.rb        |    4 +
 src/task-omatic/vnc.rb               |  136 ++++++++++++++++++++++++++++++++++
 src/test/fixtures/vms.yml            |    2 +
 src/test/unit/vm_test.rb             |   16 ++++
 9 files changed, 231 insertions(+), 1 deletions(-)
 create mode 100644 src/db/migrate/034_add_vm_vnc.rb
 create mode 100644 src/task-omatic/vnc.rb

diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index 56501fd..e92d604 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -116,6 +116,8 @@ class VmController < ApplicationController
         new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
         needs_restart = true unless current_storage_ids == new_storage_ids
       end
+      params[:vm][:forward_vnc] = params[:forward_vnc]
+      params[:vm][:forward_vnc_port] = nil unless params[:forward_vnc]
       params[:vm][:needs_restart] = 1 if needs_restart
       @vm.update_attributes!(params[:vm])
       _setup_vm_provision(params)
@@ -325,7 +327,8 @@ class VmController < ApplicationController
     newargs = {
       :vm_resource_pool_id => params[:vm_resource_pool_id],
       :vnic_mac_addr => mac.collect {|x| "%02x" %
x}.join(":"),
-      :uuid => uuid
+      :uuid => uuid,
+      :forward_vnc_port => Vm.available_forward_vnc_port
     }
     @vm = Vm.new( newargs )
     unless params[:vm_resource_pool_id]
@@ -345,6 +348,8 @@ class VmController < ApplicationController
       vm_resource_pool.create_with_parent(hardware_pool)
       params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
     end
+    params[:vm][:forward_vnc] = params[:forward_vnc]
+    params[:vm][:forward_vnc_port] = nil unless params[:forward_vnc]
     @vm = Vm.new(params[:vm])
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
@@ -356,6 +361,7 @@ class VmController < ApplicationController
   end
   def pre_edit
     @vm = Vm.find(params[:id])
+    @vm.forward_vnc_port = Vm.available_forward_vnc_port unless @vm.forward_vnc
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
     _setup_provisioning_options
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index bf99e2d..63c9232 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -40,6 +40,15 @@ class Vm < ActiveRecord::Base
   validates_format_of :uuid,
      :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
 
+  validates_numericality_of :forward_vnc_port,
+     :message => 'Forward vnc port must be >= 5900',
+     :greater_than_or_equal_to => 5900,
+     :if => Proc.new { |vm| vm.forward_vnc }
+
+  validates_uniqueness_of :forward_vnc_port,
+    :message => 'Duplicate forward vnc port exists',
+    :if => Proc.new { |vm| vm.forward_vnc }
+
   validates_numericality_of :needs_restart,
      :greater_than_or_equal_to => 0,
      :less_than_or_equal_to => 1,
@@ -335,6 +344,14 @@ class Vm < ActiveRecord::Base
     super
   end
 
+  def self.available_forward_vnc_port
+    i = 5900
+    until Vm.find(:first,  :conditions => [ "forward_vnc_port =
?", i]).nil?
+       i += 1
+    end
+    return i
+  end
+
   protected
   def validate
     resources = vm_resource_pool.max_resources_for_vm(self)
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 7cbe16d..2db6cb9 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,6 +51,21 @@
     <div class="clear_row"></div>
     <div class="clear_row"></div>
 
+    <div style="width: 50%; float: left;">
+    <%= check_box_tag_with_label "Forward vm's vnc
<b>port</b> locally", "forward_vnc", 1,
@vm.forward_vnc %>
+    </div>
+    <div style="width: 40%; float: left;">
+    <%= text_field_with_label "", "vm",
"forward_vnc_port", { :style=>"width: 80px;", :size =>
7, :disabled => ! @vm.forward_vnc } %>
+    </div>
+    <div style="clear:both;"></div>
+    <div class="clear_row"></div>
+    <script type="text/javascript">
+      $("#forward_vnc").click(function(){
+        $("#vm_forward_vnc_port").attr("disabled",
$("#forward_vnc").is(":checked") ? "" :
"disabled");
+      });
+    </script>
+
+
    <%= check_box_tag_with_label "Start VM Now? (pending current
resource availability)", "start_now", nil if create or @vm.state
== Vm::STATE_STOPPED %>
    <%= check_box_tag_with_label "Restart VM Now? (pending current
resource availability)", "restart_now", nil if @vm.state ==
Vm::STATE_RUNNING %>
 
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index f361131..add29b4 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -88,6 +88,7 @@
     <div id="vms_selection_id"
style="display:none"><%= @vm.id %></div>
     <div class="selection_key">
         Uuid:<br/>
+        <%= @vm.forward_vnc ? "VNC uri:<br/>" : ""
%>
 	Num vcpus allocated:<br/>
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
@@ -100,6 +101,9 @@
     </div>
     <div class="selection_value">
        <%=h @vm.uuid %><br/>
+       <%= url = request.url
+           url = request.url[0..(url.index('/', 8) - 1)] +
":" + @vm.forward_vnc_port.to_s
+           @vm.forward_vnc ? (url + "<br/>") : ""
%>
        <%=h @vm.num_vcpus_allocated %><br/>
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
diff --git a/src/db/migrate/034_add_vm_vnc.rb b/src/db/migrate/034_add_vm_vnc.rb
new file mode 100644
index 0000000..f23e24d
--- /dev/null
+++ b/src/db/migrate/034_add_vm_vnc.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi
+#
+# 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.
+
+class AddVmVnc < ActiveRecord::Migration
+  def self.up
+   add_column :vms, :forward_vnc, :bool, :default => false
+   add_column :vms, :forward_vnc_port, :int, :default => 0, :unique =>
true
+  end
+
+  def self.down
+   drop_column :vms, :forward_vnc
+   drop_column :vms, :forward_vnc_port
+  end
+end
+
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index bcb9bd3..6db7832 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -32,6 +32,7 @@ include Daemonize
 
 require 'task_vm'
 require 'task_storage'
+require 'vnc'
 
 class TaskOmatic
 
@@ -232,6 +233,8 @@ class TaskOmatic
       raise "Error destroying VM: #{result.text}" unless
result.status == 0
     end
 
+    VmVnc.close(vm)
+
     # undefine can fail, for instance, if we live migrated from A -> B, and
     # then we are shutting down the VM on B (because it only has
"transient"
     # XML).  Therefore, just ignore undefine errors so we do the rest
@@ -303,6 +306,7 @@ class TaskOmatic
     # of places so you'll see a lot of .reloads.
     db_vm.reload
     set_vm_vnc_port(db_vm, result.description) unless result.status != 0
+    VmVnc.forward(db_vm)
 
     # This information is not available via the libvirt interface.
     db_vm.memory_used = db_vm.memory_allocated
diff --git a/src/task-omatic/vnc.rb b/src/task-omatic/vnc.rb
new file mode 100644
index 0000000..bc3fd8f
--- /dev/null
+++ b/src/task-omatic/vnc.rb
@@ -0,0 +1,136 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# 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.
+
+# provides static 'forward' and 'close' methods to forward a
specified vm's vnc connections
+class VmVnc
+
+  private
+
+    # TODO no ruby/libiptc wrapper exists, when
+    # it does replace iptables command w/ calls to it
+    IPTABLES_CMD='/sbin/iptables '
+
+    IP_FORWARD_CMD='echo 1 > /proc/sys/net/ipv4/ip_forward'
+
+    VNC_DEBUG = false
+
+    # FIXME can this be retreived in any way
+    # since machine will have both external
+    # and internal network interface
+    LOCAL_IP = '192.168.50.2'
+
+    def self.debug(msg)
+      puts "\n" + msg + "\n" if VNC_DEBUG
+    end
+
+    def self.find_host_ip(hostname)
+      # FIXME
+      addrinfo = Socket::getaddrinfo(hostname, nil)
+      unless addrinfo.size > 0
+        raise "Could not retreive address for " + hostname
+      end
+      result = addrinfo[0][3] # return ip address of first entry
+      debug( "vm host hostname  resolved to " + result.to_s )
+      return result
+    end
+
+    def self.port_open?(port)
+      cmd=IPTABLES_CMD + ' -t nat -nL '
+      debug("vncPortOpen? iptables command: " + cmd)
+
+      `#{cmd}`.each_line do |l|
+          return true if l =~ /.*#{port}.*/
+      end
+      return false
+    end
+
+    def self.get_forward_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+      return " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j ACCEPT",
+             " -s " + ip + " -p tcp --sport " +
vm.vnc_port.to_s + " -j ACCEPT"
+    end
+
+    def self.get_nat_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+
+      return " -p tcp --dport " + vm.forward_vnc_port.to_s + "
-j DNAT --to " + ip + ":" + vm.vnc_port.to_s,
+             " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j SNAT --to " + LOCAL_IP
+    end
+
+  public
+
+    def self.forward(vm)
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       if port_open?(vm.forward_vnc_port)
+         raise "Port already open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -A FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -A FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -A PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -A POSTROUTING " +
postrouting_rule
+
+       debug(" open\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       system(forward_rule1)
+       system(forward_rule2)
+       system(prerouting_rule)
+       system(postrouting_rule)
+       system(IP_FORWARD_CMD)
+    end
+
+    def self.close(vm)
+      # FIXME forward_vnc may have been changed while the vm is running
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       unless port_open?(vm.forward_vnc_port)
+         raise "Port not open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -D FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -D FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -D PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -D POSTROUTING " +
postrouting_rule
+
+       debug(" close\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       system(forward_rule1)
+       system(forward_rule2)
+       system(prerouting_rule)
+       system(postrouting_rule)
+    end
+end
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index ca0d63f..366f192 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -11,6 +11,8 @@ production_httpd_vm:
   boot_device: hd
   host: prod_corp_com
   vm_resource_pool: corp_com_production_vmpool
+  forward_vnc: true
+  forward_vnc_port: 1234
 production_mysqld_vm:
   uuid: 89e62d32-04d9-4351-b573-b1a253397296
   description: production mysqld appliance
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index cba3188..b868dfa 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -95,6 +95,22 @@ class VmTest < Test::Unit::TestCase
        flunk 'Vm must specify valid state' if @vm.valid?
   end
 
+  # ensure duplicate forward_vnc_ports cannot exist
+  def test_invalid_without_unique_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1234 # duplicate
+     assert !vm.valid?, "forward vnc port must be unique"
+  end
+
+  # ensure bad forward_vnc_ports cannot exist
+  def test_invalid_without_bad_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1 # too small
+     assert !vm.valid?, "forward vnc port must be >= 5900"
+  end
+
   # Ensures that, if the VM does not contain the Cobbler prefix, that it
   # does not claim to be a Cobbler VM.
   #
-- 
1.6.0.6

---
 src/app/controllers/vm_controller.rb |   21 +++++-
 src/app/models/vm.rb                 |   26 ++++++
 src/app/views/vm/_form.rhtml         |   15 ++++
 src/app/views/vm/show.rhtml          |    4 +
 src/db/migrate/034_add_vm_vnc.rb     |   30 +++++++
 src/task-omatic/taskomatic.rb        |    4 +
 src/task-omatic/vnc.rb               |  140 ++++++++++++++++++++++++++++++++++
 src/test/fixtures/vms.yml            |    2 +
 src/test/unit/vm_test.rb             |   16 ++++
 9 files changed, 257 insertions(+), 1 deletions(-)
 create mode 100644 src/db/migrate/034_add_vm_vnc.rb
 create mode 100644 src/task-omatic/vnc.rb

diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index 56501fd..d4c516b 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -116,6 +116,15 @@ class VmController < ApplicationController
         new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
         needs_restart = true unless current_storage_ids == new_storage_ids
       end
+      params[:vm][:forward_vnc] = params[:forward_vnc]
+      if params[:forward_vnc]
+        if params[:vm][:forward_vnc_port].to_i == 0
+          params[:vm][:forward_vnc_port] = Vm.available_forward_vnc_port
+        end
+      else
+          params[:vm][:forward_vnc_port] = nil
+      end
+
       params[:vm][:needs_restart] = 1 if needs_restart
       @vm.update_attributes!(params[:vm])
       _setup_vm_provision(params)
@@ -325,7 +334,8 @@ class VmController < ApplicationController
     newargs = {
       :vm_resource_pool_id => params[:vm_resource_pool_id],
       :vnic_mac_addr => mac.collect {|x| "%02x" %
x}.join(":"),
-      :uuid => uuid
+      :uuid => uuid,
+      :forward_vnc_port => 0
     }
     @vm = Vm.new( newargs )
     unless params[:vm_resource_pool_id]
@@ -345,6 +355,14 @@ class VmController < ApplicationController
       vm_resource_pool.create_with_parent(hardware_pool)
       params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
     end
+    params[:vm][:forward_vnc] = params[:forward_vnc]
+    if params[:forward_vnc]
+      if params[:vm][:forward_vnc_port].to_i == 0
+        params[:vm][:forward_vnc_port] = Vm.available_forward_vnc_port
+      end
+    else
+        params[:vm][:forward_vnc_port] = nil
+    end
     @vm = Vm.new(params[:vm])
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
@@ -356,6 +374,7 @@ class VmController < ApplicationController
   end
   def pre_edit
     @vm = Vm.find(params[:id])
+    @vm.forward_vnc_port = 0 unless @vm.forward_vnc
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
     _setup_provisioning_options
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index bf99e2d..1cfcc72 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -40,6 +40,17 @@ class Vm < ActiveRecord::Base
   validates_format_of :uuid,
      :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
 
+  FORWARD_VNC_PORT_START = 5900
+
+  validates_numericality_of :forward_vnc_port,
+     :message => 'must be >= ' + FORWARD_VNC_PORT_START.to_s,
+     :greater_than_or_equal_to => FORWARD_VNC_PORT_START,
+     :if => Proc.new { |vm| vm.forward_vnc }
+
+  validates_uniqueness_of :forward_vnc_port,
+    :message => "is already in use",
+    :if => Proc.new { |vm| vm.forward_vnc }
+
   validates_numericality_of :needs_restart,
      :greater_than_or_equal_to => 0,
      :less_than_or_equal_to => 1,
@@ -335,6 +346,21 @@ class Vm < ActiveRecord::Base
     super
   end
 
+  # find the first available vnc port
+  def self.available_forward_vnc_port
+    # FIXME need a way to reserve values returned
+    #  by this until table is saved
+
+    i = FORWARD_VNC_PORT_START
+    Vm.find(:all,
+            :conditions => "forward_vnc_port is not NULL",
+            :order => 'forward_vnc_port ASC' ).collect{ |vm|
+               return i if vm.forward_vnc_port > i
+               i = i + 1
+            }
+    return i
+  end
+
   protected
   def validate
     resources = vm_resource_pool.max_resources_for_vm(self)
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 7cbe16d..f1c0239 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,6 +51,21 @@
     <div class="clear_row"></div>
     <div class="clear_row"></div>
 
+    <div style="width: 50%; float: left;">
+    <%= check_box_tag_with_label "Forward vm's vnc
<b>port</b> locally", "forward_vnc", 1,
@vm.forward_vnc %>
+     (set to 0 to autoassign port)
+    </div>
+    <div style="width: 40%; float: left;">
+    <%= text_field_with_label "", "vm",
"forward_vnc_port", { :style=>"width: 80px;", :size =>
7, :disabled => ! @vm.forward_vnc } %>
+    </div>
+    <div style="clear:both;"></div>
+    <div class="clear_row"></div>
+    <script type="text/javascript">
+      $("#forward_vnc").click(function(){
+        $("#vm_forward_vnc_port").attr("disabled",
$("#forward_vnc").is(":checked") ? "" :
"disabled");
+      });
+    </script>
+
    <%= check_box_tag_with_label "Start VM Now? (pending current
resource availability)", "start_now", nil if create or @vm.state
== Vm::STATE_STOPPED %>
    <%= check_box_tag_with_label "Restart VM Now? (pending current
resource availability)", "restart_now", nil if @vm.state ==
Vm::STATE_RUNNING %>
 
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index f361131..add29b4 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -88,6 +88,7 @@
     <div id="vms_selection_id"
style="display:none"><%= @vm.id %></div>
     <div class="selection_key">
         Uuid:<br/>
+        <%= @vm.forward_vnc ? "VNC uri:<br/>" : ""
%>
 	Num vcpus allocated:<br/>
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
@@ -100,6 +101,9 @@
     </div>
     <div class="selection_value">
        <%=h @vm.uuid %><br/>
+       <%= url = request.url
+           url = request.url[0..(url.index('/', 8) - 1)] +
":" + @vm.forward_vnc_port.to_s
+           @vm.forward_vnc ? (url + "<br/>") : ""
%>
        <%=h @vm.num_vcpus_allocated %><br/>
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
diff --git a/src/db/migrate/034_add_vm_vnc.rb b/src/db/migrate/034_add_vm_vnc.rb
new file mode 100644
index 0000000..f23e24d
--- /dev/null
+++ b/src/db/migrate/034_add_vm_vnc.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi
+#
+# 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.
+
+class AddVmVnc < ActiveRecord::Migration
+  def self.up
+   add_column :vms, :forward_vnc, :bool, :default => false
+   add_column :vms, :forward_vnc_port, :int, :default => 0, :unique =>
true
+  end
+
+  def self.down
+   drop_column :vms, :forward_vnc
+   drop_column :vms, :forward_vnc_port
+  end
+end
+
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index 0570246..380be92 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -32,6 +32,7 @@ include Daemonize
 
 require 'task_vm'
 require 'task_storage'
+require 'vnc'
 
 class TaskOmatic
 
@@ -232,6 +233,8 @@ class TaskOmatic
       raise "Error destroying VM: #{result.text}" unless
result.status == 0
     end
 
+    VmVnc.close(vm)
+
     # undefine can fail, for instance, if we live migrated from A -> B, and
     # then we are shutting down the VM on B (because it only has
"transient"
     # XML).  Therefore, just ignore undefine errors so we do the rest
@@ -303,6 +306,7 @@ class TaskOmatic
     # of places so you'll see a lot of .reloads.
     db_vm.reload
     set_vm_vnc_port(db_vm, result.description) unless result.status != 0
+    VmVnc.forward(db_vm)
 
     # This information is not available via the libvirt interface.
     db_vm.memory_used = db_vm.memory_allocated
diff --git a/src/task-omatic/vnc.rb b/src/task-omatic/vnc.rb
new file mode 100644
index 0000000..0876f36
--- /dev/null
+++ b/src/task-omatic/vnc.rb
@@ -0,0 +1,140 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# 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.
+
+# provides static 'forward' and 'close' methods to forward a
specified vm's vnc connections
+class VmVnc
+
+  private
+
+    # TODO no ruby/libiptc wrapper exists, when
+    # it does replace iptables command w/ calls to it
+    IPTABLES_CMD='/sbin/iptables '
+
+    VNC_DEBUG = false
+
+    # FIXME can this be retreived in any way
+    # since machine will have both external
+    # and internal network interface
+    LOCAL_IP = '192.168.50.2'
+
+    def self.debug(msg)
+      puts "\n" + msg + "\n" if VNC_DEBUG
+    end
+
+    def self.find_host_ip(hostname)
+      # FIXME
+      addrinfo = Socket::getaddrinfo(hostname, nil)
+      unless addrinfo.size > 0
+        raise "Could not retreive address for " + hostname
+      end
+      result = addrinfo[0][3] # return ip address of first entry
+      debug( "vm host hostname  resolved to " + result.to_s )
+      return result
+    end
+
+    def self.port_open?(port)
+      cmd=IPTABLES_CMD + ' -t nat -nL '
+      debug("vncPortOpen? iptables command: " + cmd)
+
+      `#{cmd}`.each_line do |l|
+          return true if l =~ /.*#{port}.*/
+      end
+      return false
+    end
+
+    def self.get_forward_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+      return " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j ACCEPT",
+             " -s " + ip + " -p tcp --sport " +
vm.vnc_port.to_s + " -j ACCEPT"
+    end
+
+    def self.get_nat_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+
+      return " -p tcp --dport " + vm.forward_vnc_port.to_s + "
-j DNAT --to " + ip + ":" + vm.vnc_port.to_s,
+             " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j SNAT --to " + LOCAL_IP
+    end
+
+    def self.run_command(cmd)
+      debug("Running command " + cmd)
+      status = system(cmd)
+      raise 'Command terminated with error code ' + $?.to_s unless
status
+    end
+
+  public
+
+    def self.forward(vm)
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       if port_open?(vm.forward_vnc_port)
+         raise "Port already open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -A FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -A FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -A PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -A POSTROUTING " +
postrouting_rule
+
+       debug(" open\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       File::open("/proc/sys/net/ipv4/ip_forward", "w") {
|f| f.puts "1" }
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+
+    def self.close(vm)
+      # FIXME forward_vnc may have been changed while the vm is running
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       unless port_open?(vm.forward_vnc_port)
+         raise "Port not open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -D FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -D FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -D PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -D POSTROUTING " +
postrouting_rule
+
+       debug(" close\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+end
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index ca0d63f..366f192 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -11,6 +11,8 @@ production_httpd_vm:
   boot_device: hd
   host: prod_corp_com
   vm_resource_pool: corp_com_production_vmpool
+  forward_vnc: true
+  forward_vnc_port: 1234
 production_mysqld_vm:
   uuid: 89e62d32-04d9-4351-b573-b1a253397296
   description: production mysqld appliance
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index cba3188..b868dfa 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -95,6 +95,22 @@ class VmTest < Test::Unit::TestCase
        flunk 'Vm must specify valid state' if @vm.valid?
   end
 
+  # ensure duplicate forward_vnc_ports cannot exist
+  def test_invalid_without_unique_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1234 # duplicate
+     assert !vm.valid?, "forward vnc port must be unique"
+  end
+
+  # ensure bad forward_vnc_ports cannot exist
+  def test_invalid_without_bad_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1 # too small
+     assert !vm.valid?, "forward vnc port must be >= 5900"
+  end
+
   # Ensures that, if the VM does not contain the Cobbler prefix, that it
   # does not claim to be a Cobbler VM.
   #
-- 
1.6.0.6

---
 src/app/controllers/vm_controller.rb |   29 ++++++-
 src/app/models/vm.rb                 |   26 ++++++
 src/app/views/vm/_form.rhtml         |   15 ++++
 src/app/views/vm/show.rhtml          |    4 +
 src/db/migrate/034_add_vm_vnc.rb     |   30 +++++++
 src/task-omatic/taskomatic.rb        |    4 +
 src/task-omatic/vnc.rb               |  140 ++++++++++++++++++++++++++++++++++
 src/test/fixtures/vms.yml            |    2 +
 src/test/unit/vm_test.rb             |   16 ++++
 9 files changed, 263 insertions(+), 3 deletions(-)
 create mode 100644 src/db/migrate/034_add_vm_vnc.rb
 create mode 100644 src/task-omatic/vnc.rb

diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index 56501fd..39150ae 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -62,6 +62,14 @@ class VmController < ApplicationController
   def create
     begin
       Vm.transaction do
+        if @vm.forward_vnc
+          if @vm.forward_vnc_port.to_i == 0
+            @vm.forward_vnc_port = Vm.available_forward_vnc_port
+          end
+        else
+            @vm.forward_vnc_port = nil
+        end
+
         @vm.save!
         _setup_vm_provision(params)
         @task = VmTask.new({ :user        => @user,
@@ -116,8 +124,20 @@ class VmController < ApplicationController
         new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
         needs_restart = true unless current_storage_ids == new_storage_ids
       end
-      params[:vm][:needs_restart] = 1 if needs_restart
-      @vm.update_attributes!(params[:vm])
+
+      Vm.transaction do
+        params[:vm][:forward_vnc] = params[:forward_vnc]
+        if params[:forward_vnc]
+          if params[:vm][:forward_vnc_port].to_i == 0
+            params[:vm][:forward_vnc_port] = Vm.available_forward_vnc_port
+          end
+        else
+            params[:vm][:forward_vnc_port] = nil
+        end
+
+        params[:vm][:needs_restart] = 1 if needs_restart
+        @vm.update_attributes!(params[:vm])
+      end
       _setup_vm_provision(params)
 
       if (params[:start_now] and
@vm.get_action_list.include?(VmTask::ACTION_START_VM) )
@@ -325,7 +345,8 @@ class VmController < ApplicationController
     newargs = {
       :vm_resource_pool_id => params[:vm_resource_pool_id],
       :vnic_mac_addr => mac.collect {|x| "%02x" %
x}.join(":"),
-      :uuid => uuid
+      :uuid => uuid,
+      :forward_vnc_port => 0
     }
     @vm = Vm.new( newargs )
     unless params[:vm_resource_pool_id]
@@ -345,6 +366,7 @@ class VmController < ApplicationController
       vm_resource_pool.create_with_parent(hardware_pool)
       params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
     end
+    params[:vm][:forward_vnc] = params[:forward_vnc]
     @vm = Vm.new(params[:vm])
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
@@ -356,6 +378,7 @@ class VmController < ApplicationController
   end
   def pre_edit
     @vm = Vm.find(params[:id])
+    @vm.forward_vnc_port = 0 unless @vm.forward_vnc
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
     _setup_provisioning_options
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index bf99e2d..1cfcc72 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -40,6 +40,17 @@ class Vm < ActiveRecord::Base
   validates_format_of :uuid,
      :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
 
+  FORWARD_VNC_PORT_START = 5900
+
+  validates_numericality_of :forward_vnc_port,
+     :message => 'must be >= ' + FORWARD_VNC_PORT_START.to_s,
+     :greater_than_or_equal_to => FORWARD_VNC_PORT_START,
+     :if => Proc.new { |vm| vm.forward_vnc }
+
+  validates_uniqueness_of :forward_vnc_port,
+    :message => "is already in use",
+    :if => Proc.new { |vm| vm.forward_vnc }
+
   validates_numericality_of :needs_restart,
      :greater_than_or_equal_to => 0,
      :less_than_or_equal_to => 1,
@@ -335,6 +346,21 @@ class Vm < ActiveRecord::Base
     super
   end
 
+  # find the first available vnc port
+  def self.available_forward_vnc_port
+    # FIXME need a way to reserve values returned
+    #  by this until table is saved
+
+    i = FORWARD_VNC_PORT_START
+    Vm.find(:all,
+            :conditions => "forward_vnc_port is not NULL",
+            :order => 'forward_vnc_port ASC' ).collect{ |vm|
+               return i if vm.forward_vnc_port > i
+               i = i + 1
+            }
+    return i
+  end
+
   protected
   def validate
     resources = vm_resource_pool.max_resources_for_vm(self)
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 7cbe16d..f1c0239 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,6 +51,21 @@
     <div class="clear_row"></div>
     <div class="clear_row"></div>
 
+    <div style="width: 50%; float: left;">
+    <%= check_box_tag_with_label "Forward vm's vnc
<b>port</b> locally", "forward_vnc", 1,
@vm.forward_vnc %>
+     (set to 0 to autoassign port)
+    </div>
+    <div style="width: 40%; float: left;">
+    <%= text_field_with_label "", "vm",
"forward_vnc_port", { :style=>"width: 80px;", :size =>
7, :disabled => ! @vm.forward_vnc } %>
+    </div>
+    <div style="clear:both;"></div>
+    <div class="clear_row"></div>
+    <script type="text/javascript">
+      $("#forward_vnc").click(function(){
+        $("#vm_forward_vnc_port").attr("disabled",
$("#forward_vnc").is(":checked") ? "" :
"disabled");
+      });
+    </script>
+
    <%= check_box_tag_with_label "Start VM Now? (pending current
resource availability)", "start_now", nil if create or @vm.state
== Vm::STATE_STOPPED %>
    <%= check_box_tag_with_label "Restart VM Now? (pending current
resource availability)", "restart_now", nil if @vm.state ==
Vm::STATE_RUNNING %>
 
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index f361131..add29b4 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -88,6 +88,7 @@
     <div id="vms_selection_id"
style="display:none"><%= @vm.id %></div>
     <div class="selection_key">
         Uuid:<br/>
+        <%= @vm.forward_vnc ? "VNC uri:<br/>" : ""
%>
 	Num vcpus allocated:<br/>
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
@@ -100,6 +101,9 @@
     </div>
     <div class="selection_value">
        <%=h @vm.uuid %><br/>
+       <%= url = request.url
+           url = request.url[0..(url.index('/', 8) - 1)] +
":" + @vm.forward_vnc_port.to_s
+           @vm.forward_vnc ? (url + "<br/>") : ""
%>
        <%=h @vm.num_vcpus_allocated %><br/>
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
diff --git a/src/db/migrate/034_add_vm_vnc.rb b/src/db/migrate/034_add_vm_vnc.rb
new file mode 100644
index 0000000..f23e24d
--- /dev/null
+++ b/src/db/migrate/034_add_vm_vnc.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi
+#
+# 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.
+
+class AddVmVnc < ActiveRecord::Migration
+  def self.up
+   add_column :vms, :forward_vnc, :bool, :default => false
+   add_column :vms, :forward_vnc_port, :int, :default => 0, :unique =>
true
+  end
+
+  def self.down
+   drop_column :vms, :forward_vnc
+   drop_column :vms, :forward_vnc_port
+  end
+end
+
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index 0570246..380be92 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -32,6 +32,7 @@ include Daemonize
 
 require 'task_vm'
 require 'task_storage'
+require 'vnc'
 
 class TaskOmatic
 
@@ -232,6 +233,8 @@ class TaskOmatic
       raise "Error destroying VM: #{result.text}" unless
result.status == 0
     end
 
+    VmVnc.close(vm)
+
     # undefine can fail, for instance, if we live migrated from A -> B, and
     # then we are shutting down the VM on B (because it only has
"transient"
     # XML).  Therefore, just ignore undefine errors so we do the rest
@@ -303,6 +306,7 @@ class TaskOmatic
     # of places so you'll see a lot of .reloads.
     db_vm.reload
     set_vm_vnc_port(db_vm, result.description) unless result.status != 0
+    VmVnc.forward(db_vm)
 
     # This information is not available via the libvirt interface.
     db_vm.memory_used = db_vm.memory_allocated
diff --git a/src/task-omatic/vnc.rb b/src/task-omatic/vnc.rb
new file mode 100644
index 0000000..0876f36
--- /dev/null
+++ b/src/task-omatic/vnc.rb
@@ -0,0 +1,140 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# 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.
+
+# provides static 'forward' and 'close' methods to forward a
specified vm's vnc connections
+class VmVnc
+
+  private
+
+    # TODO no ruby/libiptc wrapper exists, when
+    # it does replace iptables command w/ calls to it
+    IPTABLES_CMD='/sbin/iptables '
+
+    VNC_DEBUG = false
+
+    # FIXME can this be retreived in any way
+    # since machine will have both external
+    # and internal network interface
+    LOCAL_IP = '192.168.50.2'
+
+    def self.debug(msg)
+      puts "\n" + msg + "\n" if VNC_DEBUG
+    end
+
+    def self.find_host_ip(hostname)
+      # FIXME
+      addrinfo = Socket::getaddrinfo(hostname, nil)
+      unless addrinfo.size > 0
+        raise "Could not retreive address for " + hostname
+      end
+      result = addrinfo[0][3] # return ip address of first entry
+      debug( "vm host hostname  resolved to " + result.to_s )
+      return result
+    end
+
+    def self.port_open?(port)
+      cmd=IPTABLES_CMD + ' -t nat -nL '
+      debug("vncPortOpen? iptables command: " + cmd)
+
+      `#{cmd}`.each_line do |l|
+          return true if l =~ /.*#{port}.*/
+      end
+      return false
+    end
+
+    def self.get_forward_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+      return " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j ACCEPT",
+             " -s " + ip + " -p tcp --sport " +
vm.vnc_port.to_s + " -j ACCEPT"
+    end
+
+    def self.get_nat_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+
+      return " -p tcp --dport " + vm.forward_vnc_port.to_s + "
-j DNAT --to " + ip + ":" + vm.vnc_port.to_s,
+             " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j SNAT --to " + LOCAL_IP
+    end
+
+    def self.run_command(cmd)
+      debug("Running command " + cmd)
+      status = system(cmd)
+      raise 'Command terminated with error code ' + $?.to_s unless
status
+    end
+
+  public
+
+    def self.forward(vm)
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       if port_open?(vm.forward_vnc_port)
+         raise "Port already open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -A FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -A FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -A PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -A POSTROUTING " +
postrouting_rule
+
+       debug(" open\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       File::open("/proc/sys/net/ipv4/ip_forward", "w") {
|f| f.puts "1" }
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+
+    def self.close(vm)
+      # FIXME forward_vnc may have been changed while the vm is running
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       unless port_open?(vm.forward_vnc_port)
+         raise "Port not open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -D FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -D FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -D PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -D POSTROUTING " +
postrouting_rule
+
+       debug(" close\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+end
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index ca0d63f..366f192 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -11,6 +11,8 @@ production_httpd_vm:
   boot_device: hd
   host: prod_corp_com
   vm_resource_pool: corp_com_production_vmpool
+  forward_vnc: true
+  forward_vnc_port: 1234
 production_mysqld_vm:
   uuid: 89e62d32-04d9-4351-b573-b1a253397296
   description: production mysqld appliance
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index cba3188..b868dfa 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -95,6 +95,22 @@ class VmTest < Test::Unit::TestCase
        flunk 'Vm must specify valid state' if @vm.valid?
   end
 
+  # ensure duplicate forward_vnc_ports cannot exist
+  def test_invalid_without_unique_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1234 # duplicate
+     assert !vm.valid?, "forward vnc port must be unique"
+  end
+
+  # ensure bad forward_vnc_ports cannot exist
+  def test_invalid_without_bad_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1 # too small
+     assert !vm.valid?, "forward vnc port must be >= 5900"
+  end
+
   # Ensures that, if the VM does not contain the Cobbler prefix, that it
   # does not claim to be a Cobbler VM.
   #
-- 
1.6.0.6

---
 src/app/controllers/vm_controller.rb |   52 ++++++++++++-
 src/app/models/vm.rb                 |   26 ++++++
 src/app/views/vm/_form.rhtml         |   15 ++++
 src/app/views/vm/show.rhtml          |    4 +
 src/db/migrate/034_add_vm_vnc.rb     |   30 +++++++
 src/task-omatic/taskomatic.rb        |    4 +
 src/task-omatic/vnc.rb               |  140 ++++++++++++++++++++++++++++++++++
 src/test/fixtures/vms.yml            |    2 +
 src/test/unit/vm_test.rb             |   16 ++++
 9 files changed, 285 insertions(+), 4 deletions(-)
 create mode 100644 src/db/migrate/034_add_vm_vnc.rb
 create mode 100644 src/task-omatic/vnc.rb

diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index 56501fd..196f6a1 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -62,7 +62,26 @@ class VmController < ApplicationController
   def create
     begin
       Vm.transaction do
-        @vm.save!
+        autoassigned = false
+        continue = true
+        while(continue) do
+          begin
+            if @vm.forward_vnc
+              if @vm.forward_vnc_port.to_i == 0
+                autoassigned = true
+                @vm.forward_vnc_port = Vm.available_forward_vnc_port
+              end
+            else
+                @vm.forward_vnc_port = nil
+            end
+
+            @vm.save!
+            continue = false
+          rescue ActiveRecord::RecordInvalid => e
+             raise e unless autoassigned && !
@vm.errors[:forward_vnc_port].nil?
+          end
+        end
+
         _setup_vm_provision(params)
         @task = VmTask.new({ :user        => @user,
                              :task_target => @vm,
@@ -116,8 +135,30 @@ class VmController < ApplicationController
         new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
         needs_restart = true unless current_storage_ids == new_storage_ids
       end
-      params[:vm][:needs_restart] = 1 if needs_restart
-      @vm.update_attributes!(params[:vm])
+
+      Vm.transaction do
+        autoassigned = false
+        continue = true
+        while(continue) do
+          begin
+            params[:vm][:forward_vnc] = params[:forward_vnc]
+            if params[:forward_vnc]
+              if params[:vm][:forward_vnc_port].to_i == 0
+                autoassigned = true
+                params[:vm][:forward_vnc_port] = Vm.available_forward_vnc_port
+              end
+            else
+                params[:vm][:forward_vnc_port] = nil
+            end
+
+            params[:vm][:needs_restart] = 1 if needs_restart
+            @vm.update_attributes!(params[:vm])
+            continue = false
+          rescue ActiveRecord::RecordInvalid => e
+             raise e unless autoassigned && !
@vm.errors[:forward_vnc_port].nil?
+          end
+        end
+      end
       _setup_vm_provision(params)
 
       if (params[:start_now] and
@vm.get_action_list.include?(VmTask::ACTION_START_VM) )
@@ -325,7 +366,8 @@ class VmController < ApplicationController
     newargs = {
       :vm_resource_pool_id => params[:vm_resource_pool_id],
       :vnic_mac_addr => mac.collect {|x| "%02x" %
x}.join(":"),
-      :uuid => uuid
+      :uuid => uuid,
+      :forward_vnc_port => 0
     }
     @vm = Vm.new( newargs )
     unless params[:vm_resource_pool_id]
@@ -345,6 +387,7 @@ class VmController < ApplicationController
       vm_resource_pool.create_with_parent(hardware_pool)
       params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
     end
+    params[:vm][:forward_vnc] = params[:forward_vnc]
     @vm = Vm.new(params[:vm])
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
@@ -356,6 +399,7 @@ class VmController < ApplicationController
   end
   def pre_edit
     @vm = Vm.find(params[:id])
+    @vm.forward_vnc_port = 0 unless @vm.forward_vnc
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
     _setup_provisioning_options
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index bf99e2d..1cfcc72 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -40,6 +40,17 @@ class Vm < ActiveRecord::Base
   validates_format_of :uuid,
      :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
 
+  FORWARD_VNC_PORT_START = 5900
+
+  validates_numericality_of :forward_vnc_port,
+     :message => 'must be >= ' + FORWARD_VNC_PORT_START.to_s,
+     :greater_than_or_equal_to => FORWARD_VNC_PORT_START,
+     :if => Proc.new { |vm| vm.forward_vnc }
+
+  validates_uniqueness_of :forward_vnc_port,
+    :message => "is already in use",
+    :if => Proc.new { |vm| vm.forward_vnc }
+
   validates_numericality_of :needs_restart,
      :greater_than_or_equal_to => 0,
      :less_than_or_equal_to => 1,
@@ -335,6 +346,21 @@ class Vm < ActiveRecord::Base
     super
   end
 
+  # find the first available vnc port
+  def self.available_forward_vnc_port
+    # FIXME need a way to reserve values returned
+    #  by this until table is saved
+
+    i = FORWARD_VNC_PORT_START
+    Vm.find(:all,
+            :conditions => "forward_vnc_port is not NULL",
+            :order => 'forward_vnc_port ASC' ).collect{ |vm|
+               return i if vm.forward_vnc_port > i
+               i = i + 1
+            }
+    return i
+  end
+
   protected
   def validate
     resources = vm_resource_pool.max_resources_for_vm(self)
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 7cbe16d..f1c0239 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,6 +51,21 @@
     <div class="clear_row"></div>
     <div class="clear_row"></div>
 
+    <div style="width: 50%; float: left;">
+    <%= check_box_tag_with_label "Forward vm's vnc
<b>port</b> locally", "forward_vnc", 1,
@vm.forward_vnc %>
+     (set to 0 to autoassign port)
+    </div>
+    <div style="width: 40%; float: left;">
+    <%= text_field_with_label "", "vm",
"forward_vnc_port", { :style=>"width: 80px;", :size =>
7, :disabled => ! @vm.forward_vnc } %>
+    </div>
+    <div style="clear:both;"></div>
+    <div class="clear_row"></div>
+    <script type="text/javascript">
+      $("#forward_vnc").click(function(){
+        $("#vm_forward_vnc_port").attr("disabled",
$("#forward_vnc").is(":checked") ? "" :
"disabled");
+      });
+    </script>
+
    <%= check_box_tag_with_label "Start VM Now? (pending current
resource availability)", "start_now", nil if create or @vm.state
== Vm::STATE_STOPPED %>
    <%= check_box_tag_with_label "Restart VM Now? (pending current
resource availability)", "restart_now", nil if @vm.state ==
Vm::STATE_RUNNING %>
 
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index f361131..add29b4 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -88,6 +88,7 @@
     <div id="vms_selection_id"
style="display:none"><%= @vm.id %></div>
     <div class="selection_key">
         Uuid:<br/>
+        <%= @vm.forward_vnc ? "VNC uri:<br/>" : ""
%>
 	Num vcpus allocated:<br/>
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
@@ -100,6 +101,9 @@
     </div>
     <div class="selection_value">
        <%=h @vm.uuid %><br/>
+       <%= url = request.url
+           url = request.url[0..(url.index('/', 8) - 1)] +
":" + @vm.forward_vnc_port.to_s
+           @vm.forward_vnc ? (url + "<br/>") : ""
%>
        <%=h @vm.num_vcpus_allocated %><br/>
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
diff --git a/src/db/migrate/034_add_vm_vnc.rb b/src/db/migrate/034_add_vm_vnc.rb
new file mode 100644
index 0000000..f23e24d
--- /dev/null
+++ b/src/db/migrate/034_add_vm_vnc.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi
+#
+# 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.
+
+class AddVmVnc < ActiveRecord::Migration
+  def self.up
+   add_column :vms, :forward_vnc, :bool, :default => false
+   add_column :vms, :forward_vnc_port, :int, :default => 0, :unique =>
true
+  end
+
+  def self.down
+   drop_column :vms, :forward_vnc
+   drop_column :vms, :forward_vnc_port
+  end
+end
+
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index 0570246..380be92 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -32,6 +32,7 @@ include Daemonize
 
 require 'task_vm'
 require 'task_storage'
+require 'vnc'
 
 class TaskOmatic
 
@@ -232,6 +233,8 @@ class TaskOmatic
       raise "Error destroying VM: #{result.text}" unless
result.status == 0
     end
 
+    VmVnc.close(vm)
+
     # undefine can fail, for instance, if we live migrated from A -> B, and
     # then we are shutting down the VM on B (because it only has
"transient"
     # XML).  Therefore, just ignore undefine errors so we do the rest
@@ -303,6 +306,7 @@ class TaskOmatic
     # of places so you'll see a lot of .reloads.
     db_vm.reload
     set_vm_vnc_port(db_vm, result.description) unless result.status != 0
+    VmVnc.forward(db_vm)
 
     # This information is not available via the libvirt interface.
     db_vm.memory_used = db_vm.memory_allocated
diff --git a/src/task-omatic/vnc.rb b/src/task-omatic/vnc.rb
new file mode 100644
index 0000000..0876f36
--- /dev/null
+++ b/src/task-omatic/vnc.rb
@@ -0,0 +1,140 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# 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.
+
+# provides static 'forward' and 'close' methods to forward a
specified vm's vnc connections
+class VmVnc
+
+  private
+
+    # TODO no ruby/libiptc wrapper exists, when
+    # it does replace iptables command w/ calls to it
+    IPTABLES_CMD='/sbin/iptables '
+
+    VNC_DEBUG = false
+
+    # FIXME can this be retreived in any way
+    # since machine will have both external
+    # and internal network interface
+    LOCAL_IP = '192.168.50.2'
+
+    def self.debug(msg)
+      puts "\n" + msg + "\n" if VNC_DEBUG
+    end
+
+    def self.find_host_ip(hostname)
+      # FIXME
+      addrinfo = Socket::getaddrinfo(hostname, nil)
+      unless addrinfo.size > 0
+        raise "Could not retreive address for " + hostname
+      end
+      result = addrinfo[0][3] # return ip address of first entry
+      debug( "vm host hostname  resolved to " + result.to_s )
+      return result
+    end
+
+    def self.port_open?(port)
+      cmd=IPTABLES_CMD + ' -t nat -nL '
+      debug("vncPortOpen? iptables command: " + cmd)
+
+      `#{cmd}`.each_line do |l|
+          return true if l =~ /.*#{port}.*/
+      end
+      return false
+    end
+
+    def self.get_forward_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+      return " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j ACCEPT",
+             " -s " + ip + " -p tcp --sport " +
vm.vnc_port.to_s + " -j ACCEPT"
+    end
+
+    def self.get_nat_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+
+      return " -p tcp --dport " + vm.forward_vnc_port.to_s + "
-j DNAT --to " + ip + ":" + vm.vnc_port.to_s,
+             " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j SNAT --to " + LOCAL_IP
+    end
+
+    def self.run_command(cmd)
+      debug("Running command " + cmd)
+      status = system(cmd)
+      raise 'Command terminated with error code ' + $?.to_s unless
status
+    end
+
+  public
+
+    def self.forward(vm)
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       if port_open?(vm.forward_vnc_port)
+         raise "Port already open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -A FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -A FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -A PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -A POSTROUTING " +
postrouting_rule
+
+       debug(" open\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       File::open("/proc/sys/net/ipv4/ip_forward", "w") {
|f| f.puts "1" }
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+
+    def self.close(vm)
+      # FIXME forward_vnc may have been changed while the vm is running
+      return unless vm.forward_vnc
+      unless vm.forward_vnc_port > 0
+        raise "Must specify valid port to forward " +
vm.forward_vnc_port.to_s
+      end
+
+       unless port_open?(vm.forward_vnc_port)
+         raise "Port not open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -D FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -D FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -D PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -D POSTROUTING " +
postrouting_rule
+
+       debug(" close\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+end
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index ca0d63f..366f192 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -11,6 +11,8 @@ production_httpd_vm:
   boot_device: hd
   host: prod_corp_com
   vm_resource_pool: corp_com_production_vmpool
+  forward_vnc: true
+  forward_vnc_port: 1234
 production_mysqld_vm:
   uuid: 89e62d32-04d9-4351-b573-b1a253397296
   description: production mysqld appliance
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index cba3188..b868dfa 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -95,6 +95,22 @@ class VmTest < Test::Unit::TestCase
        flunk 'Vm must specify valid state' if @vm.valid?
   end
 
+  # ensure duplicate forward_vnc_ports cannot exist
+  def test_invalid_without_unique_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1234 # duplicate
+     assert !vm.valid?, "forward vnc port must be unique"
+  end
+
+  # ensure bad forward_vnc_ports cannot exist
+  def test_invalid_without_bad_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1 # too small
+     assert !vm.valid?, "forward vnc port must be >= 5900"
+  end
+
   # Ensures that, if the VM does not contain the Cobbler prefix, that it
   # does not claim to be a Cobbler VM.
   #
-- 
1.6.0.6

---
 src/app/controllers/vm_controller.rb |    3 +
 src/app/models/vm.rb                 |   23 +++++
 src/app/views/vm/_form.rhtml         |    3 +
 src/app/views/vm/show.rhtml          |    4 +
 src/db/migrate/034_add_vm_vnc.rb     |   30 +++++++
 src/task-omatic/taskomatic.rb        |    4 +
 src/task-omatic/vnc.rb               |  154 ++++++++++++++++++++++++++++++++++
 src/test/fixtures/vms.yml            |    2 +
 src/test/unit/vm_test.rb             |   16 ++++
 9 files changed, 239 insertions(+), 0 deletions(-)
 create mode 100644 src/db/migrate/034_add_vm_vnc.rb
 create mode 100644 src/task-omatic/vnc.rb

diff --git a/src/app/controllers/vm_controller.rb
b/src/app/controllers/vm_controller.rb
index 56501fd..f104415 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -116,6 +116,8 @@ class VmController < ApplicationController
         new_storage_ids = new_storage_ids.sort.collect {|x| x.to_i }
         needs_restart = true unless current_storage_ids == new_storage_ids
       end
+
+      params[:vm][:forward_vnc] = params[:forward_vnc]
       params[:vm][:needs_restart] = 1 if needs_restart
       @vm.update_attributes!(params[:vm])
       _setup_vm_provision(params)
@@ -345,6 +347,7 @@ class VmController < ApplicationController
       vm_resource_pool.create_with_parent(hardware_pool)
       params[:vm][:vm_resource_pool_id] = vm_resource_pool.id
     end
+    params[:vm][:forward_vnc] = params[:forward_vnc]
     @vm = Vm.new(params[:vm])
     @perm_obj = @vm.vm_resource_pool
     @current_pool_id=@perm_obj.id
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index bf99e2d..b6b5457 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -40,6 +40,17 @@ class Vm < ActiveRecord::Base
   validates_format_of :uuid,
      :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
 
+  FORWARD_VNC_PORT_START = 5900
+
+  validates_numericality_of :forward_vnc_port,
+    :message => 'must be >= ' + FORWARD_VNC_PORT_START.to_s,
+    :greater_than_or_equal_to => FORWARD_VNC_PORT_START,
+    :if => Proc.new { |vm| vm.forward_vnc &&
!vm.forward_vnc_port.nil? }
+
+  validates_uniqueness_of :forward_vnc_port,
+    :message => "is already in use",
+    :if => Proc.new { |vm| vm.forward_vnc &&
!vm.forward_vnc_port.nil? }
+
   validates_numericality_of :needs_restart,
      :greater_than_or_equal_to => 0,
      :less_than_or_equal_to => 1,
@@ -335,6 +346,18 @@ class Vm < ActiveRecord::Base
     super
   end
 
+  # find the first available vnc port
+  def self.available_forward_vnc_port
+    i = FORWARD_VNC_PORT_START
+    Vm.find(:all,
+            :conditions => "forward_vnc_port is not NULL",
+            :order => 'forward_vnc_port ASC' ).collect{ |vm|
+               return i if vm.forward_vnc_port > i
+               i = i + 1
+            }
+    return i
+  end
+
   protected
   def validate
     resources = vm_resource_pool.max_resources_for_vm(self)
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index d1c0e45..79621ca 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,6 +51,9 @@
     <div class="clear_row"></div>
     <div class="clear_row"></div>
 
+    <%= check_box_tag_with_label "Forward vm's vnc port
locally", "forward_vnc", 1, @vm.forward_vnc %>
+    <div class="clear_row"></div>
+
    <%= check_box_tag_with_label "Start VM Now? (pending current
resource availability)", "start_now", nil if create or @vm.state
== Vm::STATE_STOPPED %>
    <%= check_box_tag_with_label "Restart VM Now? (pending current
resource availability)", "restart_now", nil if @vm.state ==
Vm::STATE_RUNNING %>
 
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index f361131..add29b4 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -88,6 +88,7 @@
     <div id="vms_selection_id"
style="display:none"><%= @vm.id %></div>
     <div class="selection_key">
         Uuid:<br/>
+        <%= @vm.forward_vnc ? "VNC uri:<br/>" : ""
%>
 	Num vcpus allocated:<br/>
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
@@ -100,6 +101,9 @@
     </div>
     <div class="selection_value">
        <%=h @vm.uuid %><br/>
+       <%= url = request.url
+           url = request.url[0..(url.index('/', 8) - 1)] +
":" + @vm.forward_vnc_port.to_s
+           @vm.forward_vnc ? (url + "<br/>") : ""
%>
        <%=h @vm.num_vcpus_allocated %><br/>
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
diff --git a/src/db/migrate/034_add_vm_vnc.rb b/src/db/migrate/034_add_vm_vnc.rb
new file mode 100644
index 0000000..f23e24d
--- /dev/null
+++ b/src/db/migrate/034_add_vm_vnc.rb
@@ -0,0 +1,30 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi
+#
+# 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.
+
+class AddVmVnc < ActiveRecord::Migration
+  def self.up
+   add_column :vms, :forward_vnc, :bool, :default => false
+   add_column :vms, :forward_vnc_port, :int, :default => 0, :unique =>
true
+  end
+
+  def self.down
+   drop_column :vms, :forward_vnc
+   drop_column :vms, :forward_vnc_port
+  end
+end
+
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index 0570246..380be92 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -32,6 +32,7 @@ include Daemonize
 
 require 'task_vm'
 require 'task_storage'
+require 'vnc'
 
 class TaskOmatic
 
@@ -232,6 +233,8 @@ class TaskOmatic
       raise "Error destroying VM: #{result.text}" unless
result.status == 0
     end
 
+    VmVnc.close(vm)
+
     # undefine can fail, for instance, if we live migrated from A -> B, and
     # then we are shutting down the VM on B (because it only has
"transient"
     # XML).  Therefore, just ignore undefine errors so we do the rest
@@ -303,6 +306,7 @@ class TaskOmatic
     # of places so you'll see a lot of .reloads.
     db_vm.reload
     set_vm_vnc_port(db_vm, result.description) unless result.status != 0
+    VmVnc.forward(db_vm)
 
     # This information is not available via the libvirt interface.
     db_vm.memory_used = db_vm.memory_allocated
diff --git a/src/task-omatic/vnc.rb b/src/task-omatic/vnc.rb
new file mode 100644
index 0000000..0fd3afd
--- /dev/null
+++ b/src/task-omatic/vnc.rb
@@ -0,0 +1,154 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# 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.
+
+# provides static 'forward' and 'close' methods to forward a
specified vm's vnc connections
+class VmVnc
+
+  private
+
+    # TODO no ruby/libiptc wrapper exists, when
+    # it does replace iptables command w/ calls to it
+    IPTABLES_CMD='/sbin/iptables '
+
+    VNC_DEBUG = false
+
+    # FIXME can this be retreived in any way
+    # since machine will have both external
+    # and internal network interface
+    LOCAL_IP = '192.168.50.2'
+
+    def self.debug(msg)
+      puts "\n" + msg + "\n" if VNC_DEBUG
+    end
+
+    def self.find_host_ip(hostname)
+      # FIXME
+      addrinfo = Socket::getaddrinfo(hostname, nil)
+      unless addrinfo.size > 0
+        raise "Could not retreive address for " + hostname
+      end
+      result = addrinfo[0][3] # return ip address of first entry
+      debug( "vm host hostname  resolved to " + result.to_s )
+      return result
+    end
+
+    def self.port_open?(port)
+      cmd=IPTABLES_CMD + ' -t nat -nL '
+      debug("vncPortOpen? iptables command: " + cmd)
+
+      `#{cmd}`.each_line do |l|
+          return true if l =~ /.*#{port}.*/
+      end
+      return false
+    end
+
+    def self.allocate_forward_vnc_port(vm)
+       Vm.transaction do
+         ActiveRecord::Base.connection.execute('LOCK TABLE vms')
+         vm.forward_vnc_port = Vm.available_forward_vnc_port
+         debug("Allocating forward vnc port " +
vm.forward_vnc_port.to_s)
+         vm.save!
+       end
+    end
+
+    def self.deallocate_forward_vnc_port(vm)
+       debug("Deallocating forward vnc port " +
vm.forward_vnc_port.to_s)
+       vm.forward_vnc_port = nil
+       vm.save!
+    end
+
+    def self.get_forward_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+      return " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j ACCEPT",
+             " -s " + ip + " -p tcp --sport " +
vm.vnc_port.to_s + " -j ACCEPT"
+    end
+
+    def self.get_nat_rules(vm)
+      ip = find_host_ip(vm.host.hostname)
+
+      return " -p tcp --dport " + vm.forward_vnc_port.to_s + "
-j DNAT --to " + ip + ":" + vm.vnc_port.to_s,
+             " -d " + ip + " -p tcp --dport " +
vm.vnc_port.to_s + " -j SNAT --to " + LOCAL_IP
+    end
+
+    def self.run_command(cmd)
+      debug("Running command " + cmd)
+      status = system(cmd)
+      raise 'Command terminated with error code ' + $?.to_s unless
status
+    end
+
+  public
+
+    def self.forward(vm)
+       return unless vm.forward_vnc
+
+       allocate_forward_vnc_port(vm)
+       if port_open?(vm.forward_vnc_port)
+         deallocate_forward_vnc_port(vm)
+         raise "Port already open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -A FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -A FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -A PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -A POSTROUTING " +
postrouting_rule
+
+       debug(" open\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+
+       File::open("/proc/sys/net/ipv4/ip_forward", "w") {
|f| f.puts "1" }
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+    end
+
+    def self.close(vm)
+       return unless vm.forward_vnc
+
+       unless port_open?(vm.forward_vnc_port)
+         raise "Port not open " + vm.forward_vnc_port.to_s
+       end
+
+       forward_rule1, forward_rule2 = get_forward_rules(vm)
+       forward_rule1 = IPTABLES_CMD + " -D FORWARD " + forward_rule1
+       forward_rule2 = IPTABLES_CMD + " -D FORWARD " + forward_rule2
+
+       prerouting_rule, postrouting_rule = get_nat_rules(vm)
+       prerouting_rule = IPTABLES_CMD + " -t nat -D PREROUTING " +
prerouting_rule
+       postrouting_rule = IPTABLES_CMD + " -t nat -D POSTROUTING " +
postrouting_rule
+
+       debug(" close\n forward rule 1: "     + forward_rule1 +
+              "\n forward_rule 2: "   + forward_rule2 +
+              "\n prerouting rule: "  + prerouting_rule +
+              "\n postrouting rule: " + postrouting_rule)
+
+       run_command(forward_rule1)
+       run_command(forward_rule2)
+       run_command(prerouting_rule)
+       run_command(postrouting_rule)
+
+
+       deallocate_forward_vnc_port(vm)
+    end
+end
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index ca0d63f..40997b0 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -11,6 +11,8 @@ production_httpd_vm:
   boot_device: hd
   host: prod_corp_com
   vm_resource_pool: corp_com_production_vmpool
+  forward_vnc: true
+  forward_vnc_port: 5900
 production_mysqld_vm:
   uuid: 89e62d32-04d9-4351-b573-b1a253397296
   description: production mysqld appliance
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index cba3188..b868dfa 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -95,6 +95,22 @@ class VmTest < Test::Unit::TestCase
        flunk 'Vm must specify valid state' if @vm.valid?
   end
 
+  # ensure duplicate forward_vnc_ports cannot exist
+  def test_invalid_without_unique_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1234 # duplicate
+     assert !vm.valid?, "forward vnc port must be unique"
+  end
+
+  # ensure bad forward_vnc_ports cannot exist
+  def test_invalid_without_bad_forward_vnc_port
+     vm = vms(:production_mysqld_vm)
+     vm.forward_vnc = true
+     vm.forward_vnc_port = 1 # too small
+     assert !vm.valid?, "forward vnc port must be >= 5900"
+  end
+
   # Ensures that, if the VM does not contain the Cobbler prefix, that it
   # does not claim to be a Cobbler VM.
   #
-- 
1.6.0.6