[Ovirt-devel] [PATCH server] permit many-to-many vms / networks relationship

Mohammed Morsi mmorsi at redhat.com
Tue Jun 30 19:20:46 UTC 2009


  - db, model, service, controller, and view changes
  - many tests additions
  - various fixes / cleanup to get things working

 depends on my last two (still unacked) patches:
   - add collapsable sections to vm form
   - provide default vm allocated cpu and memory values

 ovirt-agent might need to be updated to work with the changes
---
 src/app/controllers/pool_controller.rb        |    2 +-
 src/app/controllers/vm_controller.rb          |   54 ++++++-
 src/app/helpers/application_helper.rb         |    4 +-
 src/app/models/network.rb                     |    2 -
 src/app/models/nic.rb                         |   54 +++++--
 src/app/models/vlan.rb                        |   15 ++-
 src/app/models/vm.rb                          |   17 ++-
 src/app/services/network_service.rb           |   12 +-
 src/app/services/nic_service.rb               |    2 +-
 src/app/services/vm_service.rb                |   39 +++---
 src/app/views/host/show.rhtml                 |    2 +-
 src/app/views/network/_select.rhtml           |    2 +-
 src/app/views/network/edit_nic.rhtml          |    4 +-
 src/app/views/nic/show.rhtml                  |    2 +
 src/app/views/task/_user_list.rhtml           |    2 +-
 src/app/views/vm/_form.rhtml                  |  217 ++++++++++++++++++++++++-
 src/app/views/vm/_grid.rhtml                  |    1 -
 src/app/views/vm/_list.rhtml                  |    2 -
 src/app/views/vm/show.rhtml                   |    6 +-
 src/db/migrate/040_associate_vms_with_nics.rb |  103 ++++++++++++
 src/public/stylesheets/components.css         |   39 +++++
 src/task-omatic/task_vm.rb                    |   14 +-
 src/task-omatic/taskomatic.rb                 |   43 +++---
 src/test/fixtures/nics.yml                    |   16 +-
 src/test/fixtures/vms.yml                     |    8 -
 src/test/unit/network_test.rb                 |   69 ++++++++-
 src/test/unit/nic_test.rb                     |   95 +++++++++++-
 src/test/unit/vm_test.rb                      |   14 +-
 28 files changed, 710 insertions(+), 130 deletions(-)
 create mode 100644 src/db/migrate/040_associate_vms_with_nics.rb

diff --git a/src/app/controllers/pool_controller.rb b/src/app/controllers/pool_controller.rb
index 74a958c..0d2e491 100644
--- a/src/app/controllers/pool_controller.rb
+++ b/src/app/controllers/pool_controller.rb
@@ -96,7 +96,7 @@ class PoolController < ApplicationController
   def vms_json(args)
     attr_list = [:id, :description, :uuid,
                  :num_vcpus_allocated, :memory_allocated_in_mb,
-                 :vnic_mac_addr, :state, :id]
+                 :state, :id]
     if (@pool.is_a? VmResourcePool) and @pool.get_hardware_pool.can_view(@user)
       attr_list.insert(3, [:host, :hostname])
     end
diff --git a/src/app/controllers/vm_controller.rb b/src/app/controllers/vm_controller.rb
index 197241d..2c0079c 100644
--- a/src/app/controllers/vm_controller.rb
+++ b/src/app/controllers/vm_controller.rb
@@ -55,29 +55,31 @@ class VmController < ApplicationController
   def new
     alert = svc_new(params[:vm_resource_pool_id])
     _setup_provisioning_options
+    _setup_network_options
     @storage_tree = VmResourcePool.find(params[:vm_resource_pool_id]).get_hardware_pool.storage_tree.to_json
-    @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
     render :layout => 'popup'
   end
 
   def create
     params[:vm][:forward_vnc] = params[:forward_vnc]
-    alert = svc_create(params[:vm], params[:start_now])
+    _parse_network_params(params)
+    alert = svc_create(params[:vm], params[:start_now], params[:nics])
     render :json => { :object => "vm", :success => true, :alert => alert  }
   end
 
   def edit
     svc_modify(params[:id])
     _setup_provisioning_options
-    @networks = Network.find(:all).collect{ |net| [net.name, net.id] }
+    _setup_network_options
     @storage_tree = @vm.vm_resource_pool.get_hardware_pool.storage_tree(:vm_to_include => @vm).to_json
     render :layout => 'popup'
   end
 
   def update
     params[:vm][:forward_vnc] = params[:forward_vnc]
+    _parse_network_params(params)
     alert = svc_update(params[:id], params[:vm], params[:start_now],
-                       params[:restart_now])
+                       params[:restart_now], params[:nics])
     render :json => { :object => "vm", :success => true, :alert => alert  }
   end
 
@@ -164,4 +166,48 @@ class VmController < ApplicationController
       #if cobbler doesn't respond/is misconfigured/etc just don't add profiles
     end
   end
+
+  # sets up a list of nics for the vm form
+  def _setup_network_options
+    net_conditions = ""
+    @nics = []
+
+    unless @vm.nil?
+      @vm.nics.each { |nic|
+         nnic = Nic.new(:mac => nic.mac,
+                        :vm_id => @vm.id,
+                        :network => nic.network)
+	 if(nic.network.boot_type.proto == 'static')
+	   nnic.ip_addresses << IpAddress.new(:address => nic.ip_address)
+	 end
+         @nics.push nnic
+
+         net_conditions += (net_conditions == "" ? "" : " AND ") +
+                           "id != " + nic.network_id.to_s
+      }
+    end
+
+    networks = Network.find(:all, :conditions => net_conditions)
+
+    networks.each{ |net|
+        nnic = Nic.new(:mac => Nic::gen_mac, :network => net)
+	if(net.boot_type.proto == 'static')
+	   nnic.ip_addresses << IpAddress.new(:address => '127.0.0.1') # FIXME
+	end
+        @nics.push nnic
+    }
+
+  end
+
+  # merges vm / network parameters as submitted on the vm form
+  def _parse_network_params(params)
+     params[:nics] = []
+     unless params[:networks].nil?
+       params[:networks].each { |network, id|
+         params[:nics].push({ :mac => params[("nic_"+network.to_s).intern],
+                              :network_id => network,
+			      :bandwidth => 0})
+       }
+     end
+  end
 end
diff --git a/src/app/helpers/application_helper.rb b/src/app/helpers/application_helper.rb
index d3eecd7..79d835c 100644
--- a/src/app/helpers/application_helper.rb
+++ b/src/app/helpers/application_helper.rb
@@ -78,9 +78,9 @@ module ApplicationHelper
   end
 
   # same as check_box_tag_with_label but w/ the checkbox appearing first
-  def rcheck_box_tag_with_label(label, name, value = "1", checked = false)
+  def rcheck_box_tag_with_label(label, name, value = "1", checked = false, opts={})
     %{
-      <div class="i">#{check_box_tag name, value, checked}
+      <div class="i">#{check_box_tag name, value, checked, opts}
       <label for="#{name}">#{_(label)}</label></div>
      }
   end
diff --git a/src/app/models/network.rb b/src/app/models/network.rb
index 0e2aa8c..a4b1b8b 100644
--- a/src/app/models/network.rb
+++ b/src/app/models/network.rb
@@ -22,8 +22,6 @@ 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 e8b7768..f03c525 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -19,7 +19,10 @@
 
 class Nic < ActiveRecord::Base
   belongs_to :host
-  belongs_to :physical_network
+  belongs_to :vm
+
+  belongs_to :network
+
   has_many :ip_addresses, :dependent => :destroy
 
   # FIXME bondings_nics table should just be replaced with
@@ -33,24 +36,31 @@ class Nic < ActiveRecord::Base
   validates_format_of :mac,
     :with => %r{^([0-9a-fA-F]{2}([:-]|$)){6}$}
 
-  validates_presence_of :host_id,
-    :message => 'A host must be specified.'
+  # nic must be assigned to network if associated w/ a vm
+  validates_presence_of :network_id,
+     :unless => Proc.new { |nic| nic.vm.nil? }
+
+  # nic must be associated w/ a vm if assigned to a vlan
+  validates_presence_of :vm_id,
+    :message => 'A vm must be specified.',
+    :if => Proc.new { |nic| !nic.network.nil? && nic.network.class == Vlan }
+
+  # a vm / host can't have more than one nic on a network
+  validates_uniqueness_of :network_id,
+     :scope => [:host_id, :vm_id],
+     :unless => Proc.new { |nic| nic.network.nil? }
 
   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? }
-
   # Returns whether the nic has networking defined.
   def networking?
-    (physical_network != nil)
+    (network != nil)
   end
 
   # Returns the boot protocol for the nic if networking is defined.
   def boot_protocol
-    return physical_network.boot_type.proto if networking?
+    return network.boot_type.proto if networking?
   end
 
   # Returns whether the nic is enslaved by a bonded interface.
@@ -66,30 +76,44 @@ class Nic < ActiveRecord::Base
 
   # Returns the netmask for the nic if networking is defined.
   def netmask
-    return physical_network.ip_addresses.first.netmask if networking? && !physical_network.ip_addresses.empty?
+    return network.ip_addresses.first.netmask if networking? && !network.ip_addresses.empty?
     return nil
   end
 
   # Returns the broadcast address for the nic if networking is defined.
   def broadcast
-    return physical_network.ip_addresses.first.broadcast if networking? && !physical_network.ip_addresses.empty?
+    return network.ip_addresses.first.broadcast if networking? && !network.ip_addresses.empty?
     return nil
   end
 
   # Returns the gateway address fo rthe nic if networking is defined.
   def gateway
-    return physical_network.ip_addresses.first.gateway if networking? && !physical_network.ip_addresses.empty?
+    return network.ip_addresses.first.gateway if networking? && !network.ip_addresses.empty?
     return nil
   end
 
+  def parent
+    return host if !host.nil? && vm.nil?
+    return vm   if !vm.nil? && host.nil?
+    return nil
+  end
+
+  def self.gen_mac
+    [ 0x00, 0x16, 0x3e, rand(0x7f), rand(0xff),
+       rand(0xff) ].collect {|x| "%02x" % x}.join(":")
+  end
+
   # validate 'bridge' or 'usage_type' attribute ?
 
   protected
    def validate
-    if ! physical_network.nil? and
-       physical_network.boot_type.proto == 'static' and
+    # nic must be associated with a host or vm, but not both
+    errors.add("one host or one vm must be specified") unless host.nil? ^ vm.nil?
+
+    if ! network.nil? and
+       network.boot_type.proto == 'static' and
        ip_addresses.size == 0
-           errors.add("physical_network_id",
+           errors.add("network_id",
                       "is static. Must create at least one static ip")
      end
    end
diff --git a/src/app/models/vlan.rb b/src/app/models/vlan.rb
index 2f6acba..cce897d 100644
--- a/src/app/models/vlan.rb
+++ b/src/app/models/vlan.rb
@@ -19,11 +19,24 @@
 class Vlan < Network
    has_many :bondings
 
+   has_many :nics
+
   validates_presence_of :number,
     :message => 'A number must be specified.'
 
   def is_destroyable?
-    bondings.empty?
+    bondings.empty? && nics.empty?
   end
 
+  protected
+   def validate
+     # ensure that any assigned nics only belong to vms, not hosts
+     nics.each{ |nic|
+       if nic.parent.class == Host
+         errors.add("nics", "must only be assigned to vms")
+         break
+       end
+     }
+   end
+
 end
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index f445219..a5c95bd 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -29,7 +29,7 @@ class Vm < ActiveRecord::Base
   end
   has_and_belongs_to_many :storage_volumes
 
-  belongs_to :network
+  has_many :nics, :dependent => :destroy
 
   has_many :smart_pool_tags, :as => :tagged, :dependent => :destroy
   has_many :smart_pools, :through => :smart_pool_tags
@@ -45,8 +45,8 @@ class Vm < ActiveRecord::Base
 
   validates_presence_of :uuid, :description, :num_vcpus_allocated,
                         :boot_device, :memory_allocated_in_mb,
-                        :memory_allocated, :vnic_mac_addr,
-                        :vm_resource_pool_id
+                        :memory_allocated, :vm_resource_pool_id
+
 
   validates_format_of :uuid,
      :with => %r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
@@ -88,7 +88,7 @@ class Vm < ActiveRecord::Base
      :greater_than_or_equal_to => 0,
      :unless => Proc.new{ |vm| vm.memory_allocated.nil? }
 
-  acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr, :state ],
+  acts_as_xapian :texts => [ :uuid, :description, :state ],
                  :terms => [ [ :search_users, 'U', "search_users" ] ],
                  :eager_load => :smart_pools
 
@@ -111,8 +111,8 @@ class Vm < ActiveRecord::Base
 
   NEEDS_RESTART_FIELDS = [:uuid,
                           :num_vcpus_allocated,
-                          :memory_allocated,
-                          :vnic_mac_addr]
+                          :memory_allocated]
+
 
   STATE_PENDING        = "pending"
   STATE_CREATING       = "creating"
@@ -379,6 +379,11 @@ class Vm < ActiveRecord::Base
     return i
   end
 
+  def self.gen_uuid
+    ["%02x"*4, "%02x"*2, "%02x"*2, "%02x"*2, "%02x"*6].join("-") %
+      Array.new(16) {|x| rand(0xff) }
+  end
+
   # Make method for calling paginated vms easier for clients.
   # TODO: Might want to have an optional param for per_page var
   def self.paged_with_perms(user, priv, page, order)
diff --git a/src/app/services/network_service.rb b/src/app/services/network_service.rb
index 3e7c997..221472d 100644
--- a/src/app/services/network_service.rb
+++ b/src/app/services/network_service.rb
@@ -205,13 +205,13 @@ module NetworkService
   def svc_modify_nic(id)
     authorize
     @nic = Nic.find(id)
-    @network = @nic.physical_network
+    @network = @nic.network
     network_options
     # 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)
+    @nic.parent.nics.each { |nic|
+      unless nic.network.nil? || nic.id == @nic.id
+        network_conditions.push(" id != " + nic.network.id.to_s)
       end
     }
     network_conditions = network_conditions.join(" AND ")
@@ -228,8 +228,8 @@ module NetworkService
   def svc_update_nic(id, nic_hash, ip_hash)
     authorize
     network_options
-    unless nic_hash[:physical_network_id].to_i == 0
-      @network = Network.find(nic_hash[:physical_network_id])
+    unless nic_hash[:network_id].to_i == 0
+      @network = Network.find(nic_hash[:network_id])
       if @network.boot_type.id == @static_boot_type.id
         if ip_hash[:id] == "New"
           svc_create_ip_address(ip_hash)
diff --git a/src/app/services/nic_service.rb b/src/app/services/nic_service.rb
index 12314a4..b8c5ef1 100644
--- a/src/app/services/nic_service.rb
+++ b/src/app/services/nic_service.rb
@@ -33,7 +33,7 @@ module NicService
   private
   def lookup(id, priv)
     @nic = Nic.find(id)
-    authorized!(priv, at nic.host.hardware_pool)
+    authorized!(priv, (@nic.parent.class == Host ? @nic.host.hardware_pool : @nic.vm.vm_resource_pool))
   end
 
 end
diff --git a/src/app/services/vm_service.rb b/src/app/services/vm_service.rb
index 3d9777c..24c7908 100644
--- a/src/app/services/vm_service.rb
+++ b/src/app/services/vm_service.rb
@@ -82,8 +82,8 @@ module VmService
   def svc_new(vm_resource_pool_id)
     raise ActionError.new("VM Resource Pool is required.") unless vm_resource_pool_id
 
-    new_vm_hash = {:vm_resource_pool_id => vm_resource_pool_id}
-    default_mac_and_uuid(new_vm_hash)
+    new_vm_hash = {:vm_resource_pool_id => vm_resource_pool_id,
+                   :uuid => Vm::gen_uuid}
     @vm = Vm.new(new_vm_hash)
     authorized!(Privilege::MODIFY, @vm.vm_resource_pool)
   end
@@ -94,8 +94,8 @@ module VmService
   # [<tt>@vm</tt>] the newly saved Vm
   # === Required permissions
   # [<tt>Privilege::MODIFY</tt>] for the vm's VmResourcePool
-  def svc_create(vm_hash, start_now)
-    default_mac_and_uuid(vm_hash)
+  def svc_create(vm_hash, start_now, nics)
+    vm_hash[:uuid] = Vm::gen_uuid unless vm_hash[:uuid]
     vm_hash[:state] = Vm::STATE_PENDING
     @vm = Vm.new(vm_hash)
     authorized!(Privilege::MODIFY, at vm.vm_resource_pool)
@@ -103,6 +103,10 @@ module VmService
     alert = "VM was successfully created."
     Vm.transaction do
       @vm.save!
+      nics.each{ |nic|
+        nic[:vm_id] = @vm.id
+        Nic.create(nic)
+      }
       vm_provision
       @task = VmTask.new({ :user        => @user,
                            :task_target => @vm,
@@ -129,7 +133,7 @@ module VmService
   # [<tt>@vm</tt>] stores the Vm with +id+
   # === Required permissions
   # [<tt>Privilege::MODIFY</tt>] for the Vm's VmResourcePool
-  def svc_update(id, vm_hash, start_now, restart_now)
+  def svc_update(id, vm_hash, start_now, restart_now, nics)
     lookup(id,Privilege::MODIFY)
 
     #needs restart if certain fields are changed
@@ -153,6 +157,9 @@ module VmService
 
     alert = "VM was successfully updated."
     Vm.transaction do
+      @vm.nics.clear
+      nics.each{ |nic| @vm.nics.push Nic.new(nic) }
+
       @vm.update_attributes!(vm_hash)
       vm_provision
       if start_now
@@ -256,10 +263,15 @@ module VmService
       unless system
         system = Cobbler::System.new("name" => name,
                                      @vm.cobbler_type => @vm.cobbler_name)
-        system.interfaces = [Cobbler::NetworkInterface.new(
-                                    {'mac_address' => @vm.vnic_mac_addr})]
+	system.interfaces = []
+        # do we need to do anything if vm.nics is empty ?
+	@vm.nics.each { |nic|
+	   system.interfaces.push Cobbler::NetworkInterface.new(
+                                    {'mac_address' => nic.mac})
+	}
         system.save
       end
+      # TODO update system if found
     end
   end
 
@@ -276,17 +288,4 @@ module VmService
     @vm = Vm.find(id)
     authorized!(priv, at vm.vm_resource_pool)
   end
-
-  def default_mac_and_uuid(vm_hash)
-    unless vm_hash[:uuid]
-      vm_hash[:uuid] = ["%02x"*4, "%02x"*2, "%02x"*2,
-                        "%02x"*2, "%02x"*6].join("-") %
-        Array.new(16) {|x| rand(0xff) }
-    end
-    unless vm_hash[:vnic_mac_addr]
-      vm_hash[:vnic_mac_addr] = [ 0x00, 0x16, 0x3e, rand(0x7f), rand(0xff),
-                                 rand(0xff) ].collect {|x| "%02x" % x}.join(":")
-    end
-  end
-
 end
diff --git a/src/app/views/host/show.rhtml b/src/app/views/host/show.rhtml
index f706761..ddc6481 100644
--- a/src/app/views/host/show.rhtml
+++ b/src/app/views/host/show.rhtml
@@ -64,7 +64,7 @@
 	<%=h @host.status_str %><br/>
 	<%=  @host.nics.collect{ |n|
                 n.interface_name.to_s + " " + n.mac +
-                (n.physical_network.nil? ? "" : " " + n.physical_network.name)
+                (n.network.nil? ? "" : " " + n.network.name)
               }.join("<br/>")
         %><br/>
 	<%=  @host.bondings.collect { |n| n.name }.join("<br/>") %><br/>
diff --git a/src/app/views/network/_select.rhtml b/src/app/views/network/_select.rhtml
index 4d056df..47ab319 100644
--- a/src/app/views/network/_select.rhtml
+++ b/src/app/views/network/_select.rhtml
@@ -1,5 +1,5 @@
 <% target = 'nic' unless target
-   network_id = 'physical_network_id' if target == 'nic'
+   network_id = 'network_id' if target == 'nic'
    network_id = 'vlan_id' if target == 'bonding'
 
  %>
diff --git a/src/app/views/network/edit_nic.rhtml b/src/app/views/network/edit_nic.rhtml
index 1b58c20..7f0bc02 100644
--- a/src/app/views/network/edit_nic.rhtml
+++ b/src/app/views/network/edit_nic.rhtml
@@ -6,10 +6,12 @@
 
  <%= error_messages_for 'nic' %>
 
+ <%# TODO doesn't currently break anything due to where this form is displayed
+     but @nic.host assumption should be removed so this template can be included elsewhere%>
  <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 if @nic.physical_network %>
+  <%= hidden_field_tag 'nic_network_id', @nic.network.id if @nic.network %>
 
   <div class="selected_popup_content_left">MAC:</div>
   <div class="selected_popup_content_right"><%= @nic.mac %></div>
diff --git a/src/app/views/nic/show.rhtml b/src/app/views/nic/show.rhtml
index 0bd3910..46fb2c6 100644
--- a/src/app/views/nic/show.rhtml
+++ b/src/app/views/nic/show.rhtml
@@ -7,6 +7,8 @@
 </p>
 <% end %>
 
+ <%# TODO doesn't currently break anything due to where this form is displayed
+     but @nic.host assumption should be removed so thiw template can be included elsewhere%>
 <p><b>Host:</b> <%= link_to @nic.host.hostname, { :controller => "host", :action => "show", :id => @nic.host }, { :class => "show" } %></p>
 </div>
 
diff --git a/src/app/views/task/_user_list.rhtml b/src/app/views/task/_user_list.rhtml
index e289932..1506359 100644
--- a/src/app/views/task/_user_list.rhtml
+++ b/src/app/views/task/_user_list.rhtml
@@ -7,7 +7,7 @@
     <% for task in tasks %>
       <li class="task">
         <span class="secondary">action: </span><%= link_to task.action, { :controller => "task", :action => 'show', :id => task }, { :class => "action" } -%><br/>
-        <span class="secondary">vm: </span><%= link_to task.vm.description, { :controller => "vm",  :action => 'show', :id => task }, { :class => "description", :title => "cpus: %s mem: %s vNIC: %s" % [ task.vm.num_vcpus_used, task.vm.memory_used, task.vm.vnic_mac_addr ] } -%><br/>
+        <span class="secondary">vm: </span><%= link_to task.vm.description, { :controller => "vm",  :action => 'show', :id => task }, { :class => "description", :title => "cpus: %s mem: %s" % [ task.vm.num_vcpus_used, task.vm.memory_used ] } -%><br/>
         <span class="secondary">state: </span><span class="state"><%= task.state -%></span></li>
     <% end %>
     </ul>
diff --git a/src/app/views/vm/_form.rhtml b/src/app/views/vm/_form.rhtml
index 373452d..ead26e1 100644
--- a/src/app/views/vm/_form.rhtml
+++ b/src/app/views/vm/_form.rhtml
@@ -58,19 +58,36 @@
   </div>
   <div id="vm_network_config">
     <div class="clear_row"></div>
-    <div class="clear_row"></div>
-    <div style="float:left;">
-      <%= text_field_with_label "VNIC:", "vm", "vnic_mac_addr",  {:style=>"width:250;"} %>
-    </div>
-    <div style="float:left;">
-      <%= select_with_label "Network:", 'vm', 'network_id', @networks.insert(0, ""), :style=>"width:250px;" %>
-    </div>
-    <div class="clear_row"></div>
+    <% if @nics.size > 0 %>
+      <div id="vm_network_config_header">
+        <div id="vm_network_config_header_network">
+	      Network:
+	    </div>
+	    <div id="vm_network_config_header_mac">
+	      MAC Address:
+	    </div>
+	    <div id="vm_network_config_header_ip">
+	      <%# this column is only populated if a static ip network is selected: %>
+	      IP Address:
+	    </div>
 
-    <%= rcheck_box_tag_with_label "Forward vm's vnc port locally", "forward_vnc", 1, @vm.forward_vnc %>
+        <div class="clear_row"></div><div style="clear:both;"></div>
+	  </div>
+	  <%# populated with jquery below: %>
+      <div id="vm_network_config_networks">
+	  </div>
+      <div id="vm_network_config_add">
+        Add Another Network
+      </div>
+    <% else %>
+       <b>No networks available</b>
+    <% end %>
+    <div style="clear:both;"></div>
+    <div class="clear_row"></div>
   </div>
   <div class="clear_row"></div>
 
+   <div class="form_heading"/>
    <%= rcheck_box_tag_with_label "Start VM Now? (pending current resource availability)", "start_now", nil if create or @vm.state == Vm::STATE_STOPPED %>
    <%= rcheck_box_tag_with_label "Restart VM Now? (pending current resource availability)", "restart_now", nil if @vm.state == Vm::STATE_RUNNING %>
 
@@ -135,4 +152,186 @@ ${htmlList(pools, id)}
      hide_section_with_header('#vm_network_config',   '#vm_network_section_link',   'Network');
    });
 
+   /////////////////////////////////////////////////// vm networks config
+
+   // number of rows which we are currently displaying in net config
+   var vm_network_config_rows = 0;
+
+   // last row currently being displayed
+   var vm_network_config_last_row = 0;
+
+   // value of current selectbox
+   var current_selectbox_value = 0;
+
+   // create list of nics
+   var nics = new Array();
+   <% @nics.each { |rnic| %>
+    jnic = new Object;
+    jnic.network_id = "<%= rnic.network_id.to_s %>";
+    jnic.name = "<%= rnic.network.name %>";
+    jnic.mac  = "<%= rnic.mac %>";
+    jnic.ip   = "<%= rnic.ip_address %>";
+    jnic.static_ip   = <%= rnic.network.boot_type.proto == 'static' %>;
+    jnic.selected = false;
+    nics.push(jnic);
+   <% } %>
+
+   // adds unselected network back to selectboxes indicated by selector
+   function add_unselected_network(selector, network_id){
+       for(j = 0; j < nics.length; ++j){
+         if(nics[j].network_id == network_id){
+              nics[j].selected = false;
+              $(selector).append('<option value="' + nics[j].network_id + '">' + nics[j].name + '</option>');
+              break;
+         }
+       }
+   }
+
+   // show / hide ip address column
+   function toggle_ip_address_column(){
+      for(i = 0; i < nics.length; ++i){
+        if(nics[i].selected && nics[i].static_ip){
+           $('#vm_network_config_header_ip').show();
+	   return;
+	 }
+      }
+      $('#vm_network_config_header_ip').hide();
+   }
+
+   // show a new network config row
+   function add_network_config_row(no_remove_link){
+
+      // if the number of rows is equal to the number of
+      // networks, don't show any more rows
+      if(vm_network_config_rows == nics.length)
+         return;
+
+      vm_network_config_rows += 1;
+      vm_network_config_last_row = vm_network_config_rows;
+
+      // create the content for another row to be added to the vm_network_config_networks div above.
+      // currently a row has a network select box, a mac text field, and an ip address field if a static network is selected
+      var content = '<div id="vm_network_config_row_'+vm_network_config_rows+'" class="vm_network_config_row">';
+      content    += ' <div class="vm_network_config_net">';
+      content    += '   <select id="vm_network_config_network_select_'+vm_network_config_rows+'" class="vm_network_config_network_select">';
+      content    += '     <option value="">None</option>';
+      for(i = 0; i < nics.length; ++i){
+       if(!nics[i].selected)
+        content  += '     <option value="' + nics[i].network_id + '">' + nics[i].name + '</option>';
+      }
+      content    += '   </select>';
+      content    += ' </div>';
+      content    += ' <div id="vm_network_config_mac_'+vm_network_config_rows+'" class="vm_network_config_mac">';
+      content    +=  '  <input style="width: 130px;"></input>';
+      content    += ' </div>';
+      content    += ' <div id="vm_network_config_ip_'+vm_network_config_rows+'" class="vm_network_config_ip">';
+      content    += '     ';
+      content    += ' </div>';
+
+      if(!no_remove_link){
+        content  += ' <div id="vm_network_config_remove_'+vm_network_config_rows+'" class="vm_network_config_remove">';
+        content  += '  Remove';
+        content  += ' </div>';
+      }
+      content    += ' <div class="clear_row"></div><div class="clear_row"></div><div style="clear:both;"></div>';
+      content    += '</div>';
+
+      $('#vm_network_config_networks').append(content);
+
+      $('#vm_network_config_networks').ready(function(){
+        // when vm_network_config_remove link is click remove target row
+        $('#vm_network_config_remove_'+vm_network_config_rows).bind('click', function(e){
+            remove_network_config_row(e.target.id.substr(25)); // remove vm_network_config_remove_ bit to get row num
+        });
+
+        // when select box clicked, store current value for use on change
+        $('#vm_network_config_network_select_'+vm_network_config_rows).bind('click', function(e){
+           current_selectbox_value = e.target.value;
+        });
+
+        // when value of network select box is switched
+        $('#vm_network_config_network_select_'+vm_network_config_rows).bind('change', function(e){
+	     row = e.target.id.substr(33)
+
+             // find nic w/ selected network id
+	     for(i = 0; i < nics.length; ++i){
+	       if(nics[i].network_id == e.target.value){
+		  nics[i].selected = true;
+
+	          // fill in mac / ip address textfields as necessary
+		  $('#vm_network_config_mac_'+row).html('<input id="vm_network_config_mac_field_'+nics[i].network_id+'" style="width: 130px" value="'+nics[i].mac+'"/>');
+		  if(nics[i].static_ip != ""){
+		    $('#vm_network_config_ip_'+row).html('<input id="vm_network_config_ip_field_'+nics[i].ip+'" style="width: 130px" value="'+nics[i].ip+'"/>');
+		  }else{
+                     $('#vm_network_config_ip_'+row).html(' ');
+		  }
+
+		  // for the other select boxes, removed selected network
+		  $('.vm_network_config_network_select:not(#vm_network_config_network_select_'+row+') option[@value='+nics[i].network_id+']').remove();
+
+		  break;
+	       }
+              }
+
+	      // if we are clearing the row, do so
+	      if(e.target.value == ""){
+                  $('#vm_network_config_mac_'+row).html('<input id="vm_network_config_mac_field_'+row+'" style="width: 130px" value=""/>');
+                  $('#vm_network_config_ip_'+row).html(' ');
+	      }
+
+              // add unselected network back to other selectboxes.
+	      add_unselected_network('.vm_network_config_network_select:not(#vm_network_config_network_select_'+row+')', current_selectbox_value);
+
+              // show / hide ip address column
+              toggle_ip_address_column();
+
+              // only add a new blank row if last row's select box was changed
+              if(e.target.value != "" && row == vm_network_config_last_row){
+                // add row
+                add_network_config_row();
+              }
+        });
+      });
+
+      // show / hide ip address column
+      toggle_ip_address_column();
+   }
+
+
+
+   // remove a network config row
+   function remove_network_config_row(row_num){
+      // if trying to remove the first row or a nonexistant one, fail to do so
+      if(row_num < 2 || row_num > vm_network_config_last_row)
+          return;
+
+      // get selected network, add it to other selectboxes
+      network_id = $('#vm_network_config_network_select_' + row_num).val();
+      add_unselected_network('.vm_network_config_network_select:not(#vm_network_config_network_select_'+row_num+')', network_id);
+
+      // remove the row
+      $('#vm_network_config_row_' + row_num).remove();
+
+      // when removed, set global params
+      $('#vm_network_config_networks').ready(function(){
+        vm_network_config_rows -= 1;
+        rows = $('#vm_network_config_networks').children();
+        vm_network_config_last_row = rows[rows.length - 1].id.substr(22);
+
+        // show / hide ip address column
+        toggle_ip_address_column();
+      });
+   }
+
+   // intially show only one vm network config row
+   $(document).ready(function(){
+      add_network_config_row(true);
+   });
+
+   // when vm_network_config_add link is clicked show new row
+   $('#vm_network_config_add').bind('click', function(){
+       // TODO check if there exists an empty row
+       // TODO check to see if we've already added as many rows as there are nets
+       add_network_config_row();
+   });
 </script>
diff --git a/src/app/views/vm/_grid.rhtml b/src/app/views/vm/_grid.rhtml
index b137de6..7ac3fdf 100644
--- a/src/app/views/vm/_grid.rhtml
+++ b/src/app/views/vm/_grid.rhtml
@@ -34,7 +34,6 @@
         <% end %>
         {display: 'CPUs', name : 'num_vcpus_allocated', width : 40, sortable : true, align: 'left'},
         {display: 'Memory (MB)', name : 'memory_allocated', width : 60, sortable : true, align: 'right'},
-        {display: 'vNIC Mac Addr', name : 'vnic_mac_addr', width : 60, sortable : true, align: 'right'},
         {display: 'State', name : 'state', width : 50, sortable : true, align: 'right'},
         {display: 'Load', name : 'load', width: 180, sortable : false, align: 'left', process: <%= table_id %>_load_widget }
         ],
diff --git a/src/app/views/vm/_list.rhtml b/src/app/views/vm/_list.rhtml
index 42300d6..54ae741 100644
--- a/src/app/views/vm/_list.rhtml
+++ b/src/app/views/vm/_list.rhtml
@@ -4,7 +4,6 @@
     <th class="empty">Description <span class="amp">&</span> UUID</th>
     <th>CPUs</th>
     <th>Memory (mb)</th>
-    <th>vNIC MAC Addr</th>
     <th>State</th>
   </thead>
   
@@ -15,7 +14,6 @@
     <td style="text-align:left"><%= link_to vm.description, { :controller => "vm", :action => 'show', :id => vm }, { :class => "show" } %><div class="secondary"><%= vm.uuid %></div></td>
     <td><%= vm.num_vcpus_allocated %></td>
     <td><%= vm.memory_allocated_in_mb %></td>
-    <td><%= vm.vnic_mac_addr %></td>
     <td style="text-align:left">
       <%= vm.state %>
       <%unless vm.needs_restart.nil? or vm.needs_restart == 0  -%>
diff --git a/src/app/views/vm/show.rhtml b/src/app/views/vm/show.rhtml
index 0f70da8..a7a7388 100644
--- a/src/app/views/vm/show.rhtml
+++ b/src/app/views/vm/show.rhtml
@@ -106,7 +106,7 @@
 	Num vcpus used:<br/>
 	Memory allocated:<br/>
 	Memory used:<br/>
-	vNIC MAC address:<br/>
+	vNIC MAC addresses:<br/>
 	Boot device:<br/>
         Provisioning source:<br/>
 	State:<br/>
@@ -121,7 +121,9 @@
        <%=h @vm.num_vcpus_used %><br/>
        <%=h @vm.memory_allocated_in_mb %> MB<br/>
        <%=h @vm.memory_used_in_mb %> MB<br/>
-       <%=h @vm.vnic_mac_addr %><br/>
+       <% nic_macs = ""
+          @vm.nics.each { |nic| nic_macs += nic.mac + " " } %>
+       <%=h nic_macs %><br/>
        <%=h @vm.boot_device %><br/>
        <%=h @vm.provisioning_and_boot_settings %><br/>
        <%=h @vm.state %>
diff --git a/src/db/migrate/040_associate_vms_with_nics.rb b/src/db/migrate/040_associate_vms_with_nics.rb
new file mode 100644
index 0000000..3d4af03
--- /dev/null
+++ b/src/db/migrate/040_associate_vms_with_nics.rb
@@ -0,0 +1,103 @@
+# Copyright (C) 2009 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.
+
+class AssociateVmsWithNics < ActiveRecord::Migration
+  
+  def self.up
+     # assocate nics w/ vms
+     add_column :nics, :vm_id, :integer
+     execute "alter table nics add constraint
+              fk_nics_vm_id
+              foreign key (vm_id) references
+              vms(id)"
+     execute "alter table nics alter column host_id DROP NOT NULL"
+
+     # change physical_network_id column in nic table to network_id
+     execute 'alter table nics drop constraint fk_nic_networks'
+     execute 'alter table nics rename column physical_network_id to network_id'
+     execute 'alter table nics add constraint fk_nic_networks
+              foreign key (network_id) references networks(id)'
+
+
+     # create a nic for each vm / network
+     Vm.find(:all, :conditions => "network_id IS NOT NULL").each{ |vm|
+       nic = Nic.new(:mac => vm.vnic_mac_addr,
+                     :network_id => vm.network_id, 
+                     :vm_id => vm.id, 
+                     :bandwidth => 0)
+       nic.vm = vm
+       vm.nics.push nic
+
+       vm.save!
+       nic.save!
+     }
+
+     # removes 1toM networks/vms relationship
+     # remove network_id column from vms table
+     # remove vnic_mac_addr column from vms table
+     execute 'alter table vms drop constraint fk_vm_network_id'
+     remove_column :vms, :network_id
+     remove_column :vms, :vnic_mac_addr
+     
+     # add to the db some validations
+     #   host_id is set xor vm_id  is set
+     execute 'alter table nics add constraint host_xor_vm
+              check (host_id IS NOT NULL and vm_id IS NULL or
+                     vm_id IS NOT NULL and host_id IS NULL)'
+     #   network_id is set if vm_id is
+     execute 'alter table nics add constraint vm_nic_has_network
+              check (vm_id IS NULL or network_id IS NOT NULL)'
+     #   vm_id is set if network is vlan (TBD)
+#
+  end
+
+  def self.down
+    # drop constraints added to nics table
+    execute 'alter table nics drop constraint host_xor_vm'
+    execute 'alter table nics drop constraint vm_nic_has_network'
+
+    # add network_id, vnic_mac_addr column back to vm table
+    add_column :vms, :network_id, :integer
+    add_column :vms, :vnic_mac_addr, :string
+    execute 'alter table vms add constraint fk_vm_network_id
+             foreign key (network_id) references networks(id)'
+
+    # copy network id over
+    # NOTE since we're going from a MtoM relationship to a 1toM
+    #  we're just gonna associate the last network found 
+    #  w/ the vm, so this operation is lossy
+    Nic.find(:all, :conditions => 'vm_id IS NOT NULL').each{ |nic|
+       vm = Vm.find(nic.vm_id)
+       vm.vnic_mac_addr = nic.mac
+       vm.network_id = nic.network_id
+       vm.save!
+       nic.destroy
+    }
+
+    # unassociate nics / vms
+    remove_column :nics, :vm_id
+    execute "alter table nics alter column host_id SET NOT NULL"
+
+    # change nic::network_id back to nic::physical_network_id
+    execute 'alter table nics drop constraint fk_nic_networks'
+    execute 'alter table nics rename column network_id to physical_network_id'
+    execute 'alter table nics add constraint fk_nic_networks
+              foreign key (physical_network_id) references networks(id)'
+
+  end
+end
diff --git a/src/public/stylesheets/components.css b/src/public/stylesheets/components.css
index 1409692..09f7b06 100644
--- a/src/public/stylesheets/components.css
+++ b/src/public/stylesheets/components.css
@@ -343,6 +343,45 @@
   padding-left: 30px;
 }
 
+#vm_network_config .i {
+  float: left;
+}
+
+#vm_network_config_header_network {
+  float: left;
+  width: 20%;
+}
+#vm_network_config_header_mac, #vm_network_config_header_ip{
+  float: left;
+  width: 33%;
+}
+
+.vm_network_config_net {
+  float: left;
+  width: 20%;
+}
+.vm_network_config_mac, .vm_network_config_ip{
+  float: left;
+  width: 33%;
+  min-width: 100px;
+}
+
+.vm_network_config_remove {
+  float: left;
+  padding-top: 5px;
+  color: #0033CC;
+}
+.vm_network_config_remove:hover {
+  cursor: pointer;
+}
+
+#vm_network_config_add {
+  color: #0033CC;
+}
+#vm_network_config_add:hover {
+  cursor: pointer;
+}
+
 #vm_general_section_link img, #vm_resources_section_link img, #vm_storage_section_link img, #vm_network_section_link img{
   float: left;
   padding-top: 2px;
diff --git a/src/task-omatic/task_vm.rb b/src/task-omatic/task_vm.rb
index a448d30..dd71747 100644
--- a/src/task-omatic/task_vm.rb
+++ b/src/task-omatic/task_vm.rb
@@ -35,7 +35,7 @@ end
 
 
 def create_vm_xml(name, uuid, memAllocated, memUsed, vcpus, bootDevice,
-                  macAddr, bridge, diskDevices)
+                  net_interfaces, diskDevices)
   doc = Document.new
 
   doc.add_element("domain", {"type" => "kvm"})
@@ -87,11 +87,13 @@ def create_vm_xml(name, uuid, memAllocated, memUsed, vcpus, bootDevice,
     which_device += 1
   end
 
-  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
+  net_interfaces.each { |nic|
+    interface = Element.new("interface")
+    interface.add_attribute("type", "bridge")
+    interface.add_element("mac", {"address" => nic[:mac]})
+    interface.add_element("source", {"bridge" => nic[:interface]})
+    doc.root.elements["devices"] << interface
+  }
 
   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 b3c0592..7eb8a6c 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -168,6 +168,8 @@ class TaskOmatic
          and node.memory >= db_vm.memory_allocated \
          and not curr.is_disabled.nil? and curr.is_disabled == 0 \
          and ((!vm or vm.active == 'false') or vm.node != node.object_id)
+        # FIXME ensure host is on all networks a vm's assigned to
+        # db_vm.nics.each { |nic| ignore if nic.network ! in host }
         possible_hosts.push(curr)
       end
     end
@@ -360,31 +362,34 @@ class TaskOmatic
     @logger.debug("Connecting volumes: #{volumes}")
     storagedevs = connect_storage_pools(node, volumes)
 
-    # 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
+    # loop through each nic/network assigned to vm,
+    #  finding necessary host devices to bridge
 
-    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 = "br" + device.interface_name unless device.nil?
+    net_interfaces = []
+    db_vm.nics.each { |nic|
+       device = net_device = nil
 
-      else
+       if nic.network.class == PhysicalNetwork
+         device = Nic.find(:first,
+	            :conditions => ["host_id = ? AND network_id = ?",
+                                    db_host.id, nic.network_id ])
+       else
          device = Bonding.find(:first,
-                               :conditions => ["host_id = ? AND vlan_id = ?",
-                                               db_host.id, db_vm.network_id])
-         net_device = "br" + device.interface_name unless device.nil?
-      end
-    end
+	           :conditions => ["host_id = ? AND vlan_id = ?",
+                                    db_host.id, nic.network_id ])
+       end
+
+       unless device.nil?
+         net_device = "br" + device.interface_name
+       else
+         net_device = "breth0" # FIXME remove this default at some point
+       end
+       net_interfaces.push({ :mac => nic.mac, :interface => net_device })
+    }
 
     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, net_device, storagedevs)
+              net_interfaces, storagedevs, @logger)
 
     @logger.debug("XML Domain definition: #{xml}")
 
diff --git a/src/test/fixtures/nics.yml b/src/test/fixtures/nics.yml
index 97397cd..aff7e9b 100644
--- a/src/test/fixtures/nics.yml
+++ b/src/test/fixtures/nics.yml
@@ -3,21 +3,21 @@ mailserver_nic_one:
   usage_type: 1
   bandwidth: 100
   host: mailservers_managed_node
-  physical_network: mail_network_one
+  network: mail_network_one
 
 mailserver_nic_two:
   mac: 22:11:33:66:44:55
   usage_type: 1
   bandwidth: 100
   host: mailservers_managed_node
-  physical_network: mail_network_two
+  network: mail_network_two
 
 fileserver_nic_one:
   mac: 00:99:00:99:13:07
   usage_type: 1
   bandwidth: 100
   host: fileserver_managed_node
-  physical_network: fileserver_network
+  network: fileserver_network
 
 ldapserver_nic_one:
   mac: 00:03:02:00:09:06
@@ -25,32 +25,32 @@ ldapserver_nic_one:
   bandwidth: 100
   bridge:
   host: ldapserver_managed_node
-  physical_network: static_physical_network_one
+  network: static_physical_network_one
 
 buildserver_nic_one:
   mac: 07:17:19:65:03:38
   usage_type: 1
   bandwidth: 100
   host: buildserver_managed_node
-  physical_network: dhcp_physical_network_one
+  network: dhcp_physical_network_one
 
 buildserver_nic_two:
   mac: 07:17:19:65:03:39
   usage_type: 1
   bandwidth: 100
   host: buildserver_managed_node
-  physical_network: static_physical_network_one
+  network: static_physical_network_one
 
 mediaserver_nic_one:
   mac: 07:17:19:65:03:32
   usage_type: 1
   bandwidth: 100
   host: mediaserver_managed_node
-  physical_network: mediaserver_network_one
+  network: mediaserver_network_one
 
 mediaserver_nic_two:
   mac: 07:17:19:65:03:31
   usage_type: 1
   bandwidth: 100
   host: mediaserver_managed_node
-  physical_network: mediaserver_network_two
+  network: mediaserver_network_two
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index 69b1c2b..c47eb68 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -5,7 +5,6 @@ production_httpd_vm:
   num_vcpus_used: 2
   memory_allocated: 262144
   memory_used: 131072
-  vnic_mac_addr: 23:51:90:A1:13:37
   state: stopped
   needs_restart: 0
   boot_device: hd
@@ -20,7 +19,6 @@ production_mysqld_vm:
   num_vcpus_used: 1
   memory_allocated: 2048
   memory_used: 512
-  vnic_mac_addr: 15:99:FE:ED:11:EE
   state: running
   needs_restart: 0
   boot_device: network
@@ -33,7 +31,6 @@ production_ftpd_vm:
   num_vcpus_used: 1
   memory_allocated: 1024
   memory_used: 512
-  vnic_mac_addr: FF:AA:BB:00:11:55
   state: stopped
   needs_restart: 1
   boot_device: cdrom
@@ -46,7 +43,6 @@ production_postgresql_vm:
   num_vcpus_used: 2
   memory_allocated: 2048
   memory_used: 1536
-  vnic_mac_addr: 48:24:12:93:42:11
   state: running
   needs_restart: 0
   boot_device: hd
@@ -59,7 +55,6 @@ foobar_prod1_vm:
   num_vcpus_used: 2
   memory_allocated: 4096 
   memory_used: 4096
-  vnic_mac_addr: FF:FF:FF:EE:EE:EE
   state: running
   needs_restart: 0
   boot_device: cdrom
@@ -72,7 +67,6 @@ foobar_prod2_vm:
   num_vcpus_used: 2
   memory_allocated: 4096 
   memory_used: 4096
-  vnic_mac_addr: EE:EE:EE:FF:FF:FF
   state: running
   needs_restart: 0
   boot_device: cdrom
@@ -85,7 +79,6 @@ corp_com_errata_vm:
   num_vcpus_used: 2
   memory_allocated: 2048 
   memory_used: 2048
-  vnic_mac_addr: 77:77:77:77:77:77
   state: running
   needs_restart: 0
   boot_device: network
@@ -98,7 +91,6 @@ corp_com_bugzilla_vm:
   num_vcpus_used: 2
   memory_allocated: 2048 
   memory_used: 2048
-  vnic_mac_addr: 77:77:77:77:77:77
   state: running
   needs_restart: 0
   boot_device: network
diff --git a/src/test/unit/network_test.rb b/src/test/unit/network_test.rb
index 64c5df4..9b75e8b 100644
--- a/src/test/unit/network_test.rb
+++ b/src/test/unit/network_test.rb
@@ -1,8 +1,69 @@
-require 'test_helper'
+# 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.
+
+require File.dirname(__FILE__) + '/../test_helper'
 
 class NetworkTest < ActiveSupport::TestCase
-  # Replace this with your real tests.
-  def test_truth
-    assert true
+  fixtures :networks
+  fixtures :vms
+  fixtures :hosts
+  fixtures :nics
+  fixtures :boot_types
+
+  def setup
+  end
+
+  def test_vlan_invalid_without_number
+    vl = Vlan.new({:name => 'testvlan', :boot_type_id => 2})
+    flunk "Vlan without number should not be valid" if vl.valid?
+    vl.number = 1
+    flunk "Vlan with number should be valid" unless vl.valid?
+  end
+
+  def test_vlan_nics_only_associated_with_vm
+    vl = Vlan.create({:name => 'testvlan',
+                      :boot_type => boot_types(:boot_type_dhcp),
+		      :number => 1}) # need to create for id
+    nic = Nic.new({:mac => '11:22:33:44:55:66',
+                   :bandwidth => 100,
+		   :network => vl,
+		   :host => hosts(:prod_corp_com)})
+    vl.nics.push nic
+    flunk "Nic assigned to vlan must only be associated with vm" if vl.valid?
+    nic.host = nil
+    nic.vm = vms(:production_httpd_vm)
+    flunk "Vlan consisting of only vm nics should be valid" unless vl.valid?
+  end
+
+  def test_physical_network_is_destroyable
+    pn = PhysicalNetwork.new
+    flunk "PhysicalNetwork with no nics should be destroyable" unless pn.is_destroyable?
+    pn.nics.push Nic.new
+    flunk "PhysicalNetwork with nics should not be destroyable" if pn.is_destroyable?
+  end
+
+  def test_vlan_is_destroyable
+    vl = Vlan.new
+    flunk "Vlan with no nics and bondings should be destroyable" unless vl.is_destroyable?
+    vl.nics.push Nic.new
+    flunk "Vlan with nics should not be destroyable" if vl.is_destroyable?
+    vl.nics.clear
+    vl.bondings.push Bonding.new
+    flunk "Vlan with bondings should not be destroyable" if vl.is_destroyable?
   end
 end
diff --git a/src/test/unit/nic_test.rb b/src/test/unit/nic_test.rb
index 07f54c6..b6bdb07 100644
--- a/src/test/unit/nic_test.rb
+++ b/src/test/unit/nic_test.rb
@@ -24,6 +24,7 @@ class NicTest < Test::Unit::TestCase
   fixtures :nics
   fixtures :hosts
   fixtures :networks
+  fixtures :vms
 
   def setup
     @nic = Nic.new(
@@ -31,7 +32,7 @@ class NicTest < Test::Unit::TestCase
          :usage_type => 1,
          :bandwidth => 100 )
     @nic.host = hosts(:prod_corp_com)
-    @nic.physical_network = networks(:static_physical_network_one)
+    @nic.network = networks(:static_physical_network_one)
 
     @ip_address = IpV4Address.new(
          :address => '1.2.3.4',
@@ -74,10 +75,100 @@ class NicTest < Test::Unit::TestCase
   end
 
   def test_static_network_nic_must_have_ip
-    @nic.physical_network = networks(:static_physical_network_one)
+    @nic.network = networks(:static_physical_network_one)
     @nic.ip_addresses.delete_if { true }
 
     flunk 'Nics assigned to static networks must have at least one ip' if @nic.valid?
   end
 
+  def test_vm_nic_must_have_network
+     @nic.host = nil
+     @nic.vm = vms(:production_httpd_vm)
+     flunk 'vm nic that is assigned to network is valid' unless @nic.valid?
+
+     @nic.network = nil
+     flunk 'vm nic without a network is not valid' if @nic.valid?
+  end
+
+  def test_host_nic_cant_be_assigned_to_vlan
+     @nic.network = networks(:dhcp_vlan_one)
+     flunk 'host nic cant be assgined to vlan' if @nic.valid?
+  end
+
+  def test_nic_networking
+     flunk 'nic.networking? should return true if assigned to network' unless @nic.networking?
+     @nic.network = nil
+     flunk 'nic.networking? should return false if not assigned to network' if @nic.networking?
+  end
+
+  def test_nic_boot_protocol
+      nic = Nic.new
+      nic.ip_addresses << @ip_address
+      nic.network = networks(:static_physical_network_one)
+      flunk 'incorrect nic boot protocol' unless nic.boot_protocol == 'static'
+  end
+
+  def test_nic_ip_addresses
+      nic = Nic.new
+      nic.ip_addresses << @ip_address
+      nic.network = networks(:static_physical_network_one)
+      flunk 'incorrect nic ip address' unless nic.ip_address == @ip_address.address
+  end
+
+  def test_nic_netmask
+      nic = Nic.new
+      network = Network.new
+      network.ip_addresses << @ip_address
+      nic.network = network
+
+      flunk 'incorrect nic netmask' unless nic.netmask == @ip_address.netmask
+  end
+
+
+  def test_nic_broadcast
+      nic = Nic.new
+      network = Network.new
+      network.ip_addresses << @ip_address
+      nic.network = network
+
+      flunk 'incorrect nic broadcast' unless nic.broadcast == @ip_address.broadcast
+  end
+
+  def test_nic_gateway
+      nic = Nic.new
+      network = Network.new
+      network.ip_addresses << @ip_address
+      nic.network = network
+
+      flunk 'incorrect nic gateway' unless nic.gateway == @ip_address.gateway
+  end
+
+  def test_nic_parent
+      flunk 'incorrect host nic parent' unless @nic.parent == hosts(:prod_corp_com)
+
+      @nic.host = nil
+      flunk 'incorrect nic parent' unless @nic.parent == nil
+
+      @nic.vm = vms(:production_httpd_vm)
+      flunk 'incorrect vm nic parent' unless @nic.parent == vms(:production_httpd_vm)
+  end
+
+  def test_nic_gen_mac
+      mac = Nic::gen_mac
+      flunk 'invalid generated mac' unless mac =~ /^([0-9a-fA-F]{2}([:-]|$)){6}$/
+  end
+
+  def test_nic_vm_xor_nic_host
+      flunk 'host nic without vm is valid' unless @nic.valid?
+
+      @nic.vm = vms(:production_httpd_vm)
+      flunk 'nic cannot specify both host and vm' if @nic.valid?
+
+      @nic.host = nil
+      flunk 'vm nic without host is valid' unless @nic.valid?
+
+      @nic.vm = nil
+      flunk 'nic must specify either host or vm' if @nic.valid?
+  end
+
 end
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index 5e03715..0592b57 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -37,8 +37,8 @@ class VmTest < Test::Unit::TestCase
        :num_vcpus_allocated => 1,
        :boot_device => 'hd',
        :memory_allocated_in_mb => 1,
-       :memory_allocated => 1024,
-       :vnic_mac_addr => '11:22:33:44:55:66')
+       :memory_allocated => 1024)
+
 
     @vm.vm_resource_pool = pools(:corp_com_production_vmpool)
   end
@@ -75,11 +75,6 @@ class VmTest < Test::Unit::TestCase
        flunk 'Vm must specify memory_allocated_in_mb' if @vm.valid?
   end
 
-  def test_valid_fails_without_vnic_mac_addr
-       @vm.vnic_mac_addr = ''
-       flunk 'Vm must specify vnic_mac_addr' if @vm.valid?
-  end
-
   def test_valid_fails_without_vm_resources_pool_id
        @vm.vm_resource_pool_id = ''
        flunk 'Vm must specify vm_resources_pool_id' if @vm.valid?
@@ -176,4 +171,9 @@ class VmTest < Test::Unit::TestCase
   def test_paginated_results
     assert_equal 5, Vm.paged_with_perms('ovirtadmin', Privilege::VIEW, 1, 'vms.id').size
   end
+
+  def test_vm_gen_uuid
+      uuid = Vm::gen_uuid
+      flunk 'invalid generated uuid' unless uuid =~ /[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/
+  end
 end
-- 
1.6.0.6




More information about the ovirt-devel mailing list