]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
VolumeManager: initialize even if a volume can't be found
authorFlorian Haas <florian@hastexo.com>
Mon, 20 Jan 2014 21:42:11 +0000 (22:42 +0100)
committerFlorian Haas <florian@hastexo.com>
Tue, 28 Jan 2014 17:59:06 +0000 (18:59 +0100)
If a previously volume cannot be exported (for example, because
an iSCSI target's backing LV is no longer present), VolumeManager
would previously bail out, leaving the volume service uninitialized.
This left volumes dangling in limbo: their state would be reported
as available (clearly not a reflection of reality), and they could
not be deleted (because the volume service that would be responsible
for deletion was unavailable).

Instead, catch an exception raised by ensure_export() separately,
log it, and set the volume to the error state.

For the tgt backend, this will also remove the volume_path file.
Previously this would cause an ISCSITargetRemoveFailed exception
when the volume would be removed. Instead, simply log a warning
and return, the way other backends (example: RBD) already do.

Also, add an info message that reflects the actual path and
contents of the volume_path file.

Finally, fix up the tgtadm unit test so that it tests for
identical IQNs,

Closes-bug: 1270959.

Change-Id: Id61407c9a5e020d5a823dd7b8c973d237c35cb9b

cinder/brick/iscsi/iscsi.py
cinder/volume/manager.py

index ceac1b9c2c4be0eb7cdb52800b36b55bb05095e5..fa627b0c933f968bbc5b523360f04f2b7c67b882 100644 (file)
@@ -177,6 +177,9 @@ class TgtAdm(TargetAdmin):
         f = open(volume_path, 'w+')
         f.write(volume_conf)
         f.close()
+        LOG.debug(_('Created volume path %(vp)s,\n'
+                    'content: %(vc)%')
+                  % {'vp': volume_path, 'vc': volume_conf})
 
         old_persist_file = None
         old_name = kwargs.get('old_name', None)
@@ -263,6 +266,11 @@ class TgtAdm(TargetAdmin):
         LOG.info(_('Removing iscsi_target for: %s') % vol_id)
         vol_uuid_file = vol_name
         volume_path = os.path.join(self.volumes_dir, vol_uuid_file)
+        if not os.path.exists(volume_path):
+            LOG.warning(_('Volume path %s does not exist, '
+                          'nothing to remove.') % volume_path)
+            return
+
         if os.path.isfile(volume_path):
             iqn = '%s%s' % (self.iscsi_target_prefix,
                             vol_uuid_file)
index 116b658728668eea271b82311b40a88db917b531..95bc0c307c00af98ee0be0bc527a349f0bb94cc8 100644 (file)
@@ -251,7 +251,15 @@ class VolumeManager(manager.SchedulerDependentManager):
                     # calculate allocated capacity for driver
                     sum += volume['size']
                     self.stats['allocated_capacity_gb'] = sum
-                    self.driver.ensure_export(ctxt, volume)
+                    try:
+                        self.driver.ensure_export(ctxt, volume)
+                    except Exception as export_ex:
+                        LOG.error(_("Failed to re-export volume %s: "
+                                    "setting to error state"), volume['id'])
+                        LOG.exception(export_ex)
+                        self.db.volume_update(ctxt,
+                                              volume['id'],
+                                              {'status': 'error'})
                 elif volume['status'] == 'downloading':
                     LOG.info(_("volume %s stuck in a downloading state"),
                              volume['id'])