-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Adds a first stage for the full quobyte patch
This starts a patch that covers all relevant changes for running Quobyte with a given release. This first step adds a generic image cache remotefs fix for Cinder.
- Loading branch information
1 parent
7bdc09f
commit d390ab5
Showing
1 changed file
with
281 additions
and
0 deletions.
There are no files selected for viewing
281 changes: 281 additions & 0 deletions
281
full_quobyte_patch/Ocata/Cinder/full_quobyte_ocata_cinder.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,281 @@ | ||
diff --git cinder/tests/unit/volume/drivers/test_remotefs.py cinder/tests/unit/volume/drivers/test_remotefs.py | ||
index 0cd2ad8..72d70f4 100644 | ||
--- cinder/tests/unit/volume/drivers/test_remotefs.py | ||
+++ cinder/tests/unit/volume/drivers/test_remotefs.py | ||
@@ -52,9 +52,26 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
self._fake_snapshot.id) | ||
self._fake_snapshot.volume = self._fake_volume | ||
|
||
+ @ddt.data({'current_state': 'in-use', | ||
+ 'acceptable_states': ['available', 'in-use']}, | ||
+ {'current_state': 'in-use', | ||
+ 'acceptable_states': ['available'], | ||
+ 'expected_exception': exception.InvalidVolume}) | ||
+ @ddt.unpack | ||
+ def test_validate_state(self, current_state, acceptable_states, | ||
+ expected_exception=None): | ||
+ if expected_exception: | ||
+ self.assertRaises(expected_exception, | ||
+ self._driver._validate_state, | ||
+ current_state, | ||
+ acceptable_states) | ||
+ else: | ||
+ self._driver._validate_state(current_state, acceptable_states) | ||
+ | ||
def _test_delete_snapshot(self, volume_in_use=False, | ||
stale_snapshot=False, | ||
- is_active_image=True): | ||
+ is_active_image=True, | ||
+ is_tmp_snap=False): | ||
# If the snapshot is not the active image, it is guaranteed that | ||
# another snapshot exists having it as backing file. | ||
|
||
@@ -78,6 +95,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
self._driver._local_volume_dir = mock.Mock( | ||
return_value=self._FAKE_MNT_POINT) | ||
|
||
+ self._driver._validate_state = mock.Mock() | ||
self._driver._read_info_file = mock.Mock() | ||
self._driver._write_info_file = mock.Mock() | ||
self._driver._img_commit = mock.Mock() | ||
@@ -91,12 +109,18 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
self._fake_snapshot.id: fake_snapshot_name | ||
} | ||
|
||
+ exp_acceptable_states = ['available', 'in-use', 'backing-up', | ||
+ 'deleting', 'downloading'] | ||
+ | ||
if volume_in_use: | ||
self._fake_snapshot.volume.status = 'in-use' | ||
|
||
self._driver._read_info_file.return_value = fake_info | ||
|
||
self._driver._delete_snapshot(self._fake_snapshot) | ||
+ self._driver._validate_state.assert_called_once_with( | ||
+ self._fake_snapshot.volume.status, | ||
+ exp_acceptable_states) | ||
if stale_snapshot: | ||
self._driver._delete_stale_snapshot.assert_called_once_with( | ||
self._fake_snapshot) | ||
@@ -228,7 +252,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
mock.call(*command3, run_as_root=True)] | ||
self._driver._execute.assert_has_calls(calls) | ||
|
||
- def _test_create_snapshot(self, volume_in_use=False): | ||
+ def _test_create_snapshot(self, volume_in_use=False, tmp_snap=False): | ||
fake_snapshot_info = {} | ||
fake_snapshot_file_name = os.path.basename(self._fake_snapshot_path) | ||
|
||
@@ -243,11 +267,16 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
return_value=self._fake_volume.name) | ||
self._driver._get_new_snap_path = mock.Mock( | ||
return_value=self._fake_snapshot_path) | ||
+ self._driver._validate_state = mock.Mock() | ||
|
||
expected_snapshot_info = { | ||
'active': fake_snapshot_file_name, | ||
self._fake_snapshot.id: fake_snapshot_file_name | ||
} | ||
+ exp_acceptable_states = ['available', 'in-use', 'backing-up'] | ||
+ if tmp_snap: | ||
+ exp_acceptable_states.append('downloading') | ||
+ self._fake_snapshot.id = 'tmp-snap-%s' % self._fake_snapshot.id | ||
|
||
if volume_in_use: | ||
self._fake_snapshot.volume.status = 'in-use' | ||
@@ -258,6 +287,9 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
|
||
self._driver._create_snapshot(self._fake_snapshot) | ||
|
||
+ self._driver._validate_state.assert_called_once_with( | ||
+ self._fake_snapshot.volume.status, | ||
+ exp_acceptable_states) | ||
fake_method = getattr(self._driver, expected_method_called) | ||
fake_method.assert_called_with( | ||
self._fake_snapshot, self._fake_volume.name, | ||
@@ -428,52 +460,59 @@ class RemoteFsSnapDriverTestCase(test.TestCase): | ||
basedir=basedir, | ||
valid_backing_file=False) | ||
|
||
- def test_create_cloned_volume(self): | ||
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, | ||
+ '_validate_state') | ||
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_create_snapshot') | ||
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, '_delete_snapshot') | ||
+ @mock.patch.object(remotefs.RemoteFSSnapDriver, | ||
+ '_copy_volume_from_snapshot') | ||
+ def test_create_cloned_volume(self, mock_copy_volume_from_snapshot, | ||
+ mock_delete_snapshot, | ||
+ mock_create_snapshot, | ||
+ mock_validate_state): | ||
drv = self._driver | ||
|
||
- with mock.patch.object(drv, '_create_snapshot') as \ | ||
- mock_create_snapshot,\ | ||
- mock.patch.object(drv, '_delete_snapshot') as \ | ||
- mock_delete_snapshot,\ | ||
- mock.patch.object(drv, '_copy_volume_from_snapshot') as \ | ||
- mock_copy_volume_from_snapshot: | ||
- | ||
- volume = fake_volume.fake_volume_obj(self.context) | ||
- src_vref_id = '375e32b2-804a-49f2-b282-85d1d5a5b9e1' | ||
- src_vref = fake_volume.fake_volume_obj( | ||
- self.context, | ||
- id=src_vref_id, | ||
- name='volume-%s' % src_vref_id) | ||
- | ||
- vol_attrs = ['provider_location', 'size', 'id', 'name', 'status', | ||
- 'volume_type', 'metadata'] | ||
- Volume = collections.namedtuple('Volume', vol_attrs) | ||
- | ||
- snap_attrs = ['volume_name', 'volume_size', 'name', | ||
- 'volume_id', 'id', 'volume'] | ||
- Snapshot = collections.namedtuple('Snapshot', snap_attrs) | ||
- | ||
- volume_ref = Volume(id=volume.id, | ||
- name=volume.name, | ||
- status=volume.status, | ||
- provider_location=volume.provider_location, | ||
- size=volume.size, | ||
- volume_type=volume.volume_type, | ||
- metadata=volume.metadata) | ||
- | ||
- snap_ref = Snapshot(volume_name=volume.name, | ||
- name='clone-snap-%s' % src_vref.id, | ||
- volume_size=src_vref.size, | ||
- volume_id=src_vref.id, | ||
- id='tmp-snap-%s' % src_vref.id, | ||
- volume=src_vref) | ||
- | ||
- drv.create_cloned_volume(volume, src_vref) | ||
- | ||
- mock_create_snapshot.assert_called_once_with(snap_ref) | ||
- mock_copy_volume_from_snapshot.assert_called_once_with( | ||
- snap_ref, volume_ref, volume['size']) | ||
- self.assertTrue(mock_delete_snapshot.called) | ||
+ volume = fake_volume.fake_volume_obj(self.context) | ||
+ src_vref_id = '375e32b2-804a-49f2-b282-85d1d5a5b9e1' | ||
+ src_vref = fake_volume.fake_volume_obj( | ||
+ self.context, | ||
+ id=src_vref_id, | ||
+ name='volume-%s' % src_vref_id) | ||
+ | ||
+ vol_attrs = ['provider_location', 'size', 'id', 'name', 'status', | ||
+ 'volume_type', 'metadata'] | ||
+ Volume = collections.namedtuple('Volume', vol_attrs) | ||
+ | ||
+ snap_attrs = ['volume_name', 'volume_size', 'name', | ||
+ 'volume_id', 'id', 'volume'] | ||
+ Snapshot = collections.namedtuple('Snapshot', snap_attrs) | ||
+ | ||
+ volume_ref = Volume(id=volume.id, | ||
+ name=volume.name, | ||
+ status=volume.status, | ||
+ provider_location=volume.provider_location, | ||
+ size=volume.size, | ||
+ volume_type=volume.volume_type, | ||
+ metadata=volume.metadata) | ||
+ | ||
+ snap_ref = Snapshot(volume_name=volume.name, | ||
+ name='clone-snap-%s' % src_vref.id, | ||
+ volume_size=src_vref.size, | ||
+ volume_id=src_vref.id, | ||
+ id='tmp-snap-%s' % src_vref.id, | ||
+ volume=src_vref) | ||
+ | ||
+ drv.create_cloned_volume(volume, src_vref) | ||
+ | ||
+ exp_acceptable_states = ['available', 'backing-up', 'downloading'] | ||
+ mock_validate_state.assert_called_once_with( | ||
+ src_vref.status, | ||
+ exp_acceptable_states, | ||
+ obj_description='source volume') | ||
+ mock_create_snapshot.assert_called_once_with(snap_ref) | ||
+ mock_copy_volume_from_snapshot.assert_called_once_with( | ||
+ snap_ref, volume_ref, volume['size']) | ||
+ self.assertTrue(mock_delete_snapshot.called) | ||
|
||
def test_create_regular_file(self): | ||
self._driver._create_regular_file('/path', 1) | ||
diff --git cinder/volume/drivers/remotefs.py cinder/volume/drivers/remotefs.py | ||
index 6f07cb9..e76c945 100644 | ||
--- cinder/volume/drivers/remotefs.py | ||
+++ cinder/volume/drivers/remotefs.py | ||
@@ -227,6 +227,23 @@ class RemoteFSDriver(driver.BaseVD): | ||
" mount_point_base.") | ||
return None | ||
|
||
+ @staticmethod | ||
+ def _validate_state(current_state, | ||
+ acceptable_states, | ||
+ obj_description='volume', | ||
+ invalid_exc=exception.InvalidVolume): | ||
+ if current_state not in acceptable_states: | ||
+ message = _('Invalid %(obj_description)s state. ' | ||
+ 'Acceptable states for this operation: ' | ||
+ '%(acceptable_states)s. ' | ||
+ 'Current %(obj_description)s state: ' | ||
+ '%(current_state)s.') | ||
+ raise invalid_exc( | ||
+ message=message % | ||
+ dict(obj_description=obj_description, | ||
+ acceptable_states=acceptable_states, | ||
+ current_state=current_state)) | ||
+ | ||
@utils.trace | ||
def create_volume(self, volume): | ||
"""Creates a volume. | ||
@@ -937,12 +954,10 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): | ||
{'src': src_vref.id, | ||
'dst': volume.id}) | ||
|
||
- if src_vref.status not in ['available', 'backing-up']: | ||
- msg = _("Source volume status must be 'available', or " | ||
- "'backing-up' but is: " | ||
- "%(status)s.") % {'status': src_vref.status} | ||
- raise exception.InvalidVolume(msg) | ||
- | ||
+ acceptable_states = ['available', 'backing-up', 'downloading'] | ||
+ self._validate_state(src_vref.status, | ||
+ acceptable_states, | ||
+ obj_description='source volume') | ||
volume_name = CONF.volume_name_template % volume.id | ||
|
||
# Create fake volume and snapshot objects | ||
@@ -1017,12 +1032,9 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): | ||
else 'offline')}) | ||
|
||
volume_status = snapshot.volume.status | ||
- if volume_status not in ['available', 'in-use', 'backing-up']: | ||
- msg = _("Volume status must be 'available', 'in-use' or " | ||
- "'backing-up' but is: " | ||
- "%(status)s.") % {'status': volume_status} | ||
- | ||
- raise exception.InvalidVolume(msg) | ||
+ acceptable_states = ['available', 'in-use', 'backing-up', 'deleting', | ||
+ 'downloading'] | ||
+ self._validate_state(volume_status, acceptable_states) | ||
|
||
vol_path = self._local_volume_dir(snapshot.volume) | ||
self._ensure_share_writable(vol_path) | ||
@@ -1327,12 +1339,15 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): | ||
else 'offline')}) | ||
|
||
status = snapshot.volume.status | ||
- if status not in ['available', 'in-use', 'backing-up']: | ||
- msg = _("Volume status must be 'available', 'in-use' or " | ||
- "'backing-up' but is: " | ||
- "%(status)s.") % {'status': status} | ||
|
||
- raise exception.InvalidVolume(msg) | ||
+ acceptable_states = ['available', 'in-use', 'backing-up'] | ||
+ if snapshot.id.startswith('tmp-snap-'): | ||
+ # This is an internal volume snapshot. In order to support | ||
+ # image caching, we'll allow creating/deleting such snapshots | ||
+ # while having volumes in 'downloading' state. | ||
+ acceptable_states.append('downloading') | ||
+ | ||
+ self._validate_state(status, acceptable_states) | ||
|
||
info_path = self._local_path_volume_info(snapshot.volume) | ||
snap_info = self._read_info_file(info_path, empty_if_missing=True) |