From 2177cbbe0692d5dadc22a717d46ae0dd755cedaa Mon Sep 17 00:00:00 2001 From: "Walter A. Boring IV" Date: Mon, 16 Sep 2013 17:05:27 -0700 Subject: [PATCH] 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 --- cinder/backup/manager.py | 25 +++++++++++++++--- cinder/exception.py | 4 +++ cinder/tests/test_backup.py | 1 + cinder/tests/test_gpfs.py | 1 + cinder/tests/test_rbd.py | 2 ++ cinder/tests/test_volume.py | 2 ++ cinder/utils.py | 11 ++++++++ cinder/volume/flows/create_volume/__init__.py | 6 +++++ cinder/volume/manager.py | 26 ++++++++++++++++--- 9 files changed, 71 insertions(+), 7 deletions(-) diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 5ff6c44b0..ceab83b06 100644 --- 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 77c0e192a..0460f37e7 100644 --- 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 4e4085003..a01adb5d8 100644 --- 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 85bf212ea..a836326de 100644 --- 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 d782fdc73..53a310185 100644 --- 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 3f7f95636..8ce5b7357 100644 --- 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 a4375d659..0ecf06426 100644 --- 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 6394f1158..de2192c48 100644 --- 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 54ac3825b..fda0fb953 100644 --- 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'] -- 2.45.2