[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
Mohammed Morsi
mmorsi at redhat.com
Fri Mar 6 14:13:11 UTC 2009
(major changes)
- new db migration (035) updating db model to contain additional
fields / constraints
- add host_id / network_id uniqueness constraint to nics and
bondings table (a host may only have one nic / bonding on
any given network). added validations to models to support
this, and filter network list presented on edit nics / bonding
form to only present networks not already selected on host
- associate vm / network tables (many vms to one network)
- add network drop down to vm form
- bonding form bugfix, only display nics not already in a bonding
- edit host browser to store interface name upon retreiving nic
- remove network validation from nics / bondings, changes to
net controller to remove assumption that nics / bondings are
always associated w/ network
- remove routing info related fields (netmask, broadcast, gateway)
from create / edit ip address form when displayed as part of
edit nics / bonding interface
(minor changes)
- display associate usages on network details pane
- change wording on network details pane from 'edit ip addresses'
to 'edit routing info'
- add interface name and network name (if a network is associated)
next to each nic in host details pane
- move vm uuid up to top of vm form as it doesn't belong under
networks section
---
src/app/controllers/network_controller.rb | 105 +++++++++++++++-----
src/app/controllers/vm_controller.rb | 2 +
src/app/models/bonding.rb | 9 +-
src/app/models/ip_address.rb | 4 -
src/app/models/network.rb | 2 +
src/app/models/nic.rb | 10 ++-
src/app/models/vm.rb | 2 +
src/app/views/host/edit_network.rhtml | 2 +-
src/app/views/host/show.rhtml | 6 +-
src/app/views/network/_bonding_form.rhtml | 2 +-
src/app/views/network/_ip_address_form.rhtml | 40 ++++----
src/app/views/network/_select.rhtml | 2 +-
.../views/network/edit_network_ip_addresses.rhtml | 2 +-
src/app/views/network/edit_nic.rhtml | 4 +-
src/app/views/network/show.rhtml | 7 +-
src/app/views/vm/_form.rhtml | 3 +-
.../035_networks_nics_bondings_additions.rb | 55 ++++++++++
src/host-browser/host-browser.rb | 10 +--
src/task-omatic/task_vm.rb | 9 +-
src/task-omatic/taskomatic.rb | 25 +++++-
src/test/fixtures/networks.yml | 9 ++
src/test/unit/bonding_test.rb | 4 +-
22 files changed, 235 insertions(+), 79 deletions(-)
create mode 100644 src/db/migrate/035_networks_nics_bondings_additions.rb
diff --git a/src/app/controllers/network_controller.rb b/src/app/controllers/network_controller.rb
index 7328e66..a99cf8d 100644
--- a/src/app/controllers/network_controller.rb
+++ b/src/app/controllers/network_controller.rb
@@ -272,7 +272,16 @@ class NetworkController < ApplicationController
@nic = Nic.find(params[:id])
@network = @nic.physical_network
- @networks = PhysicalNetwork.find(:all)
+ # filter out networks already assigned to nics on host
+ network_conditions = []
+ @nic.host.nics.each { |nic|
+ unless nic.physical_network.nil? || nic.id == @nic.id
+ network_conditions.push(" id != " + nic.physical_network.id.to_s)
+ end
+ }
+ network_conditions = network_conditions.join(" AND ")
+
+ @networks = PhysicalNetwork.find(:all, :conditions => network_conditions)
network_options
render :layout => false
@@ -281,14 +290,16 @@ class NetworkController < ApplicationController
def update_nic
begin
network_options
- @network = Network.find(params[:nic][:physical_network_id])
- if @network.boot_type.id == @static_boot_type.id
- if params[:ip_address][:id] == "New"
- _create_ip_address
- elsif params[:ip_address][:id] != ""
- _update_ip_address(params[:ip_address][:id])
- end
+ unless params[:nic][:physical_network_id].nil? || params[:nic][:physical_network_id].to_i == 0
+ @network = Network.find(params[:nic][:physical_network_id])
+ if @network.boot_type.id == @static_boot_type.id
+ if params[:ip_address][:id] == "New"
+ _create_ip_address
+ elsif params[:ip_address][:id] != ""
+ _update_ip_address(params[:ip_address][:id])
+ end
+ end
end
@nic = Nic.find(params[:id])
@@ -313,13 +324,31 @@ class NetworkController < ApplicationController
########################## Bonding related actions
def new_bonding
- unless params[:host_id]
+ unless params[:host_id]
flash[:notice] = "Host is required."
redirect_to :controller => 'dashboard'
end
@host = Host.find(params[:host_id])
- @networks = Vlan.find(:all)
+
+ # FIXME when bonding_nics table is removed, and
+ # bondings_id column added to nics table, simplify
+ # (select where bonding.nil?)
+ @nics = []
+ @host.nics.each{ |nic|
+ @nics.push(nic) if nic.bondings.nil? || nic.bondings.size == 0
+ }
+
+ # filter out networks already assigned to bondings on host
+ network_conditions = []
+ @host.bondings.each { |bonding|
+ unless bonding.vlan.nil?
+ network_conditions.push(" id != " + bonding.vlan.id.to_s)
+ end
+ }
+ network_conditions = network_conditions.join(" AND ")
+
+ @networks = Vlan.find(:all, :conditions => network_conditions)
network_options
render :layout => false
@@ -328,13 +357,15 @@ class NetworkController < ApplicationController
def create_bonding
begin
network_options
- @network = Network.find(params[:bonding][:vlan_id])
- if @network.boot_type.id == @static_boot_type.id
- if params[:ip_address][:id] == "New"
- _create_ip_address
- elsif params[:ip_address][:id] != ""
- _update_ip_address(params[:ip_address][:id])
+ unless params[:bonding][:vlan_id].nil? || params[:bonding][:vlan_id].to_i == 0
+ @network = Network.find(params[:bonding][:vlan_id])
+ if @network.boot_type.id == @static_boot_type.id
+ if params[:ip_address][:id] == "New"
+ _create_ip_address
+ elsif params[:ip_address][:id] != ""
+ _update_ip_address(params[:ip_address][:id])
+ end
end
end
@@ -368,7 +399,29 @@ class NetworkController < ApplicationController
@network = @bonding.vlan
@host = @bonding.host
- @networks = Vlan.find(:all)
+
+ # FIXME when bonding_nics table is removed, and
+ # bondings_id column added to nics table, simplify
+ # (select where bonding.nil? or bonding has nic)
+ @nics = []
+ @host.nics.each{ |nic|
+ if nic.bondings.nil? ||
+ nic.bondings.size == 0 ||
+ nic.bondings[0].id == @bonding.id
+ @nics.push(nic)
+ end
+ }
+
+ # filter out networks already assigned to bondings on host
+ network_conditions = []
+ @host.bondings.each { |bonding|
+ unless bonding.vlan.nil? || bonding.id == @bonding.id
+ network_conditions.push(" id != " + bonding.vlan.id.to_s)
+ end
+ }
+ network_conditions = network_conditions.join(" AND ")
+
+ @networks = Vlan.find(:all, :conditions => network_conditions)
network_options
render :layout => false
@@ -377,13 +430,15 @@ class NetworkController < ApplicationController
def update_bonding
begin
network_options
- @network = Network.find(params[:bonding][:vlan_id])
- if @network.boot_type.id == @static_boot_type.id
- if params[:ip_address][:id] == "New"
- _create_ip_address
- elsif params[:ip_address][:id] != ""
- _update_ip_address(params[:ip_address][:id])
+ unless params[:bonding][:vlan_id].nil? || params[:bonding][:vlan_id].to_i == 0
+ @network = Network.find(params[:bonding][:vlan_id])
+ if @network.boot_type.id == @static_boot_type.id
+ if params[:ip_address][:id] == "New"
+ _create_ip_address
+ elsif params[:ip_address][:id] != ""
+ _update_ip_address(params[:ip_address][:id])
+ end
end
end
@@ -394,13 +449,13 @@ class NetworkController < ApplicationController
alert = "Bonding was successfully updated."
render :json => { :object => "bonding", :success => true,
:alert => alert }
- rescue
+ rescue Exception => e
if @ip_address and @ip_address.errors.size != 0
render :json => { :object => "ip_address", :success => false,
:errors =>
@ip_address.errors.localize_error_messages.to_a}
else
- render :json => { :object => "bonding", :success => false,
+ render :json => { :object => "bonding", :success => false,
:errors =>
@bonding.errors.localize_error_messages.to_a }
end
diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb
index f104415..aa575c6 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -335,6 +335,7 @@ class VmController < ApplicationController
end
@perm_obj = @vm.vm_resource_pool
@current_pool_id=@perm_obj.id
+ @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
_setup_provisioning_options
end
def pre_create
@@ -361,6 +362,7 @@ class VmController < ApplicationController
@vm = Vm.find(params[:id])
@perm_obj = @vm.vm_resource_pool
@current_pool_id=@perm_obj.id
+ @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
_setup_provisioning_options
end
def pre_vm_action
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index a67b97b..32b9a37 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -37,11 +37,13 @@ class Bonding < ActiveRecord::Base
belongs_to :vlan
has_many :ip_addresses, :dependent => :destroy
+ # FIXME bondings_nics table should just be replaced with
+ # bonding_id column in nics table, and relationship changed
+ # here to has_many
has_and_belongs_to_many :nics,
:join_table => 'bondings_nics',
:foreign_key => :bonding_id
-
validates_presence_of :name,
:message => 'A name is required.'
@@ -57,8 +59,9 @@ class Bonding < ActiveRecord::Base
validates_presence_of :bonding_type_id,
:message => 'A bonding type must be specified.'
- validates_presence_of :vlan_id,
- :message => 'A vlan must be specified.'
+ validates_uniqueness_of :vlan_id,
+ :scope => :host_id,
+ :unless => Proc.new { |bonding| bonding.vlan.nil? }
# verify arp ping address to be ipv4 if set
validates_format_of :arp_ping_address,
diff --git a/src/app/models/ip_address.rb b/src/app/models/ip_address.rb
index 3f246b1..5d2e6af 100644
--- a/src/app/models/ip_address.rb
+++ b/src/app/models/ip_address.rb
@@ -24,8 +24,4 @@ class IpAddress < ActiveRecord::Base
belongs_to :network
belongs_to :nic
belongs_to :bonding
-
- def validate
- errors.add("id", "ip must be associated with foreign entity") if network_id.nil? and nic_id.nil? and bonding_id.nil?
- end
end
diff --git a/src/app/models/network.rb b/src/app/models/network.rb
index 404633d..0ad38bb 100644
--- a/src/app/models/network.rb
+++ b/src/app/models/network.rb
@@ -22,6 +22,8 @@ class Network < ActiveRecord::Base
has_and_belongs_to_many :usages, :join_table => 'networks_usages'
+ has_many :vms
+
validates_presence_of :type,
:message => 'A type must be specified.'
validates_presence_of :name,
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index e43f067..e26c110 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -22,6 +22,9 @@ class Nic < ActiveRecord::Base
belongs_to :physical_network
has_many :ip_addresses, :dependent => :destroy
+ # FIXME bondings_nics table should just be replaced with
+ # bonding_id column in nics table, and relationship changed
+ # here to belongs_to
has_and_belongs_to_many :bondings, :join_table => 'bondings_nics'
validates_presence_of :mac,
@@ -33,12 +36,13 @@ class Nic < ActiveRecord::Base
validates_presence_of :host_id,
:message => 'A host must be specified.'
- validates_presence_of :physical_network_id,
- :message => 'A network must be specified.'
-
validates_numericality_of :bandwidth,
:greater_than_or_equal_to => 0
+ validates_uniqueness_of :physical_network_id,
+ :scope => :host_id,
+ :unless => Proc.new { |nic| nic.physical_network_id.nil? }
+
# validate 'bridge' or 'usage_type' attribute ?
protected
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index 83aa410..e56d6dd 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -29,6 +29,8 @@ class Vm < ActiveRecord::Base
end
has_and_belongs_to_many :storage_volumes
+ belongs_to :network
+
has_many :smart_pool_tags, :as => :tagged, :dependent => :destroy
has_many :smart_pools, :through => :smart_pool_tags
diff --git a/src/app/views/host/edit_network.rhtml b/src/app/views/host/edit_network.rhtml
index 7ec3180..760f508 100644
--- a/src/app/views/host/edit_network.rhtml
+++ b/src/app/views/host/edit_network.rhtml
@@ -9,7 +9,7 @@
<div id="select-host-nic" class="popup-content-selection">
<%= select_with_label "NICs", "nic", "id",
@host.nics.
- collect{ |nic| [nic.mac, nic.id] }.
+ collect{ |nic| [nic.interface_name.to_s + " " + nic.mac, nic.id] }.
insert(0, "") %>
</div>
diff --git a/src/app/views/host/show.rhtml b/src/app/views/host/show.rhtml
index b671578..1e0787e 100644
--- a/src/app/views/host/show.rhtml
+++ b/src/app/views/host/show.rhtml
@@ -62,7 +62,11 @@
<%=h @host.arch %><br/>
<%=h @host.hypervisor_type %><br/>
<%=h @host.status_str %><br/>
- <%= @host.nics.collect{ |n| n.mac }.join("<br/>") %><br/>
+ <%= @host.nics.collect{ |n|
+ n.interface_name.to_s + " " + n.mac +
+ (n.physical_network.nil? ? "" : " " + n.physical_network.name)
+ }.join("<br/>")
+ %><br/>
<%= @host.bondings.collect { |n| n.name }.join("<br/>") %><br/>
<%= @host.vms.collect{|x| x.uuid }.join(" <br/> ") %><br/>
diff --git a/src/app/views/network/_bonding_form.rhtml b/src/app/views/network/_bonding_form.rhtml
index 4ec0df1..663c537 100644
--- a/src/app/views/network/_bonding_form.rhtml
+++ b/src/app/views/network/_bonding_form.rhtml
@@ -24,7 +24,7 @@
<div class="selected_popup_content_left">NICs</div>
<div class="selected_popup_content_right">
<select id="bonding_nic_ids" name="bonding[nic_ids][]" multiple="true">
- <%= options_from_collection_for_select @host.nics, "id", "mac",
+ <%= options_from_collection_for_select @nics, "id", "mac",
@bonding ? @bonding.nics.collect{ |x| x.nic_id.to_i } : [] %>
</select>
</div>
diff --git a/src/app/views/network/_ip_address_form.rhtml b/src/app/views/network/_ip_address_form.rhtml
index b952807..a3b3a95 100644
--- a/src/app/views/network/_ip_address_form.rhtml
+++ b/src/app/views/network/_ip_address_form.rhtml
@@ -21,31 +21,33 @@
</div>
</div>
- <div id="static_ip_v4_options">
- <div class="selected_nic_bonding_left">Netmask</div>
- <div class="selected_nic_bonding_right">
- <%= text_field_with_label "", "ip_address", "netmask" %>
- </div>
+ <% if @parent_type == 'network' %>
+ <div id="static_ip_v4_options">
+ <div class="selected_nic_bonding_left">Netmask</div>
+ <div class="selected_nic_bonding_right">
+ <%= text_field_with_label "", "ip_address", "netmask" %>
+ </div>
- <div class="selected_nic_bonding_left">Broadcast</div>
- <div class="selected_nic_bonding_right">
- <%= text_field_with_label "", "ip_address", "broadcast" %>
+ <div class="selected_nic_bonding_left">Broadcast</div>
+ <div class="selected_nic_bonding_right">
+ <%= text_field_with_label "", "ip_address", "broadcast" %>
+ </div>
</div>
- </div>
- <div id="static_ip_v6_options" style="display: none;">
- <div class="selected_nic_bonding_left">Prefix</div>
- <div class="selected_nic_bonding_right">
- <%= text_field_with_label "", "ip_address", "prefix" %>
+ <div id="static_ip_v6_options" style="display: none;">
+ <div class="selected_nic_bonding_left">Prefix</div>
+ <div class="selected_nic_bonding_right">
+ <%= text_field_with_label "", "ip_address", "prefix" %>
+ </div>
</div>
- </div>
- <div class="static_ip_common_options">
- <div class="selected_nic_bonding_left">Gateway</div>
- <div class="selected_nic_bonding_right">
- <%= text_field_with_label "", "ip_address", "gateway" %>
+ <div class="static_ip_common_options">
+ <div class="selected_nic_bonding_left">Gateway</div>
+ <div class="selected_nic_bonding_right">
+ <%= text_field_with_label "", "ip_address", "gateway" %>
+ </div>
</div>
- </div>
+ <% end %>
</div>
diff --git a/src/app/views/network/_select.rhtml b/src/app/views/network/_select.rhtml
index e69d9b0..4d056df 100644
--- a/src/app/views/network/_select.rhtml
+++ b/src/app/views/network/_select.rhtml
@@ -7,7 +7,7 @@
<div class="selected_popup_content_left">Network:</div>
<div class="selected_popup_content_right">
<%= select_with_label "", target, network_id,
- @networks.collect { |n| [n.name + ' - ' + n.boot_type.label, n.id ] } %>
+ @networks.collect { |n| [n.name + ' - ' + n.boot_type.label, n.id ] }.insert(0, "") %>
</div>
diff --git a/src/app/views/network/edit_network_ip_addresses.rhtml b/src/app/views/network/edit_network_ip_addresses.rhtml
index 7a1e4cb..78d6ad1 100644
--- a/src/app/views/network/edit_network_ip_addresses.rhtml
+++ b/src/app/views/network/edit_network_ip_addresses.rhtml
@@ -1,5 +1,5 @@
<%- content_for :title do -%>
- Edit Network IP Addresses
+ Edit Network Routing Info
<%- end -%>
<%- content_for :description do -%>
<%- end -%>
diff --git a/src/app/views/network/edit_nic.rhtml b/src/app/views/network/edit_nic.rhtml
index 75af6fb..1b58c20 100644
--- a/src/app/views/network/edit_nic.rhtml
+++ b/src/app/views/network/edit_nic.rhtml
@@ -9,7 +9,7 @@
<div id="selected_popup_content_expanded" class="dialog_form">
<%= hidden_field_tag 'id', @nic.id %>
<%= hidden_field_tag 'nic_host_id', @nic.host.id %>
- <%= hidden_field_tag 'nic_network_id', @nic.physical_network.id %>
+ <%= hidden_field_tag 'nic_network_id', @nic.physical_network.id if @nic.physical_network %>
<div class="selected_popup_content_left">MAC:</div>
<div class="selected_popup_content_right"><%= @nic.mac %></div>
@@ -30,7 +30,7 @@
<%= render :partial => 'select' %>
<div id="static_ip_options"
- style="<% if @network.boot_type_id != @static_boot_type.id %>
+ style="<% if @network.nil? || @network.boot_type_id != @static_boot_type.id %>
display: none;<%end %>">
<%= render :partial => 'ip_addresses_form',
:locals => { :parent_type => 'nic',
diff --git a/src/app/views/network/show.rhtml b/src/app/views/network/show.rhtml
index ebb9402..9d07151 100644
--- a/src/app/views/network/show.rhtml
+++ b/src/app/views/network/show.rhtml
@@ -7,7 +7,7 @@
{:action => 'edit', :id => @network.id },
:rel=>"facebox[.bolder]", :class=>"selection_facebox" %>
- <%= link_to image_tag("icon_edit.png") + "Edit IP Addresses",
+ <%= link_to image_tag("icon_edit.png") + "Edit Routing Info",
{:action => 'edit_network_ip_addresses', :id => @network.id },
:rel=>"facebox[.bolder]", :class=>"selection_facebox" %>
<%- end -%>
@@ -17,13 +17,16 @@
Type:<br/>
Boot Type:<br/>
IP Addresses:<br/>
+ Usage(s):<br/>
</div>
<div class="selection_value">
<%=h @network.name %><br/>
<%=h @network.type %><br/>
<%=h @network.boot_type.label %><br/>
<%=@network.ip_addresses.collect{ |ip|
- ip.address}.join("<br/>") %>
+ ip.address}.join("<br/>") %><br/>
+ <%=@network.usages.collect{ |usage|
+ usage.label}.join(" ") %><br/>
</div>
<%- content_for :right do -%>
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 79621ca..58a3d61 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -8,6 +8,7 @@
<%= hidden_field_tag 'hardware_pool_id', @hardware_pool.id if @hardware_pool %>
<%= text_field_with_label "Name:", "vm", "description", {:style=>"width:250px;"} %>
+ <%= text_field_with_label "UUID:", "vm", "uuid", {:style=>"width:250px;"} %>
<%= select_with_label "Operating System:", 'vm', 'provisioning_and_boot_settings', @provisioning_options, :style=>"width:250px;" %>
<% if controller.action_name == "edit" %><b style="color: #FF0000">*Warning* Editing provision could overwrite vm</b><% end %>
<div class="clear_row" style="height:15px;"></div>
@@ -45,7 +46,7 @@
<%= text_field_with_label "VNIC:", "vm", "vnic_mac_addr", {:style=>"width:250;"} %>
</div>
<div style="float:left;">
- <%= text_field_with_label "UUID:", "vm", "uuid", {:style=>"width:200px;"} %>
+ <%= select_with_label "Network:", 'vm', 'network_id', @networks.insert(0, ""), :style=>"width:250px;" %>
</div>
<div style="clear:both;"></div>
<div class="clear_row"></div>
diff --git a/src/db/migrate/035_networks_nics_bondings_additions.rb b/src/db/migrate/035_networks_nics_bondings_additions.rb
new file mode 100644
index 0000000..242eb50
--- /dev/null
+++ b/src/db/migrate/035_networks_nics_bondings_additions.rb
@@ -0,0 +1,55 @@
+# 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.
+
+# adds
+# 'interface_name' column to the nics table
+# 'network_id' to vms table
+# host_id / network_id constraints to nics / bondings
+#
+class NetworksNicsBondingsAdditions < ActiveRecord::Migration
+ def self.up
+ add_column :nics, :interface_name, :string
+ add_column :vms, :network_id, :integer
+
+ dhcp_boot_type = BootType.find(:first, :conditions=>"proto='dhcp'")
+ usages = Usage.find(:all)
+
+ # drop all physical_network_ids and vlan_ids
+ execute 'update nics set physical_network_id = NULL'
+ execute 'update bondings set vlan_id = NULL'
+
+ execute "alter table nics add constraint
+ host_physical_network_unique unique
+ (host_id, physical_network_id)"
+
+ execute "alter table bondings add constraint
+ host_vlan_unique unique
+ (host_id, vlan_id)"
+
+ execute "alter table vms add constraint
+ fk_vm_network_id
+ foreign key (network_id) references
+ networks(id)"
+ end
+
+ def self.down
+ remove_column :nics, :interface_name
+ remove_column :vms, :network_id
+ end
+end
+
diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
index 579f241..1c5cf83 100755
--- a/src/host-browser/host-browser.rb
+++ b/src/host-browser/host-browser.rb
@@ -264,13 +264,6 @@ class HostBrowser
host.cpus << detail
end
- # Create a new network for the host
- boot_type = BootType.find_by_proto('dhcp')
- network_name = (host.uuid ? host.uuid : "") + ' Physical Network'
- network = PhysicalNetwork.create(
- :name => network_name,
- :boot_type_id => boot_type.id)
-
# Update the NIC details for this host:
# -if the NIC exists, then update the IP address
# -if the NIC does not exist, create it
@@ -291,6 +284,7 @@ class HostBrowser
updated_nic = Nic.find_by_id(nic.id)
updated_nic.bandwidth = detail['BANDWIDTH'].to_i
+ updated_nic.interface_name = detail['IFACE_NAME']
updated_nic.save!
found=true
@@ -311,8 +305,8 @@ class HostBrowser
detail = Nic.new(
'mac' => nic['MAC'].upcase,
'bandwidth' => nic['BANDWIDTH'].to_i,
+ 'interface_name' => nic['IFACE_NAME'],
'usage_type' => 1)
- detail.physical_network = network
host.nics << detail
end
diff --git a/src/task-omatic/task_vm.rb b/src/task-omatic/task_vm.rb
index 74cf862..507818f 100644
--- a/src/task-omatic/task_vm.rb
+++ b/src/task-omatic/task_vm.rb
@@ -87,9 +87,12 @@ def create_vm_xml(name, uuid, memAllocated, memUsed, vcpus, bootDevice,
which_device += 1
end
- doc.root.elements["devices"].add_element("interface", {"type" => "bridge"})
- doc.root.elements["devices"].elements["interface"].add_element("mac", {"address" => macAddr})
- doc.root.elements["devices"].elements["interface"].add_element("source", {"bridge" => bridge})
+ unless macAddr.nil? || bridge.nil? || macAddr == "" || bridge == ""
+ doc.root.elements["devices"].add_element("interface", {"type" => "bridge"})
+ doc.root.elements["devices"].elements["interface"].add_element("mac", {"address" => macAddr})
+ doc.root.elements["devices"].elements["interface"].add_element("source", {"bridge" => bridge})
+ end
+
doc.root.elements["devices"].add_element("input", {"type" => "mouse", "bus" => "ps2"})
doc.root.elements["devices"].add_element("graphics", {"type" => "vnc", "port" => "-1", "listen" => "0.0.0.0"})
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index a88dbdc..40d494a 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -330,10 +330,31 @@ class TaskOmatic
volumes << image_volume if image_volume
storagedevs = connect_storage_pools(node, volumes)
- # FIXME: get rid of the hardcoded bridge
+ # determine if vm has been assigned to physical or
+ # virtual network and assign nic / bonding accordingly
+ # FIXME instead of trying to find a nic or bonding here, given
+ # a specified host and network, we should try earlier on to find a host
+ # that has a nic / bonding on the specified network
+
+ net_device = "breth0" # FIXME remove this default value at some point, tho net_device can't be nil
+ unless db_vm.network.nil?
+ if db_vm.network.class == PhysicalNetwork
+ device = Nic.find(:first,
+ :conditions => ["host_id = ? AND physical_network_id = ?",
+ db_host.id, db_vm.network_id ])
+ net_device = device.interface_name unless device.nil?
+
+ else
+ device = Bonding.find(:first,
+ :conditions => ["host_id = ? AND vlan_id = ?",
+ db_host.id, db_vm.network_id])
+ net_device = device.interface_name unless device.nil?
+ end
+ end
+
xml = create_vm_xml(db_vm.description, db_vm.uuid, db_vm.memory_allocated,
db_vm.memory_used, db_vm.num_vcpus_allocated, db_vm.boot_device,
- db_vm.vnic_mac_addr, "breth0", storagedevs)
+ db_vm.vnic_mac_addr, net_device, storagedevs)
result = node.domainDefineXML(xml.to_s)
raise "Error defining virtual machine: #{result.text}" unless result.status == 0
diff --git a/src/test/fixtures/networks.yml b/src/test/fixtures/networks.yml
index 1ea2c7c..f78d1b4 100644
--- a/src/test/fixtures/networks.yml
+++ b/src/test/fixtures/networks.yml
@@ -31,3 +31,12 @@ bootp_physical_network_one:
name: BOOTP Physical Network 1
boot_type: boot_type_bootp
+default_physical_network:
+ type: PhysicalNetwork
+ name: Default Physical Network
+ boot_type: boot_type_dhcp
+
+default_vlan:
+ type: Vlan
+ name: Default VLAN
+ boot_type: boot_type_dhcp
diff --git a/src/test/unit/bonding_test.rb b/src/test/unit/bonding_test.rb
index 33bfd56..a9aa4fc 100644
--- a/src/test/unit/bonding_test.rb
+++ b/src/test/unit/bonding_test.rb
@@ -75,9 +75,9 @@ class BondingTest < ActiveSupport::TestCase
flunk 'Bonding must specify bonding type' if @bonding.valid?
end
- def test_valid_fails_without_vlan
+ def test_valid_without_vlan
@bonding.vlan = nil
- flunk 'Bonding must specify vlan' if @bonding.valid?
+ flunk 'Bonding should not require vlan' if !@bonding.valid?
end
# Ensures that an arp ping address must have the correct format
--
1.6.0.6
More information about the ovirt-devel
mailing list