libvirt : Fix slightly misleading parameter name, validate param

We actually need transport_iface, not transport as the param
(iscsi_tcp & iser are exceptions to this, where transport and
transport_iface are one and the same), so change it so that user is
not confused as to what must be provided. Also fixes a typo in
param help section.

Also added code to ensure that non-existing transport_iface is not
provided, in which case we we log a warning and fall back to the
default instead of failing as would have previously happened.

There is no change in functionality, this will just ensure that
documentation is not misleading.

Originally added in commit 554647a4de.

DocImpact - due to the renamed config option
Closes-Bug: #1420971
Change-Id: I6c190328803e8efcde34d522e2c1814ef6afc220
This commit is contained in:
Anish Bhatt 2015-02-06 16:40:36 -08:00 committed by Matt Riedemann
parent 51cfe7def3
commit 3b8a2cf781
2 changed files with 76 additions and 11 deletions

View File

@ -348,13 +348,35 @@ Setting up iSCSI targets: unused
expected_device = self.generate_device(transport, 1, False)
if transport:
self.stubs.Set(glob, 'glob', lambda x: [expected_device])
self.stubs.Set(libvirt_driver, '_validate_transport',
lambda x: True)
device = libvirt_driver._get_host_device(iscsi_properties)
self.assertEqual(expected_device, device)
def test_libvirt_iscsi_get_host_device_with_transport(self):
self.flags(iscsi_transport='fake_transport', group='libvirt')
self.flags(iscsi_iface='fake_transport', group='libvirt')
self.test_libvirt_iscsi_get_host_device('fake_transport')
@mock.patch.object(volume.utils, 'execute')
def test_libvirt_iscsi_validate_transport(self, mock_execute):
libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn)
sample_output = ('# BEGIN RECORD 2.0-872\n'
'iface.iscsi_ifacename = %s.fake_suffix\n'
'iface.net_ifacename = <empty>\n'
'iface.ipaddress = <empty>\n'
'iface.hwaddress = 00:53:00:00:53:00\n'
'iface.transport_name = %s\n'
'iface.initiatorname = <empty>\n'
'# END RECORD')
for tport in libvirt_driver.supported_transports:
mock_execute.return_value = (sample_output % (tport, tport), '')
self.assertTrue(libvirt_driver._validate_transport(
tport + '.fake_suffix'))
mock_execute.return_value = ("", 'iscsiadm: Could not '
'read iface fake_transport (6)')
self.assertFalse(libvirt_driver._validate_transport('fake_transport'))
def test_libvirt_iscsi_driver(self, transport=None):
# NOTE(vish) exists is to make driver assume connecting worked
self.stubs.Set(os.path, 'exists', lambda x: True)
@ -364,6 +386,8 @@ Setting up iSCSI targets: unused
if transport is not None:
self.stubs.Set(libvirt_driver, '_get_host_device',
lambda x: self.generate_device(transport, 1, False))
self.stubs.Set(libvirt_driver, '_validate_transport',
lambda x: True)
libvirt_driver.connect_volume(connection_info, self.disk_info)
libvirt_driver.disconnect_volume(connection_info, "vde")
expected_commands = [('iscsiadm', '-m', 'node', '-T', self.iqn,
@ -386,7 +410,7 @@ Setting up iSCSI targets: unused
self.assertEqual(expected_commands, self.executes)
def test_libvirt_iscsi_driver_with_transport(self):
self.flags(iscsi_transport='fake_transport', group='libvirt')
self.flags(iscsi_iface='fake_transport', group='libvirt')
self.test_libvirt_iscsi_driver('fake_transport')
def test_libvirt_iscsi_driver_still_in_use(self, transport=None):
@ -397,6 +421,8 @@ Setting up iSCSI targets: unused
if transport is not None:
self.stubs.Set(libvirt_driver, '_get_host_device',
lambda x: self.generate_device(transport, 1, False))
self.stubs.Set(libvirt_driver, '_validate_transport',
lambda x: True)
devs = [self.generate_device(transport, 2, False)]
self.stubs.Set(self.fake_conn, '_get_all_block_devices', lambda: devs)
vol = {'id': 1, 'name': self.name}
@ -418,7 +444,7 @@ Setting up iSCSI targets: unused
self.assertEqual(self.executes, expected_commands)
def test_libvirt_iscsi_driver_still_in_use_with_transport(self):
self.flags(iscsi_transport='fake_transport', group='libvirt')
self.flags(iscsi_iface='fake_transport', group='libvirt')
self.test_libvirt_iscsi_driver_still_in_use('fake_transport')
def test_libvirt_iscsi_driver_disconnect_multipath_error(self,
@ -479,7 +505,7 @@ Setting up iSCSI targets: unused
self.assertEqual(dev_path, tree.find('./source').get('dev'))
def test_libvirt_iscsi_driver_get_config_with_transport(self):
self.flags(iscsi_transport = 'fake_transport', group='libvirt')
self.flags(iscsi_iface = 'fake_transport', group='libvirt')
self.test_libvirt_iscsi_driver_get_config('fake_transport')
def test_libvirt_iscsi_driver_multipath_id(self):

View File

@ -102,11 +102,11 @@ volume_opts = [
'compute node'),
cfg.StrOpt('quobyte_client_cfg',
help='Path to a Quobyte Client configuration file.'),
cfg.StrOpt('iscsi_transport',
default=None,
help='The iSCSI transport to use to connect to target in case '
'offload support is desired. Supported transports are '
'be2iscsi, bnx2i, cxgb3i, cxgb4i, qla4xx and ocs. '
cfg.StrOpt('iscsi_iface',
deprecated_name='iscsi_transport',
help='The iSCSI transport iface to use to connect to target in '
'case offload support is desired. Supported transports '
'are be2iscsi, bnx2i, cxgb3i, cxgb4i, qla4xxx and ocs. '
'Default format is transport_name.hwaddress and can be '
'generated manually or via iscsiadm -m iface'),
# iser is also supported, but use LibvirtISERVolumeDriver
@ -296,18 +296,57 @@ class LibvirtNetVolumeDriver(LibvirtBaseVolumeDriver):
class LibvirtISCSIVolumeDriver(LibvirtBaseVolumeDriver):
"""Driver to attach Network volumes to libvirt."""
supported_transports = ['be2iscsi', 'bnx2i', 'cxgb3i',
'cxgb4i', 'qla4xxx', 'ocs']
def __init__(self, connection):
super(LibvirtISCSIVolumeDriver, self).__init__(connection,
is_block_dev=True)
self.num_scan_tries = CONF.libvirt.num_iscsi_scan_tries
self.use_multipath = CONF.libvirt.iscsi_use_multipath
if CONF.libvirt.iscsi_transport:
self.transport = CONF.libvirt.iscsi_transport
if CONF.libvirt.iscsi_iface:
self.transport = CONF.libvirt.iscsi_iface
else:
self.transport = 'default'
def _get_transport(self):
if self._validate_transport(self.transport):
return self.transport
else:
return 'default'
def _validate_transport(self, transport_iface):
"""Check that given iscsi_iface uses only supported transports
Accepted transport names for provided iface param are
be2iscsi, bnx2i, cxgb3i, cxgb4i, qla4xxx and ocs. iSER uses it's
own separate driver. Note the difference between transport and
iface; unlike iscsi_tcp/iser, this is not one and the same for
offloaded transports, where the default format is
transport_name.hwaddress
"""
# We can support iser here as well, but currently reject it as the
# separate iser driver has not yet been deprecated.
if transport_iface == 'default':
return True
# Will return (6) if iscsi_iface file was not found, or (2) if iscsid
# could not be contacted
out = self._run_iscsiadm_bare(['-m',
'iface',
'-I',
transport_iface],
check_exit_code=[0, 2, 6])[0] or ""
LOG.debug("iscsiadm %(iface)s configuration: stdout=%(out)s",
{'iface': transport_iface, 'out': out})
for data in [line.split() for line in out.splitlines()]:
if data[0] == 'iface.transport_name':
if data[2] in self.supported_transports:
return True
LOG.warn(_LW("No useable transport found for iscsi iface %s. "
"Falling back to default transport"),
transport_iface)
return False
def _run_iscsiadm(self, iscsi_properties, iscsi_command, **kwargs):
check_exit_code = kwargs.pop('check_exit_code', 0)