Permalink
Browse files

Merge "Fixes nova-volume support for multiple luns"

  • Loading branch information...
2 parents 492acb2 + 509e5ad commit 53b2cc83c6329c112ce69d4d2c45fa7686347d00 Jenkins committed with openstack-gerrit Feb 17, 2012
View
@@ -124,7 +124,11 @@ def __init__(self, hyperv="QEMU"):
def get_hypervisor_type(self):
return self.hyperv
- self.fake_conn = FakeLibvirtConnection("Xen")
+
+ def get_all_block_devices(self):
+ return []
+
+ self.fake_conn = FakeLibvirtConnection()
self.connr = {
'ip': '127.0.0.1',
'initiator': 'fake_initiator'
@@ -167,6 +171,38 @@ def test_libvirt_iscsi_driver(self):
'-p', location, '--op', 'delete')]
self.assertEqual(self.executes, expected_commands)
+ def test_libvirt_iscsi_driver_still_in_use(self):
+ # NOTE(vish) exists is to make driver assume connecting worked
+ self.stubs.Set(os.path, 'exists', lambda x: True)
+ vol_driver = volume_driver.ISCSIDriver()
+ libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn)
+ location = '10.0.2.15:3260'
+ name = 'volume-00000001'
+ iqn = 'iqn.2010-10.org.openstack:%s' % name
+ devs = ['/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location, iqn)]
+ self.stubs.Set(self.fake_conn, 'get_all_block_devices', lambda: devs)
+ vol = {'id': 1,
+ 'name': name,
+ 'provider_auth': None,
+ 'provider_location': '%s,fake %s' % (location, iqn)}
+ connection_info = vol_driver.initialize_connection(vol, self.connr)
+ mount_device = "vde"
+ xml = libvirt_driver.connect_volume(connection_info, mount_device)
+ tree = xml_to_tree(xml)
+ dev_str = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-0' % (location, iqn)
+ self.assertEqual(tree.get('type'), 'block')
+ self.assertEqual(tree.find('./source').get('dev'), dev_str)
+ libvirt_driver.disconnect_volume(connection_info, mount_device)
+ connection_info = vol_driver.terminate_connection(vol, self.connr)
+ expected_commands = [('iscsiadm', '-m', 'node', '-T', iqn,
+ '-p', location),
+ ('iscsiadm', '-m', 'node', '-T', iqn,
+ '-p', location, '--login'),
+ ('iscsiadm', '-m', 'node', '-T', iqn,
+ '-p', location, '--op', 'update',
+ '-n', 'node.startup', '-v', 'automatic')]
+ self.assertEqual(self.executes, expected_commands)
+
def test_libvirt_sheepdog_driver(self):
vol_driver = volume_driver.SheepdogDriver()
libvirt_driver = volume.LibvirtNetVolumeDriver(self.fake_conn)
@@ -469,6 +505,57 @@ def test_list_instances(self):
# Only one should be listed, since domain with ID 0 must be skiped
self.assertEquals(len(instances), 1)
+ def test_get_all_block_devices(self):
+ xml = [
+ # NOTE(vish): id 0 is skipped
+ None,
+ """
+ <domain type='kvm'>
+ <devices>
+ <disk type='file'>
+ <source file='filename'/>
+ </disk>
+ <disk type='block'>
+ <source dev='/path/to/dev/1'/>
+ </disk>
+ </devices>
+ </domain>
+ """,
+ """
+ <domain type='kvm'>
+ <devices>
+ <disk type='file'>
+ <source file='filename'/>
+ </disk>
+ </devices>
+ </domain>
+ """,
+ """
+ <domain type='kvm'>
+ <devices>
+ <disk type='file'>
+ <source file='filename'/>
+ </disk>
+ <disk type='block'>
+ <source dev='/path/to/dev/3'/>
+ </disk>
+ </devices>
+ </domain>
+ """,
+ ]
+
+ def fake_lookup(id):
+ return FakeVirtDomain(xml[id])
+
+ self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn')
+ connection.LibvirtConnection._conn.listDomainsID = lambda: range(4)
+ connection.LibvirtConnection._conn.lookupByID = fake_lookup
+
+ self.mox.ReplayAll()
+ conn = connection.LibvirtConnection(False)
+ devices = conn.get_all_block_devices()
+ self.assertEqual(devices, ['/path/to/dev/1', '/path/to/dev/3'])
+
@test.skip_if(missing_libvirt(), "Test requires libvirt")
def test_snapshot_in_ami_format(self):
self.flags(image_service='nova.image.fake.FakeImageService')
@@ -132,6 +132,7 @@ def _make_info():
'volume_id': 1,
'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
'target_portal': '127.0.0.1:3260,fake',
+ 'target_lun': None,
'auth_method': 'CHAP',
'auth_method': 'fake',
'auth_method': 'fake',
@@ -1392,6 +1392,26 @@ def _create_new_domain(self, xml, persistent=True, launch_flags=0):
return domain
+ def get_all_block_devices(self):
+ """
+ Return all block devices in use on this node.
+ """
+ devices = []
+ for dom_id in self._conn.listDomainsID():
+ domain = self._conn.lookupByID(dom_id)
+ try:
+ doc = ElementTree.fromstring(domain.XMLDesc(0))
+ except Exception:
+ continue
+ ret = doc.findall('./devices/disk')
+ for node in ret:
+ if node.get('type') != 'block':
+ continue
+ for child in node.getchildren():
+ if child.tag == 'source':
+ devices.append(child.get('dev'))
+ return devices
+
def get_disks(self, instance_name):
"""
Note that this function takes an instance name.
@@ -103,14 +103,16 @@ def _iscsiadm_update(self, iscsi_properties, property_key, property_value):
'-v', property_value)
return self._run_iscsiadm(iscsi_properties, iscsi_command)
+ @utils.synchronized('connect_volume')
def connect_volume(self, connection_info, mount_device):
"""Attach the volume to instance_name"""
iscsi_properties = connection_info['data']
- # NOTE(vish): if we are on the same host as nova volume, the
+ # NOTE(vish): If we are on the same host as nova volume, the
# discovery makes the target so we don't need to
# run --op new. Therefore, we check to see if the
# target exists, and if we get 255 (Not Found), then
- # we run --op new
+ # we run --op new. This will also happen if another
+ # volume is using the same target.
try:
self._run_iscsiadm(iscsi_properties, ())
except exception.ProcessExecutionError as exc:
@@ -131,18 +133,17 @@ def connect_volume(self, connection_info, mount_device):
"node.session.auth.password",
iscsi_properties['auth_password'])
- self._run_iscsiadm(iscsi_properties, ("--login",))
+ # NOTE(vish): If we have another lun on the same target, we may
+ # have a duplicate login
+ self._run_iscsiadm(iscsi_properties, ("--login",),
+ check_exit_code=[0, 255])
self._iscsiadm_update(iscsi_properties, "node.startup", "automatic")
- if FLAGS.iscsi_helper == 'tgtadm':
- host_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-1" %
- (iscsi_properties['target_portal'],
- iscsi_properties['target_iqn']))
- else:
- host_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-0" %
- (iscsi_properties['target_portal'],
- iscsi_properties['target_iqn']))
+ host_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-%s" %
+ (iscsi_properties['target_portal'],
+ iscsi_properties['target_iqn'],
+ iscsi_properties.get('target_lun', 0)))
# The /dev/disk/by-path/... node is not always present immediately
# TODO(justinsb): This retry-with-delay is a pattern, move to utils?
@@ -172,13 +173,22 @@ def connect_volume(self, connection_info, mount_device):
sup = super(LibvirtISCSIVolumeDriver, self)
return sup.connect_volume(connection_info, mount_device)
+ @utils.synchronized('connect_volume')
def disconnect_volume(self, connection_info, mount_device):
"""Detach the volume from instance_name"""
sup = super(LibvirtISCSIVolumeDriver, self)
sup.disconnect_volume(connection_info, mount_device)
iscsi_properties = connection_info['data']
- self._iscsiadm_update(iscsi_properties, "node.startup", "manual")
- self._run_iscsiadm(iscsi_properties, ("--logout",),
- check_exit_code=[0, 255])
- self._run_iscsiadm(iscsi_properties, ('--op', 'delete'),
- check_exit_code=[0, 255])
+ # NOTE(vish): Only disconnect from the target if no luns from the
+ # target are in use.
+ device_prefix = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-" %
+ (iscsi_properties['target_portal'],
+ iscsi_properties['target_iqn']))
+ devices = self.connection.get_all_block_devices()
+ devices = [dev for dev in devices if dev.startswith(device_prefix)]
+ if not devices:
+ self._iscsiadm_update(iscsi_properties, "node.startup", "manual")
+ self._run_iscsiadm(iscsi_properties, ("--logout",),
+ check_exit_code=[0, 255])
+ self._run_iscsiadm(iscsi_properties, ('--op', 'delete'),
+ check_exit_code=[0, 255])
View
@@ -346,7 +346,7 @@ def get_vnc_console(self, instance):
def get_volume_connector(self, _instance):
"""Return volume connector information"""
if not self._initiator:
- stats = self.get_host_stats(update=True)
+ stats = self.get_host_stats(refresh=True)
try:
self._initiator = stats['host_other-config']['iscsi_iqn']
except (TypeError, KeyError):
View
@@ -295,8 +295,12 @@ def create_export(self, context, volume):
self.tgtadm.new_logicalunit(iscsi_target, 0, volume_path)
model_update = {}
+ if FLAGS.iscsi_helper == 'tgtadm':
+ lun = 1
+ else:
+ lun = 0
model_update['provider_location'] = _iscsi_location(
- FLAGS.iscsi_ip_address, iscsi_target, iscsi_name)
+ FLAGS.iscsi_ip_address, iscsi_target, iscsi_name, lun)
return model_update
def remove_export(self, context, volume):
@@ -349,6 +353,8 @@ def _get_iscsi_properties(self, volume):
:target_portal: the portal of the iSCSI target
+ :target_lun: the lun of the iSCSI target
+
:volume_id: the id of the volume (currently used by xen)
:auth_method:, :auth_username:, :auth_password:
@@ -376,16 +382,20 @@ def _get_iscsi_properties(self, volume):
LOG.debug(_("ISCSI Discovery: Found %s") % (location))
properties['target_discovered'] = True
- (iscsi_target, _sep, iscsi_name) = location.partition(" ")
-
- iscsi_portal = iscsi_target.split(",")[0]
+ results = location.split(" ")
+ properties['target_portal'] = results[0].split(",")[0]
+ properties['target_iqn'] = results[1]
+ try:
+ properties['target_lun'] = int(results[2])
+ except (IndexError, ValueError):
+ if FLAGS.iscsi_helper == 'tgtadm':
+ properties['target_lun'] = 1
+ else:
+ properties['target_lun'] = 0
properties['volume_id'] = volume['id']
- properties['target_iqn'] = iscsi_name
- properties['target_portal'] = iscsi_portal
auth = volume['provider_auth']
-
if auth:
(auth_method, auth_username, auth_secret) = auth.split()
@@ -794,14 +804,10 @@ def _discover_volume(self, context, volume):
self._iscsiadm_update(iscsi_properties, "node.startup", "automatic")
- if FLAGS.iscsi_helper == 'tgtadm':
- mount_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-1" %
- (iscsi_properties['target_portal'],
- iscsi_properties['target_iqn']))
- else:
- mount_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-0" %
- (iscsi_properties['target_portal'],
- iscsi_properties['target_iqn']))
+ mount_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-%s" %
+ (iscsi_properties['target_portal'],
+ iscsi_properties['target_iqn'],
+ iscsi_properties['target_lun']))
# The /dev/disk/by-path/... node is not always present immediately
# TODO(justinsb): This retry-with-delay is a pattern, move to utils?
@@ -1007,5 +1013,5 @@ def get_volume_stats(self, refresh=False):
return {'drive_qos_info': drive_info}
-def _iscsi_location(ip, target, iqn):
- return "%s:%s,%s %s" % (ip, FLAGS.iscsi_port, target, iqn)
+def _iscsi_location(ip, target, iqn, lun=None):
+ return "%s:%s,%s %s %s" % (ip, FLAGS.iscsi_port, target, iqn, lun)

0 comments on commit 53b2cc8

Please sign in to comment.