[Ovirt-devel] [PATCH server] improvements / fixes to ovirt network ui
Mohammed Morsi
mmorsi at redhat.com
Tue Feb 10 01:07:36 UTC 2009
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
More information about the ovirt-devel
mailing list