Merge lp:~billy-olsen/cinder/icehouse-1336568 into lp:~ubuntu-server-dev/cinder/icehouse

[Billy Olsen]

This change includes the option to specify the iscsi_write_cache behavior, which is a backport of LP: #1336568

[Corey Bryant (corey.bryant)]

Looks good, thanks! I had to make a very minor tweak by moving the asterisk in d/changelog right by one space in order for the merge to work.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2015-06-23 00:11:54 +0000
+++ debian/changelog	2015-08-07 22:28:11 +0000
@@ -1,3 +1,12 @@
+cinder (1:2014.1.5-0ubuntu2) trusty; urgency=medium
+
+ * Enable iscsi_write_cache option for tgtadm backends (LP: #1336568):
+    - d/p/tgtadmin-iscsi-write-cache-config.patch - Includes backport of
+      change from the juno release for enabling iscsi write cache policy
+      for tgtadm.
+
+ -- Billy Olsen <[e-mail]>  Mon, 27 Jul 2015 17:35:57 -0700
+
 cinder (1:2014.1.5-0ubuntu1) trusty; urgency=medium
 
   * Resynchronize with stable/icehouse (d03ca5d) (LP: #1467533):

=== modified file 'debian/patches/series'
--- debian/patches/series	2014-03-17 17:37:44 +0000
+++ debian/patches/series	2015-08-07 22:28:11 +0000
@@ -1,1 +1,2 @@
 fix-requirements.patch
+tgtadmin-iscsi-write-cache-config.patch

=== added file 'debian/patches/tgtadmin-iscsi-write-cache-config.patch'
--- debian/patches/tgtadmin-iscsi-write-cache-config.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/tgtadmin-iscsi-write-cache-config.patch	2015-08-07 22:28:11 +0000
@@ -0,0 +1,180 @@
+From fb077f5fe2d64c8c75ff4b507ab56b16c722ef77 Mon Sep 17 00:00:00 2001
+From: Mitsuhiro Tanino <[e-mail]>
+Date: Tue, 8 Jul 2014 15:52:11 -0400
+Subject: [PATCH 1/1] Configure write cache option of tgtd iscsi driver
+
+Cinder LVMiSCSI driver is using default value of write-cache parameter
+of tgtd iscsi driver. In this setting, write I/O from guest instance is
+cached on dirty cache of a host.(write-back mode)
+
+In this case, data lost may be occurred if the host crashes before
+flushing dirty cache. This may cause a lot of instances to lose
+their data.
+
+In order to avoid this issue, it is better to turn off the write cache.
+(write-through mode)
+
+This patch adds "iscsi_write_cache" parameter to configure a behavior of
+write cache. The default value is "iscsi_write_cache=on".(write-back mode)
+
+Closes-Bug: 1336568
+DocImpact
+
+Change-Id: I7a495bc6118d4254576bdf1620a04ac537b3078d
+Signed-off-by: Mitsuhiro Tanino <[e-mail]>
+(cherry picked from commit 617e59bde660f919df0818611b911ae35f8b7247)
+
+The following conflicts were due to local vs global config changes
+from upstream commit (1b21d499bb3bb1e94e2815cbada86db8e8e21406), which
+is not needed in order for change in icehouse. Will not include changes
+from this commit in order to avoid changing the API.
+
+Conflicts:
+	cinder/volume/driver.py
+	cinder/volume/drivers/lvm.py
+	cinder/volume/iscsi.py
+	etc/cinder/cinder.conf.sample
+---
+ cinder/brick/iscsi/iscsi.py   | 10 ++++++++--
+ cinder/tests/test_iscsi.py    |  4 +++-
+ cinder/volume/driver.py       |  6 ++++++
+ cinder/volume/iscsi.py        |  7 +++++--
+ etc/cinder/cinder.conf.sample |  6 ++++++
+ 5 files changed, 28 insertions(+), 5 deletions(-)
+
+diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py
+index e5ae6ce..ab0caa6 100644
+--- a/cinder/brick/iscsi/iscsi.py
++++ b/cinder/brick/iscsi/iscsi.py
+@@ -83,6 +83,7 @@ class TgtAdm(TargetAdmin):
+                 <target %s>
+                     backing-store %s
+                     lld iscsi
++                    write-cache %s
+                 </target>
+                   """
+     VOLUME_CONF_WITH_CHAP_AUTH = """
+@@ -90,6 +91,7 @@ class TgtAdm(TargetAdmin):
+                                     backing-store %s
+                                     lld iscsi
+                                     %s
++                                    write-cache %s
+                                 </target>
+                                  """
+ 
+@@ -165,11 +167,13 @@ class TgtAdm(TargetAdmin):
+         fileutils.ensure_tree(self.volumes_dir)
+ 
+         vol_id = name.split(':')[1]
++        write_cache = kwargs.get('write_cache', 'on')
+         if chap_auth is None:
+-            volume_conf = self.VOLUME_CONF % (name, path)
++            volume_conf = self.VOLUME_CONF % (name, path, write_cache)
+         else:
+             volume_conf = self.VOLUME_CONF_WITH_CHAP_AUTH % (name,
+-                                                             path, chap_auth)
++                                                             path, chap_auth,
++                                                             write_cache)
+ 
+         LOG.info(_('Creating iscsi_target for: %s') % vol_id)
+         volumes_dir = self.volumes_dir
+@@ -600,6 +604,7 @@ class ISERTgtAdm(TgtAdm):
+                 <target %s>
+                     driver iser
+                     backing-store %s
++                    write_cache %s
+                 </target>
+                   """
+     VOLUME_CONF_WITH_CHAP_AUTH = """
+@@ -607,6 +612,7 @@ class ISERTgtAdm(TgtAdm):
+                                     driver iser
+                                     backing-store %s
+                                     %s
++                                    write_cache %s
+                                 </target>
+                                  """
+ 
+diff --git a/cinder/tests/test_iscsi.py b/cinder/tests/test_iscsi.py
+index d24989b..74e40f1 100644
+--- a/cinder/tests/test_iscsi.py
++++ b/cinder/tests/test_iscsi.py
+@@ -34,6 +34,7 @@ class TargetAdminTestCase(object):
+         self.path = '/foo'
+         self.vol_id = 'blaa'
+         self.vol_name = 'volume-blaa'
++        self.write_cache = 'off'
+         self.db = {}
+ 
+         self.script_template = None
+@@ -93,7 +94,8 @@ class TargetAdminTestCase(object):
+         target_helper = self.driver.get_target_helper(self.db)
+         target_helper.set_execute(self.fake_execute)
+         target_helper.create_iscsi_target(self.target_name, self.tid,
+-                                          self.lun, self.path)
++                                          self.lun, self.path,
++                                          write_cache=self.write_cache)
+         target_helper.show_target(self.tid, iqn=self.target_name)
+         target_helper.remove_iscsi_target(self.tid, self.lun, self.vol_id,
+                                           self.vol_name)
+diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
+index aa0fb20..1f08dc3 100644
+--- a/cinder/volume/driver.py
++++ b/cinder/volume/driver.py
+@@ -103,6 +103,12 @@ volume_opts = [
+                default='1M',
+                help='The default block size used when copying/clearing '
+                     'volumes'),
++    cfg.StrOpt('iscsi_write_cache',
++               default='on',
++               help='Sets the behavior of the iSCSI target to either '
++                    'perform write-back(on) or write-through(off). '
++                    'This parameter is valid if iscsi_helper is set '
++                    'to tgtadm or iseradm.'),
+ ]
+ 
+ # for backward compatibility
+diff --git a/cinder/volume/iscsi.py b/cinder/volume/iscsi.py
+index 7ba7aad..70fd695 100644
+--- a/cinder/volume/iscsi.py
++++ b/cinder/volume/iscsi.py
+@@ -50,7 +50,9 @@ class _ExportMixin(object):
+                                        iscsi_target,
+                                        0,
+                                        volume_path,
+-                                       chap_auth)
++                                       chap_auth,
++                                       write_cache=
++                                       CONF.iscsi_write_cache)
+         data = {}
+         data['location'] = self._iscsi_location(
+             CONF.iscsi_ip_address, tid, iscsi_name, lun)
+@@ -104,7 +106,8 @@ class _ExportMixin(object):
+                 old_name = None
+         self.create_iscsi_target(iscsi_name, iscsi_target, 0, volume_path,
+                                  chap_auth, check_exit_code=False,
+-                                 old_name=old_name)
++                                 old_name=old_name,
++                                 write_cache=CONF.iscsi_write_cache)
+ 
+     def _ensure_iscsi_targets(self, context, host):
+         """Ensure that target ids have been created in datastore."""
+diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample
+index 22404f4..f1e9f9f 100644
+--- a/etc/cinder/cinder.conf.sample
++++ b/etc/cinder/cinder.conf.sample
+@@ -1009,6 +1009,12 @@
+ # (string value)
+ #volume_dd_blocksize=1M
+ 
++# Sets the behavior of the iSCSI target to either perform
++# write-back(on) or write-through(off). This parameter is
++# valid if iscsi_helper is set to tgtadm or iseradm. (string
++# value)
++#iscsi_write_cache=on
++
+ 
+ #
+ # Options defined in cinder.volume.drivers.block_device
+-- 
+2.1.4
+