diff --git a/nova/tests/virt/baremetal/test_volume_driver.py b/nova/tests/virt/baremetal/test_volume_driver.py index 4adf0f749d78..8ea358804ba8 100644 --- a/nova/tests/virt/baremetal/test_volume_driver.py +++ b/nova/tests/virt/baremetal/test_volume_driver.py @@ -204,43 +204,24 @@ class BareMetalLibVirtVolumeDriverTestCase(test.TestCase): def test_fake_connect_volume(self): """Check connect_volume returns without exceptions.""" - self.driver._volume_driver_method('connect_volume', - self.connection_info, - self.disk_info) + self.driver._connect_volume(self.connection_info, + self.disk_info) def test_volume_driver_method_ok(self): fake_driver = self.driver.volume_drivers['fake'] self.mox.StubOutWithMock(fake_driver, 'connect_volume') fake_driver.connect_volume(self.connection_info, self.disk_info) self.mox.ReplayAll() - self.driver._volume_driver_method('connect_volume', - self.connection_info, - self.disk_info) + self.driver._connect_volume(self.connection_info, + self.disk_info) def test_volume_driver_method_driver_type_not_found(self): self.connection_info['driver_volume_type'] = 'qwerty' self.assertRaises(exception.VolumeDriverNotFound, - self.driver._volume_driver_method, - 'connect_volume', + self.driver._connect_volume, self.connection_info, self.disk_info) - def test_connect_volume(self): - self.mox.StubOutWithMock(self.driver, '_volume_driver_method') - self.driver._volume_driver_method('connect_volume', - self.connection_info, - self.disk_info) - self.mox.ReplayAll() - self.driver._connect_volume(self.connection_info, self.disk_info) - - def test_disconnect_volume(self): - self.mox.StubOutWithMock(self.driver, '_volume_driver_method') - self.driver._volume_driver_method('disconnect_volume', - self.connection_info, - self.mount_device) - self.mox.ReplayAll() - self.driver._disconnect_volume(self.connection_info, self.mount_device) - def test_publish_iscsi(self): self.mox.StubOutWithMock(volume_driver, '_get_iqn') self.mox.StubOutWithMock(volume_driver, '_get_next_tid') diff --git a/nova/tests/virt/libvirt/test_driver.py b/nova/tests/virt/libvirt/test_driver.py index af133df03aeb..2012a155ea76 100644 --- a/nova/tests/virt/libvirt/test_driver.py +++ b/nova/tests/virt/libvirt/test_driver.py @@ -3393,10 +3393,10 @@ class LibvirtConnTestCase(test.TestCase): fakelibvirt.VIR_DOMAIN_AFFECT_LIVE) with contextlib.nested( - mock.patch.object(conn, 'volume_driver_method', + mock.patch.object(conn, '_connect_volume', return_value=mock_conf), mock.patch.object(conn, '_set_cache_mode') - ) as (mock_volume_driver_method, mock_set_cache_mode): + ) as (mock_connect_volume, mock_set_cache_mode): for state in (power_state.RUNNING, power_state.PAUSED): mock_dom.info.return_value = [state, 512, 512, 2, 1234, 5678] @@ -3406,8 +3406,8 @@ class LibvirtConnTestCase(test.TestCase): mock_lookup_by_name.assert_called_with(instance['name']) mock_get_info.assert_called_with(CONF.libvirt.virt_type, bdm) - mock_volume_driver_method.assert_called_with( - 'connect_volume', connection_info, disk_info) + mock_connect_volume.assert_called_with( + connection_info, disk_info) mock_set_cache_mode.assert_called_with(mock_conf) mock_dom.attachDeviceFlags.assert_called_with( mock_conf.to_xml(), flags) @@ -3434,8 +3434,8 @@ class LibvirtConnTestCase(test.TestCase): flags = (fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG | fakelibvirt.VIR_DOMAIN_AFFECT_LIVE) - with mock.patch.object(conn, 'volume_driver_method') as \ - mock_volume_driver_method: + with mock.patch.object(conn, '_disconnect_volume') as \ + mock_disconnect_volume: for state in (power_state.RUNNING, power_state.PAUSED): mock_dom.info.return_value = [state, 512, 512, 2, 1234, 5678] mock_lookup_by_name.return_value = mock_dom @@ -3446,8 +3446,8 @@ class LibvirtConnTestCase(test.TestCase): mock_get_disk_xml.assert_called_with(mock_dom.XMLDesc(0), 'vdc') mock_dom.detachDeviceFlags.assert_called_with(mock_xml, flags) - mock_volume_driver_method.assert_called_with( - 'disconnect_volume', connection_info, 'vdc') + mock_disconnect_volume.assert_called_with( + connection_info, 'vdc') def test_multi_nic(self): instance_data = dict(self.test_instance) @@ -4301,16 +4301,15 @@ class LibvirtConnTestCase(test.TestCase): self.mox.StubOutWithMock(driver, "block_device_info_get_mapping") driver.block_device_info_get_mapping(vol ).AndReturn(vol['block_device_mapping']) - self.mox.StubOutWithMock(conn, "volume_driver_method") + self.mox.StubOutWithMock(conn, "_connect_volume") for v in vol['block_device_mapping']: disk_info = { 'bus': "scsi", 'dev': v['mount_device'].rpartition("/")[2], 'type': "disk" } - conn.volume_driver_method('connect_volume', - v['connection_info'], - disk_info) + conn._connect_volume(v['connection_info'], + disk_info) self.mox.StubOutWithMock(conn, 'plug_vifs') conn.plug_vifs(mox.IsA(inst_ref), nw_info) @@ -4358,16 +4357,15 @@ class LibvirtConnTestCase(test.TestCase): c = context.get_admin_context() nw_info = FakeNetworkInfo() # Creating mocks - self.mox.StubOutWithMock(conn, "volume_driver_method") + self.mox.StubOutWithMock(conn, "_connect_volume") for v in vol['block_device_mapping']: disk_info = { 'bus': "scsi", 'dev': v['mount_device'].rpartition("/")[2], 'type': "disk" } - conn.volume_driver_method('connect_volume', - v['connection_info'], - disk_info) + conn._connect_volume(v['connection_info'], + disk_info) self.mox.StubOutWithMock(conn, 'plug_vifs') conn.plug_vifs(mox.IsA(inst_ref), nw_info) self.mox.ReplayAll() @@ -4491,15 +4489,14 @@ class LibvirtConnTestCase(test.TestCase): with contextlib.nested( mock.patch.object(driver, 'block_device_info_get_mapping', return_value=vol['block_device_mapping']), - mock.patch.object(conn, 'volume_driver_method') - ) as (block_device_info_get_mapping, volume_driver_method): + mock.patch.object(conn, '_disconnect_volume') + ) as (block_device_info_get_mapping, _disconnect_volume): conn.post_live_migration(cntx, inst_ref, vol) block_device_info_get_mapping.assert_has_calls([ mock.call(vol)]) - volume_driver_method.assert_has_calls([ - mock.call('disconnect_volume', - v['connection_info'], + _disconnect_volume.assert_has_calls([ + mock.call(v['connection_info'], v['mount_device'].rpartition("/")[2]) for v in vol['block_device_mapping']]) @@ -5201,13 +5198,13 @@ class LibvirtConnTestCase(test.TestCase): driver.block_device_info_get_mapping(vol ).AndReturn(vol['block_device_mapping']) self.mox.StubOutWithMock(libvirt_driver.LibvirtDriver, - "volume_driver_method") + "_disconnect_volume") if volume_fail: - libvirt_driver.LibvirtDriver.volume_driver_method( + libvirt_driver.LibvirtDriver._disconnect_volume( mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg()).\ AndRaise(exception.VolumeNotFound('vol')) else: - libvirt_driver.LibvirtDriver.volume_driver_method( + libvirt_driver.LibvirtDriver._disconnect_volume( mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg()) self.mox.StubOutWithMock(shutil, "rmtree") shutil.rmtree(os.path.join(CONF.instances_path, @@ -6880,14 +6877,13 @@ class LibvirtConnTestCase(test.TestCase): mock.patch.object(conn, '_lookup_by_name', side_effect=exception.InstanceNotFound( instance_id=instance.name)), - mock.patch.object(conn, 'volume_driver_method') - ) as (_lookup_by_name, volume_driver_method): + mock.patch.object(conn, '_disconnect_volume') + ) as (_lookup_by_name, _disconnect_volume): connection_info = {'driver_volume_type': 'fake'} conn.detach_volume(connection_info, instance, '/dev/sda') _lookup_by_name.assert_called_once_with(instance.name) - volume_driver_method.assert_called_once_with('disconnect_volume', - connection_info, - 'sda') + _disconnect_volume.assert_called_once_with(connection_info, + 'sda') def _test_attach_detach_interface_get_config(self, method_name): """Tests that the get_config() method is properly called in diff --git a/nova/virt/baremetal/volume_driver.py b/nova/virt/baremetal/volume_driver.py index 2a35e126873a..3cbe87877060 100644 --- a/nova/virt/baremetal/volume_driver.py +++ b/nova/virt/baremetal/volume_driver.py @@ -218,15 +218,6 @@ class LibvirtVolumeDriver(VolumeDriver): driver_class = importutils.import_class(driver) self.volume_drivers[driver_type] = driver_class(self) - def _volume_driver_method(self, method_name, connection_info, - *args, **kwargs): - driver_type = connection_info.get('driver_volume_type') - if driver_type not in self.volume_drivers: - raise exception.VolumeDriverNotFound(driver_type=driver_type) - driver = self.volume_drivers[driver_type] - method = getattr(driver, method_name) - return method(connection_info, *args, **kwargs) - def attach_volume(self, connection_info, instance, mountpoint): fixed_ips = _get_fixed_ips(instance) if not fixed_ips: @@ -245,9 +236,11 @@ class LibvirtVolumeDriver(VolumeDriver): conf.source_path) def _connect_volume(self, connection_info, disk_info): - return self._volume_driver_method('connect_volume', - connection_info, - disk_info) + driver_type = connection_info.get('driver_volume_type') + if driver_type not in self.volume_drivers: + raise exception.VolumeDriverNotFound(driver_type=driver_type) + driver = self.volume_drivers[driver_type] + return driver.connect_volume(connection_info, disk_info) def _publish_iscsi(self, instance, mountpoint, fixed_ips, device_path): iqn = _get_iqn(instance['name'], mountpoint) @@ -275,9 +268,11 @@ class LibvirtVolumeDriver(VolumeDriver): self._disconnect_volume(connection_info, mount_device) def _disconnect_volume(self, connection_info, disk_dev): - return self._volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + driver_type = connection_info.get('driver_volume_type') + if driver_type not in self.volume_drivers: + raise exception.VolumeDriverNotFound(driver_type=driver_type) + driver = self.volume_drivers[driver_type] + return driver.connect_volume(connection_info, disk_dev) def _depublish_iscsi(self, instance, mountpoint): iqn = _get_iqn(instance['name'], mountpoint) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 033d96f7e2b3..bd21f86120b3 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1013,9 +1013,7 @@ class LibvirtDriver(driver.ComputeDriver): encryptor.detach_volume(**encryption) try: - self.volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + self._disconnect_volume(connection_info, disk_dev) except Exception as exc: with excutils.save_and_reraise_exception() as ctxt: if destroy_disks: @@ -1150,14 +1148,19 @@ class LibvirtDriver(driver.ComputeDriver): self.unplug_vifs(instance, network_info) self.firewall_driver.unfilter_instance(instance, network_info) - def volume_driver_method(self, method_name, connection_info, - *args, **kwargs): + def _connect_volume(self, connection_info, disk_info): driver_type = connection_info.get('driver_volume_type') if driver_type not in self.volume_drivers: raise exception.VolumeDriverNotFound(driver_type=driver_type) driver = self.volume_drivers[driver_type] - method = getattr(driver, method_name) - return method(connection_info, *args, **kwargs) + return driver.connect_volume(connection_info, disk_info) + + def _disconnect_volume(self, connection_info, disk_dev): + driver_type = connection_info.get('driver_volume_type') + if driver_type not in self.volume_drivers: + raise exception.VolumeDriverNotFound(driver_type=driver_type) + driver = self.volume_drivers[driver_type] + return driver.disconnect_volume(connection_info, disk_dev) def _get_volume_encryptor(self, connection_info, encryption): encryptor = encryptors.get_volume_encryptor(connection_info, @@ -1196,9 +1199,7 @@ class LibvirtDriver(driver.ComputeDriver): raise exception.Invalid(msg) disk_info = blockinfo.get_info_from_bdm(CONF.libvirt.virt_type, bdm) - conf = self.volume_driver_method('connect_volume', - connection_info, - disk_info) + conf = self._connect_volume(connection_info, disk_info) self._set_cache_mode(conf) try: @@ -1224,15 +1225,11 @@ class LibvirtDriver(driver.ComputeDriver): if isinstance(ex, libvirt.libvirtError): errcode = ex.get_error_code() if errcode == libvirt.VIR_ERR_OPERATION_FAILED: - self.volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + self._disconnect_volume(connection_info, disk_dev) raise exception.DeviceIsBusy(device=disk_dev) with excutils.save_and_reraise_exception(): - self.volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + self._disconnect_volume(connection_info, disk_dev) def _swap_volume(self, domain, disk_path, new_path): """Swap existing disk with a new block device.""" @@ -1282,19 +1279,13 @@ class LibvirtDriver(driver.ComputeDriver): CONF.libvirt.virt_type, disk_dev), 'type': 'disk', } - conf = self.volume_driver_method('connect_volume', - new_connection_info, - disk_info) + conf = self._connect_volume(new_connection_info, disk_info) if not conf.source_path: - self.volume_driver_method('disconnect_volume', - new_connection_info, - disk_dev) + self._disconnect_volume(new_connection_info, disk_dev) raise NotImplementedError(_("Swap only supports host devices")) self._swap_volume(virt_dom, disk_dev, conf.source_path) - self.volume_driver_method('disconnect_volume', - old_connection_info, - disk_dev) + self._disconnect_volume(old_connection_info, disk_dev) @staticmethod def _get_disk_xml(xml, device): @@ -1366,9 +1357,7 @@ class LibvirtDriver(driver.ComputeDriver): else: raise - self.volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + self._disconnect_volume(connection_info, disk_dev) def attach_interface(self, instance, image_meta, vif): virt_dom = self._lookup_by_name(instance['name']) @@ -3029,9 +3018,7 @@ class LibvirtDriver(driver.ComputeDriver): connection_info = vol['connection_info'] vol_dev = block_device.prepend_dev(vol['mount_device']) info = disk_mapping[vol_dev] - cfg = self.volume_driver_method('connect_volume', - connection_info, - info) + cfg = self._connect_volume(connection_info, info) devices.append(cfg) vol['connection_info'] = connection_info vol.save(nova_context.get_admin_context()) @@ -3614,9 +3601,7 @@ class LibvirtDriver(driver.ComputeDriver): connection_info = vol['connection_info'] disk_info = blockinfo.get_info_from_bdm( CONF.libvirt.virt_type, vol) - conf = self.volume_driver_method('connect_volume', - connection_info, - disk_info) + conf = self._connect_volume(connection_info, disk_info) # cache device_path in connection_info -- required by encryptors if (not reboot and 'data' in connection_info and @@ -4575,9 +4560,7 @@ class LibvirtDriver(driver.ComputeDriver): connection_info = vol['connection_info'] disk_info = blockinfo.get_info_from_bdm( CONF.libvirt.virt_type, vol) - self.volume_driver_method('connect_volume', - connection_info, - disk_info) + self._connect_volume(connection_info, disk_info) # We call plug_vifs before the compute manager calls # ensure_filtering_rules_for_instance, to ensure bridge is set up @@ -4667,9 +4650,7 @@ class LibvirtDriver(driver.ComputeDriver): for vol in block_device_mapping: connection_info = vol['connection_info'] disk_dev = vol['mount_device'].rpartition("/")[2] - self.volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + self._disconnect_volume(connection_info, disk_dev) def post_live_migration_at_destination(self, context, instance, @@ -4927,9 +4908,7 @@ class LibvirtDriver(driver.ComputeDriver): for vol in block_device_mapping: connection_info = vol['connection_info'] disk_dev = vol['mount_device'].rpartition("/")[2] - self.volume_driver_method('disconnect_volume', - connection_info, - disk_dev) + self._disconnect_volume(connection_info, disk_dev) try: utils.execute('mv', inst_base, inst_base_resize)