From f0e075dd1f732525ee09efb7723944d715e9cdf9 Mon Sep 17 00:00:00 2001 From: sds03 Date: Wed, 16 Sep 2015 18:00:45 +0900 Subject: [PATCH 1/5] Sheepdog: Implement abstract methods of manage_existing We add the following SheepdogDriver method. - manage_existing - manage_existing_get_size --- cinder/tests/unit/test_sheepdog.py | 247 +++++++++++++++++++++++++++++ cinder/volume/drivers/sheepdog.py | 92 ++++++++++- 2 files changed, 333 insertions(+), 6 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index f18ac4a5302..de5626cef6e 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -105,6 +105,10 @@ def cmd_dog_vdi_resize(self, name, size): return ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'vdi', 'resize', name, size, '-a', SHEEP_ADDR, '-p', SHEEP_PORT) + def cmd_dog_vdi_list(self, vdiname): + return ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'vdi', 'list', vdiname, + '-a', SHEEP_ADDR, '-p', SHEEP_PORT) + CMD_DOG_CLUSTER_INFO = ('env', 'LC_ALL=C', 'LANG=C', 'dog', 'cluster', 'info', '-a', SHEEP_ADDR, '-p', SHEEP_PORT) @@ -173,6 +177,11 @@ def cmd_dog_vdi_resize(self, name, size): 'QoS_support': False, } + TEST_EXISTING_REF = { + 'source-name': 'test-vdi', + 'source-id': '', + } + COLLIE_CLUSTER_INFO_0_5 = """\ Cluster status: running @@ -260,6 +269,27 @@ def cmd_dog_vdi_resize(self, name, size): DOG_NODE_INFO_ERROR_GET_NO_NODE_INFO = """\ Cannot get information from any nodes +""" + + DOG_VDI_LIST = """\ += test-vdi 0 10737418240 0 0 1442379755 7be7f9 3 +""" + + DOG_VDI_LIST_WITH_SNAPSHOT = """\ +s test-vdi 1 10737418240 0 0 1442379755 7be7f9 3 += test-vdi 0 16106127360 0 0 1442381817 7be7fb 3 +""" + + DOG_VDI_LIST_CLONE = """\ +c test-clone 0 10737418240 0 0 1442381753 ce8575 3 +""" + + DOG_VDI_LIST_SIZE_10G_MORE_1 = """\ += test-vdi 0 10737418241 0 0 1442379755 7be7f9 3 +""" + + DOG_VDI_LIST_SIZE_11G_LESS_1 = """\ += test-vdi 0 11811160063 0 0 1442379755 7be7f9 3 """ DOG_COMMAND_ERROR_VDI_NOT_EXISTS = """\ @@ -971,6 +1001,17 @@ def test_clone_success(self, fake_execute): self.client.clone(*args) fake_execute.assert_called_once_with(*expected_cmd) + @mock.patch.object(sheepdog.SheepdogClient, '_run_qemu_img') + def test_clone_success_without_size(self, fake_execute): + args = (self._src_vdiname, self._snapname, self._dst_vdiname) + src_volume = 'sheepdog:%(src_vdiname)s:%(snapname)s' % { + 'src_vdiname': self._src_vdiname, 'snapname': self._snapname} + dst_volume = 'sheepdog:%s' % self._dst_vdiname + expected_cmd = ('create', '-b', src_volume, dst_volume) + fake_execute.return_code = ("", "") + self.client.clone(*args) + fake_execute.assert_called_once_with(*expected_cmd) + @mock.patch.object(sheepdog.SheepdogClient, '_run_qemu_img') @mock.patch.object(sheepdog, 'LOG') def test_clone_fail_to_connect(self, fake_logger, fake_execute): @@ -1603,6 +1644,113 @@ def test_get_disk_capacity_parse_stdout_error(self, fake_logger, self.assertRaises(AttributeError, self.client.get_disk_capacity) self.assertTrue(fake_logger.exception.called) + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_vdi_size_success(self, fake_execute): + expected_cmd = ('vdi', 'list', '-r', self._vdiname) + fake_execute.return_value = (self.test_data.DOG_VDI_LIST, '') + + actual = self.client.get_vdi_size(self._vdiname) + fake_execute.assert_called_once_with(*expected_cmd) + self.assertEqual(10, actual) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_vdi_size_with_snapshot_success(self, fake_execute): + expected_cmd = ('vdi', 'list', '-r', self._vdiname) + fake_execute.return_value = (self.test_data.DOG_VDI_LIST_WITH_SNAPSHOT, + '') + actual = self.client.get_vdi_size(self._vdiname) + fake_execute.assert_called_once_with(*expected_cmd) + self.assertEqual(15, actual) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_vdi_size_clone_success(self, fake_execute): + expected_cmd = ('vdi', 'list', '-r', self._vdiname) + fake_execute.return_value = (self.test_data.DOG_VDI_LIST_CLONE, '') + + actual = self.client.get_vdi_size(self._vdiname) + fake_execute.assert_called_once_with(*expected_cmd) + self.assertEqual(10, actual) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_vdi_size_round_up_1_success(self, fake_execute): + expected_cmd = ('vdi', 'list', '-r', self._vdiname) + fake_execute.return_value = ( + self.test_data.DOG_VDI_LIST_SIZE_10G_MORE_1, '') + + actual = self.client.get_vdi_size(self._vdiname) + fake_execute.assert_called_once_with(*expected_cmd) + self.assertEqual(11, actual) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_vdi_size_round_up_2_success(self, fake_execute): + expected_cmd = ('vdi', 'list', '-r', self._vdiname) + fake_execute.return_value = ( + self.test_data.DOG_VDI_LIST_SIZE_11G_LESS_1, '') + + actual = self.client.get_vdi_size(self._vdiname) + fake_execute.assert_called_once_with(*expected_cmd) + self.assertEqual(11, actual) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + def test_get_vdi_size_not_found_error(self, fake_execute): + stdout = '' + stderr = '' + fake_execute.return_value = (stdout, stderr) + + self.assertRaises(exception.NotFound, self.client.get_vdi_size, + self._vdiname) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + @mock.patch.object(sheepdog, 'LOG') + def test_get_vdi_size_stdout_parse_error(self, fake_logger, fake_execute): + stdout = 'dummy' + stderr = '' + fake_execute.return_value = (stdout, stderr) + + self.assertRaises(IndexError, self.client.get_vdi_size, self._vdiname) + self.assertTrue(fake_logger.exception.called) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + @mock.patch.object(sheepdog, 'LOG') + def test_get_vdi_size_failed_to_connect(self, fake_logger, fake_execute): + cmd = self.test_data.cmd_dog_vdi_list(self._vdiname) + exit_code = 2 + stdout = 'stdout_dummy' + stderr = self.test_data.DOG_COMMAND_ERROR_FAIL_TO_CONNECT + expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, + exit_code=exit_code, + stdout=stdout, + stderr=stderr) + fake_execute.side_effect = exception.SheepdogCmdError( + cmd=cmd, exit_code=exit_code, stdout=stdout.replace('\n', '\\n'), + stderr=stderr.replace('\n', '\\n')) + + ex = self.assertRaises(exception.SheepdogCmdError, + self.client.get_vdi_size, self._vdiname) + self.assertTrue(fake_logger.exception.called) + self.assertEqual(expected_msg, ex.msg) + + @mock.patch.object(sheepdog.SheepdogClient, '_run_dog') + @mock.patch.object(sheepdog, 'LOG') + def test_get_vdi_size_unknown_error(self, fake_logger, fake_execute): + cmd = self.test_data.cmd_dog_vdi_list(self._vdiname) + exit_code = 2 + stdout = 'stdout_dummy' + stderr = 'unknown' + expected_msg = self.test_data.sheepdog_cmd_error(cmd=cmd, + exit_code=exit_code, + stdout=stdout, + stderr=stderr) + fake_execute.side_effect = exception.SheepdogCmdError( + cmd=cmd, exit_code=exit_code, + stdout=stdout.replace('\n', '\\n'), + stderr=stderr.replace('\n', '\\n')) + + ex = self.assertRaises(exception.SheepdogCmdError, + self.client.get_vdi_size, self._vdiname) + self.assertTrue(fake_logger.exception.called) + self.assertEqual(expected_msg, ex.msg) + class SheepdogDriverTestCase(test.TestCase): def setUp(self): @@ -2014,3 +2162,102 @@ def test_restore_backup(self, fake_backup_service): self.assertEqual(fake_backup, call_backup) self.assertEqual(fake_volume.id, call_volume_id) self.assertIsInstance(call_sheepdog_fd, sheepdog.SheepdogIOWrapper) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') + @mock.patch.object(sheepdog.SheepdogClient, 'clone') + @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') + def test_manage_existing_success(self, fake_create_snapshot, fake_clone, + fake_delete_snapshot, fake_delete): + vol = self.test_data.TEST_VOLUME + ref = self.test_data.TEST_EXISTING_REF + source_name = ref['source-name'] + snapshot_name = 'temp-snapshot-' + source_name + + self.driver.manage_existing(vol, ref) + fake_create_snapshot.assert_called_once_with(source_name, + snapshot_name) + fake_clone.assert_called_once_with(source_name, snapshot_name, + vol.name) + fake_delete_snapshot.assert_called_once_with(source_name, + snapshot_name) + fake_delete.assert_called_once_with(source_name) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') + @mock.patch.object(sheepdog.SheepdogClient, 'clone') + @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') + @mock.patch.object(sheepdog, 'LOG') + def test_manage_existing_failed_create_snapshot(self, fake_logger, + fake_create_snapshot, + fake_clone, + fake_delete_snapshot, + fake_delete): + vol = self.test_data.TEST_VOLUME + ref = self.test_data.TEST_EXISTING_REF + fake_create_snapshot.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.driver.manage_existing, vol, ref) + self.assertTrue(fake_logger.error.called) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') + @mock.patch.object(sheepdog.SheepdogClient, 'clone') + @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') + @mock.patch.object(sheepdog, 'LOG') + def test_manage_existing_failed_clone(self, fake_logger, + fake_create_snapshot, + fake_clone, fake_delete_snapshot, + fake_delete): + vol = self.test_data.TEST_VOLUME + ref = self.test_data.TEST_EXISTING_REF + source_name = ref['source-name'] + snapshot_name = 'temp-snapshot-' + source_name + fake_clone.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.driver.manage_existing, vol, ref) + fake_delete_snapshot.assert_called_once_with(source_name, + snapshot_name) + self.assertTrue(fake_logger.error.called) + + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + def test_manage_existing_get_size_success(self, fake_get_vdi_size): + fake_get_vdi_size.return_value = 10 + ref = self.test_data.TEST_EXISTING_REF + + size = self.driver.manage_existing_get_size(self.test_data.TEST_VOLUME, + ref) + fake_get_vdi_size.assert_called_once_with(ref['source-name']) + self.assertEqual(10, size) + + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + def test_manage_existing_get_size_sourcename_key_empty( + self, fake_get_vdi_size): + ref = {'source-id': ''} + + self.assertRaises(exception.ManageExistingInvalidReference, + self.driver.manage_existing_get_size, + self.test_data.TEST_VOLUME, ref) + + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + def test_manage_existing_get_size_sourcename_value_empty( + self, fake_get_vdi_size): + ref = {'source-name': '', 'source-id': ''} + + self.assertRaises(exception.ManageExistingInvalidReference, + self.driver.manage_existing_get_size, + self.test_data.TEST_VOLUME, ref) + + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + def test_manage_existing_get_size_vdi_not_found( + self, fake_get_vdi_size): + fake_get_vdi_size.side_effect = exception.NotFound() + + self.assertRaises(exception.ManageExistingInvalidReference, + self.driver.manage_existing_get_size, + self.test_data.TEST_VOLUME, + self.test_data.TEST_EXISTING_REF) diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 5e959ebe2ea..2ee246d0c24 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -22,6 +22,7 @@ import errno import eventlet import io +import math import re from oslo_concurrency import processutils @@ -267,13 +268,15 @@ def delete_snapshot(self, vdiname, snapname): LOG.error(_LE('Failed to delete snapshot. (command: %s)'), cmd) - def clone(self, src_vdiname, src_snapname, dst_vdiname, size): + def clone(self, src_vdiname, src_snapname, dst_vdiname, size=None): + params = ['-b', 'sheepdog:%(src_vdiname)s:%(src_snapname)s' % + {'src_vdiname': src_vdiname, + 'src_snapname': src_snapname}, + 'sheepdog:%s' % dst_vdiname] + if size is not None: + params.append('%sG' % size) try: - self._run_qemu_img('create', '-b', - 'sheepdog:%(src_vdiname)s:%(src_snapname)s' % - {'src_vdiname': src_vdiname, - 'src_snapname': src_snapname}, - 'sheepdog:%s' % dst_vdiname, '%sG' % size) + self._run_qemu_img('create', *params) except exception.SheepdogCmdError as e: cmd = e.kwargs['cmd'] _stderr = e.kwargs['stderr'] @@ -470,6 +473,39 @@ def get_disk_capacity(self): return (total_gb, used_gb, free_gb) + def get_vdi_size(self, vdiname): + """Get size(GB) of volume.""" + try: + (_stdout, _stderr) = self._run_dog('vdi', 'list', '-r', vdiname) + except exception.SheepdogCmdError as e: + _stderr = e.kwargs['stderr'] + with excutils.save_and_reraise_exception(): + if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): + LOG.exception(_LE('Failed to connect sheep daemon. ' + 'addr: %(addr)s, port: %(port)s'), + {'addr': self.addr, 'port': self.port}) + else: + LOG.exception(_LE('Failed to get vdi size. ' + 'vdi: %(vdiname)s, addr: %(addr)s, ' + 'port: %(port)s'), + {'vdiname': vdiname, 'addr': self.addr, + 'port': self.port}) + + # Delete snapshots of record from list + r = re.compile('^s .*', re.MULTILINE) + _stdout = re.sub(r, '', _stdout).strip() + if not _stdout: + raise exception.NotFound(_('Not found vdi. vdi: %s') % vdiname) + + try: + size = float(_stdout.split(' ')[3]) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.exception(_LE('Failed to parse stdout of ' + '"dog vdi list -r". stdout: %s'), _stdout) + + return int(math.ceil(size / units.Gi)) + class SheepdogIOWrapper(io.RawIOBase): """File-like object with Sheepdog backend.""" @@ -789,3 +825,47 @@ def restore_backup(self, context, backup, volume, backup_service): """Restore an existing backup to a new or existing volume.""" sheepdog_fd = SheepdogIOWrapper(volume) backup_service.restore(backup, volume.id, sheepdog_fd) + + def manage_existing(self, volume, existing_ref): + """Manages an existing volume. + + Renames the vdi name to match the expected name for the volume. + Error checking done by manage_existing_get_size is not repeated. + """ + source_name = existing_ref['source-name'] + + snapshot_name = 'temp-snapshot-' + source_name + try: + self.client.create_snapshot(source_name, snapshot_name) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Failed to create temporary snapshot for ' + 'volume "%s".'), source_name) + try: + self.client.clone(source_name, snapshot_name, volume.name) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Failed to clone volume %s.'), volume.name) + finally: + self.client.delete_snapshot(source_name, snapshot_name) + + self.client.delete(source_name) + + def manage_existing_get_size(self, volume, existing_ref): + """Return size of an existing volume for manage_existing.""" + if 'source-name' in existing_ref and existing_ref['source-name']: + source_name = existing_ref['source-name'] + else: + reason = _("Reference must contain source-name.") + raise exception.ManageExistingInvalidReference( + existing_ref=existing_ref, + reason=reason) + + try: + size_gb = self.client.get_vdi_size(source_name) + except exception.NotFound: + reason = _("Specified volume of source-name does not exist.") + raise exception.ManageExistingInvalidReference( + existing_ref=existing_ref, reason=reason) + + return size_gb From 92b53645d11d342ca30b56bc524f97583a1306f4 Mon Sep 17 00:00:00 2001 From: sds03 Date: Thu, 17 Sep 2015 15:58:58 +0900 Subject: [PATCH 2/5] Fixed comments --- cinder/tests/unit/test_sheepdog.py | 144 ++++++++++++++++------------- cinder/volume/drivers/sheepdog.py | 51 +++++----- 2 files changed, 108 insertions(+), 87 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index de5626cef6e..e03c6afd8ad 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -1751,6 +1751,62 @@ def test_get_vdi_size_unknown_error(self, fake_logger, fake_execute): self.assertTrue(fake_logger.exception.called) self.assertEqual(expected_msg, ex.msg) + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') + @mock.patch.object(sheepdog.SheepdogClient, 'clone') + @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') + def test_rename_success(self, fake_create_snapshot, fake_clone, + fake_delete_snapshot, fake_delete): + old_vdi_name = self._vdiname + new_vdi_name = self.test_data.TEST_EXISTING_REF['source-name'] + snapshot_name = 'temp-rename-snapshot-' + old_vdi_name + + self.client.rename(old_vdi_name, new_vdi_name) + fake_create_snapshot.assert_called_once_with(old_vdi_name, + snapshot_name) + fake_clone.assert_called_once_with(old_vdi_name, snapshot_name, + new_vdi_name) + fake_delete_snapshot.assert_called_once_with(old_vdi_name, + snapshot_name) + fake_delete.assert_called_once_with(old_vdi_name) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') + @mock.patch.object(sheepdog.SheepdogClient, 'clone') + @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') + @mock.patch.object(sheepdog, 'LOG') + def test_rename_failed_create_snapshot(self, fake_logger, + fake_create_snapshot, fake_clone, + fake_delete_snapshot, fake_delete): + old_vdi_name = self._vdiname + new_vdi_name = self.test_data.TEST_EXISTING_REF['source-name'] + fake_create_snapshot.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.client.rename, old_vdi_name, new_vdi_name) + self.assertTrue(fake_logger.error.called) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') + @mock.patch.object(sheepdog.SheepdogClient, 'clone') + @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') + @mock.patch.object(sheepdog, 'LOG') + def test_rename_failed_clone(self, fake_logger, fake_create_snapshot, + fake_clone, fake_delete_snapshot, + fake_delete): + old_vdi_name = self._vdiname + new_vdi_name = self.test_data.TEST_EXISTING_REF['source-name'] + snapshot_name = 'temp-rename-snapshot-' + old_vdi_name + fake_clone.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.client.rename, old_vdi_name, new_vdi_name) + fake_delete_snapshot.assert_called_once_with(old_vdi_name, + snapshot_name) + self.assertTrue(fake_logger.error.called) + class SheepdogDriverTestCase(test.TestCase): def setUp(self): @@ -2163,94 +2219,42 @@ def test_restore_backup(self, fake_backup_service): self.assertEqual(fake_volume.id, call_volume_id) self.assertIsInstance(call_sheepdog_fd, sheepdog.SheepdogIOWrapper) - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') - @mock.patch.object(sheepdog.SheepdogClient, 'clone') - @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') - def test_manage_existing_success(self, fake_create_snapshot, fake_clone, - fake_delete_snapshot, fake_delete): - vol = self.test_data.TEST_VOLUME - ref = self.test_data.TEST_EXISTING_REF - source_name = ref['source-name'] - snapshot_name = 'temp-snapshot-' + source_name - - self.driver.manage_existing(vol, ref) - fake_create_snapshot.assert_called_once_with(source_name, - snapshot_name) - fake_clone.assert_called_once_with(source_name, snapshot_name, - vol.name) - fake_delete_snapshot.assert_called_once_with(source_name, - snapshot_name) - fake_delete.assert_called_once_with(source_name) - - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') - @mock.patch.object(sheepdog.SheepdogClient, 'clone') - @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') - @mock.patch.object(sheepdog, 'LOG') - def test_manage_existing_failed_create_snapshot(self, fake_logger, - fake_create_snapshot, - fake_clone, - fake_delete_snapshot, - fake_delete): - vol = self.test_data.TEST_VOLUME - ref = self.test_data.TEST_EXISTING_REF - fake_create_snapshot.side_effect = exception.SheepdogCmdError( - cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') - - self.assertRaises(exception.SheepdogCmdError, - self.driver.manage_existing, vol, ref) - self.assertTrue(fake_logger.error.called) - - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') - @mock.patch.object(sheepdog.SheepdogClient, 'clone') - @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') - @mock.patch.object(sheepdog, 'LOG') - def test_manage_existing_failed_clone(self, fake_logger, - fake_create_snapshot, - fake_clone, fake_delete_snapshot, - fake_delete): - vol = self.test_data.TEST_VOLUME - ref = self.test_data.TEST_EXISTING_REF - source_name = ref['source-name'] - snapshot_name = 'temp-snapshot-' + source_name - fake_clone.side_effect = exception.SheepdogCmdError( - cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') + @mock.patch.object(sheepdog.SheepdogClient, 'rename') + def test_manage_existing_success(self, fake_rename): + fake_volume = self.test_data.TEST_VOLUME + fake_ref = self.test_data.TEST_EXISTING_REF - self.assertRaises(exception.SheepdogCmdError, - self.driver.manage_existing, vol, ref) - fake_delete_snapshot.assert_called_once_with(source_name, - snapshot_name) - self.assertTrue(fake_logger.error.called) + self.driver.manage_existing(fake_volume, fake_ref) + fake_rename.assert_called_once_with(fake_ref['source-name'], + fake_volume.name) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_success(self, fake_get_vdi_size): fake_get_vdi_size.return_value = 10 - ref = self.test_data.TEST_EXISTING_REF + fake_ref = self.test_data.TEST_EXISTING_REF size = self.driver.manage_existing_get_size(self.test_data.TEST_VOLUME, - ref) - fake_get_vdi_size.assert_called_once_with(ref['source-name']) + fake_ref) + fake_get_vdi_size.assert_called_once_with(fake_ref['source-name']) self.assertEqual(10, size) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_sourcename_key_empty( self, fake_get_vdi_size): - ref = {'source-id': ''} + fake_ref = {'source-id': ''} self.assertRaises(exception.ManageExistingInvalidReference, self.driver.manage_existing_get_size, - self.test_data.TEST_VOLUME, ref) + self.test_data.TEST_VOLUME, fake_ref) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_sourcename_value_empty( self, fake_get_vdi_size): - ref = {'source-name': '', 'source-id': ''} + fake_ref = {'source-name': '', 'source-id': ''} self.assertRaises(exception.ManageExistingInvalidReference, self.driver.manage_existing_get_size, - self.test_data.TEST_VOLUME, ref) + self.test_data.TEST_VOLUME, fake_ref) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_vdi_not_found( @@ -2261,3 +2265,11 @@ def test_manage_existing_get_size_vdi_not_found( self.driver.manage_existing_get_size, self.test_data.TEST_VOLUME, self.test_data.TEST_EXISTING_REF) + + @mock.patch.object(sheepdog.SheepdogClient, 'rename') + def test_unmanage_success(self, fake_rename): + fake_volume = self.test_data.TEST_VOLUME + new_volume_name = fake_volume.name + '-unmanaged' + + self.driver.unmanage(fake_volume) + fake_rename.assert_called_once_with(fake_volume.name, new_volume_name) diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 2ee246d0c24..2bc4304608e 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -474,14 +474,14 @@ def get_disk_capacity(self): return (total_gb, used_gb, free_gb) def get_vdi_size(self, vdiname): - """Get size(GB) of volume.""" + """Get size(GB) of the volume.""" try: (_stdout, _stderr) = self._run_dog('vdi', 'list', '-r', vdiname) except exception.SheepdogCmdError as e: _stderr = e.kwargs['stderr'] with excutils.save_and_reraise_exception(): if _stderr.startswith(self.DOG_RESP_CONNECTION_ERROR): - LOG.exception(_LE('Failed to connect sheep daemon. ' + LOG.exception(_LE('Failed to connect to sheep daemon. ' 'addr: %(addr)s, port: %(port)s'), {'addr': self.addr, 'port': self.port}) else: @@ -491,7 +491,7 @@ def get_vdi_size(self, vdiname): {'vdiname': vdiname, 'addr': self.addr, 'port': self.port}) - # Delete snapshots of record from list + # Omit the snapshot entries from the list r = re.compile('^s .*', re.MULTILINE) _stdout = re.sub(r, '', _stdout).strip() if not _stdout: @@ -506,6 +506,28 @@ def get_vdi_size(self, vdiname): return int(math.ceil(size / units.Gi)) + def rename(self, old_vdi_name, new_vdi_name): + """Rename vdi name.""" + snapshot_name = 'temp-rename-snapshot-' + old_vdi_name + try: + self.create_snapshot(old_vdi_name, snapshot_name) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Failed to create temporary snapshot for ' + 'rename "%s".'), old_vdi_name) + try: + self.clone(old_vdi_name, snapshot_name, new_vdi_name) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Failed to clone for rename from ' + '"%(old_vdi_name)s" to "%(new_vdi_name)s".'), + {'old_vdi_name': old_vdi_name, + 'new_vdi_name': new_vdi_name}) + finally: + self.delete_snapshot(old_vdi_name, snapshot_name) + + self.delete(old_vdi_name) + class SheepdogIOWrapper(io.RawIOBase): """File-like object with Sheepdog backend.""" @@ -832,24 +854,7 @@ def manage_existing(self, volume, existing_ref): Renames the vdi name to match the expected name for the volume. Error checking done by manage_existing_get_size is not repeated. """ - source_name = existing_ref['source-name'] - - snapshot_name = 'temp-snapshot-' + source_name - try: - self.client.create_snapshot(source_name, snapshot_name) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Failed to create temporary snapshot for ' - 'volume "%s".'), source_name) - try: - self.client.clone(source_name, snapshot_name, volume.name) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Failed to clone volume %s.'), volume.name) - finally: - self.client.delete_snapshot(source_name, snapshot_name) - - self.client.delete(source_name) + self.client.rename(existing_ref['source-name'], volume.name) def manage_existing_get_size(self, volume, existing_ref): """Return size of an existing volume for manage_existing.""" @@ -869,3 +874,7 @@ def manage_existing_get_size(self, volume, existing_ref): existing_ref=existing_ref, reason=reason) return size_gb + + def unmanage(self, volume): + """Removes the specified volume from Cinder management.""" + self.client.rename(volume.name, volume.name + '-unmanaged') From cba6cae162741aaef82e8b3586bf9cd738f09c21 Mon Sep 17 00:00:00 2001 From: sds03 Date: Thu, 17 Sep 2015 18:22:46 +0900 Subject: [PATCH 3/5] Fixed a bug in rename the already managed volume --- cinder/tests/unit/test_sheepdog.py | 19 ++++++++++++++++++- cinder/volume/drivers/sheepdog.py | 9 ++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index e03c6afd8ad..202941440e5 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -36,6 +36,7 @@ from cinder import utils from cinder.volume import configuration as conf from cinder.volume.drivers import sheepdog +from cinder.volume import utils as volutils SHEEP_ADDR = '127.0.0.1' SHEEP_PORT = 7000 @@ -2220,14 +2221,30 @@ def test_restore_backup(self, fake_backup_service): self.assertIsInstance(call_sheepdog_fd, sheepdog.SheepdogIOWrapper) @mock.patch.object(sheepdog.SheepdogClient, 'rename') - def test_manage_existing_success(self, fake_rename): + @mock.patch.object(volutils, 'check_already_managed_volume') + def test_manage_existing_success(self, fake_check_already_managed_volume, + fake_rename): fake_volume = self.test_data.TEST_VOLUME fake_ref = self.test_data.TEST_EXISTING_REF + fake_check_already_managed_volume.return_value = False self.driver.manage_existing(fake_volume, fake_ref) + fake_check_already_managed_volume.assert_called_once_with( + self.db, fake_ref['source-name']) fake_rename.assert_called_once_with(fake_ref['source-name'], fake_volume.name) + @mock.patch.object(sheepdog.SheepdogClient, 'rename') + @mock.patch.object(volutils, 'check_already_managed_volume') + def test_manage_existing_fail_already_managed_volume( + self, fake_check_already_managed_volume, fake_rename): + fake_volume = self.test_data.TEST_VOLUME + fake_ref = self.test_data.TEST_EXISTING_REF + fake_check_already_managed_volume.return_value = True + self.assertRaises(exception.ManageExistingAlreadyManaged, + self.driver.manage_existing, + fake_volume, fake_ref) + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_success(self, fake_get_vdi_size): fake_get_vdi_size.return_value = 10 diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 2bc4304608e..92ad9c14c4a 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -37,6 +37,7 @@ from cinder.image import image_utils from cinder import utils from cinder.volume import driver +from cinder.volume import utils as volutils # snapshot name of glance image GLANCE_SNAPNAME = 'glance-image' @@ -854,7 +855,13 @@ def manage_existing(self, volume, existing_ref): Renames the vdi name to match the expected name for the volume. Error checking done by manage_existing_get_size is not repeated. """ - self.client.rename(existing_ref['source-name'], volume.name) + source_name = existing_ref['source-name'] + + if volutils.check_already_managed_volume(self.db, source_name): + raise exception.ManageExistingAlreadyManaged( + volume_ref=source_name) + + self.client.rename(source_name, volume.name) def manage_existing_get_size(self, volume, existing_ref): """Return size of an existing volume for manage_existing.""" From e0621aae739ee37f24f03c05cb0a46d087d55475 Mon Sep 17 00:00:00 2001 From: sds03 Date: Fri, 18 Sep 2015 16:05:03 +0900 Subject: [PATCH 4/5] Changes to resize at manage_existing --- cinder/tests/unit/test_sheepdog.py | 225 +++++++++++++++++------------ cinder/volume/drivers/sheepdog.py | 69 +++++---- 2 files changed, 172 insertions(+), 122 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index 202941440e5..840b8e71dd1 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -1002,17 +1002,6 @@ def test_clone_success(self, fake_execute): self.client.clone(*args) fake_execute.assert_called_once_with(*expected_cmd) - @mock.patch.object(sheepdog.SheepdogClient, '_run_qemu_img') - def test_clone_success_without_size(self, fake_execute): - args = (self._src_vdiname, self._snapname, self._dst_vdiname) - src_volume = 'sheepdog:%(src_vdiname)s:%(snapname)s' % { - 'src_vdiname': self._src_vdiname, 'snapname': self._snapname} - dst_volume = 'sheepdog:%s' % self._dst_vdiname - expected_cmd = ('create', '-b', src_volume, dst_volume) - fake_execute.return_code = ("", "") - self.client.clone(*args) - fake_execute.assert_called_once_with(*expected_cmd) - @mock.patch.object(sheepdog.SheepdogClient, '_run_qemu_img') @mock.patch.object(sheepdog, 'LOG') def test_clone_fail_to_connect(self, fake_logger, fake_execute): @@ -1752,62 +1741,6 @@ def test_get_vdi_size_unknown_error(self, fake_logger, fake_execute): self.assertTrue(fake_logger.exception.called) self.assertEqual(expected_msg, ex.msg) - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') - @mock.patch.object(sheepdog.SheepdogClient, 'clone') - @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') - def test_rename_success(self, fake_create_snapshot, fake_clone, - fake_delete_snapshot, fake_delete): - old_vdi_name = self._vdiname - new_vdi_name = self.test_data.TEST_EXISTING_REF['source-name'] - snapshot_name = 'temp-rename-snapshot-' + old_vdi_name - - self.client.rename(old_vdi_name, new_vdi_name) - fake_create_snapshot.assert_called_once_with(old_vdi_name, - snapshot_name) - fake_clone.assert_called_once_with(old_vdi_name, snapshot_name, - new_vdi_name) - fake_delete_snapshot.assert_called_once_with(old_vdi_name, - snapshot_name) - fake_delete.assert_called_once_with(old_vdi_name) - - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') - @mock.patch.object(sheepdog.SheepdogClient, 'clone') - @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') - @mock.patch.object(sheepdog, 'LOG') - def test_rename_failed_create_snapshot(self, fake_logger, - fake_create_snapshot, fake_clone, - fake_delete_snapshot, fake_delete): - old_vdi_name = self._vdiname - new_vdi_name = self.test_data.TEST_EXISTING_REF['source-name'] - fake_create_snapshot.side_effect = exception.SheepdogCmdError( - cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') - - self.assertRaises(exception.SheepdogCmdError, - self.client.rename, old_vdi_name, new_vdi_name) - self.assertTrue(fake_logger.error.called) - - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogClient, 'delete_snapshot') - @mock.patch.object(sheepdog.SheepdogClient, 'clone') - @mock.patch.object(sheepdog.SheepdogClient, 'create_snapshot') - @mock.patch.object(sheepdog, 'LOG') - def test_rename_failed_clone(self, fake_logger, fake_create_snapshot, - fake_clone, fake_delete_snapshot, - fake_delete): - old_vdi_name = self._vdiname - new_vdi_name = self.test_data.TEST_EXISTING_REF['source-name'] - snapshot_name = 'temp-rename-snapshot-' + old_vdi_name - fake_clone.side_effect = exception.SheepdogCmdError( - cmd='dummy', exit_code=1, stdout='out dummy', stderr='err dummy') - - self.assertRaises(exception.SheepdogCmdError, - self.client.rename, old_vdi_name, new_vdi_name) - fake_delete_snapshot.assert_called_once_with(old_vdi_name, - snapshot_name) - self.assertTrue(fake_logger.error.called) - class SheepdogDriverTestCase(test.TestCase): def setUp(self): @@ -2220,58 +2153,138 @@ def test_restore_backup(self, fake_backup_service): self.assertEqual(fake_volume.id, call_volume_id) self.assertIsInstance(call_sheepdog_fd, sheepdog.SheepdogIOWrapper) - @mock.patch.object(sheepdog.SheepdogClient, 'rename') + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') @mock.patch.object(volutils, 'check_already_managed_volume') def test_manage_existing_success(self, fake_check_already_managed_volume, - fake_rename): + fake_get_vdi_size, + fake_create_cloned_volume, fake_delete): fake_volume = self.test_data.TEST_VOLUME - fake_ref = self.test_data.TEST_EXISTING_REF + fake_existing_ref = self.test_data.TEST_EXISTING_REF + source_name = fake_existing_ref['source-name'] + fake_vref = {'name': source_name, 'size': fake_volume.size} fake_check_already_managed_volume.return_value = False + fake_get_vdi_size.return_value = fake_volume.size - self.driver.manage_existing(fake_volume, fake_ref) - fake_check_already_managed_volume.assert_called_once_with( - self.db, fake_ref['source-name']) - fake_rename.assert_called_once_with(fake_ref['source-name'], - fake_volume.name) + self.driver.manage_existing(fake_volume, fake_existing_ref) + fake_check_already_managed_volume.assert_called_once_with(self.db, + source_name) + fake_get_vdi_size.assert_called_once_with(source_name) + fake_create_cloned_volume.assert_called_once_with(fake_volume, + fake_vref) + fake_delete.assert_called_once_with(source_name) - @mock.patch.object(sheepdog.SheepdogClient, 'rename') + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') @mock.patch.object(volutils, 'check_already_managed_volume') def test_manage_existing_fail_already_managed_volume( - self, fake_check_already_managed_volume, fake_rename): + self, fake_check_already_managed_volume, fake_get_vdi_size, + fake_create_cloned_volume, fake_delete): fake_volume = self.test_data.TEST_VOLUME - fake_ref = self.test_data.TEST_EXISTING_REF + fake_existing_ref = self.test_data.TEST_EXISTING_REF fake_check_already_managed_volume.return_value = True + self.assertRaises(exception.ManageExistingAlreadyManaged, self.driver.manage_existing, - fake_volume, fake_ref) + fake_volume, fake_existing_ref) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + @mock.patch.object(volutils, 'check_already_managed_volume') + @mock.patch.object(sheepdog, 'LOG') + def test_manage_existing_fail_get_vdi_size( + self, fake_logger, fake_check_already_managed_volume, + fake_get_vdi_size, fake_create_cloned_volume, fake_delete): + fake_volume = self.test_data.TEST_VOLUME + fake_existing_ref = self.test_data.TEST_EXISTING_REF + fake_check_already_managed_volume.return_value = False + fake_get_vdi_size.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='dummy', stderr='dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.driver.manage_existing, fake_volume, + fake_existing_ref) + self.assertTrue(fake_logger.error.called) + self.assertTrue(fake_check_already_managed_volume.called) + self.assertTrue(fake_get_vdi_size.called) + self.assertFalse(fake_create_cloned_volume.called) + self.assertFalse(fake_delete.called) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + @mock.patch.object(volutils, 'check_already_managed_volume') + @mock.patch.object(sheepdog, 'LOG') + def test_manage_existing_fail_create_cloned_volume( + self, fake_logger, fake_check_already_managed_volume, + fake_get_vdi_size, fake_create_cloned_volume, fake_delete): + fake_volume = self.test_data.TEST_VOLUME + fake_existing_ref = self.test_data.TEST_EXISTING_REF + fake_check_already_managed_volume.return_value = False + fake_create_cloned_volume.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='dummy', stderr='dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.driver.manage_existing, fake_volume, + fake_existing_ref) + self.assertTrue(fake_logger.error.called) + self.assertTrue(fake_check_already_managed_volume.called) + self.assertTrue(fake_get_vdi_size.called) + self.assertTrue(fake_create_cloned_volume.called) + self.assertFalse(fake_delete.called) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') + @mock.patch.object(volutils, 'check_already_managed_volume') + @mock.patch.object(sheepdog, 'LOG') + def test_manage_existing_fail_delete( + self, fake_logger, fake_check_already_managed_volume, + fake_get_vdi_size, fake_create_cloned_volume, fake_delete): + fake_volume = self.test_data.TEST_VOLUME + fake_existing_ref = self.test_data.TEST_EXISTING_REF + fake_check_already_managed_volume.return_value = False + fake_delete.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='dummy', stderr='dummy') + + self.driver.manage_existing(fake_volume, fake_existing_ref) + self.assertTrue(fake_logger.warning.called) + self.assertTrue(fake_check_already_managed_volume.called) + self.assertTrue(fake_get_vdi_size.called) + self.assertTrue(fake_create_cloned_volume.called) + self.assertTrue(fake_delete.called) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_success(self, fake_get_vdi_size): fake_get_vdi_size.return_value = 10 - fake_ref = self.test_data.TEST_EXISTING_REF + fake_existing_ref = self.test_data.TEST_EXISTING_REF size = self.driver.manage_existing_get_size(self.test_data.TEST_VOLUME, - fake_ref) - fake_get_vdi_size.assert_called_once_with(fake_ref['source-name']) + fake_existing_ref) + fake_get_vdi_size.assert_called_once_with( + fake_existing_ref['source-name']) self.assertEqual(10, size) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_sourcename_key_empty( self, fake_get_vdi_size): - fake_ref = {'source-id': ''} + fake_existing_ref = {'source-id': ''} self.assertRaises(exception.ManageExistingInvalidReference, self.driver.manage_existing_get_size, - self.test_data.TEST_VOLUME, fake_ref) + self.test_data.TEST_VOLUME, fake_existing_ref) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_sourcename_value_empty( self, fake_get_vdi_size): - fake_ref = {'source-name': '', 'source-id': ''} + fake_existing_ref = {'source-name': '', 'source-id': ''} self.assertRaises(exception.ManageExistingInvalidReference, self.driver.manage_existing_get_size, - self.test_data.TEST_VOLUME, fake_ref) + self.test_data.TEST_VOLUME, fake_existing_ref) @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') def test_manage_existing_get_size_vdi_not_found( @@ -2283,10 +2296,42 @@ def test_manage_existing_get_size_vdi_not_found( self.test_data.TEST_VOLUME, self.test_data.TEST_EXISTING_REF) - @mock.patch.object(sheepdog.SheepdogClient, 'rename') - def test_unmanage_success(self, fake_rename): + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + def test_unmanage_success(self, fake_create_cloned_volume, fake_delete): + fake_volume = self.test_data.TEST_VOLUME + vref = {'name': fake_volume.name, 'size': fake_volume.size} + self.driver.unmanage(fake_volume) + fake_create_cloned_volume.assert_called_once_with(fake_volume, + vref) + fake_delete.assert_called_once_with(vref['name']) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog, 'LOG') + def test_unmanage_fail_create_cloned_volume(self, fake_logger, + fake_create_cloned_volume, + fake_delete): + fake_volume = self.test_data.TEST_VOLUME + fake_create_cloned_volume.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='dummy', stderr='dummy') + + self.assertRaises(exception.SheepdogCmdError, + self.driver.unmanage, fake_volume) + self.assertTrue(fake_logger.error.called) + self.assertTrue(fake_create_cloned_volume.called) + self.assertFalse(fake_delete.called) + + @mock.patch.object(sheepdog.SheepdogClient, 'delete') + @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') + @mock.patch.object(sheepdog, 'LOG') + def test_unmanage_fail_delete(self, fake_logger, fake_create_cloned_volume, + fake_delete): fake_volume = self.test_data.TEST_VOLUME - new_volume_name = fake_volume.name + '-unmanaged' + fake_delete.side_effect = exception.SheepdogCmdError( + cmd='dummy', exit_code=1, stdout='dummy', stderr='dummy') self.driver.unmanage(fake_volume) - fake_rename.assert_called_once_with(fake_volume.name, new_volume_name) + self.assertTrue(fake_logger.warning.called) + self.assertTrue(fake_create_cloned_volume.called) + self.assertTrue(fake_delete.called) diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 92ad9c14c4a..37c2948ce89 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -269,15 +269,13 @@ def delete_snapshot(self, vdiname, snapname): LOG.error(_LE('Failed to delete snapshot. (command: %s)'), cmd) - def clone(self, src_vdiname, src_snapname, dst_vdiname, size=None): - params = ['-b', 'sheepdog:%(src_vdiname)s:%(src_snapname)s' % - {'src_vdiname': src_vdiname, - 'src_snapname': src_snapname}, - 'sheepdog:%s' % dst_vdiname] - if size is not None: - params.append('%sG' % size) + def clone(self, src_vdiname, src_snapname, dst_vdiname, size): try: - self._run_qemu_img('create', *params) + self._run_qemu_img('create', '-b', + 'sheepdog:%(src_vdiname)s:%(src_snapname)s' % + {'src_vdiname': src_vdiname, + 'src_snapname': src_snapname}, + 'sheepdog:%s' % dst_vdiname, '%sG' % size) except exception.SheepdogCmdError as e: cmd = e.kwargs['cmd'] _stderr = e.kwargs['stderr'] @@ -507,28 +505,6 @@ def get_vdi_size(self, vdiname): return int(math.ceil(size / units.Gi)) - def rename(self, old_vdi_name, new_vdi_name): - """Rename vdi name.""" - snapshot_name = 'temp-rename-snapshot-' + old_vdi_name - try: - self.create_snapshot(old_vdi_name, snapshot_name) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Failed to create temporary snapshot for ' - 'rename "%s".'), old_vdi_name) - try: - self.clone(old_vdi_name, snapshot_name, new_vdi_name) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Failed to clone for rename from ' - '"%(old_vdi_name)s" to "%(new_vdi_name)s".'), - {'old_vdi_name': old_vdi_name, - 'new_vdi_name': new_vdi_name}) - finally: - self.delete_snapshot(old_vdi_name, snapshot_name) - - self.delete(old_vdi_name) - class SheepdogIOWrapper(io.RawIOBase): """File-like object with Sheepdog backend.""" @@ -861,7 +837,21 @@ def manage_existing(self, volume, existing_ref): raise exception.ManageExistingAlreadyManaged( volume_ref=source_name) - self.client.rename(source_name, volume.name) + try: + size_gb = self.client.get_vdi_size(source_name) + volume.size = size_gb + vref = {'name': source_name, 'size': size_gb} + self.create_cloned_volume(volume, vref) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Failed to manage existing volume. ' + 'source_name: %s.'), source_name) + try: + self.client.delete(source_name) + except Exception: + LOG.warning(_LW('Failed to delete working backend volume.' + 'It remains as trash. source_name: %s'), + source_name) def manage_existing_get_size(self, volume, existing_ref): """Return size of an existing volume for manage_existing.""" @@ -884,4 +874,19 @@ def manage_existing_get_size(self, volume, existing_ref): def unmanage(self, volume): """Removes the specified volume from Cinder management.""" - self.client.rename(volume.name, volume.name + '-unmanaged') + source_name = volume.name + vref = {'name': source_name, 'size': volume.size} + volume.name_id += '-unmanaged' + + try: + self.create_cloned_volume(volume, vref) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Failed to unmanage volume. volume: %s.'), + source_name) + + try: + self.client.delete(source_name) + except Exception: + LOG.warning(_LW('Failed to delete working backend volume.' + 'It remains as trash. volume: %s'), source_name) From 4137786a97219a8a0e7d7de83ba3d08afa328322 Mon Sep 17 00:00:00 2001 From: sds03 Date: Fri, 25 Sep 2015 11:05:30 +0900 Subject: [PATCH 5/5] Modify to give the volume size to manage_existing() --- cinder/tests/unit/test_sheepdog.py | 40 +++--------------------------- cinder/volume/drivers/sheepdog.py | 5 ++-- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/cinder/tests/unit/test_sheepdog.py b/cinder/tests/unit/test_sheepdog.py index 840b8e71dd1..9f921dfb633 100644 --- a/cinder/tests/unit/test_sheepdog.py +++ b/cinder/tests/unit/test_sheepdog.py @@ -2155,33 +2155,28 @@ def test_restore_backup(self, fake_backup_service): @mock.patch.object(sheepdog.SheepdogClient, 'delete') @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') - @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') @mock.patch.object(volutils, 'check_already_managed_volume') def test_manage_existing_success(self, fake_check_already_managed_volume, - fake_get_vdi_size, fake_create_cloned_volume, fake_delete): fake_volume = self.test_data.TEST_VOLUME fake_existing_ref = self.test_data.TEST_EXISTING_REF source_name = fake_existing_ref['source-name'] fake_vref = {'name': source_name, 'size': fake_volume.size} fake_check_already_managed_volume.return_value = False - fake_get_vdi_size.return_value = fake_volume.size self.driver.manage_existing(fake_volume, fake_existing_ref) fake_check_already_managed_volume.assert_called_once_with(self.db, source_name) - fake_get_vdi_size.assert_called_once_with(source_name) fake_create_cloned_volume.assert_called_once_with(fake_volume, fake_vref) fake_delete.assert_called_once_with(source_name) @mock.patch.object(sheepdog.SheepdogClient, 'delete') @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') - @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') @mock.patch.object(volutils, 'check_already_managed_volume') def test_manage_existing_fail_already_managed_volume( - self, fake_check_already_managed_volume, fake_get_vdi_size, - fake_create_cloned_volume, fake_delete): + self, fake_check_already_managed_volume, fake_create_cloned_volume, + fake_delete): fake_volume = self.test_data.TEST_VOLUME fake_existing_ref = self.test_data.TEST_EXISTING_REF fake_check_already_managed_volume.return_value = True @@ -2192,35 +2187,11 @@ def test_manage_existing_fail_already_managed_volume( @mock.patch.object(sheepdog.SheepdogClient, 'delete') @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') - @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') - @mock.patch.object(volutils, 'check_already_managed_volume') - @mock.patch.object(sheepdog, 'LOG') - def test_manage_existing_fail_get_vdi_size( - self, fake_logger, fake_check_already_managed_volume, - fake_get_vdi_size, fake_create_cloned_volume, fake_delete): - fake_volume = self.test_data.TEST_VOLUME - fake_existing_ref = self.test_data.TEST_EXISTING_REF - fake_check_already_managed_volume.return_value = False - fake_get_vdi_size.side_effect = exception.SheepdogCmdError( - cmd='dummy', exit_code=1, stdout='dummy', stderr='dummy') - - self.assertRaises(exception.SheepdogCmdError, - self.driver.manage_existing, fake_volume, - fake_existing_ref) - self.assertTrue(fake_logger.error.called) - self.assertTrue(fake_check_already_managed_volume.called) - self.assertTrue(fake_get_vdi_size.called) - self.assertFalse(fake_create_cloned_volume.called) - self.assertFalse(fake_delete.called) - - @mock.patch.object(sheepdog.SheepdogClient, 'delete') - @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') - @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') @mock.patch.object(volutils, 'check_already_managed_volume') @mock.patch.object(sheepdog, 'LOG') def test_manage_existing_fail_create_cloned_volume( self, fake_logger, fake_check_already_managed_volume, - fake_get_vdi_size, fake_create_cloned_volume, fake_delete): + fake_create_cloned_volume, fake_delete): fake_volume = self.test_data.TEST_VOLUME fake_existing_ref = self.test_data.TEST_EXISTING_REF fake_check_already_managed_volume.return_value = False @@ -2232,18 +2203,16 @@ def test_manage_existing_fail_create_cloned_volume( fake_existing_ref) self.assertTrue(fake_logger.error.called) self.assertTrue(fake_check_already_managed_volume.called) - self.assertTrue(fake_get_vdi_size.called) self.assertTrue(fake_create_cloned_volume.called) self.assertFalse(fake_delete.called) @mock.patch.object(sheepdog.SheepdogClient, 'delete') @mock.patch.object(sheepdog.SheepdogDriver, 'create_cloned_volume') - @mock.patch.object(sheepdog.SheepdogClient, 'get_vdi_size') @mock.patch.object(volutils, 'check_already_managed_volume') @mock.patch.object(sheepdog, 'LOG') def test_manage_existing_fail_delete( self, fake_logger, fake_check_already_managed_volume, - fake_get_vdi_size, fake_create_cloned_volume, fake_delete): + fake_create_cloned_volume, fake_delete): fake_volume = self.test_data.TEST_VOLUME fake_existing_ref = self.test_data.TEST_EXISTING_REF fake_check_already_managed_volume.return_value = False @@ -2253,7 +2222,6 @@ def test_manage_existing_fail_delete( self.driver.manage_existing(fake_volume, fake_existing_ref) self.assertTrue(fake_logger.warning.called) self.assertTrue(fake_check_already_managed_volume.called) - self.assertTrue(fake_get_vdi_size.called) self.assertTrue(fake_create_cloned_volume.called) self.assertTrue(fake_delete.called) diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 37c2948ce89..08e24b5b847 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -837,10 +837,8 @@ def manage_existing(self, volume, existing_ref): raise exception.ManageExistingAlreadyManaged( volume_ref=source_name) + vref = {'name': source_name, 'size': volume.size} try: - size_gb = self.client.get_vdi_size(source_name) - volume.size = size_gb - vref = {'name': source_name, 'size': size_gb} self.create_cloned_volume(volume, vref) except Exception: with excutils.save_and_reraise_exception(): @@ -870,6 +868,7 @@ def manage_existing_get_size(self, volume, existing_ref): raise exception.ManageExistingInvalidReference( existing_ref=existing_ref, reason=reason) + volume.size = size_gb return size_gb def unmanage(self, volume):