RBD: Improve IOWrapper's close & flush methods

The RBDVolumeIOWrapper class inherits from io.RawIOBase but its close
method doesn't flush and after being called the "closed" attribute still
returns False.

Trying to set the "closed" attribute ourselves in
RBDVolumeIOWrapper.close results in an error, so we call the parent's
implementation to do it.

The parent's close method will also call the flush method.

This change ensures that rbd_image.close is only called once, that the
closed attribute is correct, and that the data is flushed on close.

Without these changes we could have, unintentionally, multiple calls to
the rbd_image.close method.  If we called close on the wrapper
ourselves, then when the Garbage Collector is cleaning up the wrapper it
will call the io.RawIOBase finalizer that will see that the file is not
closed and call close again.

Now that we correctly set the closed flag on the instance, the flush
method should also behave like the parent implementation and fail if it
has already been closed.

Change-Id: Ib3b066a7da071b1c2de78a1a4e569676539bd335
This commit is contained in:
Gorka Eguileor 2022-08-23 11:09:25 +02:00
parent b3f89a304f
commit fafd1a6f76
2 changed files with 21 additions and 1 deletions

View File

@ -226,6 +226,8 @@ class RBDVolumeIOWrapper(io.RawIOBase):
return self._offset
def flush(self):
# Raise ValueError if already closed
super().flush()
try:
self._rbd_volume.image.flush()
except AttributeError:
@ -240,4 +242,8 @@ class RBDVolumeIOWrapper(io.RawIOBase):
raise IOError(_("fileno() not supported by RBD()"))
def close(self):
self.rbd_image.close()
if not self.closed:
# Can't set closed attribute ourselves, call parent to flush and
# change it.
super().close()
self.rbd_image.close()

View File

@ -194,6 +194,14 @@ class RBDVolumeIOWrapperTestCase(base.TestCase):
self.assertEqual(1, self.mock_volume.image.flush.call_count)
self.assertEqual(1, mock_logger.warning.call_count)
def test_flush_on_closed(self):
self.mock_volume_wrapper.close()
self.mock_volume.image.flush.assert_called_once_with()
self.assertTrue(self.mock_volume_wrapper.closed)
self.mock_volume.image.flush.reset_mock()
self.assertRaises(ValueError, self.mock_volume_wrapper.flush)
self.mock_volume.image.flush.assert_not_called()
def test_fileno(self):
self.assertRaises(IOError, self.mock_volume_wrapper.fileno)
@ -207,6 +215,12 @@ class RBDVolumeIOWrapperTestCase(base.TestCase):
linuxrbd.RBDImageMetadata(rbd_volume, 'pool', 'user', None))
rbd_handle.close()
self.assertEqual(1, rbd_disconnect.call_count)
# Confirm the handle now reports that is closed (this attribute cannot
# be modified directly)
self.assertTrue(rbd_handle.closed)
# New call to close shouldn't create additional calls
rbd_handle.close()
self.assertEqual(1, rbd_disconnect.call_count)
class RBDVolumeTestCase(base.TestCase):