Enforce driver is initialized

This updates the volume manager and the
backup manager to enforce that the driver
has been initialized.

The managers call the driver in many places
without ensuring that the driver has been
properly initialized.   When the driver fails
inside of do_setup(), the managers shouldn't
call the driver's set_initialized() method.
The managers now dump out the exception, and
exit the init_host, leaving the driver in
an uninitialized state.

Fixes bug #1225897

Change-Id: I77b947f2a9fbe1b38f321511dba10fcd2fe1fe90

diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py
index 5ff6c44b0e68c249d736b9650c1f46e1472e6a2c..ceab83b06c14941eea2d5ba7195b245f801810ed 100644 (file)
--- a/cinder/backup/manager.py
+++ b/cinder/backup/manager.py
@@ -41,6 +41,7 @@ from cinder import manager
 from cinder.openstack.common import excutils
 from cinder.openstack.common import importutils
 from cinder.openstack.common import log as logging
+from cinder import utils
 
 
 LOG = logging.getLogger(__name__)
@@ -95,8 +96,22 @@ class BackupManager(manager.SchedulerDependentManager):
         """
 
         ctxt = context.get_admin_context()
-        self.driver.do_setup(ctxt)
-        self.driver.check_for_setup_error()
+        LOG.info(_("Starting volume driver %(driver_name)s (%(version)s)") %
+                 {'driver_name': self.driver.__class__.__name__,
+                  'version': self.driver.get_version()})
+        try:
+            self.driver.do_setup(ctxt)
+            self.driver.check_for_setup_error()
+        except Exception as ex:
+            LOG.error(_("Error encountered during "
+                        "initialization of driver: %(name)s") %
+                      {'name': self.driver.__class__.__name__})
+            LOG.exception(ex)
+            # we don't want to continue since we failed
+            # to initialize the driver correctly.
+            return
+
+        self.driver.set_initialized()
 
         LOG.info(_("Cleaning up incomplete backup operations"))
         volumes = self.db.volume_get_all_by_host(ctxt, self.host)
@@ -131,6 +146,7 @@ class BackupManager(manager.SchedulerDependentManager):
                 LOG.info(_('Resuming delete on backup: %s') % backup['id'])
                 self.delete_backup(ctxt, backup['id'])
 
+    @utils.require_driver_initialized
     def create_backup(self, context, backup_id):
         """Create volume backups using configured backup service."""
         backup = self.db.backup_get(context, backup_id)
@@ -186,11 +202,13 @@ class BackupManager(manager.SchedulerDependentManager):
                                                    self.az})
         LOG.info(_('create_backup finished. backup: %s'), backup_id)
 
+    @utils.require_driver_initialized
     def restore_backup(self, context, backup_id, volume_id):
         """Restore volume backups from configured backup service."""
         LOG.info(_('restore_backup started, restoring backup: %(backup_id)s'
                    ' to volume: %(volume_id)s') %
                  {'backup_id': backup_id, 'volume_id': volume_id})
+
         backup = self.db.backup_get(context, backup_id)
         volume = self.db.volume_get(context, volume_id)
         self.db.backup_update(context, backup_id, {'host': self.host})
@@ -256,10 +274,11 @@ class BackupManager(manager.SchedulerDependentManager):
                    ' to volume: %(volume_id)s') %
                  {'backup_id': backup_id, 'volume_id': volume_id})
 
+    @utils.require_driver_initialized
     def delete_backup(self, context, backup_id):
         """Delete volume backup from configured backup service."""
-        backup = self.db.backup_get(context, backup_id)
         LOG.info(_('delete_backup started, backup: %s'), backup_id)
+        backup = self.db.backup_get(context, backup_id)
         self.db.backup_update(context, backup_id, {'host': self.host})
 
         expected_status = 'deleting'

diff --git a/cinder/exception.py b/cinder/exception.py
index 77c0e192a0bf7dc3506c566c1edfc35f9cb7c79d..0460f37e784dbd48756a2b6055b8f3fae180df55 100644 (file)
--- a/cinder/exception.py
+++ b/cinder/exception.py
@@ -126,6 +126,10 @@ class ImageNotAuthorized(CinderException):
     message = _("Not authorized for image %(image_id)s.")
 
 
+class DriverNotInitialized(CinderException):
+    message = _("Volume driver '%(driver)s' not initialized.")
+
+
 class Invalid(CinderException):
     message = _("Unacceptable parameters.")
     code = 400

diff --git a/cinder/tests/test_backup.py b/cinder/tests/test_backup.py
index 4e4085003d408874156340beedb813951407aeb8..a01adb5d8c18533a0baf56018736dfb49b6d2573 100644 (file)
--- a/cinder/tests/test_backup.py
+++ b/cinder/tests/test_backup.py
@@ -51,6 +51,7 @@ class BackupTestCase(test.TestCase):
             importutils.import_object(CONF.backup_manager)
         self.backup_mgr.host = 'testhost'
         self.ctxt = context.get_admin_context()
+        self.backup_mgr.driver.set_initialized()
 
     def tearDown(self):
         super(BackupTestCase, self).tearDown()

diff --git a/cinder/tests/test_gpfs.py b/cinder/tests/test_gpfs.py
index 85bf212ea899aa935e9bc82003d1e6505d0b7828..a836326de945f255e7ec976ba6e15848de08b000 100644 (file)
--- a/cinder/tests/test_gpfs.py
+++ b/cinder/tests/test_gpfs.py
@@ -86,6 +86,7 @@ class GPFSDriverTestCase(test.TestCase):
                    gpfs_mount_point_base=self.volumes_path)
         self.volume = importutils.import_object(CONF.volume_manager)
         self.volume.driver.set_execute(self._execute_wrapper)
+        self.volume.driver.set_initialized()
 
         self.stubs.Set(GPFSDriver, '_create_gpfs_snap',
                        self._fake_gpfs_snap)

diff --git a/cinder/tests/test_rbd.py b/cinder/tests/test_rbd.py
index d782fdc7396b185dfb38dfbbcb326ec3c43b3276..53a310185c19dee319726b0d7c1a6f1e13a9908b 100644 (file)
--- a/cinder/tests/test_rbd.py
+++ b/cinder/tests/test_rbd.py
@@ -102,6 +102,7 @@ class RBDTestCase(test.TestCase):
                                        configuration=self.configuration,
                                        rados=self.rados,
                                        rbd=self.rbd)
+        self.driver.set_initialized()
 
     def test_create_volume(self):
         name = u'volume-00000001'
@@ -498,6 +499,7 @@ class ManagedRBDTestCase(DriverTestCase):
     def setUp(self):
         super(ManagedRBDTestCase, self).setUp()
         fake_image.stub_out_image_service(self.stubs)
+        self.volume.driver.set_initialized()
 
     def _clone_volume_from_image(self, expected_status,
                                  clone_works=True):

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 3f7f956368426f604b639666db7454ede3d8c884..8ce5b73577a4ff10a1b62068d74569cf785b169b 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -105,6 +105,7 @@ class BaseVolumeTestCase(test.TestCase):
         test_notifier.NOTIFICATIONS = []
         self.stubs.Set(brick_lvm.LVM, '_vg_exists', lambda x: True)
         self.stubs.Set(os.path, 'exists', lambda x: True)
+        self.volume.driver.set_initialized()
 
     def tearDown(self):
         try:
@@ -1959,6 +1960,7 @@ class DriverTestCase(test.TestCase):
             """Fake _execute."""
             return self.output, None
         self.volume.driver.set_execute(_fake_execute)
+        self.volume.driver.set_initialized()
 
     def tearDown(self):
         try:

diff --git a/cinder/utils.py b/cinder/utils.py
index a4375d659d0b1c27529df1bb3815262ae130505c..0ecf06426ec44071e86179640c14bd9368dddf93 100644 (file)
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -792,3 +792,14 @@ def brick_get_connector(protocol, driver=None,
                                                 driver=driver,
                                                 execute=execute,
                                                 use_multipath=use_multipath)
+
+
+def require_driver_initialized(func):
+    @functools.wraps(func)
+    def wrapper(self, *args, **kwargs):
+        # we can't do anything if the driver didn't init
+        if not self.driver.initialized:
+            driver_name = self.driver.__class__.__name__
+            raise exception.DriverNotInitialized(driver=driver_name)
+        return func(self, *args, **kwargs)
+    return wrapper

diff --git a/cinder/volume/flows/create_volume/__init__.py b/cinder/volume/flows/create_volume/__init__.py
index 6394f1158d66a6422b9aaa3d5726879deab19e24..de2192c48b918507cbeea40dd68f7039ca36a936 100644 (file)
--- a/cinder/volume/flows/create_volume/__init__.py
+++ b/cinder/volume/flows/create_volume/__init__.py
@@ -1439,6 +1439,12 @@ class CreateVolumeFromSpecTask(base.CinderTask):
         return self.driver.create_volume(volume_ref)
 
     def __call__(self, context, volume_ref, volume_spec):
+        # we can't do anything if the driver didn't init
+        if not self.driver.initialized:
+            LOG.error(_("Unable to create volume, driver not initialized"))
+            driver_name = self.driver.__class__.__name__
+            raise exception.DriverNotInitialized(driver=driver_name)
+
         create_type = volume_spec.pop('type', None)
         create_functor = self._create_func_mapping.get(create_type)
         if not create_functor:

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 54ac3825b356ac1b94312ea1a2e4d48c5f08da7e..fda0fb9530b3ccd5060aeaaccc4139c8aa77d558 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -169,10 +169,14 @@ class VolumeManager(manager.SchedulerDependentManager):
         try:
             self.driver.do_setup(ctxt)
             self.driver.check_for_setup_error()
-        except Exception:
+        except Exception as ex:
             LOG.error(_("Error encountered during "
-                        "initialization of driver: %s"),
-                      self.driver.__class__.__name__)
+                        "initialization of driver: %(name)s") %
+                      {'name': self.driver.__class__.__name__})
+            LOG.exception(ex)
+            # we don't want to continue since we failed
+            # to initialize the driver correctly.
+            return
 
         volumes = self.db.volume_get_all_by_host(ctxt, self.host)
         LOG.debug(_("Re-exporting %s volumes"), len(volumes))
@@ -198,6 +202,7 @@ class VolumeManager(manager.SchedulerDependentManager):
         # collect and publish service capabilities
         self.publish_service_capabilities(ctxt)
 
+    @utils.require_driver_initialized
     def create_volume(self, context, volume_id, request_spec=None,
                       filter_properties=None, allow_reschedule=True,
                       snapshot_id=None, image_id=None, source_volid=None):
@@ -227,6 +232,7 @@ class VolumeManager(manager.SchedulerDependentManager):
         self._reset_stats()
         return volume_id
 
+    @utils.require_driver_initialized
     def delete_volume(self, context, volume_id):
         """Deletes and unexports volume."""
         context = context.elevated()
@@ -304,12 +310,14 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         return True
 
+    @utils.require_driver_initialized
     def create_snapshot(self, context, volume_id, snapshot_id):
         """Creates and exports the snapshot."""
         caller_context = context
         context = context.elevated()
         snapshot_ref = self.db.snapshot_get(context, snapshot_id)
         LOG.info(_("snapshot %s: creating"), snapshot_ref['id'])
+
         self._notify_about_snapshot_usage(
             context, snapshot_ref, "create.start")
 
@@ -352,12 +360,14 @@ class VolumeManager(manager.SchedulerDependentManager):
         self._notify_about_snapshot_usage(context, snapshot_ref, "create.end")
         return snapshot_id
 
+    @utils.require_driver_initialized
     def delete_snapshot(self, context, snapshot_id):
         """Deletes and unexports snapshot."""
         caller_context = context
         context = context.elevated()
         snapshot_ref = self.db.snapshot_get(context, snapshot_id)
         project_id = snapshot_ref['project_id']
+
         LOG.info(_("snapshot %s: deleting"), snapshot_ref['id'])
         self._notify_about_snapshot_usage(
             context, snapshot_ref, "delete.start")
@@ -412,6 +422,7 @@ class VolumeManager(manager.SchedulerDependentManager):
             QUOTAS.commit(context, reservations, project_id=project_id)
         return True
 
+    @utils.require_driver_initialized
     def attach_volume(self, context, volume_id, instance_uuid, host_name,
                       mountpoint, mode):
         """Updates db to show volume is attached"""
@@ -484,6 +495,7 @@ class VolumeManager(manager.SchedulerDependentManager):
                                     mountpoint)
         return do_attach()
 
+    @utils.require_driver_initialized
     def detach_volume(self, context, volume_id):
         """Updates db to show volume is detached"""
         # TODO(vish): refactor this into a more general "unreserve"
@@ -508,6 +520,7 @@ class VolumeManager(manager.SchedulerDependentManager):
                 volume['name'] not in volume['provider_location']):
             self.driver.ensure_export(context, volume)
 
+    @utils.require_driver_initialized
     def copy_volume_to_image(self, context, volume_id, image_meta):
         """Uploads the specified volume to Glance.
 
@@ -538,6 +551,7 @@ class VolumeManager(manager.SchedulerDependentManager):
                 self.db.volume_update(context, volume_id,
                                       {'status': 'in-use'})
 
+    @utils.require_driver_initialized
     def initialize_connection(self, context, volume_id, connector):
         """Prepare volume for connection from host represented by connector.
 
@@ -604,6 +618,7 @@ class VolumeManager(manager.SchedulerDependentManager):
             conn_info['data']['access_mode'] = access_mode
         return conn_info
 
+    @utils.require_driver_initialized
     def terminate_connection(self, context, volume_id, connector, force=False):
         """Cleanup connection from host represented by connector.
 
@@ -612,6 +627,7 @@ class VolumeManager(manager.SchedulerDependentManager):
         volume_ref = self.db.volume_get(context, volume_id)
         self.driver.terminate_connection(volume_ref, connector, force=force)
 
+    @utils.require_driver_initialized
     def accept_transfer(self, context, volume_id, new_user, new_project):
         # NOTE(jdg): need elevated context as we haven't "given" the vol
         # yet
@@ -708,6 +724,7 @@ class VolumeManager(manager.SchedulerDependentManager):
         self.db.volume_update(ctxt, volume_id, {'migration_status': None})
         return volume['id']
 
+    @utils.require_driver_initialized
     def migrate_volume(self, ctxt, volume_id, host, force_host_copy=False):
         """Migrate the volume to the specified host (called on source host)."""
         volume_ref = self.db.volume_get(ctxt, volume_id)
@@ -753,7 +770,7 @@ class VolumeManager(manager.SchedulerDependentManager):
     def _report_driver_status(self, context):
         LOG.info(_("Updating volume status"))
         if not self.driver.initialized:
-            LOG.warning(_('Unabled to update stats, driver is '
+            LOG.warning(_('Unable to update stats, driver is '
                           'uninitialized'))
         else:
             volume_stats = self.driver.get_volume_stats(refresh=True)
@@ -793,6 +810,7 @@ class VolumeManager(manager.SchedulerDependentManager):
             context, snapshot, event_suffix,
             extra_usage_info=extra_usage_info, host=self.host)
 
+    @utils.require_driver_initialized
     def extend_volume(self, context, volume_id, new_size):
         volume = self.db.volume_get(context, volume_id)
         size_increase = (int(new_size)) - volume['size']