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

diff --git a/cinder/volume/drivers/block_device.py b/cinder/volume/drivers/block_device.py
index 0e86efb0c81ae7aa040f7f3cea28425c6d9fbb19..a9cf269499620d63c32164dad791d6ebde172f1e 100644 (file)
--- 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 ed27fecce91438ffcfa93d2f633533eb23416400..a87f66371968ccf43fd131a65fe695f2b9f383ec 100644 (file)
--- 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 b0409a7c01d14dc95b9cbd1d3b0ee53f97f84e46..cb229e6c247cdb58053252e96b52150e9bc8afa6 100644 (file)
--- 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 18053c4d25bf0786ecdfbe8932a72409fa85042f..7e1a43d24a67c2a04d3eb37d4e869b42111f0152 100644 (file)
--- 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 4147bba49598efd1b891022105e1bb87a1805884..056288061cd39e34c508218d51362aa788642830 100644 (file)
--- 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 d1bcbe9e0d6c4d9907b16b1f2f9e0c0834c74dae..08257e1e1d8aa13473f5cb2fcbbbf7bcbba6f019 100644 (file)
--- 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 6c3f5e6d19b7ac6078b62ed2442b776505bffd4d..80f04334d03da2a4019054a77b204c9204c3f23c 100644 (file)
--- 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 0676d6fbd04e6b3c4ed6d0a414d2818e82371f7e..a01951e5022bde3d2b2f91d504c09c0ab5566094 100644 (file)
--- a/cinder/volume/targets/iser.py
+++ b/cinder/volume/targets/iser.py
@@ -25,6 +25,7 @@ class ISERTgtAdm(TgtAdm):
                 <target %s>
                     driver iser
                     backing-store %s
+                    write_cache %s
                 </target>
                   """
     VOLUME_CONF_WITH_CHAP_AUTH = """
@@ -32,6 +33,7 @@ class ISERTgtAdm(TgtAdm):
                                     driver iser
                                     backing-store %s
                                     %s
+                                    write_cache %s
                                 </target>
                                  """
 

diff --git a/cinder/volume/targets/lio.py b/cinder/volume/targets/lio.py
index 7b8930f8eef58645d10e5957a04849461204116f..6a8c71eba639cbaf10b7d35d21522e23bc4f703a 100644 (file)
--- 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 02ff1d385061ba0fca8fe287af69021c3447f9b2..4d0489da3cc807bfb7279effb9c8260d9ae29c97 100644 (file)
--- 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)