openstack
/
nova
Code Issues Proposed changes

Detect and fix issues caused by vol ID migration 

The migration from volume ID to UUID neglected to update the provider_location
field on the volume.  As a result the iqn and volume name no long match and
existing volumes are no longer able to be attached after an upgrade
(essex -> folsom and then nova-vol->cinder).

This patch adds a method to the volume driver that will check for the
mismatch of volume name in the iqn during service start up.  If
detected it will update the provider_location field in the database
to include the new ID.  Also it will create a symlink to the device backing
file that also has the correct naming convention.

Note: We don't disturb an connections that are currently attached.
For this case we add a check in manager.detach and do any provider_location
cleanup that's needed at that time.  This ensures that connections
persist on restarts of tgtd and reboot.

Change-Id: Ib41ebdc849ebc31a9225bdc4902209c5a23da104
Fixes: bug 1065702
This commit is contained in:
John Griffith 2012-10-29 17:57:40 -06:00
parent ce20672df4
commit e3d7f8c7de
5 changed files with 98 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
etc/nova/rootwrap.d/volume.filters
View File

@ -34,3 +34,4 @@ dmsetup_usr: CommandFilter, /usr/sbin/dmsetup, root
#nova/volume/.py: utils.temporary_chown(path, 0), ...
chown: CommandFilter, /bin/chown, root
ln: CommandFilter, /bin/ln, root

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

@ -101,6 +101,8 @@ class CloudTestCase(test.TestCase):
self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target)
self.stubs.Set(iscsi.TgtAdm, 'remove_iscsi_target',
self.fake_remove_iscsi_target)
self.stubs.Set(iscsi.TgtAdm, 'create_iscsi_target',
self.fake_create_iscsi_target)
def fake_show(meh, context, id):
return {'id': id,
@ -168,6 +170,9 @@ class CloudTestCase(test.TestCase):
def fake_remove_iscsi_target(obj, tid, lun, vol_id, **kwargs):
pass
def fake_create_iscsi_target(obj, name, tid, lun, path, **kwargs):
pass
def _stub_instance_get_with_fixed_ips(self, func_name):
orig_func = getattr(self.cloud.compute_api, func_name)
@ -1203,8 +1208,8 @@ class CloudTestCase(test.TestCase):
'name': 'fake_name',
'container_format': 'ami',
'properties': {
'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
'ramdisk_id': 'cedef40a-ed67-4d10-800e-17455edce175',
'kernel_id': 'cedef40a-ed67-4d10-800e-17455edce175',
'ramdisk_id': 'cedef40a-ed67-4d10-800e-17455edce175',
'type': 'machine'}}]
def fake_show_none(meh, context, id):
@ -2036,15 +2041,24 @@ class CloudTestCase(test.TestCase):
def _volume_create(self, volume_id=None):
location = '10.0.2.15:3260'
iqn = 'iqn.2010-10.org.openstack:%s' % volume_id
iqn = 'iqn.2010-10.org.openstack:volume-%s' % volume_id
kwargs = {'status': 'available',
'host': self.volume.host,
'size': 1,
'provider_location': '1 %s,fake %s' % (location, iqn),
'attach_status': 'detached', }
if volume_id:
kwargs['id'] = volume_id
return db.volume_create(self.context, kwargs)
volume_ref = db.volume_create(self.context, kwargs)
if volume_id is None:
model_update = {}
model_update['provider_location'] =\
volume_ref['provider_location'].replace('volume-None',
volume_ref['name'])
db.volume_update(self.context, volume_ref['id'], model_update)
return volume_ref
def _assert_volume_attached(self, vol, instance_uuid, mountpoint):
self.assertEqual(vol['instance_uuid'], instance_uuid)
@ -2066,6 +2080,7 @@ class CloudTestCase(test.TestCase):
vol1 = self._volume_create()
vol2 = self._volume_create()
kwargs = {'image_id': 'ami-1',
'instance_type': FLAGS.default_instance_type,
'max_count': 1,
@ -2132,6 +2147,7 @@ class CloudTestCase(test.TestCase):
vol1 = self._volume_create()
vol2 = self._volume_create()
kwargs = {'image_id': 'ami-1',
'instance_type': FLAGS.default_instance_type,
'max_count': 1,

66
nova/volume/driver.py
View File

@ -21,6 +21,7 @@ Drivers for volumes.
"""
import os
import re
import tempfile
import time
import urllib
@ -327,14 +328,72 @@ class ISCSIDriver(VolumeDriver):
else:
iscsi_target = 1 # dummy value when using TgtAdm
iscsi_name = "%s%s" % (FLAGS.iscsi_target_prefix, volume['name'])
volume_path = "/dev/%s/%s" % (FLAGS.volume_group, volume['name'])
# Check for https://bugs.launchpad.net/nova/+bug/1065702
old_name = None
volume_name = volume['name']
if (volume['provider_location'] is not None and
volume['name'] not in volume['provider_location']):
msg = _('Detected inconsistency in provider_location id')
LOG.debug(msg)
old_name = self._fix_id_migration(context, volume)
if 'in-use' in volume['status']:
volume_name = old_name
old_name = None
iscsi_name = "%s%s" % (FLAGS.iscsi_target_prefix, volume_name)
volume_path = "/dev/%s/%s" % (FLAGS.volume_group, volume_name)
# NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need
# should clean this all up at some point in the future
self.tgtadm.create_iscsi_target(iscsi_name, iscsi_target,
0, volume_path,
check_exit_code=False)
check_exit_code=False,
old_name=old_name)
def _fix_id_migration(self, context, volume):
"""Fix provider_location and dev files to address bug 1065702.
For volumes that the provider_location has NOT been updated
and are not currently in-use we'll create a new iscsi target
and remove the persist file.
If the volume is in-use, we'll just stick with the old name
and when detach is called we'll feed back into ensure_export
again if necessary and fix things up then.
Details at: https://bugs.launchpad.net/nova/+bug/1065702
"""
model_update = {}
pattern = re.compile(r":|\s")
fields = pattern.split(volume['provider_location'])
old_name = fields[3]
volume['provider_location'] = \
volume['provider_location'].replace(old_name, volume['name'])
model_update['provider_location'] = volume['provider_location']
self.db.volume_update(context, volume['id'], model_update)
start = os.getcwd()
os.chdir('/dev/%s' % FLAGS.volume_group)
try:
(out, err) = self._execute('readlink', old_name)
except exception.ProcessExecutionError:
link_path = '/dev/%s/%s' % (FLAGS.volume_group, old_name)
LOG.debug(_('Symbolic link %s not found') % link_path)
os.chdir(start)
return
rel_path = out.rstrip()
self._execute('ln',
'-s',
rel_path, volume['name'],
run_as_root=True)
os.chdir(start)
return old_name
def _ensure_iscsi_targets(self, context, host):
"""Ensure that target ids have been created in datastore."""
@ -354,7 +413,6 @@ class ISCSIDriver(VolumeDriver):
def create_export(self, context, volume):
"""Creates an export for a logical volume."""
#BOOKMARK(jdg)
iscsi_name = "%s%s" % (FLAGS.iscsi_target_prefix, volume['name'])
volume_path = "/dev/%s/%s" % (FLAGS.volume_group, volume['name'])

10
nova/volume/iscsi.py
View File

@ -126,6 +126,11 @@ class TgtAdm(TargetAdmin):
f.write(volume_conf)
f.close()
old_persist_file = None
old_name = kwargs.get('old_name', None)
if old_name is not None:
old_persist_file = os.path.join(volumes_dir, old_name)
try:
(out, err) = self._execute('tgt-admin',
'--update',
@ -147,6 +152,9 @@ class TgtAdm(TargetAdmin):
"contains 'include %(volumes_dir)s/*'") % locals())
raise exception.NotFound()
if old_persist_file is not None and os.path.exists(old_persist_file):
os.unlink(old_persist_file)
return tid
def remove_iscsi_target(self, tid, lun, vol_id, **kwargs):
@ -164,7 +172,7 @@ class TgtAdm(TargetAdmin):
iqn,
run_as_root=True)
except exception.ProcessExecutionError, e:
LOG.error(_("Failed to create iscsi target for volume "
LOG.error(_("Failed to remove iscsi target for volume "
"id:%(volume_id)s.") % locals())
raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)

7
nova/volume/manager.py
View File

@ -309,7 +309,12 @@ class VolumeManager(manager.SchedulerDependentManager):
volume_id,
{'status': 'error_detaching'})
self.db.volume_detached(context, volume_id)
self.db.volume_detached(context.elevated(), volume_id)
# Check for https://bugs.launchpad.net/nova/+bug/1065702
volume_ref = self.db.volume_get(context, volume_id)
if volume_ref['name'] not in volume_ref['provider_location']:
self.driver.ensure_export(context, volume_ref)
def _copy_image_to_volume(self, context, volume, image_id):
"""Downloads Glance image to the specified volume. """