Code review comment for lp:~tr3buchet/nova/multi_nic

Thanks Trey. Couple of the my concerns are addressed by you.

Pending and new problems I found out in rev. 838 are listed below:-

1) Typo problem
patch:-
=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py 2011-06-21 16:59:22 +0000
+++ nova/compute/manager.py 2011-06-21 18:09:06 +0000
@@ -301,7 +301,7 @@
         self._update_state(context, instance_id, power_state.BUILDING)

         try:
- self.driver.spawn(instance_ref, network_info, block_device_mapping)
+ self.driver.spawn(instance, network_info, block_device_mapping)
         except Exception as ex: # pylint: disable=W0702
             msg = _("Instance '%(instance_id)s' failed to spawn. Is "
                     "virtualization enabled in the BIOS? Details: "

2) Floating IP addresses are not disassociated when you terminate an instance.

Patch:-
=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py 2011-06-21 16:59:22 +0000
+++ nova/db/sqlalchemy/api.py 2011-06-21 19:38:45 +0000
@@ -754,6 +754,7 @@
 def fixed_ip_get_by_instance(context, instance_id):
     session = get_session()
     rv = session.query(models.FixedIp).\
+ options(joinedload('floating_ips')).\
                  filter_by(instance_id=instance_id).\
                  filter_by(deleted=False).\
                  all()

=== modified file 'nova/network/api.py'
--- nova/network/api.py 2011-06-17 18:47:28 +0000
+++ nova/network/api.py 2011-06-21 19:31:35 +0000
@@ -107,7 +107,7 @@
             return
         if not floating_ip.get('fixed_ip'):
             raise exception.ApiError('Address is not associated.')
- host = floating_ip['host']
+ host = floating_ip['fixed_ip']['network']['host']
         rpc.call(context,
                  self.db.queue_get_for(context, FLAGS.network_topic, host),
                  {'method': 'disassociate_floating_ip',

3) Virtual interfaces db records should be deleted in the release_fixed_ip method instead of deallocate_for_instance method in the class NetworkManager. Also while updating dhcp information for a particular network filter associated fixed IPs whose virtual interfaces are set to NULL.

Patch:-
=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py 2011-06-21 16:59:22 +0000
+++ nova/db/sqlalchemy/api.py 2011-06-21 19:38:45 +0000
@@ -1566,6 +1567,7 @@
                    options(joinedload_all('instance')).\
                    filter_by(network_id=network_id).\
                    filter(models.FixedIp.instance_id != None).\
+ filter(models.FixedIp.virtual_interface_id != None).\
                    filter_by(deleted=False).\
                    all()

=== modified file 'nova/network/manager.py'
--- nova/network/manager.py 2011-06-21 16:51:08 +0000
+++ nova/network/manager.py 2011-06-21 19:21:15 +0000
@@ -379,8 +379,6 @@
                   self.db.fixed_ip_get_by_instance(context, instance_id)
         LOG.debug(_("network deallocation for instance |%s|"), instance_id,
                                                                context=context)
- # deallocate mac addresses
- self.db.virtual_interface_delete_by_instance(context, instance_id)

         # deallocate fixed ips
         for fixed_ip in fixed_ips:
@@ -534,11 +532,16 @@
         if not fixed_ip['leased']:
             LOG.warn(_('IP %s released that was not leased'), address,
                      context=context)
+
         self.db.fixed_ip_update(context,
                                 fixed_ip['address'],
                                 {'leased': False})
         if not fixed_ip['allocated']:
             self.db.fixed_ip_disassociate(context, address)
+
+ # deallocate mac addresses
+ self.db.virtual_interface_delete(context, fixed_ip['virtual_interface']['id'])
+
             # NOTE(vish): dhcp server isn't updated until next setup, this
             # means there will stale entries in the conf file
             # the code below will update the file if necessary