merged trunk

8 files changed, 230 insertions, 140 deletions

diff --git a/nova/db/api.py b/nova/db/api.py
index 3e5b9fdac..608b92d70 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -144,12 +144,20 @@ def floating_ip_get_instance(context, address):
 ####################
 
 
-def fixed_ip_allocate(context, network_id):
-    """Allocate free fixed ip and return the address.
+def fixed_ip_associate(context, address, instance_id):
+    """Associate fixed ip to instance.
+
+    Raises if fixed ip is not available.
+    """
+    return IMPL.fixed_ip_associate(context, address, instance_id)
+
+
+def fixed_ip_associate_pool(context, network_id, instance_id):
+    """Find free ip in network and associate it to instance.
 
     Raises if one is not available.
     """
-    return IMPL.fixed_ip_allocate(context, network_id)
+    return IMPL.fixed_ip_associate_pool(context, network_id, instance_id)
 
 
 def fixed_ip_create(context, values):
@@ -157,9 +165,9 @@ def fixed_ip_create(context, values):
     return IMPL.fixed_ip_create(context, values)
 
 
-def fixed_ip_deallocate(context, address):
-    """Deallocate a fixed ip by address."""
-    return IMPL.fixed_ip_deallocate(context, address)
+def fixed_ip_disassociate(context, address):
+    """Disassociate a fixed ip from an instance by address."""
+    return IMPL.fixed_ip_disassociate(context, address)
 
 
 def fixed_ip_get_by_address(context, address):
@@ -177,16 +185,6 @@ def fixed_ip_get_network(context, address):
     return IMPL.fixed_ip_get_network(context, address)
 
 
-def fixed_ip_instance_associate(context, address, instance_id):
-    """Associate a fixed ip to an instance by address."""
-    return IMPL.fixed_ip_instance_associate(context, address, instance_id)
-
-
-def fixed_ip_instance_disassociate(context, address):
-    """Disassociate a fixed ip from an instance by address."""
-    return IMPL.fixed_ip_instance_disassociate(context, address)
-
-
 def fixed_ip_update(context, address, values):
     """Create a fixed ip from the values dictionary."""
     return IMPL.fixed_ip_update(context, address, values)
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index c0237e9e4..d593abbcc 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -183,7 +183,25 @@ def floating_ip_get_instance(_context, address):
 ###################
 
 
-def fixed_ip_allocate(_context, network_id):
+def fixed_ip_associate(_context, address, instance_id):
+    session = get_session()
+    with session.begin():
+        fixed_ip_ref = session.query(models.FixedIp
+                             ).filter_by(address=address
+                             ).filter_by(deleted=False
+                             ).filter_by(instance=None
+                             ).with_lockmode('update'
+                             ).first()
+        # NOTE(vish): if with_lockmode isn't supported, as in sqlite,
+        #             then this has concurrency issues
+        if not fixed_ip_ref:
+            raise db.NoMoreAddresses()
+        fixed_ip_ref.instance = models.Instance.find(instance_id,
+                                                     session=session)
+        session.add(fixed_ip_ref)
+
+
+def fixed_ip_associate_pool(_context, network_id, instance_id):
     session = get_session()
     with session.begin():
         network_or_none = or_(models.FixedIp.network_id == network_id,
@@ -191,9 +209,8 @@ def fixed_ip_allocate(_context, network_id):
         fixed_ip_ref = session.query(models.FixedIp
                              ).filter(network_or_none
                              ).filter_by(reserved=False
-                             ).filter_by(allocated=False
-                             ).filter_by(leased=False
                              ).filter_by(deleted=False
+                             ).filter_by(instance=None
                              ).with_lockmode('update'
                              ).first()
         # NOTE(vish): if with_lockmode isn't supported, as in sqlite,
@@ -203,7 +220,8 @@ def fixed_ip_allocate(_context, network_id):
         if not fixed_ip_ref.network:
             fixed_ip_ref.network = models.Network.find(network_id,
                                                        session=session)
-        fixed_ip_ref['allocated'] = True
+        fixed_ip_ref.instance = models.Instance.find(instance_id,
+                                                     session=session)
         session.add(fixed_ip_ref)
     return fixed_ip_ref['address']
 
@@ -216,6 +234,14 @@ def fixed_ip_create(_context, values):
     return fixed_ip_ref['address']
 
 
+def fixed_ip_disassociate(_context, address):
+    session = get_session()
+    with session.begin():
+        fixed_ip_ref = models.FixedIp.find_by_str(address, session=session)
+        fixed_ip_ref.instance = None
+        fixed_ip_ref.save(session=session)
+
+
 def fixed_ip_get_by_address(_context, address):
     return models.FixedIp.find_by_str(address)
 
@@ -232,27 +258,6 @@ def fixed_ip_get_network(_context, address):
         return models.FixedIp.find_by_str(address, session=session).network
 
 
-def fixed_ip_deallocate(context, address):
-    db.fixed_ip_update(context, address, {'allocated': False})
-
-
-def fixed_ip_instance_associate(_context, address, instance_id):
-    session = get_session()
-    with session.begin():
-        fixed_ip_ref = models.FixedIp.find_by_str(address, session=session)
-        instance_ref = models.Instance.find(instance_id, session=session)
-        fixed_ip_ref.instance = instance_ref
-        fixed_ip_ref.save(session=session)
-
-
-def fixed_ip_instance_disassociate(_context, address):
-    session = get_session()
-    with session.begin():
-        fixed_ip_ref = models.FixedIp.find_by_str(address, session=session)
-        fixed_ip_ref.instance = None
-        fixed_ip_ref.save(session=session)
-
-
 def fixed_ip_update(_context, address, values):
     session = get_session()
     with session.begin():
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 7f510301a..c559159da 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -24,8 +24,7 @@ import sys
 import datetime
 
 # TODO(vish): clean up these imports
-from sqlalchemy.orm import relationship, backref, validates, exc
-from sqlalchemy.sql import func
+from sqlalchemy.orm import relationship, backref, exc, object_mapper
 from sqlalchemy import Column, Integer, String
 from sqlalchemy import ForeignKey, DateTime, Boolean, Text
 from sqlalchemy.ext.declarative import declarative_base
@@ -113,6 +112,14 @@ class NovaBase(object):
     def __getitem__(self, key):
         return getattr(self, key)
 
+    def __iter__(self):
+        self._i = iter(object_mapper(self).columns)
+        return self
+
+    def next(self):
+        n = self._i.next().name
+        return n, getattr(self, n)
+
 # TODO(vish): Store images in the database instead of file system
 #class Image(BASE, NovaBase):
 #    """Represents an image in the datastore"""
diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py
index adb63351f..749bf5f9c 100644
--- a/nova/endpoint/cloud.py
+++ b/nova/endpoint/cloud.py
@@ -90,13 +90,14 @@ class CloudController(object):
 
     def _get_mpi_data(self, project_id):
         result = {}
-        for instance in db.instance_get_by_project(project_id):
-            line = '%s slots=%d' % (instance.fixed_ip['str_id'],
-                INSTANCE_TYPES[instance['instance_type']]['vcpus'])
-            if instance['key_name'] in result:
-                result[instance['key_name']].append(line)
-            else:
-                result[instance['key_name']] = [line]
+        for instance in db.instance_get_by_project(None, project_id):
+            if instance['fixed_ip']:
+                line = '%s slots=%d' % (instance['fixed_ip']['str_id'],
+                    INSTANCE_TYPES[instance['instance_type']]['vcpus'])
+                if instance['key_name'] in result:
+                    result[instance['key_name']].append(line)
+                else:
+                    result[instance['key_name']] = [line]
         return result
 
     def get_metadata(self, address):
@@ -114,13 +115,13 @@ class CloudController(object):
         else:
             keys = ''
         hostname = instance_ref['hostname']
-        floating_ip = db.instance_get_floating_ip_address(None,
-                                                          instance_ref['id'])
+        floating_ip = db.instance_get_floating_address(None,
+                                                       instance_ref['id'])
         data = {
             'user-data': base64.b64decode(instance_ref['user_data']),
             'meta-data': {
                 'ami-id': instance_ref['image_id'],
-                'ami-launch-index': instance_ref['ami_launch_index'],
+                'ami-launch-index': instance_ref['launch_index'],
                 'ami-manifest-path': 'FIXME',
                 'block-device-mapping': {  # TODO(vish): replace with real data
                     'ami': 'sda1',
@@ -136,7 +137,7 @@ class CloudController(object):
                 'local-ipv4': address,
                 'kernel-id': instance_ref['kernel_id'],
                 'placement': {
-                    'availaibility-zone': instance_ref['availability_zone'],
+                    'availability-zone': 'nova' # TODO(vish): real zone
                 },
                 'public-hostname': hostname,
                 'public-ipv4': floating_ip or '',
@@ -376,12 +377,14 @@ class CloudController(object):
     def _format_instances(self, context, reservation_id=None):
         reservations = {}
         if reservation_id:
-            instances = db.instance_get_by_reservation(context, reservation_id)
+            instances = db.instance_get_by_reservation(context,
+                                                       reservation_id)
         else:
             if not context.user.is_admin():
                 instances = db.instance_get_all(context)
             else:
-                instances = db.instance_get_by_project(context, context.project.id)
+                instances = db.instance_get_by_project(context,
+                                                       context.project.id)
         for instance in instances:
             if not context.user.is_admin():
                 if instance['image_id'] == FLAGS.vpn_image_id:
@@ -393,15 +396,16 @@ class CloudController(object):
                 'code': instance['state'],
                 'name': instance['state_description']
             }
+            fixed_addr = None
             floating_addr = None
-            if instance['fixed_ip']['floating_ips']:
-                floating_addr = instance['fixed_ip']['floating_ips'][0]['str_id']
-            i['publicDnsName'] = floating_addr
-            fixed_addr = instance['fixed_ip']['str_id']
+            if instance['fixed_ip']:
+                fixed_addr = instance['fixed_ip']['str_id']
+                if instance['fixed_ip']['floating_ips']:
+                    fixed = instance['fixed_ip']
+                    floating_addr = fixed['floating_ips'][0]['str_id']
             i['privateDnsName'] = fixed_addr
-            if not i['publicDnsName']:
-                i['publicDnsName'] = i['privateDnsName']
-            i['dnsName'] = None
+            i['publicDnsName'] = floating_addr
+            i['dnsName'] = i['publicDnsName'] or i['privateDnsName']
             i['keyName'] = instance['key_name']
             if context.user.is_admin():
                 i['keyName'] = '%s (%s, %s)' % (i['keyName'],
@@ -435,8 +439,10 @@ class CloudController(object):
                                                      context.project.id)
         for floating_ip_ref in iterator:
             address = floating_ip_ref['str_id']
-            instance_ref = db.floating_ip_get_instance(context, address)
-            instance_id = instance_ref['str_id']
+            instance_id = None
+            if (floating_ip_ref['fixed_ip']
+                and floating_ip_ref['fixed_ip']['instance']):
+                instance_id = floating_ip_ref['fixed_ip']['instance']['str_id']
             address_rv = {'public_ip': address,
                           'instance_id': instance_id}
             if context.user.is_admin():
@@ -584,12 +590,13 @@ class CloudController(object):
         base_options['local_gb'] = type_data['local_gb']
 
         for num in range(num_instances):
-            inst_id = db.instance_create(context, base_options)['id']
+            instance_ref = db.instance_create(context, base_options)
+            inst_id = instance_ref['id']
 
             inst = {}
             inst['mac_address'] = utils.generate_mac()
             inst['launch_index'] = num
-            inst['hostname'] = inst_id
+            inst['hostname'] = instance_ref['str_id']
             db.instance_update(context, inst_id, inst)
             address = self.network_manager.allocate_fixed_ip(context,
                                                              inst_id,
@@ -647,7 +654,7 @@ class CloudController(object):
                 # NOTE(vish): Currently, nothing needs to be done on the
                 #             network node until release. If this changes,
                 #             we will need to cast here.
-                db.fixed_ip_deallocate(context, address)
+                self.network.deallocate_fixed_ip(context, address)
 
             host = instance_ref['host']
             if host:
diff --git a/nova/network/manager.py b/nova/network/manager.py
index 83de5d023..7a3bcfc2f 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -61,6 +61,8 @@ flags.DEFINE_integer('cnt_vpn_clients', 5,
                      'Number of addresses reserved for vpn clients')
 flags.DEFINE_string('network_driver', 'nova.network.linux_net',
                     'Driver to use for network creation')
+flags.DEFINE_bool('update_dhcp_on_disassociate', False,
+                  'Whether to update dhcp when fixed_ip is disassocated')
 
 
 class AddressAlreadyAllocated(exception.Error):
@@ -68,11 +70,6 @@ class AddressAlreadyAllocated(exception.Error):
     pass
 
 
-class AddressNotAllocated(exception.Error):
-    """Address has not been allocated"""
-    pass
-
-
 class NetworkManager(manager.Manager):
     """Implements common network manager functionality
 
@@ -101,6 +98,10 @@ class NetworkManager(manager.Manager):
         """Gets a fixed ip from the pool"""
         raise NotImplementedError()
 
+    def deallocate_fixed_ip(self, context, instance_id, *args, **kwargs):
+        """Returns a fixed ip to the pool"""
+        raise NotImplementedError()
+
     def setup_fixed_ip(self, context, address):
         """Sets up rules for fixed ip"""
         raise NotImplementedError()
@@ -175,10 +176,17 @@ class FlatManager(NetworkManager):
     def allocate_fixed_ip(self, context, instance_id, *args, **kwargs):
         """Gets a fixed ip from the pool"""
         network_ref = self.db.project_get_network(context, context.project.id)
-        address = self.db.fixed_ip_allocate(context, network_ref['id'])
-        self.db.fixed_ip_instance_associate(context, address, instance_id)
+        address = self.db.fixed_ip_associate_pool(context,
+                                                  network_ref['id'],
+                                                  instance_id)
+        self.db.fixed_ip_update(context, address, {'allocated': True})
         return address
 
+    def deallocate_fixed_ip(self, context, address, *args, **kwargs):
+        """Returns a fixed ip to the pool"""
+        self.db.fixed_ip_update(context, address, {'allocated': False})
+        self.db.fixed_ip_disassociate(context, address)
+
     def setup_compute_network(self, context, project_id):
         """Network is created manually"""
         pass
@@ -214,13 +222,29 @@ class VlanManager(NetworkManager):
         """Gets a fixed ip from the pool"""
         network_ref = self.db.project_get_network(context, context.project.id)
         if kwargs.get('vpn', None):
-            address = self._allocate_vpn_ip(context, network_ref['id'])
+            address = network_ref['vpn_private_address']
+            self.db.fixed_ip_associate(context, address, instance_id)
         else:
-            address = self.db.fixed_ip_allocate(context,
-                                                network_ref['id'])
-        self.db.fixed_ip_instance_associate(context, address, instance_id)
+            address = self.db.fixed_ip_associate_pool(context,
+                                                      network_ref['id'],
+                                                      instance_id)
+        self.db.fixed_ip_update(context, address, {'allocated': True})
         return address
 
+    def deallocate_fixed_ip(self, context, address, *args, **kwargs):
+        """Returns a fixed ip to the pool"""
+        self.db.fixed_ip_update(context, address, {'allocated': False})
+        fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
+        if not fixed_ip_ref['leased']:
+            self.db.fixed_ip_disassociate(context, address)
+            # 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
+            if FLAGS.update_dhcp_on_disassociate:
+                network_ref = self.db.fixed_ip_get_network(context, address)
+                self.driver.update_dhcp(context, network_ref['id'])
+
+
     def setup_fixed_ip(self, context, address):
         """Sets forwarding rules and dhcp for fixed ip"""
         fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
@@ -231,22 +255,47 @@ class VlanManager(NetworkManager):
                                             network_ref['vpn_private_address'])
         self.driver.update_dhcp(context, network_ref['id'])
 
-    def lease_fixed_ip(self, context, address):
+    def lease_fixed_ip(self, context, mac, address):
         """Called by dhcp-bridge when ip is leased"""
         logging.debug("Leasing IP %s", address)
         fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
         if not fixed_ip_ref['allocated']:
-            raise AddressNotAllocated(address)
+            logging.warn("IP %s leased that was already deallocated", address)
+            return
+        instance_ref = self.db.fixed_ip_get_instance(context, address)
+        if not instance_ref:
+            raise exception.Error("IP %s leased that isn't associated" %
+                                  address)
+        if instance_ref['mac_address'] != mac:
+            raise exception.Error("IP %s leased to bad mac %s vs %s" %
+                                  (address, instance_ref['mac_address'], mac))
         self.db.fixed_ip_update(context,
                                 fixed_ip_ref['str_id'],
                                 {'leased': True})
 
-    def release_fixed_ip(self, context, address):
+    def release_fixed_ip(self, context, mac, address):
         """Called by dhcp-bridge when ip is released"""
         logging.debug("Releasing IP %s", address)
-        self.db.fixed_ip_update(context, address, {'allocated': False,
-                                                   'leased': False})
-        self.db.fixed_ip_instance_disassociate(context, address)
+        fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
+        if not fixed_ip_ref['leased']:
+            logging.warn("IP %s released that was not leased", address)
+            return
+        instance_ref = self.db.fixed_ip_get_instance(context, address)
+        if not instance_ref:
+            raise exception.Error("IP %s released that isn't associated" %
+                                  address)
+        if instance_ref['mac_address'] != mac:
+            raise exception.Error("IP %s released from bad mac %s vs %s" %
+                                  (address, instance_ref['mac_address'], mac))
+        self.db.fixed_ip_update(context, address, {'leased': False})
+        if not fixed_ip_ref['allocated']:
+            self.db.fixed_ip_disassociate(context, address)
+            # 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
+            if FLAGS.update_dhcp_on_disassociate:
+                network_ref = self.db.fixed_ip_get_network(context, address)
+                self.driver.update_dhcp(context, network_ref['id'])
 
     def allocate_network(self, context, project_id):
         """Set up the network"""
@@ -288,19 +337,6 @@ class VlanManager(NetworkManager):
         # TODO(vish): Implement this
         pass
 
-    @staticmethod
-    def _allocate_vpn_ip(context, network_id):
-        """Allocate vpn ip for network"""
-        # TODO(vish): There is a possible concurrency issue here.
-        network_ref = db.network_get(context, network_id)
-        address = network_ref['vpn_private_address']
-        fixed_ip_ref = db.fixed_ip_get_by_address(context, address)
-        # TODO(vish): Should this be fixed_ip_is_allocated?
-        if fixed_ip_ref['allocated']:
-            raise AddressAlreadyAllocated()
-        db.fixed_ip_update(context, fixed_ip_ref['id'], {'allocated': True})
-        return fixed_ip_ref['str_id']
-
     def _ensure_indexes(self, context):
         """Ensure the indexes for the network exist
 
diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py
index 29947e03c..c36d5a34f 100644
--- a/nova/tests/cloud_unittest.py
+++ b/nova/tests/cloud_unittest.py
@@ -29,6 +29,7 @@ from nova import rpc
 from nova import test
 from nova import utils
 from nova.auth import manager
+from nova.compute import power_state
 from nova.endpoint import api
 from nova.endpoint import cloud
 
@@ -94,7 +95,7 @@ class CloudTestCase(test.BaseTestCase):
             rv = yield defer.succeed(time.sleep(1))
             info = self.cloud._get_instance(instance['instance_id'])
             logging.debug(info['state'])
-            if info['state'] == node.Instance.RUNNING:
+            if info['state'] == power_state.RUNNING:
                 break
         self.assert_(rv)
 
diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py
index 9958600e0..dc5277f02 100644
--- a/nova/tests/network_unittest.py
+++ b/nova/tests/network_unittest.py
@@ -28,6 +28,7 @@ from nova import flags
 from nova import test
 from nova import utils
 from nova.auth import manager
+from nova.endpoint import api
 
 FLAGS = flags.FLAGS
 
@@ -48,7 +49,7 @@ class NetworkTestCase(test.TrialTestCase):
         self.user = self.manager.create_user('netuser', 'netuser', 'netuser')
         self.projects = []
         self.network = utils.import_object(FLAGS.network_manager)
-        self.context = None
+        self.context = api.APIRequestContext(None, project=None, user=self.user)
         for i in range(5):
             name = 'project%s' % i
             self.projects.append(self.manager.create_project(name,
@@ -75,12 +76,10 @@ class NetworkTestCase(test.TrialTestCase):
 
     def _create_address(self, project_num, instance_id=None):
         """Create an address in given project num"""
-        net = db.project_get_network(None, self.projects[project_num].id)
-        address = db.fixed_ip_allocate(None, net['id'])
         if instance_id is None:
             instance_id = self.instance_id
-        db.fixed_ip_instance_associate(None, address, instance_id)
-        return address
+        self.context.project = self.projects[project_num]
+        return self.network.allocate_fixed_ip(self.context, instance_id)
 
     def test_public_network_association(self):
         """Makes sure that we can allocaate a public ip"""
@@ -103,14 +102,14 @@ class NetworkTestCase(test.TrialTestCase):
         address = db.instance_get_floating_address(None, self.instance_id)
         self.assertEqual(address, None)
         self.network.deallocate_floating_ip(self.context, float_addr)
-        db.fixed_ip_deallocate(None, fix_addr)
+        self.network.deallocate_fixed_ip(self.context, fix_addr)
 
     def test_allocate_deallocate_fixed_ip(self):
         """Makes sure that we can allocate and deallocate a fixed ip"""
         address = self._create_address(0)
         self.assertTrue(is_allocated_in_project(address, self.projects[0].id))
         lease_ip(address)
-        db.fixed_ip_deallocate(None, address)
+        self.network.deallocate_fixed_ip(self.context, address)
 
         # Doesn't go away until it's dhcp released
         self.assertTrue(is_allocated_in_project(address, self.projects[0].id))
@@ -131,14 +130,14 @@ class NetworkTestCase(test.TrialTestCase):
         lease_ip(address)
         lease_ip(address2)
 
-        db.fixed_ip_deallocate(None, address)
+        self.network.deallocate_fixed_ip(self.context, address)
         release_ip(address)
         self.assertFalse(is_allocated_in_project(address, self.projects[0].id))
 
         # First address release shouldn't affect the second
         self.assertTrue(is_allocated_in_project(address2, self.projects[1].id))
 
-        db.fixed_ip_deallocate(None, address2)
+        self.network.deallocate_fixed_ip(self.context, address2)
         release_ip(address2)
         self.assertFalse(is_allocated_in_project(address2,
                                                  self.projects[1].id))
@@ -147,10 +146,23 @@ class NetworkTestCase(test.TrialTestCase):
         """Makes sure that private ips don't overlap"""
         first = self._create_address(0)
         lease_ip(first)
+        instance_ids = []
         for i in range(1, 5):
-            address = self._create_address(i)
-            address2 = self._create_address(i)
-            address3 = self._create_address(i)
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address = self._create_address(i, instance_ref['id'])
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address2 = self._create_address(i, instance_ref['id'])
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address3 = self._create_address(i, instance_ref['id'])
             lease_ip(address)
             lease_ip(address2)
             lease_ip(address3)
@@ -160,14 +172,16 @@ class NetworkTestCase(test.TrialTestCase):
                                                      self.projects[0].id))
             self.assertFalse(is_allocated_in_project(address3,
                                                      self.projects[0].id))
-            db.fixed_ip_deallocate(None, address)
-            db.fixed_ip_deallocate(None, address2)
-            db.fixed_ip_deallocate(None, address3)
+            self.network.deallocate_fixed_ip(self.context, address)
+            self.network.deallocate_fixed_ip(self.context, address2)
+            self.network.deallocate_fixed_ip(self.context, address3)
             release_ip(address)
             release_ip(address2)
             release_ip(address3)
+        for instance_id in instance_ids:
+            db.instance_destroy(None, instance_id)
         release_ip(first)
-        db.fixed_ip_deallocate(None, first)
+        self.network.deallocate_fixed_ip(self.context, first)
 
     def test_vpn_ip_and_port_looks_valid(self):
         """Ensure the vpn ip and port are reasonable"""
@@ -194,12 +208,12 @@ class NetworkTestCase(test.TrialTestCase):
         """Makes sure that ip addresses that are deallocated get reused"""
         address = self._create_address(0)
         lease_ip(address)
-        db.fixed_ip_deallocate(None, address)
+        self.network.deallocate_fixed_ip(self.context, address)
         release_ip(address)
 
         address2 = self._create_address(0)
         self.assertEqual(address, address2)
-        db.fixed_ip_deallocate(None, address2)
+        self.network.deallocate_fixed_ip(self.context, address2)
 
     def test_available_ips(self):
         """Make sure the number of available ips for the network is correct
@@ -226,21 +240,27 @@ class NetworkTestCase(test.TrialTestCase):
         num_available_ips = db.network_count_available_ips(None,
                                                            network['id'])
         addresses = []
+        instance_ids = []
         for i in range(num_available_ips):
-            address = self._create_address(0)
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address = self._create_address(0, instance_ref['id'])
             addresses.append(address)
             lease_ip(address)
 
         self.assertEqual(db.network_count_available_ips(None,
                                                         network['id']), 0)
         self.assertRaises(db.NoMoreAddresses,
-                          db.fixed_ip_allocate,
-                          None,
-                          network['id'])
+                          self.network.allocate_fixed_ip,
+                          self.context,
+                          'foo')
 
-        for i in range(len(addresses)):
-            db.fixed_ip_deallocate(None, addresses[i])
+        for i in range(num_available_ips):
+            self.network.deallocate_fixed_ip(self.context, addresses[i])
             release_ip(addresses[i])
+            db.instance_destroy(None, instance_ids[i])
         self.assertEqual(db.network_count_available_ips(None,
                                                         network['id']),
                          num_available_ips)
@@ -263,7 +283,10 @@ def binpath(script):
 def lease_ip(private_ip):
     """Run add command on dhcpbridge"""
     network_ref = db.fixed_ip_get_network(None, private_ip)
-    cmd = "%s add fake %s fake" % (binpath('nova-dhcpbridge'), private_ip)
+    instance_ref = db.fixed_ip_get_instance(None, private_ip)
+    cmd = "%s add %s %s fake" % (binpath('nova-dhcpbridge'),
+                                 instance_ref['mac_address'],
+                                 private_ip)
     env = {'DNSMASQ_INTERFACE': network_ref['bridge'],
            'TESTING': '1',
            'FLAGFILE': FLAGS.dhcpbridge_flagfile}
@@ -274,7 +297,10 @@ def lease_ip(private_ip):
 def release_ip(private_ip):
     """Run del command on dhcpbridge"""
     network_ref = db.fixed_ip_get_network(None, private_ip)
-    cmd = "%s del fake %s fake" % (binpath('nova-dhcpbridge'), private_ip)
+    instance_ref = db.fixed_ip_get_instance(None, private_ip)
+    cmd = "%s del %s %s fake" % (binpath('nova-dhcpbridge'),
+                                 instance_ref['mac_address'],
+                                 private_ip)
     env = {'DNSMASQ_INTERFACE': network_ref['bridge'],
            'TESTING': '1',
            'FLAGFILE': FLAGS.dhcpbridge_flagfile}
diff --git a/nova/virt/xenapi.py b/nova/virt/xenapi.py
index b44ac383a..1c6de4403 100644
--- a/nova/virt/xenapi.py
+++ b/nova/virt/xenapi.py
@@ -274,9 +274,19 @@ class XenAPIConnection(object):
     def destroy(self, instance):
         vm = yield self._lookup(instance.name)
         if vm is None:
-            raise Exception('instance not present %s' % instance.name)
-        task = yield self._call_xenapi('Async.VM.destroy', vm)
-        yield self._wait_for_task(task)
+            # Don't complain, just return.  This lets us clean up instances
+            # that have already disappeared from the underlying platform.
+            defer.returnValue(None)
+        try:
+            task = yield self._call_xenapi('Async.VM.hard_shutdown', vm)
+            yield self._wait_for_task(task)
+        except Exception, exc:
+            logging.warn(exc)
+        try:
+            task = yield self._call_xenapi('Async.VM.destroy', vm)
+            yield self._wait_for_task(task)
+        except Exception, exc:
+            logging.warn(exc)
 
     def get_info(self, instance_id):
         vm = self._lookup_blocking(instance_id)
@@ -330,9 +340,9 @@ class XenAPIConnection(object):
                              error_info)
                 deferred.errback(XenAPI.Failure(error_info))
             #logging.debug('Polling task %s done.', task)
-        except Exception, exn:
-            logging.warn(exn)
-            deferred.errback(exn)
+        except Exception, exc:
+            logging.warn(exc)
+            deferred.errback(exc)
 
     @utils.deferredToThread
     def _call_xenapi(self, method, *args):
@@ -358,21 +368,21 @@ class XenAPIConnection(object):
 def _unwrap_plugin_exceptions(func, *args, **kwargs):
     try:
         return func(*args, **kwargs)
-    except XenAPI.Failure, exn:
-        logging.debug("Got exception: %s", exn)
-        if (len(exn.details) == 4 and
-            exn.details[0] == 'XENAPI_PLUGIN_EXCEPTION' and
-            exn.details[2] == 'Failure'):
+    except XenAPI.Failure, exc:
+        logging.debug("Got exception: %s", exc)
+        if (len(exc.details) == 4 and
+            exc.details[0] == 'XENAPI_PLUGIN_EXCEPTION' and
+            exc.details[2] == 'Failure'):
             params = None
             try:
-                params = eval(exn.details[3])
+                params = eval(exc.details[3])
             except:
-                raise exn
+                raise exc
             raise XenAPI.Failure(params)
         else:
             raise
-    except xmlrpclib.ProtocolError, exn:
-        logging.debug("Got exception: %s", exn)
+    except xmlrpclib.ProtocolError, exc:
+        logging.debug("Got exception: %s", exc)
         raise