openstack
/
nova
Code Issues Proposed changes

libvirt: remove volume_driver_method API 

Replace the 'volume_driver_method' API with _connect_volume
and _disconnect_volume methods to remove the pointless
introspection based calling technique. This adds a '_' to
the names to show that they are internal methods only.

Related-bug: #1333219
Change-Id: Ife3fb60dd8aef8adf6bbdd2465971e156430fa0d
This commit is contained in:
Daniel P. Berrange 2014-06-20 15:05:16 +01:00
parent be97af9a30
commit 07eb0e25e4
4 changed files with 62 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
nova/tests/virt/baremetal/test_volume_driver.py
View File

@ -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')

54
nova/tests/virt/libvirt/test_driver.py
View File

@ -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

25
nova/virt/baremetal/volume_driver.py
View File

@ -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)

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

@ -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)