Fix issues with device autoassignment in xenapi

This is a workaround for two issues in xenapi. The first is that
does not set the instance default_root_device to /dev/xvda so it
defaults to /dev/sda. The proper fix for this involves setting the
default_root_device in xenapi and a db migration to set the proper
default_root_device for existing instances.

This patch works around this issue by explicitly setting the prefix
to /dev/xvd if the compute driver is xenapi.

The second issue is that xenapi never updates the instance record
to include default_swap_device and default_ephemeral device. The
fix for this involes adding the appropriate update to the instance
record and a migration that sets the proper values for all existing
instances.

This patch works around this issue by explicily checking the
instance_type and removing the devices from the list if the compute
driver is xenapi.

Fixes bug 1055715 and bug 1055712

Change-Id: I61aa15e69eb0a22430bb22ea5149b1f0735b3328
(cherry picked from commit 6956476396)

nova/compute/utils.py

@@ -21,6 +21,7 @@ import string
 import traceback
 
 from nova import block_device
+from nova.compute import instance_types
 from nova import db
 from nova import exception
 from nova import flags
@@ -79,6 +80,10 @@ def get_device_name_for_instance(context, instance, device):
         prefix = block_device.match_device(mappings['root'])[0]
     except (TypeError, AttributeError, ValueError):
         raise exception.InvalidDevicePath(path=mappings['root'])
+    # NOTE(vish): remove this when xenapi is setting default_root_device
+    if (FLAGS.connection_type == 'xenapi' or
+        FLAGS.compute_driver.endswith('xenapi.XenAPIDriver')):
+        prefix = '/dev/xvd'
     if req_prefix != prefix:
         LOG.debug(_("Using %(prefix)s instead of %(req_prefix)s") % locals())
     letters_list = []
@@ -89,6 +94,18 @@ def get_device_name_for_instance(context, instance, device):
         letter = re.sub("\d+", "", letter)
         letters_list.append(letter)
     used_letters = set(letters_list)
+
+    # NOTE(vish): remove this when xenapi is properly setting
+    #             default_ephemeral_device and default_swap_device
+    if (FLAGS.connection_type == 'xenapi' or
+        FLAGS.compute_driver.endswith('xenapi.XenAPIDriver')):
+        instance_type_id = instance['instance_type_id']
+        instance_type = instance_types.get_instance_type(instance_type_id)
+        if instance_type['ephemeral_gb']:
+            used_letters.update('b')
+        if instance_type['swap']:
+            used_letters.update('c')
+
     if not req_letters:
         req_letters = _get_unused_letters(used_letters)
     if req_letters in used_letters:

nova/tests/compute/test_compute_utils.py

@@ -46,8 +46,16 @@ class ComputeValidateDeviceTestCase(test.TestCase):
         self.instance = {
                 'uuid': 'fake',
                 'root_device_name': '/dev/vda',
-                'default_ephemeral_device': '/dev/vdb'
+                'default_ephemeral_device': '/dev/vdb',
+                'instance_type_id': 'fake',
         }
+        self.data = []
+
+        def fake_get(instance_type_id, ctxt=None):
+            return self.instance_type
+
+        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
+                       lambda context, instance: self.data)
 
     def _validate_device(self, device=None):
         return compute_utils.get_device_name_for_instance(self.context,
@@ -64,68 +72,51 @@ class ComputeValidateDeviceTestCase(test.TestCase):
         }
 
     def test_wrap(self):
-        data = []
+        self.data = []
         for letter in string.ascii_lowercase[2:]:
-            data.append(self._fake_bdm('/dev/vd' + letter))
+            self.data.append(self._fake_bdm('/dev/vd' + letter))
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device()
         self.assertEqual(device, '/dev/vdaa')
 
     def test_wrap_plus_one(self):
-        data = []
+        self.data = []
         for letter in string.ascii_lowercase[2:]:
-            data.append(self._fake_bdm('/dev/vd' + letter))
+            self.data.append(self._fake_bdm('/dev/vd' + letter))
-        data.append(self._fake_bdm('/dev/vdaa'))
+        self.data.append(self._fake_bdm('/dev/vdaa'))
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device()
         self.assertEqual(device, '/dev/vdab')
 
     def test_later(self):
-        data = [
+        self.data = [
             self._fake_bdm('/dev/vdc'),
             self._fake_bdm('/dev/vdd'),
             self._fake_bdm('/dev/vde'),
         ]
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device()
         self.assertEqual(device, '/dev/vdf')
 
     def test_gap(self):
-        data = [
+        self.data = [
             self._fake_bdm('/dev/vdc'),
             self._fake_bdm('/dev/vde'),
         ]
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device()
         self.assertEqual(device, '/dev/vdd')
 
     def test_no_bdms(self):
-        data = []
+        self.data = []
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device()
         self.assertEqual(device, '/dev/vdc')
 
     def test_lxc_names_work(self):
-        self.instance = {
+        self.instance['root_device_name'] = '/dev/a'
-                'uuid': 'fake',
+        self.instance['ephemeral_device_name'] = '/dev/b'
-                'root_device_name': '/dev/a',
+        self.data = []
-                'default_ephemeral_device': '/dev/b'
-        }
-        data = []
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device()
         self.assertEqual(device, '/dev/c')
 
     def test_name_conversion(self):
-        data = []
+        self.data = []
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: data)
         device = self._validate_device('/dev/c')
         self.assertEqual(device, '/dev/vdc')
         device = self._validate_device('/dev/sdc')
@@ -134,24 +125,65 @@ class ComputeValidateDeviceTestCase(test.TestCase):
         self.assertEqual(device, '/dev/vdc')
 
     def test_invalid_bdms(self):
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: [])
         self.instance['root_device_name'] = "baddata"
         self.assertRaises(exception.InvalidDevicePath,
                           self._validate_device)
 
     def test_invalid_device_prefix(self):
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: [])
         self.assertRaises(exception.InvalidDevicePath,
                           self._validate_device, '/baddata/vdc')
 
     def test_device_in_use(self):
-        self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
-                       lambda context, instance: [])
         self.assertRaises(exception.DevicePathInUse,
                           self._validate_device, '/dev/vdb')
 
+    def test_swap(self):
+        self.instance['default_swap_device'] = "/dev/vdc"
+        device = self._validate_device()
+        self.assertEqual(device, '/dev/vdd')
+
+    def test_swap_no_ephemeral(self):
+        del self.instance['default_ephemeral_device']
+        self.instance['default_swap_device'] = "/dev/vdb"
+        device = self._validate_device()
+        self.assertEqual(device, '/dev/vdc')
+
+    def test_ephemeral_xenapi(self):
+        self.flags(compute_driver='xenapi.XenAPIDriver')
+        del self.instance['default_ephemeral_device']
+        self.instance_type = {
+            'ephemeral_gb': 10,
+            'swap': 0,
+        }
+        self.stubs.Set(instance_types, 'get_instance_type',
+                       lambda instance_type_id, ctxt=None: self.instance_type)
+        device = self._validate_device()
+        self.assertEqual(device, '/dev/xvdc')
+
+    def test_swap_xenapi(self):
+        self.flags(compute_driver='xenapi.XenAPIDriver')
+        del self.instance['default_ephemeral_device']
+        self.instance_type = {
+            'ephemeral_gb': 0,
+            'swap': 10,
+        }
+        self.stubs.Set(instance_types, 'get_instance_type',
+                       lambda instance_type_id, ctxt=None: self.instance_type)
+        device = self._validate_device()
+        self.assertEqual(device, '/dev/xvdb')
+
+    def test_swap_and_ephemeral_xenapi(self):
+        self.flags(compute_driver='xenapi.XenAPIDriver')
+        del self.instance['default_ephemeral_device']
+        self.instance_type = {
+            'ephemeral_gb': 10,
+            'swap': 10,
+        }
+        self.stubs.Set(instance_types, 'get_instance_type',
+                       lambda instance_type_id, ctxt=None: self.instance_type)
+        device = self._validate_device()
+        self.assertEqual(device, '/dev/xvdd')
+
 
 class UsageInfoTestCase(test.TestCase):