diff --git a/cinder/tests/unit/targets/test_tgt_driver.py b/cinder/tests/unit/targets/test_tgt_driver.py
index 71ee09b51be..58727501751 100644
--- a/cinder/tests/unit/targets/test_tgt_driver.py
+++ b/cinder/tests/unit/targets/test_tgt_driver.py
@@ -178,20 +178,6 @@ class TestTgtAdmDriver(tf.TargetDriverFixture):
self.testvol_path,
chap_auth=('chap_foo', 'chap_bar')))
- mock_open.assert_called_once_with(
- os.path.join(self.fake_volumes_dir, self.test_vol.split(':')[1]),
- 'w+')
- expected = ('\n\n'
- ' backing-store %(bspath)s\n'
- ' driver iscsi\n'
- ' incominguser chap_foo chap_bar\n'
- ' bsoflags foo\n'
- ' write-cache bar\n'
- '\n' % {'id': self.VOLUME_ID,
- 'bspath': self.testvol_path})
- self.assertEqual(expected,
- mock_open.return_value.write.call_args[0][0])
-
def test_create_iscsi_target_already_exists(self):
def _fake_execute(*args, **kwargs):
if 'update' in args:
diff --git a/cinder/tests/unit/test_utils.py b/cinder/tests/unit/test_utils.py
index 95ea919f518..0bddc52c9df 100644
--- a/cinder/tests/unit/test_utils.py
+++ b/cinder/tests/unit/test_utils.py
@@ -191,6 +191,50 @@ class GenericUtilsTestCase(test.TestCase):
utils.read_file_as_root,
test_filepath)
+ @mock.patch('tempfile.NamedTemporaryFile')
+ @mock.patch.object(os, 'open')
+ @mock.patch.object(os, 'fdatasync')
+ @mock.patch.object(os, 'fsync')
+ @mock.patch.object(os, 'rename')
+ @mock.patch.object(os, 'close')
+ @mock.patch.object(os.path, 'isfile')
+ @mock.patch.object(os, 'unlink')
+ def test_write_configfile(self, mock_unlink, mock_isfile, mock_close,
+ mock_rename, mock_fsync, mock_fdatasync,
+ mock_open, mock_tmp):
+ filename = 'foo'
+ directory = '/some/random/path'
+ filepath = os.path.join(directory, filename)
+ expected = ('\n\n'
+ ' backing-store %(bspath)s\n'
+ ' driver iscsi\n'
+ ' incominguser chap_foo chap_bar\n'
+ ' bsoflags foo\n'
+ ' write-cache bar\n'
+ '\n' % {'id': filename,
+ 'bspath': filepath})
+
+ # Normal case
+ utils.robust_file_write(directory, filename, expected)
+ mock_open.assert_called_once_with(directory, os.O_DIRECTORY)
+ mock_rename.assert_called_once_with(mock.ANY, filepath)
+ self.assertEqual(
+ expected.encode('utf-8'),
+ mock_tmp.return_value.__enter__.return_value.write.call_args[0][0]
+ )
+
+ # Failure to write persistent file.
+ tempfile = '/some/tempfile'
+ mock_tmp.return_value.__enter__.return_value.name = tempfile
+ mock_rename.side_effect = OSError
+ self.assertRaises(OSError,
+ utils.robust_file_write,
+ directory,
+ filename,
+ mock.MagicMock())
+ mock_isfile.assert_called_once_with(tempfile)
+ mock_unlink.assert_called_once_with(tempfile)
+
@mock.patch('oslo_utils.timeutils.utcnow')
def test_service_is_up(self, mock_utcnow):
fts_func = datetime.datetime.fromtimestamp
diff --git a/cinder/utils.py b/cinder/utils.py
index 3eef2ac3c0b..e2865a429ac 100644
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -46,6 +46,7 @@ from oslo_concurrency import processutils
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import encodeutils
+from oslo_utils import excutils
from oslo_utils import importutils
from oslo_utils import strutils
from oslo_utils import timeutils
@@ -452,6 +453,49 @@ def read_file_as_root(file_path):
raise exception.FileNotFound(file_path=file_path)
+def robust_file_write(directory, filename, data):
+ """Robust file write.
+
+ Use "write to temp file and rename" model for writing the
+ persistence file.
+
+ :param directory: Target directory to create a file.
+ :param filename: File name to store specified data.
+ :param data: String data.
+ """
+ tempname = None
+ dirfd = None
+ try:
+ dirfd = os.open(directory, os.O_DIRECTORY)
+
+ # write data to temporary file
+ with tempfile.NamedTemporaryFile(prefix=filename,
+ dir=directory,
+ delete=False) as tf:
+ tempname = tf.name
+ tf.write(data.encode('utf-8'))
+ tf.flush()
+ os.fdatasync(tf.fileno())
+ tf.close()
+
+ # Fsync the directory to ensure the fact of the existence of
+ # the temp file hits the disk.
+ os.fsync(dirfd)
+ # If destination file exists, it will be replaced silently.
+ os.rename(tempname, os.path.join(directory, filename))
+ # Fsync the directory to ensure the rename hits the disk.
+ os.fsync(dirfd)
+ except OSError:
+ with excutils.save_and_reraise_exception():
+ LOG.error(_LE("Failed to write persistence file: %(path)s."),
+ {'path': os.path.join(directory, filename)})
+ if os.path.isfile(tempname):
+ os.unlink(tempname)
+ finally:
+ if dirfd:
+ os.close(dirfd)
+
+
@contextlib.contextmanager
def temporary_chown(path, owner_uid=None):
"""Temporarily chown a path.
diff --git a/cinder/volume/targets/cxt.py b/cinder/volume/targets/cxt.py
index c08974f21fd..d129f5f38c8 100644
--- a/cinder/volume/targets/cxt.py
+++ b/cinder/volume/targets/cxt.py
@@ -130,8 +130,7 @@ class CxtAdm(iscsi.ISCSITarget):
if os.path.exists(volume_path):
LOG.warning(_LW('Persistence file already exists for volume, '
'found file at: %s'), volume_path)
- with open(volume_path, 'w+') as f:
- f.write(volume_conf)
+ utils.robust_file_write(volumes_dir, vol_id, volume_conf)
LOG.debug('Created volume path %(vp)s,\n'
'content: %(vc)s',
{'vp': volume_path, 'vc': volume_conf})
diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py
index 57c02efd6ad..ee3ab9cc2ce 100644
--- a/cinder/volume/targets/tgt.py
+++ b/cinder/volume/targets/tgt.py
@@ -165,8 +165,7 @@ class TgtAdm(iscsi.ISCSITarget):
if os.path.exists(volume_path):
LOG.warning(_LW('Persistence file already exists for volume, '
'found file at: %s'), volume_path)
- with open(volume_path, 'w+') as f:
- f.write(volume_conf)
+ utils.robust_file_write(volumes_dir, vol_id, volume_conf)
LOG.debug(('Created volume path %(vp)s,\n'
'content: %(vc)s'),
{'vp': volume_path, 'vc': volume_conf})