]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Detect and fix issues caused by vol ID migration
authorJohn Griffith <john.griffith@solidfire.com>
Wed, 31 Oct 2012 22:43:09 +0000 (16:43 -0600)
committerJohn Griffith <john.griffith@solidfire.com>
Wed, 31 Oct 2012 22:43:09 +0000 (16:43 -0600)
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: I4683df4ef489972752dc58cb4e91d458a79a8ef2
Fixes: bug 1065702enter the commit message for your changes. Lines starting
cinder/volume/driver.py
cinder/volume/iscsi.py
cinder/volume/manager.py
etc/cinder/rootwrap.d/volume.filters

index 76077c6f870056b228a18dd1514bb14621b2508c..4461dcf11907f149451dd60e7cad872bea117434 100644 (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/cinder/+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/cinder/+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,8 +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'])
         model_update = {}
index 68e21a07ade8487de1c221d69bc82860accc67cf..48a8e6092c6b4f17370991562562843cb8acdb1e 100644 (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 delete iscsi target for volume "
                         "id:%(volume_id)s.") % locals())
             raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
 
index fbefce21303895b6a7708b16a870adc33104cbe2..d64014d5fd3556e8f0c3002bdd633c65845a138c 100644 (file)
@@ -310,6 +310,11 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         self.db.volume_detached(context.elevated(), volume_id)
 
+        # Check for https://bugs.launchpad.net/cinder/+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. """
         volume_id = volume['id']
index 20d7e9badcde60a9314112f210e69c2fc30f8273..936dbbd3c42e2b853260fe01db20271ee4b02aa6 100644 (file)
@@ -34,3 +34,4 @@ chown: CommandFilter, /bin/chown, root
 # cinder/volume/driver.py
 dmsetup: CommandFilter, /sbin/dmsetup, root
 dmsetup_usr: CommandFilter, /usr/sbin/dmsetup, root
+ln: CommandFilter, /bin/ln, root