[Ovirt-devel] [PATCH server] network-related fixes
Hugh O. Brock
hbrock at redhat.com
Fri Mar 27 14:32:05 UTC 2009
On Wed, Mar 25, 2009 at 10:25:23PM -0400, Mohammed Morsi wrote:
> - 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
On testing it appears at the very least we are not correctly
identifying what is a bridge and what is not, so NACK until we get
that sorted out. For the very short term we simply want to be able to
define the bridges that the node creates by default correctly.
--Hugh
More information about the ovirt-devel
mailing list