openstack
/
nova
Code Issues Proposed changes

libvirt: Rename _is_storage_shared_with to _is_path_shared_with 

This method only checks if a specific path is shared between two hosts
and has been renamed accordingly to avoid confusion.

Additionally the shared_storage variable used to store the returned
value from this method within migrate_disk_and_power_off is renamed to
shared_instance_path.

Change-Id: I426de20864321d664d3fe0e08a14e1af509c8a2b
This commit is contained in:
Lee Yarwood 2019-11-07 11:01:51 +00:00
parent 014c1ab864
commit afebcdc950
2 changed files with 19 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -17544,7 +17544,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
mock.patch.object(drvr._remotefs, 'create_file'), mock.patch.object(drvr._remotefs, 'create_file'),
mock.patch.object(drvr._remotefs, 'remove_file') mock.patch.object(drvr._remotefs, 'remove_file')
) as (mock_rem_fs_create, mock_rem_fs_remove): ) as (mock_rem_fs_create, mock_rem_fs_remove):
result = drvr._is_storage_shared_with('host', '/path') result = drvr._is_path_shared_with('host', '/path')
mock_rem_fs_create.assert_any_call('host', mock.ANY) mock_rem_fs_create.assert_any_call('host', mock.ANY)
create_args, create_kwargs = mock_rem_fs_create.call_args create_args, create_kwargs = mock_rem_fs_create.call_args
self.assertTrue(create_args[1].startswith('/path')) self.assertTrue(create_args[1].startswith('/path'))
@ -17570,7 +17570,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
def test_shared_storage_detection_easy(self, mock_get, mock_exec, def test_shared_storage_detection_easy(self, mock_get, mock_exec,
mock_exists, mock_unlink): mock_exists, mock_unlink):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
self.assertTrue(drvr._is_storage_shared_with('foo', '/path')) self.assertTrue(drvr._is_path_shared_with('foo', '/path'))
mock_get.assert_called_once_with() mock_get.assert_called_once_with()
mock_exec.assert_not_called() mock_exec.assert_not_called()
mock_exists.assert_not_called() mock_exists.assert_not_called()
@ -20295,7 +20295,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_host_ip_addr', @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_host_ip_addr',
return_value='10.0.0.1') return_value='10.0.0.1')
@mock.patch(('nova.virt.libvirt.driver.LibvirtDriver.' @mock.patch(('nova.virt.libvirt.driver.LibvirtDriver.'
'_is_storage_shared_with'), return_value=False) '_is_path_shared_with'), return_value=False)
@mock.patch('os.rename') @mock.patch('os.rename')
@mock.patch('os.path.exists', return_value=True) @mock.patch('os.path.exists', return_value=True)
@mock.patch('oslo_concurrency.processutils.execute', @mock.patch('oslo_concurrency.processutils.execute',
@ -20322,7 +20322,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_host_ip_addr', @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.get_host_ip_addr',
return_value='10.0.0.1') return_value='10.0.0.1')
@mock.patch(('nova.virt.libvirt.driver.LibvirtDriver.' @mock.patch(('nova.virt.libvirt.driver.LibvirtDriver.'
'_is_storage_shared_with'), return_value=False) '_is_path_shared_with'), return_value=False)
@mock.patch('os.rename') @mock.patch('os.rename')
@mock.patch('os.path.exists', return_value=True) @mock.patch('os.path.exists', return_value=True)
@mock.patch('oslo_concurrency.processutils.execute') @mock.patch('oslo_concurrency.processutils.execute')
@ -20474,7 +20474,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver' @mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
'._get_instance_disk_info') '._get_instance_disk_info')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver' @mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
'._is_storage_shared_with') '._is_path_shared_with')
def _test_migrate_disk_and_power_off_backing_file(self, def _test_migrate_disk_and_power_off_backing_file(self,
shared_storage, shared_storage,
mock_is_shared_storage, mock_is_shared_storage,
@ -20520,8 +20520,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
expected_exc = exception.InstanceFaultRollback expected_exc = exception.InstanceFaultRollback
self._test_migrate_disk_and_power_off_resize_check(expected_exc) self._test_migrate_disk_and_power_off_resize_check(expected_exc)
@mock.patch.object(libvirt_driver.LibvirtDriver, '_is_storage_shared_with', @mock.patch.object(libvirt_driver.LibvirtDriver,
return_value=False) '_is_path_shared_with', return_value=False)
def test_migrate_disk_and_power_off_resize_cannot_ssh(self, def test_migrate_disk_and_power_off_resize_cannot_ssh(self,
mock_is_shared): mock_is_shared):
def fake_execute(*args, **kwargs): def fake_execute(*args, **kwargs):
@ -20648,7 +20648,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._destroy') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._destroy')
@mock.patch('nova.virt.libvirt.utils.get_instance_path') @mock.patch('nova.virt.libvirt.utils.get_instance_path')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver' @mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
'._is_storage_shared_with') '._is_path_shared_with')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver' @mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
'._get_instance_disk_info') '._get_instance_disk_info')
def test_migrate_disk_and_power_off_resize_copy_disk_info( def test_migrate_disk_and_power_off_resize_copy_disk_info(

22
nova/virt/libvirt/driver.py
View File

@ -9592,26 +9592,26 @@ class LibvirtDriver(driver.ComputeDriver):
images.fetch_to_raw(context, image_id, path) images.fetch_to_raw(context, image_id, path)
return True return True
def _is_storage_shared_with(self, dest, inst_base): def _is_path_shared_with(self, dest, path):
# NOTE (rmk): There are two methods of determining whether we are # NOTE (rmk): There are two methods of determining whether we are
# on the same filesystem: the source and dest IP are the # on the same filesystem: the source and dest IP are the
# same, or we create a file on the dest system via SSH # same, or we create a file on the dest system via SSH
# and check whether the source system can also see it. # and check whether the source system can also see it.
shared_storage = (dest == self.get_host_ip_addr()) shared_path = (dest == self.get_host_ip_addr())
if not shared_storage: if not shared_path:
tmp_file = uuidutils.generate_uuid(dashed=False) + '.tmp' tmp_file = uuidutils.generate_uuid(dashed=False) + '.tmp'
tmp_path = os.path.join(inst_base, tmp_file) tmp_path = os.path.join(path, tmp_file)
try: try:
self._remotefs.create_file(dest, tmp_path) self._remotefs.create_file(dest, tmp_path)
if os.path.exists(tmp_path): if os.path.exists(tmp_path):
shared_storage = True shared_path = True
os.unlink(tmp_path) os.unlink(tmp_path)
else: else:
self._remotefs.remove_file(dest, tmp_path) self._remotefs.remove_file(dest, tmp_path)
except Exception: except Exception:
pass pass
return shared_storage return shared_path
def migrate_disk_and_power_off(self, context, instance, dest, def migrate_disk_and_power_off(self, context, instance, dest,
flavor, network_info, flavor, network_info,
@ -9651,12 +9651,12 @@ class LibvirtDriver(driver.ComputeDriver):
# shared storage for instance dir (eg. NFS). # shared storage for instance dir (eg. NFS).
inst_base = libvirt_utils.get_instance_path(instance) inst_base = libvirt_utils.get_instance_path(instance)
inst_base_resize = inst_base + "_resize" inst_base_resize = inst_base + "_resize"
shared_storage = self._is_storage_shared_with(dest, inst_base) shared_instance_path = self._is_path_shared_with(dest, inst_base)
# try to create the directory on the remote compute node # try to create the directory on the remote compute node
# if this fails we pass the exception up the stack so we can catch # if this fails we pass the exception up the stack so we can catch
# failures here earlier # failures here earlier
if not shared_storage: if not shared_instance_path:
try: try:
self._remotefs.create_dir(dest, inst_base) self._remotefs.create_dir(dest, inst_base)
except processutils.ProcessExecutionError as e: except processutils.ProcessExecutionError as e:
@ -9676,10 +9676,10 @@ class LibvirtDriver(driver.ComputeDriver):
try: try:
os.rename(inst_base, inst_base_resize) os.rename(inst_base, inst_base_resize)
# if we are migrating the instance with shared storage then # if we are migrating the instance with shared instance path then
# create the directory. If it is a remote node the directory # create the directory. If it is a remote node the directory
# has already been created # has already been created
if shared_storage: if shared_instance_path:
dest = None dest = None
fileutils.ensure_tree(inst_base) fileutils.ensure_tree(inst_base)
@ -9721,7 +9721,7 @@ class LibvirtDriver(driver.ComputeDriver):
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
self._cleanup_remote_migration(dest, inst_base, self._cleanup_remote_migration(dest, inst_base,
inst_base_resize, inst_base_resize,
shared_storage) shared_instance_path)
return jsonutils.dumps(disk_info) return jsonutils.dumps(disk_info)