[Ovirt-devel] Re: [PATCH server] network integration into ovirt server db and wui

Darryl Pierce dpierce at redhat.com
Mon Nov 3 14:06:07 UTC 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alan Pevec wrote:
> Yeah, Darryl's NAK was about 1)
> Here's my naive fix (just takes first ip_address and assumes IPv4),
> please review it:

ACK. Just a single style note.

> 
> diff --git a/src/lib/managed_node_configuration.rb
> b/src/lib/managed_node_configuration.rb
> index 101be9f..d0642d1 100644
> --- a/src/lib/managed_node_configuration.rb
> +++ b/src/lib/managed_node_configuration.rb
> @@ -67,10 +67,11 @@ class ManagedNodeConfiguration
>     host.bondings.each do |bonding|
>       entry =
> "ifcfg=none|#{bonding.interface_name}|BONDING_OPTS=\"mode=#{bonding.bonding_type.mode}
> miimon=100\""
> 
> -      if bonding.ip_addr == nil || bonding.ip_addr.empty?
> +      if bonding.ip_addresses.empty?
>         entry += "|BOOTPROTO=dhcp"
>       else
> -        entry +=
> "|BOOTPROTO=static|IPADDR=#{bonding.ip_addr}|NETMASK=#{bonding.netmask}|BROADCAST=#{bonding.broadcast}"
> 
> +        ip = bonding.ip_addresses[0]

Stylistic note: you can do, to avoid an index out of bounds error:

ip = bonding.ip_addresses.first

> +        entry +=
> "|BOOTPROTO=static|IPADDR=#{ip.address}|NETMASK=#{ip.netmask}|BROADCAST=#{ip.broadcast}"
> 
>       end
> 
>       result.puts "#{entry}|ONBOOT=yes"
> @@ -111,7 +112,10 @@ class ManagedNodeConfiguration
>         entry += "|MASTER=#{bonding.interface_name}|SLAVE=yes"
>       else
>         entry += "|BOOTPROTO=#{nic.boot_type.proto}"
> -        entry +=
> "|IPADDR=#{nic.ip_addr}|NETMASK=#{nic.netmask}|BROADCAST=#{nic.broadcast}"
> if nic.boot_type.proto == 'static'
> +        if nic.boot_type.proto == 'static'

I think we ought to hide this detail in the BootType class itself; i.e.,

def static?
  static == 'static'
end

so we don't couple ourselves to the value of proto directly.

> +          ip = nic.ip_addresses[0]

Same here with using .first.

> +          entry +=
> "|IPADDR=#{ip.address}|NETMASK=#{ip.netmask}|BROADCAST=#{ip.broadcast}"
> +        end
>         entry += "|BRIDGE=#{nic.bridge}" if nic.bridge && !is_bridge
>         entry += "|BRIDGE=ovirtbr0" if !nic.bridge && !is_bridge
>         entry += "|TYPE=bridge" if is_bridge


- --
Darryl L. Pierce <dpierce at redhat.com> : GPG KEYID: 6C4E7F1B
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkkPBUwACgkQjaT4DmxOfxtCQwCfXlQnV9yH42aw1TlG8201W4oJ
2YkAoMAxCG6CauRUMWQtVTpcdN8qLhUl
=934m
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dpierce.vcf
Type: text/x-vcard
Size: 333 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20081103/62106797/attachment.vcf>


More information about the ovirt-devel mailing list