[Ovirt-devel] [PATCH server] remove vm forward vnc and vm host history
Mohammed Morsi
mmorsi at redhat.com
Mon Jul 13 18:25:53 UTC 2009
since the introduction of ovirt-vnc-proxy and the updating
of ovirt-viewer to user it the forward vnc functionality is
no longer necessary / and incompatable w/ the current system.
---
src/app/controllers/vm_controller.rb | 2 -
src/app/models/host.rb | 9 -
src/app/models/vm.rb | 36 ----
src/app/models/vm_host_history.rb | 22 ---
src/app/views/vm/_form.rhtml | 3 -
src/app/views/vm/show.rhtml | 4 -
src/db-omatic/db_omatic.rb | 31 ----
src/db-omatic/vnc.rb | 173 --------------------
.../040_remove_vm_history_and_forward_vnc.rb | 65 ++++++++
src/test/fixtures/vms.yml | 2 -
src/test/unit/vm_test.rb | 16 --
11 files changed, 65 insertions(+), 298 deletions(-)
delete mode 100644 src/app/models/vm_host_history.rb
delete mode 100644 src/db-omatic/vnc.rb
create mode 100644 src/db/migrate/040_remove_vm_history_and_forward_vnc.rb
diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb
index 197241d..11b68ed 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -61,7 +61,6 @@ class VmController < ApplicationController
end
def create
- params[:vm][:forward_vnc] = params[:forward_vnc]
alert = svc_create(params[:vm], params[:start_now])
render :json => { :object => "vm", :success => true, :alert => alert }
end
@@ -75,7 +74,6 @@ class VmController < ApplicationController
end
def update
- params[:vm][:forward_vnc] = params[:forward_vnc]
alert = svc_update(params[:id], params[:vm], params[:start_now],
params[:restart_now])
render :json => { :object => "vm", :success => true, :alert => alert }
diff --git a/src/app/models/host.rb b/src/app/models/host.rb
index 588137b..4975205 100644
--- a/src/app/models/host.rb
+++ b/src/app/models/host.rb
@@ -54,15 +54,6 @@ class Host < ActiveRecord::Base
has_many :smart_pool_tags, :as => :tagged, :dependent => :destroy
has_many :smart_pools, :through => :smart_pool_tags
- # reverse cronological collection of vm history
- # each collection item contains vm that was running on host
- # time started, and time ended (see VmHostHistory)
- has_many :vm_host_histories,
- :order => 'time_started DESC',
- :dependent => :destroy
-
- alias history vm_host_histories
-
acts_as_xapian :texts => [ :hostname, :uuid, :hypervisor_type, :arch ],
:values => [ [ :created_at, 0, "created_at", :date ],
[ :updated_at, 1, "updated_at", :date ] ],
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index 6d0f864..14ad14c 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -34,14 +34,6 @@ class Vm < ActiveRecord::Base
has_many :smart_pool_tags, :as => :tagged, :dependent => :destroy
has_many :smart_pools, :through => :smart_pool_tags
- # reverse cronological collection of vm history
- # each collection item contains host vm was running on,
- # time started, and time ended (see VmHostHistory)
- has_many :vm_host_histories,
- :order => 'time_started DESC',
- :dependent => :destroy
-
-
has_many :vm_state_change_events,
:order => 'created_at' do
def previous_state_with_type(state_type)
@@ -59,22 +51,6 @@ 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 = 5901
-
- 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? &&
- vm.forward_vnc_port != 0 }
-
- validates_uniqueness_of :forward_vnc_port,
- :message => "is already in use",
- :if => Proc.new { |vm| vm.forward_vnc &&
- !vm.forward_vnc_port.nil? &&
- vm.forward_vnc_port != 0 }
-
-
validates_numericality_of :needs_restart,
:greater_than_or_equal_to => 0,
:less_than_or_equal_to => 1,
@@ -408,18 +384,6 @@ 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' ).each{ |vm|
- break if vm.forward_vnc_port > i
- i = i + 1
- }
- return i
- end
-
def self.calc_uptime
"vms.*, case when state='running' then
(cast(total_uptime || ' sec' as interval) +
diff --git a/src/app/models/vm_host_history.rb b/src/app/models/vm_host_history.rb
deleted file mode 100644
index bd61ddc..0000000
--- a/src/app/models/vm_host_history.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-# 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.
-
-class VmHostHistory < ActiveRecord::Base
- belongs_to :vm
- belongs_to :host
-end
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 034c3df..a08f2a4 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -51,9 +51,6 @@
<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 ffe5055..0bdf1e9 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -101,7 +101,6 @@
<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/>
@@ -115,9 +114,6 @@
</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-omatic/db_omatic.rb b/src/db-omatic/db_omatic.rb
index 155ff5e..b469695 100755
--- a/src/db-omatic/db_omatic.rb
+++ b/src/db-omatic/db_omatic.rb
@@ -10,7 +10,6 @@ require 'dutils'
require 'daemons'
require 'optparse'
require 'logger'
-require 'vnc'
include Daemonize
@@ -159,36 +158,6 @@ class DbOmatic < Qpid::Qmf::Console
end
end
- begin
- # find open vm host history for this vm,
- history = VmHostHistory.find(:first, :conditions => ["vm_id = ? AND time_ended is NULL", vm.id])
-
- if state == Vm::STATE_RUNNING
- if history.nil?
- history = VmHostHistory.new
- history.vm = vm
- history.host = vm.host
- history.vnc_port = vm.vnc_port
- history.state = state
- history.time_started = Time.now
- history.save!
- end
-
- VmVnc.forward(vm)
- elsif state != Vm::STATE_PENDING
- VmVnc.close(vm, history)
-
- unless history.nil? # throw an exception if this fails?
- history.time_ended = Time.now
- history.state = state
- history.save!
- end
- end
-
- rescue Exception => e # just log any errors here
- @logger.error "Error with VM #{domain['name']} operation: " + e
- end
-
@logger.info "Updating VM #{domain['name']} to state #{state}"
if state == Vm::STATE_STOPPED
diff --git a/src/db-omatic/vnc.rb b/src/db-omatic/vnc.rb
deleted file mode 100644
index d826841..0000000
--- a/src/db-omatic/vnc.rb
+++ /dev/null
@@ -1,173 +0,0 @@
-# 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
-
- 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, history_record = nil)
- if history_record.nil?
- ip = find_host_ip(vm.host.hostname)
- vnc_port = vm.vnc_port.to_s
- else
- ip = find_host_ip(history_record.host.hostname)
- vnc_port = history_record.vnc_port.to_s
- end
-
- return " -d " + ip + " -p tcp --dport " + vnc_port + " -j ACCEPT",
- " -s " + ip + " -p tcp --sport " + vnc_port + " -j ACCEPT"
- end
-
- def self.get_nat_rules(vm, history_record = nil)
- if history_record.nil?
- ip = find_host_ip(vm.host.hostname)
- vnc_port = vm.vnc_port.to_s
- else
- ip = find_host_ip(history_record.host.hostname)
- vnc_port = history_record.vnc_port.to_s
- end
-
- server,port = get_srv('ovirt', 'tcp')
- local_ip = find_host_ip(server)
- return " -p tcp --dport " + vm.forward_vnc_port.to_s + " -j DNAT --to " + ip + ":" + vnc_port,
- " -d " + ip + " -p tcp --dport " + vnc_port + " -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, history_record = nil)
- 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, history_record)
- forward_rule1 = IPTABLES_CMD + " -D FORWARD " + forward_rule1
- forward_rule2 = IPTABLES_CMD + " -D FORWARD " + forward_rule2
-
- prerouting_rule, postrouting_rule = get_nat_rules(vm, history_record)
- 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
-
- # should be used in cases which
- # closing iptables ports is not needed
- # (ex. on server startup)
- def self.deallocate_all
- Vm.find(:all).each{ |vm|
- deallocate_forward_vnc_port(vm)
- }
- end
-end
diff --git a/src/db/migrate/040_remove_vm_history_and_forward_vnc.rb b/src/db/migrate/040_remove_vm_history_and_forward_vnc.rb
new file mode 100644
index 0000000..b815f18
--- /dev/null
+++ b/src/db/migrate/040_remove_vm_history_and_forward_vnc.rb
@@ -0,0 +1,65 @@
+# 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.
+
+# reverses 034_add_vm_vnc migration and 036_vm_host_history
+# pretty much copied the self.up from 034 / 036 into self.down
+# and self.down from 034 / 036 into self.up
+class RemoveVmHistoryAndForwardVnc < ActiveRecord::Migration
+ def self.up
+ # 034 self.down
+ remove_column :vms, :forward_vnc
+ remove_column :vms, :forward_vnc_port
+
+ # 036 self.down
+ drop_table :vm_host_histories
+ end
+
+ def self.down
+ # 034 self.up
+ add_column :vms, :forward_vnc, :bool, :default => false
+ add_column :vms, :forward_vnc_port, :int, :default => 0, :unique => true
+
+ # 036 self.up
+ # this table gets populated in db-omatic
+ create_table :vm_host_histories do |t|
+ # vm / host association
+ t.integer :vm_id
+ t.integer :host_id
+
+ # records operating info of vm
+ # (most likey we will want to add a
+ # slew of more info here or in a future
+ # migration)
+ t.integer :vnc_port
+ t.string :state
+
+ # start / end timestamps
+ t.timestamp :time_started
+ t.timestamp :time_ended
+ end
+
+ execute "alter table vm_host_histories add constraint
+ fk_vm_host_histories_vms foreign key (vm_id) references vms(id)"
+
+ execute "alter table vm_host_histories add constraint
+ fk_vm_host_histories_hosts foreign key (host_id) references
+ hosts(id)"
+
+ end
+end
+
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index b2711b2..0d2caa3 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -11,8 +11,6 @@ production_httpd_vm:
boot_device: hd
host: prod_corp_com
vm_resource_pool: corp_com_production_vmpool
- forward_vnc: true
- forward_vnc_port: 5901
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 a28f183..d7b9d40 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -96,22 +96,6 @@ class VmTest < ActiveSupport::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
More information about the ovirt-devel
mailing list