[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
Mohammed Morsi
mmorsi at redhat.com
Tue Feb 17 22:44:01 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
- require user specify network if boot_device is not 'hd',
changes made to model and taskomatic
- upon starting vm, determine the nic or bonding on the host / network
which the vm is running on and assign its interface name to the
libvirt xml to be sent to node. if no nic / bonding if found and
boot device is not set to 'hd', fail to start vm w/ a raised exception
- 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
Note the 'default' networks concept has been removed from this
patch as assigning every new nic / bonding to a default
network would violate the 'one nic or bonding per host / network'
constraint.
---
src/app/controllers/network_controller.rb | 105 +++++++++++++++-----
src/app/controllers/vm_controller.rb | 2 +
src/app/models/bonding.rb | 9 +-
src/app/models/network.rb | 2 +
src/app/models/nic.rb | 10 ++-
src/app/models/vm.rb | 3 +
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 +-
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 | 51 ++++++++++
src/host-browser/host-browser.rb | 10 +--
src/task-omatic/task_vm.rb | 9 +-
src/task-omatic/taskomatic.rb | 28 +++++-
src/test/fixtures/networks.yml | 9 ++
src/test/unit/bonding_test.rb | 4 +-
20 files changed, 234 insertions(+), 74 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/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 9610ace..72bd0fb 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
@@ -368,6 +370,7 @@ class Vm < ActiveRecord::Base
errors.add("memory_allocated_in_mb", "violates quota") unless not(memory_allocated) or resources[:memory].nil? or memory_allocated <= resources[:memory]
errors.add("num_vcpus_allocated", "violates quota") unless not(num_vcpus_allocated) or resources[:cpus].nil? or num_vcpus_allocated <= resources[:cpus]
errors.add_to_base("No available nics in quota") unless resources[:nics].nil? or resources[:nics] >= 1
+ errors.add("network_id", "must be specified when not booting off hd") unless boot_device == BOOT_DEV_HD || !network.nil?
# no need to validate VM limit here
# need to enforce storage differently since obj is saved first
storage_size = 0
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_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..5ef6ed9 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, :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..883d1ab
--- /dev/null
+++ b/src/db/migrate/035_networks_nics_bondings_additions.rb
@@ -0,0 +1,51 @@
+# 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)
+
+ 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 65aac32..b4031d7 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -333,10 +333,34 @@ 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
+ net_device = ""
+ 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
+
+ # if we're not booting off hd, ensure network has been
+ # specified and an accessible network device is found
+ if db_vm.boot_device != Vm::BOOT_DEV_HD &&
+ (db_vm.network.nil? || net_device == "")
+ raise "Network must be specifed and device accessible when not booting off hd"
+ 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