Mohammed Morsi
2009-Feb-10 01:07 UTC
[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
based on discussion on ovirt-devel, the following is included in this patch:
- new db migration adding 'interface_name' column to nics table,
and default physical and virtual networks
- add static method to physical and virtual network model classes,
allowing retreival of default networks
- edit host browser to assign new nics to default network, as
opposed to creating a seperate network for each
- 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
- 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
- remove routing info related fields (netmask, broadcast, gateway)
from create / edit ip address form when displayed as part of
edit nics / bonding interface
---
src/app/controllers/network_controller.rb | 44 +++++++++++--------
src/app/models/bonding.rb | 3 -
src/app/models/nic.rb | 3 -
src/app/models/physical_network.rb | 6 +++
src/app/models/vlan.rb | 6 +++
src/app/views/host/edit_network.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 | 4 +-
..._add_default_networks_and_nic_interface_name.rb | 43 +++++++++++++++++++
src/host-browser/host-browser.rb | 11 +----
src/test/fixtures/networks.yml | 9 ++++
src/test/unit/bonding_test.rb | 4 +-
15 files changed, 125 insertions(+), 63 deletions(-)
create mode 100644
src/db/migrate/035_add_default_networks_and_nic_interface_name.rb
diff --git a/src/app/controllers/network_controller.rb
b/src/app/controllers/network_controller.rb
index 7328e66..e669e8f 100644
--- a/src/app/controllers/network_controller.rb
+++ b/src/app/controllers/network_controller.rb
@@ -281,14 +281,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])
@@ -328,13 +330,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
@@ -377,13 +381,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
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index a67b97b..ba14aae 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -57,9 +57,6 @@ 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.'
-
# verify arp ping address to be ipv4 if set
validates_format_of :arp_ping_address,
:with => %r{^(\d{1,3}\.){3}\d{1,3}$},
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index e43f067..7789afc 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -33,9 +33,6 @@ 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
diff --git a/src/app/models/physical_network.rb
b/src/app/models/physical_network.rb
index 6923e40..66aaa6e 100644
--- a/src/app/models/physical_network.rb
+++ b/src/app/models/physical_network.rb
@@ -18,4 +18,10 @@
class PhysicalNetwork < Network
has_many :nics
+
+ DEFAULT_NETWORK_NAME = 'Default Physical Network'
+
+ def self.get_default_network
+ return find(:first, :conditions => "name='" +
DEFAULT_NETWORK_NAME + "'")
+ end
end
diff --git a/src/app/models/vlan.rb b/src/app/models/vlan.rb
index f7889f4..d7643cf 100644
--- a/src/app/models/vlan.rb
+++ b/src/app/models/vlan.rb
@@ -21,4 +21,10 @@ class Vlan < Network
validates_presence_of :number,
:message => 'A number must be specified.'
+
+ DEFAULT_NETWORK_NAME = 'Default VLAN'
+
+ def self.get_default_network
+ return find(:first, :conditions => "name='" +
DEFAULT_NETWORK_NAME + "'")
+ end
end
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/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..3b5a614 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>
@@ -44,9 +45,6 @@
<div style="float:left;">
<%= 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;"} %>
- </div>
<div style="clear:both;"></div>
<div class="clear_row"></div>
<div class="clear_row"></div>
diff --git a/src/db/migrate/035_add_default_networks_and_nic_interface_name.rb
b/src/db/migrate/035_add_default_networks_and_nic_interface_name.rb
new file mode 100644
index 0000000..f1e1214
--- /dev/null
+++ b/src/db/migrate/035_add_default_networks_and_nic_interface_name.rb
@@ -0,0 +1,43 @@
+# 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 'default' networks to the application and a
'interface_name'
+# column to the nics table
+class AddDefaultNetworksAndNicInterfaceName < ActiveRecord::Migration
+ def self.up
+ add_column :nics, :interface_name, :string
+
+
+ dhcp_boot_type = BootType.find(:first,
:conditions=>"proto='dhcp'")
+ usages = Usage.find(:all)
+
+ PhysicalNetwork.create( :name => PhysicalNetwork::DEFAULT_NETWORK_NAME,
+ :boot_type => dhcp_boot_type,
+ :usages => usages)
+
+ Vlan.create( :name => Vlan::DEFAULT_NETWORK_NAME,
+ :number => 1,
+ :boot_type => dhcp_boot_type,
+ :usages => usages)
+ end
+
+ def self.down
+ remove_column :nics, :interface_name
+ end
+end
+
diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
index 579f241..884babc 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,9 @@ 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
+ detail.physical_network = PhysicalNetwork.get_default_network
host.nics << detail
end
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
Mohammed Morsi
2009-Feb-17 22:44 UTC
[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
(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
Mohammed Morsi
2009-Mar-03 19:07 UTC
[Ovirt-devel] Re: [PATCH server] improvements / fixes to ovirt network ui
Patch bump. Know everyone's busy w/ the installer and other things but was wondering if anyone had a second to take a look at this before it got stale. I made sure to thoroughly test everything included when I wrote it so a brief code review and a quick functionality verification should be all that's needed. I had also sent out an additional email with a few notes relating to these changes. It can be found here https://www.redhat.com/archives/ovirt-devel/2009-February/msg00178.html Thanks alot, -Mo
Mohammed Morsi
2009-Mar-06 05:30 UTC
[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
-------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: improvements-fixes-network-ui.patch URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20090306/43c34fa7/attachment.ksh>
Mohammed Morsi
2009-Mar-06 14:13 UTC
[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
(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