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
parent
be97af9a30
commit
07eb0e25e4
@ -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')
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user