From 509e5ad9ef6ed38caa69d2c76b1adccedcdd315e Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 9 Feb 2012 15:05:21 -0800 Subject: [PATCH] Fixes nova-volume support for multiple luns * stores lun in provider_location if specified * passes lun in iscsi_properties instead of hard coding * adds call to libvirt to list all used block devices * make sure to synchronize connect and disconnect commands * only disconnect from target if no luns are in use * allow double logins to targets * fixes typo in get_volume_connector in xenapi_connection * fixes bug 929790 Change-Id: I2466dc750a6fa5e0b07f94314d38873740aa6b29 --- nova/tests/test_libvirt.py | 89 ++++++++++++++++++++++++++++++++- nova/tests/test_xenapi.py | 1 + nova/virt/libvirt/connection.py | 20 ++++++++ nova/virt/libvirt/volume.py | 42 ++++++++++------ nova/virt/xenapi_conn.py | 2 +- nova/volume/driver.py | 40 ++++++++------- 6 files changed, 159 insertions(+), 35 deletions(-) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 2eede7ed5930..f0f1520ee9f5 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -124,7 +124,11 @@ class LibvirtVolumeTestCase(test.TestCase): 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 @@ class LibvirtVolumeTestCase(test.TestCase): '-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 @@ class LibvirtConnTestCase(test.TestCase): # 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, + """ + + + + + + + + + + + """, + """ + + + + + + + + """, + """ + + + + + + + + + + + """, + ] + + 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') diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 2673977f4fa3..f3b48a56c184 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -132,6 +132,7 @@ class XenAPIVolumeTestCase(test.TestCase): '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', diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py index c3bfdedda68c..349faa157020 100644 --- a/nova/virt/libvirt/connection.py +++ b/nova/virt/libvirt/connection.py @@ -1388,6 +1388,26 @@ class LibvirtConnection(driver.ComputeDriver): 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. diff --git a/nova/virt/libvirt/volume.py b/nova/virt/libvirt/volume.py index 75fa0e5a95f4..055e34abc796 100644 --- a/nova/virt/libvirt/volume.py +++ b/nova/virt/libvirt/volume.py @@ -103,14 +103,16 @@ class LibvirtISCSIVolumeDriver(LibvirtVolumeDriver): '-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 @@ class LibvirtISCSIVolumeDriver(LibvirtVolumeDriver): "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 @@ class LibvirtISCSIVolumeDriver(LibvirtVolumeDriver): 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]) diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py index 1e2d11bb08fe..08542268c2fe 100644 --- a/nova/virt/xenapi_conn.py +++ b/nova/virt/xenapi_conn.py @@ -346,7 +346,7 @@ class XenAPIConnection(driver.ComputeDriver): 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): diff --git a/nova/volume/driver.py b/nova/volume/driver.py index a92c51c6b169..9249918607f3 100644 --- a/nova/volume/driver.py +++ b/nova/volume/driver.py @@ -295,8 +295,12 @@ class ISCSIDriver(VolumeDriver): 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 @@ class ISCSIDriver(VolumeDriver): :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 @@ class ISCSIDriver(VolumeDriver): 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 @@ class ZadaraBEDriver(ISCSIDriver): 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 @@ class ZadaraBEDriver(ISCSIDriver): 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)

AltStyle によって変換されたページ (->オリジナル) /