[Ovirt-devel] [PATCH server] network-related fixes

[Mohammed Morsi]

  - removed mac dependency from managed_node_controller, since we already
     have nic interface names from host browser, node doesn't need to pass
     them in when invoking the managed node controller
  - fixes to managed node configuration concerning proper usage of networks
     associated w/ nics
  - fix to host browser correcting comparison of nic mac addresses between
     those received and those in db
---
 src/app/controllers/managed_node_controller.rb |   17 +-----------
 src/host-browser/host-browser.rb               |    2 +-
 src/lib/managed_node_configuration.rb          |   34 ++++++++++++------------
 3 files changed, 19 insertions(+), 34 deletions(-)

diff --git a/src/app/controllers/managed_node_controller.rb b/src/app/controllers/managed_node_controller.rb
index f5948be..72908e7 100644
--- a/src/app/controllers/managed_node_controller.rb
+++ b/src/app/controllers/managed_node_controller.rb
@@ -21,7 +21,6 @@
 #
 class ManagedNodeController < ApplicationController
   before_filter :load_host, :only => :config
-  before_filter :load_macs, :only => :config
 
   def is_logged_in
     # this overrides the default which is to force the client to log in
@@ -30,7 +29,7 @@ class ManagedNodeController < ApplicationController
   # Generates a configuration file for the managed node.
   #
   def config
-    context = ManagedNodeConfiguration.generate(@host, @macs)
+    context = ManagedNodeConfiguration.generate(@host)
 
     send_data("#{context}", :type => 'text', :disposition => 'inline')
   end
@@ -46,18 +45,4 @@ class ManagedNodeController < ApplicationController
 
     render :nothing => true, :status => :error unless @host
   end
-
-  def load_macs
-    @macs = Hash.new
-    mac_text = params[:macs]
-
-    if mac_text != nil
-      mac_text.scan(/([^,]+)\,?/).each do |line|
-        key, value = line.first.split("=")
-        @macs[key] = value
-      end
-    end
-
-    render :nothing => true, :status => :error if @macs.empty?
-  end
 end
diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
index 1c5cf83..c3ddeaa 100755
--- a/src/host-browser/host-browser.rb
+++ b/src/host-browser/host-browser.rb
@@ -278,7 +278,7 @@ class HostBrowser
             nic_info.collect do |detail|
                 # if we have a match, then update the database and remove
                 # the received data to avoid creating a dupe later
-                if detail['MAC'] == nic.mac
+                if detail['MAC'].upcase == nic.mac && detail['IFACE_NAME'] == nic.interface_name
                     nic_info.delete(detail)
 
                     updated_nic = Nic.find_by_id(nic.id)
diff --git a/src/lib/managed_node_configuration.rb b/src/lib/managed_node_configuration.rb
index c412917..84ecab2 100644
--- a/src/lib/managed_node_configuration.rb
+++ b/src/lib/managed_node_configuration.rb
@@ -47,10 +47,11 @@ require 'stringio'
 # +00:11:22:33:44+. It will use DHCP for retrieving it's IP address details,
 # and will use the +breth0+ interface as a bridge.
 #
+
 class ManagedNodeConfiguration
   NIC_ENTRY_PREFIX='/files/etc/sysconfig/network-scripts'
 
-  def self.generate(host, macs)
+  def self.generate(host)
     result = StringIO.new
 
     result.puts "# THIS FILE IS GENERATED!"
@@ -77,23 +78,15 @@ class ManagedNodeConfiguration
       result.puts "#{entry}|ONBOOT=yes"
 
       bonding.nics.each do |nic|
-        process_nic result, nic, macs, bonding
+        process_nic result, nic, bonding
       end
     end
 
-    has_bridge = false
     host.nics.each do |nic|
       # only process this nic if it doesn't have a bonding
       # TODO remove the hack to force a bridge into the picture
       if nic.bondings.empty?
-        process_nic result, nic, macs, nil, false, true
-
-	# TODO remove this when bridges are properly supported
-	unless has_bridge
-	  macs[nic.mac] = "breth0"
-	  process_nic result, nic, macs, nil, true, false
-	  has_bridge = true
-	end
+        process_nic result, nic, nil, false, true
       end
     end
 
@@ -102,8 +95,8 @@ class ManagedNodeConfiguration
 
   private
 
-  def self.process_nic(result, nic, macs, bonding = nil, is_bridge = false, bridged = true)
-    iface_name = macs[nic.mac]
+  def self.process_nic(result, nic, bonding = nil, is_bridge = false, bridged = true)
+    iface_name = nic.interface_name
 
     if iface_name
       entry = "ifcfg=#{nic.mac}|#{iface_name}"
@@ -111,10 +104,17 @@ class ManagedNodeConfiguration
       if bonding
         entry += "|MASTER=#{bonding.interface_name}|SLAVE=yes"
       else
-        entry += "|BOOTPROTO=#{nic.physical_network.boot_type.proto}"
-        if nic.physical_network.boot_type.proto == 'static'
-          ip = nic.ip_addresses[0]
-          entry += "|IPADDR=#{ip.address}|NETMASK=#{ip.netmask}|BROADCAST=#{ip.broadcast}"
+        boot_proto = nic.physical_network.nil? ? 'dhcp' : nic.physical_network.boot_type.proto
+        entry += "|BOOTPROTO=#{boot_proto}"
+        if boot_proto == 'static'
+          ip = nic.ip_addresses[0].address
+          netmask = '255.255.255.0'
+          broadcast = '255.255.255.255'
+          unless nic.physical_network.nil? || nic.physical_network.ip_addresses.size == 0
+            netmask = nic.physical_network.ip_addresses[0].netmask
+            broadcast = nic.physical_network.ip_addresses[0].broadcast
+          end
+          entry += "|IPADDR=#{ip}|NETMASK=#{netmask}|BROADCAST=#{broadcast}"
         end
         entry += "|BRIDGE=#{nic.bridge}" if nic.bridge && !is_bridge
         entry += "|BRIDGE=breth0" if !nic.bridge && !is_bridge
-- 
1.6.0.6