The proposed fix puts a VM which fails to spawn in a (new) 'FAILED' power state. It does not perform a clean-up.

This because the user needs to know what has happened to the VM he/she was trying to run. Normally, API users do not have access to log files. In this case, the only way for the user to know what happened to the instance is to query its state (e.g.: doing euca-describe-instances). If we perform a complete clean-up, no information about the instance which failed to spawn will be left.

For the NO_HOST_AVAILABLE error, which occurs when there is not enough RAM left on the host, the amount of available memory is now checked at the beginning of the spawn process. This way, if there is not enough RAM left on the host, the spawn process returns immediately.
A test case (spawn_not_enough_memory) has been added as well.

I understand adding a new value to the power_state enumeration might not be acceptable. In that case I will propose for merge a different branch in which we perform a complete clean-up.
However, I reckon we still to provide a way to inform the user the spawn process has failed.

6 files changed, 38 insertions, 6 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index f4418af26..bb999931c 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -127,7 +127,7 @@ class ComputeManager(manager.Manager):
             info = self.driver.get_info(instance_ref['name'])
             state = info['state']
         except exception.NotFound:
-            state = power_state.NOSTATE
+            state = power_state.FAILED
         self.db.instance_set_state(context, instance_id, state)
 
     def get_console_topic(self, context, **_kwargs):
diff --git a/nova/compute/power_state.py b/nova/compute/power_state.py
index 37039d2ec..adfc2dff0 100644
--- a/nova/compute/power_state.py
+++ b/nova/compute/power_state.py
@@ -27,6 +27,7 @@ SHUTDOWN = 0x04
 SHUTOFF = 0x05
 CRASHED = 0x06
 SUSPENDED = 0x07
+FAILED = 0x08
 
 
 def name(code):
@@ -38,5 +39,6 @@ def name(code):
         SHUTDOWN: 'shutdown',
         SHUTOFF: 'shutdown',
         CRASHED: 'crashed',
-        SUSPENDED: 'suspended'}
+        SUSPENDED: 'suspended',
+        FAILED: 'failed to spawn'}
     return d[code]
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index 9f5b266f3..d5660c5d1 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -243,7 +243,8 @@ class XenAPIVMTestCase(test.TestCase):
         # Check that the VM is running according to XenAPI.
         self.assertEquals(vm['power_state'], 'Running')
 
-    def _test_spawn(self, image_id, kernel_id, ramdisk_id):
+    def _test_spawn(self, image_id, kernel_id, ramdisk_id,
+                    instance_type="m1.large"):
         stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
         values = {'name': 1,
                   'id': 1,
@@ -252,7 +253,7 @@ class XenAPIVMTestCase(test.TestCase):
                   'image_id': image_id,
                   'kernel_id': kernel_id,
                   'ramdisk_id': ramdisk_id,
-                  'instance_type': 'm1.large',
+                  'instance_type': instance_type,
                   'mac_address': 'aa:bb:cc:dd:ee:ff',
                   }
         conn = xenapi_conn.get_connection(False)
@@ -260,6 +261,12 @@ class XenAPIVMTestCase(test.TestCase):
         conn.spawn(instance)
         self.check_vm_record(conn)
 
+    def test_spawn_not_enough_memory(self):
+        FLAGS.xenapi_image_service = 'glance'
+        self.assertRaises(Exception,
+                          self._test_spawn,
+                          1, 2, 3, "m1.xlarge")
+
     def test_spawn_raw_objectstore(self):
         FLAGS.xenapi_image_service = 'objectstore'
         self._test_spawn(1, None, None)
diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py
index e8352771c..018d0dcd3 100644
--- a/nova/virt/xenapi/fake.py
+++ b/nova/virt/xenapi/fake.py
@@ -286,6 +286,10 @@ class SessionBase(object):
         rec['currently_attached'] = False
         rec['device'] = ''
 
+    def host_compute_free_memory(self, _1, ref):
+        #Always return 12GB available
+        return 12 * 1024 * 1024 * 1024
+
     def xenapi_request(self, methodname, params):
         if methodname.startswith('login'):
             self._login(methodname, params)
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index f5c19099a..574ef0944 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -139,6 +139,16 @@ class VMHelper(HelperBase):
         return vm_ref
 
     @classmethod
+    def ensure_free_mem(cls, session, instance):
+        instance_type = instance_types.INSTANCE_TYPES[instance.instance_type]
+        mem = long(instance_type['memory_mb']) * 1024 * 1024
+        #get free memory from host
+        host = session.get_xenapi_host()
+        host_free_mem = long(session.get_xenapi().host.
+                             compute_free_memory(host))
+        return host_free_mem >= mem
+
+    @classmethod
     def create_vbd(cls, session, vm_ref, vdi_ref, userdevice, bootable):
         """Create a VBD record.  Returns a Deferred that gives the new
         VBD reference."""
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index fe95d881b..98f8ab46e 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -66,7 +66,15 @@ class VMOps(object):
         if vm is not None:
             raise exception.Duplicate(_('Attempted to create'
             ' non-unique name %s') % instance.name)
-
+        #ensure enough free memory is available
+        if not VMHelper.ensure_free_mem(self._session, instance):
+                name = instance['name']
+                LOG.exception(_('instance %(name)s: not enough free memory')
+                              % locals())
+                db.instance_set_state(context.get_admin_context(),
+                                      instance['id'],
+                                      power_state.SHUTDOWN)
+                return
         bridge = db.network_get_by_instance(context.get_admin_context(),
                                             instance['id'])['bridge']
         network_ref = \
@@ -161,7 +169,8 @@ class VMOps(object):
                 instance_name = instance_or_vm.name
         vm = VMHelper.lookup(self._session, instance_name)
         if vm is None:
-            raise Exception(_('Instance not present %s') % instance_name)
+            raise exception.NotFound(
+                            _('Instance not present %s') % instance_name)
         return vm
 
     def snapshot(self, instance, image_id):