openstack
/
nova
Code Issues Proposed changes

Automatically convert device names 

In the past, users have been able to specify xvda or xvdb and it
has worked. We are now validating device names against the expected
name from the backend, and this causes confusion, especially for lxc
which expecting device names to be /dev/a /dev/b /dev/c etc.

This patch addresses the issue by automatically converting between
different formats. The proper format for the backend will be returned
by the api. Includes tests to verify that the conversion works and
that the lxc values work as expected.

Fixes bug 1046020

Change-Id: Iffa552ba05f89f70b6fb93043edf8882c8412215
This commit is contained in:
Vishvananda Ishaya 2012-09-04 15:25:35 -07:00
parent 51f5b8c28e
commit 07e0b779fd
7 changed files with 69 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
nova/block_device.py
View File

@ -131,3 +131,11 @@ def instance_block_mapping(instance, bdms):
nebs += 1
return mappings
def match_device(device):
"""Matches device name and returns prefix, suffix"""
match = re.match("(^/dev/x{0,1}[a-z]{0,1}d{0,1})([a-z]+)[0-9]*$", device)
if not match:
return None
return match.groups()

2
nova/compute/api.py
View File

@ -1722,7 +1722,7 @@ class API(base.Base):
# will need to be removed along with the test if we
# change the logic in the manager for what constitutes
# a valid device.
if device and not re.match("^/dev/x{0,1}[a-z]d[a-z]+$", device):
if device and not block_device.match_device(device):
raise exception.InvalidDevicePath(path=device)
# NOTE(vish): This is done on the compute host because we want
# to avoid a race where two devices are requested at

25
nova/compute/utils.py
View File

@ -34,28 +34,31 @@ LOG = log.getLogger(__name__)
def get_device_name_for_instance(context, instance, device):
# NOTE(vish): this will generate a unique device name that is not
# in use already. It is a reasonable guess at where
# it will show up in a linux guest, but it may not
# always be correct
"""Validates (or generates) a device name for instance.
If device is not set, it will generate a unique device appropriate
for the instance. It uses the block device mapping table to find
valid device names. If the device name is valid but applicable to
a different backend (for example /dev/vdc is specified but the
backend uses /dev/xvdc), the device name will be converted to the
appropriate format.
"""
req_prefix = None
req_letters = None
if device:
try:
match = re.match("(^/dev/x{0,1}[a-z]d)([a-z]+)$", device)
req_prefix, req_letters = match.groups()
req_prefix, req_letters = block_device.match_device(device)
except (TypeError, AttributeError, ValueError):
raise exception.InvalidDevicePath(path=device)
bdms = db.block_device_mapping_get_all_by_instance(context,
instance['uuid'])
mappings = block_device.instance_block_mapping(instance, bdms)
try:
match = re.match("(^/dev/x{0,1}[a-z]d)[a-z]+[0-9]*$", mappings['root'])
prefix = match.groups()[0]
prefix = block_device.match_device(mappings['root'])[0]
except (TypeError, AttributeError, ValueError):
raise exception.InvalidDevicePath(path=mappings['root'])
if not req_prefix:
req_prefix = prefix
if req_prefix != prefix:
LOG.debug(_("Using %(prefix)s instead of %(req_prefix)s") % locals())
letters_list = []
for _name, device in mappings.iteritems():
letter = block_device.strip_prefix(device)
@ -68,7 +71,7 @@ def get_device_name_for_instance(context, instance, device):
req_letters = _get_unused_letters(used_letters)
if req_letters in used_letters:
raise exception.DevicePathInUse(path=device)
return req_prefix + req_letters
return prefix + req_letters
def _get_unused_letters(used_letters):

34
nova/tests/api/ec2/test_cinder_cloud.py
View File

@ -676,10 +676,10 @@ class CinderCloudTestCase(test.TestCase):
kwargs = {'image_id': 'ami-1',
'instance_type': FLAGS.default_instance_type,
'max_count': 1,
'block_device_mapping': [{'device_name': '/dev/vdb',
'block_device_mapping': [{'device_name': '/dev/sdb',
'volume_id': vol1_uuid,
'delete_on_termination': False},
{'device_name': '/dev/vdc',
{'device_name': '/dev/sdc',
'volume_id': vol2_uuid,
'delete_on_termination': True},
]}
@ -695,28 +695,28 @@ class CinderCloudTestCase(test.TestCase):
str(vol['id']) == str(vol2_uuid))
if(str(vol['id']) == str(vol1_uuid)):
self.volume_api.attach(self.context, vol,
instance_uuid, '/dev/vdb')
instance_uuid, '/dev/sdb')
elif(str(vol['id']) == str(vol2_uuid)):
self.volume_api.attach(self.context, vol,
instance_uuid, '/dev/vdc')
instance_uuid, '/dev/sdc')
vol = self.volume_api.get(self.context, vol1_uuid)
self._assert_volume_attached(vol, instance_uuid, '/dev/vdb')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdb')
vol = self.volume_api.get(self.context, vol2_uuid)
self._assert_volume_attached(vol, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdc')
result = self.cloud.stop_instances(self.context, [ec2_instance_id])
self.assertTrue(result)
vol = self.volume_api.get(self.context, vol1_uuid)
self._assert_volume_attached(vol, instance_uuid, '/dev/vdb')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdb')
vol = self.volume_api.get(self.context, vol1_uuid)
self._assert_volume_attached(vol, instance_uuid, '/dev/vdb')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdb')
vol = self.volume_api.get(self.context, vol2_uuid)
self._assert_volume_attached(vol, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdc')
self.cloud.start_instances(self.context, [ec2_instance_id])
vols = self.volume_api.get_all(self.context)
@ -725,8 +725,8 @@ class CinderCloudTestCase(test.TestCase):
for vol in vols:
self.assertTrue(str(vol['id']) == str(vol1_uuid) or
str(vol['id']) == str(vol2_uuid))
self.assertTrue(vol['mountpoint'] == '/dev/vdb' or
vol['mountpoint'] == '/dev/vdc')
self.assertTrue(vol['mountpoint'] == '/dev/sdb' or
vol['mountpoint'] == '/dev/sdc')
self.assertEqual(vol['instance_uuid'], instance_uuid)
self.assertEqual(vol['status'], "in-use")
self.assertEqual(vol['attach_status'], "attached")
@ -758,7 +758,7 @@ class CinderCloudTestCase(test.TestCase):
kwargs = {'image_id': 'ami-1',
'instance_type': FLAGS.default_instance_type,
'max_count': 1,
'block_device_mapping': [{'device_name': '/dev/vdb',
'block_device_mapping': [{'device_name': '/dev/sdb',
'volume_id': vol1_uuid,
'delete_on_termination': True}]}
ec2_instance_id = self._run_instance(**kwargs)
@ -771,7 +771,7 @@ class CinderCloudTestCase(test.TestCase):
self.assertEqual(len(vols), 1)
for vol in vols:
self.assertEqual(vol['id'], vol1_uuid)
self._assert_volume_attached(vol, instance_uuid, '/dev/vdb')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdb')
vol = self.volume_api.get(self.context, vol2_uuid)
self._assert_volume_detached(vol)
@ -779,13 +779,13 @@ class CinderCloudTestCase(test.TestCase):
self.cloud.compute_api.attach_volume(self.context,
instance,
volume_id=vol2_uuid,
device='/dev/vdc')
device='/dev/sdc')
vol1 = self.volume_api.get(self.context, vol1_uuid)
self._assert_volume_attached(vol1, instance_uuid, '/dev/vdb')
self._assert_volume_attached(vol1, instance_uuid, '/dev/sdb')
vol2 = self.volume_api.get(self.context, vol2_uuid)
self._assert_volume_attached(vol2, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol2, instance_uuid, '/dev/sdc')
self.cloud.compute_api.detach_volume(self.context,
volume_id=vol1_uuid)
@ -797,7 +797,7 @@ class CinderCloudTestCase(test.TestCase):
self.assertTrue(result)
vol2 = self.volume_api.get(self.context, vol2_uuid)
self._assert_volume_attached(vol2, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol2, instance_uuid, '/dev/sdc')
self.cloud.start_instances(self.context, [ec2_instance_id])
vols = self.volume_api.get_all(self.context)

10
nova/tests/api/ec2/test_cloud.py
View File

@ -2090,7 +2090,7 @@ class CloudTestCase(test.TestCase):
kwargs = {'image_id': 'ami-1',
'instance_type': FLAGS.default_instance_type,
'max_count': 1,
'block_device_mapping': [{'device_name': '/dev/vdb',
'block_device_mapping': [{'device_name': '/dev/sdb',
'volume_id': vol1['id'],
'delete_on_termination': True}]}
ec2_instance_id = self._run_instance(**kwargs)
@ -2102,7 +2102,7 @@ class CloudTestCase(test.TestCase):
self.assertEqual(len(vols), 1)
for vol in vols:
self.assertEqual(vol['id'], vol1['id'])
self._assert_volume_attached(vol, instance_uuid, '/dev/vdb')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdb')
vol = db.volume_get(self.context, vol2['id'])
self._assert_volume_detached(vol)
@ -2113,7 +2113,7 @@ class CloudTestCase(test.TestCase):
volume_id=vol2['id'],
device='/dev/vdc')
vol = db.volume_get(self.context, vol2['id'])
self._assert_volume_attached(vol, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdc')
self.cloud.compute_api.detach_volume(self.context,
volume_id=vol1['id'])
@ -2124,14 +2124,14 @@ class CloudTestCase(test.TestCase):
self.assertTrue(result)
vol = db.volume_get(self.context, vol2['id'])
self._assert_volume_attached(vol, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdc')
self.cloud.start_instances(self.context, [ec2_instance_id])
vols = db.volume_get_all_by_instance_uuid(self.context, instance_uuid)
self.assertEqual(len(vols), 1)
for vol in vols:
self.assertEqual(vol['id'], vol2['id'])
self._assert_volume_attached(vol, instance_uuid, '/dev/vdc')
self._assert_volume_attached(vol, instance_uuid, '/dev/sdc')
vol = db.volume_get(self.context, vol1['id'])
self._assert_volume_detached(vol)

2
nova/tests/compute/test_compute.py
View File

@ -4093,7 +4093,7 @@ class ComputeAPITestCase(BaseTestCase):
self.context,
{'locked': False},
None,
'/dev/invalid')
'/invalid')
def test_vnc_console(self):
"""Make sure we can a vnc console for an instance."""

23
nova/tests/compute/test_compute_utils.py
View File

@ -110,6 +110,29 @@ class ComputeValidateDeviceTestCase(test.TestCase):
device = self._validate_device()
self.assertEqual(device, '/dev/vdc')
def test_lxc_names_work(self):
self.instance = {
'uuid': 'fake',
'root_device_name': '/dev/a',
'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.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')
self.assertEqual(device, '/dev/vdc')
device = self._validate_device('/dev/xvdc')
self.assertEqual(device, '/dev/vdc')
def test_invalid_bdms(self):
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
lambda context, instance: [])