Ovirt devel - Jul 2009 - permit many-to-many vms / networks relationship

This patchset contains changes to the ovirt server
frontend, backend, and tests components, permitting vms
to be associated with multiple networks and vice versa. 

Also included are two patches which are required for the frontend
bits; a patch adding collapsable sections to the vm form, which
in itself depends on the second patch that provides default values
for the cpu and memory vm table fields

implements changes to the controllers, models,
  services, and taskomatic to permit vms to be
  associated with multiple networks.

  additional various fixes / cleanup to get
  things working
---
 src/app/controllers/pool_controller.rb        |    2 +-
 src/app/controllers/vm_controller.rb          |   54 ++++++++++++-
 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/db/migrate/041_associate_vms_with_nics.rb |  103 +++++++++++++++++++++++++
 src/task-omatic/task_vm.rb                    |   14 ++--
 src/task-omatic/taskomatic.rb                 |   43 ++++++-----
 12 files changed, 276 insertions(+), 81 deletions(-)
 create mode 100644 src/db/migrate/041_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/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 d4696cf..b4035b4 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
@@ -53,8 +53,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})
@@ -96,7 +96,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
 
@@ -119,8 +119,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"
@@ -420,6 +420,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 073b819..54b7787 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
@@ -285,10 +292,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
 
@@ -305,17 +317,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/db/migrate/041_associate_vms_with_nics.rb
b/src/db/migrate/041_associate_vms_with_nics.rb
new file mode 100644
index 0000000..ce8ad07
--- /dev/null
+++ b/src/db/migrate/041_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/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}")
 
-- 
1.6.0.6

On Thu, 2009-07-09 at 17:56 -0400, Mohammed Morsi wrote:
> This patchset contains changes to the ovirt server
> frontend, backend, and tests components, permitting vms
> to be associated with multiple networks and vice versa. 
> 
> Also included are two patches which are required for the frontend
> bits; a patch adding collapsable sections to the vm form, which
> in itself depends on the second patch that provides default values
> for the cpu and memory vm table fields
I am about to send feedback on the 'add collapsable sections patch, and
likely then moving on to 'provide default vm..' patch.  However, once
you make the request changes to the form, could you please resend the
entire series with '--no-chain-reply-to' so the patches are numbered?
This will make it a lot easier for me to know what order to test them
in.  Thanks,

-j
> 
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/ovirt-devel