From 942981cdd3627c3ccd5dfe5db463f4ee2ccc73cb Mon Sep 17 00:00:00 2001 From: John Griffith Date: Tue, 6 Jan 2015 22:47:01 -0700 Subject: [PATCH] Fix iscsi_write_cache setting for iscsi targets While transitioning to the new driver and target model (change 9651f547147188645942466602c92cce06666483) some things got lost not surprisingly. One of those things that wasn't migrated correctly was the iscsi_write_cache option due to ensure_export not being called properly on service restart. This patch cleans up the ensure/create_export methods and their signatures. Most importantly esnure_export should now actually work and check the iscsi_write_cache settings. While fixing this I also cleaned up the two methods mentioned above to eliminate the unnecessary and duplicate info in their arguments. Closes-Bug: #1408171 Change-Id: Ibfd1feefd72c43ef316b267e9d6645f2e67e2558 --- cinder/volume/drivers/block_device.py | 10 +++++----- cinder/volume/drivers/drbdmanagedrv.py | 8 ++++++-- cinder/volume/drivers/lvm.py | 16 ++++++++-------- cinder/volume/drivers/srb.py | 18 +++++------------- cinder/volume/targets/driver.py | 4 +--- cinder/volume/targets/fake.py | 4 +--- cinder/volume/targets/iet.py | 4 +--- cinder/volume/targets/iser.py | 2 ++ cinder/volume/targets/lio.py | 7 +++---- cinder/volume/targets/tgt.py | 14 ++++++++------ 10 files changed, 40 insertions(+), 47 deletions(-) diff --git a/cinder/volume/drivers/block_device.py b/cinder/volume/drivers/block_device.py index 0e86efb0c..a9cf26949 100644 --- a/cinder/volume/drivers/block_device.py +++ b/cinder/volume/drivers/block_device.py @@ -189,13 +189,13 @@ class BlockDeviceDriver(driver.VolumeDriver): # ####### Interface methods for DataPath (Target Driver) ######## def ensure_export(self, context, volume): - volume_name = volume['name'] volume_path = "/dev/%s/%s" % (self.configuration.volume_group, - volume_name) + volume['name']) model_update = \ - self.target_driver.ensure_export(context, - volume, - volume_path=volume_path) + self.target_driver.ensure_export( + context, + volume, + volume_path) return model_update def create_export(self, context, volume): diff --git a/cinder/volume/drivers/drbdmanagedrv.py b/cinder/volume/drivers/drbdmanagedrv.py index ed27fecce..a87f66371 100644 --- a/cinder/volume/drivers/drbdmanagedrv.py +++ b/cinder/volume/drivers/drbdmanagedrv.py @@ -481,15 +481,19 @@ class DrbdManageDriver(driver.VolumeDriver): # ####### Interface methods for DataPath (Target Driver) ######## def ensure_export(self, context, volume): + volume_path = self.local_path(volume) return self.target_driver.ensure_export( context, volume, - volume_path=self.local_path(volume)) + volume_path) def create_export(self, context, volume): + volume_path = self.local_path(volume), export_info = self.target_driver.create_export( context, - volume, volume_path=self.local_path(volume)) + volume, + volume_path) + return {'provider_location': export_info['location'], 'provider_auth': export_info['auth'], } diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index b0409a7c0..cb229e6c2 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -542,13 +542,11 @@ class LVMVolumeDriver(driver.VolumeDriver): # ####### Interface methods for DataPath (Target Driver) ######## def ensure_export(self, context, volume): - volume_name = volume['name'] volume_path = "/dev/%s/%s" % (self.configuration.volume_group, - volume_name) + volume['name']) + model_update = \ - self.target_driver.ensure_export(context, - volume, - volume_path=volume_path) + self.target_driver.ensure_export(context, volume, volume_path) return model_update def create_export(self, context, volume, vg=None): @@ -556,9 +554,11 @@ class LVMVolumeDriver(driver.VolumeDriver): vg = self.configuration.volume_group volume_path = "/dev/%s/%s" % (vg, volume['name']) - export_info = self.target_driver.create_export(context, - volume, - volume_path) + + export_info = self.target_driver.create_export( + context, + volume, + volume_path) return {'provider_location': export_info['location'], 'provider_auth': export_info['auth'], } diff --git a/cinder/volume/drivers/srb.py b/cinder/volume/drivers/srb.py index 18053c4d2..7e1a43d24 100644 --- a/cinder/volume/drivers/srb.py +++ b/cinder/volume/drivers/srb.py @@ -824,18 +824,11 @@ class SRBISCSIDriver(SRBDriver, driver.ISCSIDriver): self.target_driver.set_execute(execute) def ensure_export(self, context, volume): - volume_name = volume['name'] - iscsi_name = "%s%s" % (self.configuration.iscsi_target_prefix, - volume_name) device_path = self._mapper_path(volume) - # NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need - # should clean this all up at some point in the future - model_update = self.target_driver.ensure_export( - context, volume, - iscsi_name, - device_path, - None, - self.configuration) + + model_update = self.target_driver.ensure_export(context, + volume, + device_path) if model_update: self.db.volume_update(context, volume['id'], model_update) @@ -850,8 +843,7 @@ class SRBISCSIDriver(SRBDriver, driver.ISCSIDriver): data = self.target_driver.create_export(context, volume, - volume_path, - self.configuration) + volume_path) return { 'provider_location': data['location'], 'provider_auth': data['auth'], diff --git a/cinder/volume/targets/driver.py b/cinder/volume/targets/driver.py index 4147bba49..056288061 100644 --- a/cinder/volume/targets/driver.py +++ b/cinder/volume/targets/driver.py @@ -40,9 +40,7 @@ class Target(object): CONF.rootwrap_config) @abc.abstractmethod - def ensure_export(self, context, volume, - iscsi_name, volume_path, - volume_group, config): + def ensure_export(self, context, volume, volume_path): """Synchronously recreates an export for a volume.""" pass diff --git a/cinder/volume/targets/fake.py b/cinder/volume/targets/fake.py index d1bcbe9e0..08257e1e1 100644 --- a/cinder/volume/targets/fake.py +++ b/cinder/volume/targets/fake.py @@ -19,9 +19,7 @@ class FakeTarget(iscsi.ISCSITarget): def __init__(self, *args, **kwargs): super(FakeTarget, self).__init__(*args, **kwargs) - def ensure_export(self, context, volume, - iscsi_name, volume_path, - volume_group, config): + def ensure_export(self, context, volume, volume_path): pass def create_export(self, context, volume, volume_path): diff --git a/cinder/volume/targets/iet.py b/cinder/volume/targets/iet.py index 6c3f5e6d1..80f04334d 100644 --- a/cinder/volume/targets/iet.py +++ b/cinder/volume/targets/iet.py @@ -17,9 +17,7 @@ class IetAdm(object): def __init__(self, *args, **kwargs): super(IetAdm, self).__init__(*args, **kwargs) - def ensure_export(self, context, volume, - iscsi_name, volume_path, - volume_group, config): + def ensure_export(self, context, volume, volume_path): pass def create_export(self, context, volume, volume_path): diff --git a/cinder/volume/targets/iser.py b/cinder/volume/targets/iser.py index 0676d6fbd..a01951e50 100644 --- a/cinder/volume/targets/iser.py +++ b/cinder/volume/targets/iser.py @@ -25,6 +25,7 @@ class ISERTgtAdm(TgtAdm): driver iser backing-store %s + write_cache %s """ VOLUME_CONF_WITH_CHAP_AUTH = """ @@ -32,6 +33,7 @@ class ISERTgtAdm(TgtAdm): driver iser backing-store %s %s + write_cache %s """ diff --git a/cinder/volume/targets/lio.py b/cinder/volume/targets/lio.py index 7b8930f8e..6a8c71eba 100644 --- a/cinder/volume/targets/lio.py +++ b/cinder/volume/targets/lio.py @@ -49,9 +49,7 @@ class LioAdm(TgtAdm): self.remove_iscsi_target(iscsi_target, 0, volume['id'], volume['name']) - def ensure_export(self, context, volume, - iscsi_name, volume_path, - volume_group, config): + def ensure_export(self, context, volume, volume_path): try: volume_info = self.db.volume_get(context, volume['id']) (auth_method, @@ -66,7 +64,8 @@ class LioAdm(TgtAdm): "provision for volume: %s"), volume['id']) iscsi_target = 1 - + iscsi_name = "%s%s" % (self.configuration.iscsi_target_prefix, + volume['name']) self.create_iscsi_target(iscsi_name, iscsi_target, 0, volume_path, chap_auth, check_exit_code=False) diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py index 02ff1d385..4d0489da3 100644 --- a/cinder/volume/targets/tgt.py +++ b/cinder/volume/targets/tgt.py @@ -160,9 +160,7 @@ class TgtAdm(iscsi.ISCSITarget): LOG.debug('Failed to find CHAP auth from config for %s' % vol_id) return None - def ensure_export(self, context, volume, - iscsi_name, volume_path, - volume_group, config): + def ensure_export(self, context, volume, volume_path): chap_auth = None old_name = None @@ -173,11 +171,13 @@ class TgtAdm(iscsi.ISCSITarget): iscsi_name = "%s%s" % (self.configuration.iscsi_target_prefix, volume['name']) + iscsi_write_cache = self.configuration.get('iscsi_write_cache', 'on') self.create_iscsi_target( iscsi_name, 1, 0, volume_path, chap_auth, check_exit_code=False, - old_name=old_name) + old_name=old_name, + iscsi_write_cache=iscsi_write_cache) def create_iscsi_target(self, name, tid, lun, path, chap_auth=None, **kwargs): @@ -186,7 +186,7 @@ class TgtAdm(iscsi.ISCSITarget): fileutils.ensure_tree(self.volumes_dir) vol_id = name.split(':')[1] - write_cache = kwargs.get('write_cache', 'on') + write_cache = kwargs.get('iscsi_write_cache', 'on') if chap_auth is None: volume_conf = self.VOLUME_CONF % (name, path, write_cache) else: @@ -296,11 +296,13 @@ class TgtAdm(iscsi.ISCSITarget): chap_password) # NOTE(jdg): For TgtAdm case iscsi_name is the ONLY param we need # should clean this all up at some point in the future + iscsi_write_cache = self.configuration.get('iscsi_write_cache', 'on') tid = self.create_iscsi_target(iscsi_name, iscsi_target, 0, volume_path, - chap_auth) + chap_auth, + iscsi_write_cache=iscsi_write_cache) data = {} data['location'] = self._iscsi_location( self.configuration.iscsi_ip_address, tid, iscsi_name, lun) -- 2.45.2