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: I8224824b793c98a9767c5d8dd741d892be720c4f
Fixes: bug 1065702

diff --git a/bin/cinder-volume-usage-audit b/bin/cinder-volume-usage-audit
index 6f39e1124b3d2de780fa567cb92991ffffb55604..89bd3c519512131d4213e9324a18e940c12ff92d 100755 (executable)
--- a/bin/cinder-volume-usage-audit
+++ b/bin/cinder-volume-usage-audit
@@ -65,7 +65,8 @@ if __name__ == '__main__':
     logging.setup("cinder")
     begin, end = utils.last_completed_audit_period()
     print _("Starting volume usage audit")
-    print _("Creating usages for %s until %s") % (str(begin), str(end))
+    msg = _("Creating usages for %(begin_period) until %(end_period)")
+    print (msg % {"begin_period": str(begin), "end_period": str(end)})
     volumes = db.volume_get_active_by_window(admin_context,
                                              begin,
                                              end)

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index 14022bfbfbb2b223c82c3874e71937d07edf7aec..eb331f7ea0e798877fa36f77d7c65f334ece324b 100644 (file)
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -21,6 +21,7 @@ Drivers for volumes.
 """
 
 import os
+import re
 import tempfile
 import time
 import urllib
@@ -323,14 +324,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."""
@@ -350,7 +409,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'])

diff --git a/cinder/volume/iscsi.py b/cinder/volume/iscsi.py
index edf548f45197885905af7362a977ef48e60f2b17..cc7d520639b83588f1a630d2889c0d6e163a03f4 100644 (file)
--- a/cinder/volume/iscsi.py
+++ b/cinder/volume/iscsi.py
@@ -127,6 +127,7 @@ class TgtAdm(TargetAdmin):
                     %s
                 </target>
             """ % (name, path, chap_auth)
+
         LOG.info(_('Creating volume: %s') % vol_id)
         volumes_dir = FLAGS.volumes_dir
         volume_path = os.path.join(volumes_dir, vol_id)
@@ -135,6 +136,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',
@@ -156,6 +162,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):
@@ -173,7 +182,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:%(vol_id)s.") % locals())
             raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
 

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index ddf86e6954d451163e4f07a95fec2949912bffee..915e5cf016826a6ff247125b50f1f8bc4e0c0cdb 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -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']

diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters
index 20d7e9badcde60a9314112f210e69c2fc30f8273..936dbbd3c42e2b853260fe01db20271ee4b02aa6 100644 (file)
--- a/etc/cinder/rootwrap.d/volume.filters
+++ b/etc/cinder/rootwrap.d/volume.filters
@@ -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