From 8c33a55a886389280d80fdb8017598e2d08b10d1 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Mon, 24 Jul 2023 12:00:07 +0200 Subject: [PATCH] Blivet3.6.0-7 Backport iSCSI initiator name related fixes: - Allow changing iSCSI initiator name after setting it Resolves: rhbz#2083139 - Add a basic test case for the iscsi module Related: rhbz#2083139 - tests: Use blivet-specific prefix for targetcli backing files Related: rhbz#2083139 - iscsi: Save firmware initiator name to /etc/iscsi/initiatorname.iscsi Resolves: rhbz#2084043 - tests: Improve iscsi_test.ISCSITestCase Related: rhbz#2083139 --- ...t-iSCSI-initiator-name-related-fixes.patch | 382 ++++++++++++++++++ python-blivet.spec | 16 +- 2 files changed, 397 insertions(+), 1 deletion(-) create mode 100644 0016-Backport-iSCSI-initiator-name-related-fixes.patch diff --git a/0016-Backport-iSCSI-initiator-name-related-fixes.patch b/0016-Backport-iSCSI-initiator-name-related-fixes.patch new file mode 100644 index 0000000..f61a188 --- /dev/null +++ b/0016-Backport-iSCSI-initiator-name-related-fixes.patch @@ -0,0 +1,382 @@ +From d06c45db59d0e917dbab4c283f2f04c8f9206a6e Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Mon, 6 Mar 2023 10:51:42 +0100 +Subject: [PATCH 1/5] Allow changing iSCSI initiator name after setting it + +Resolves: rhbz#2083139 +--- + blivet/iscsi.py | 13 +++++++++++-- + 1 file changed, 11 insertions(+), 2 deletions(-) + +diff --git a/blivet/iscsi.py b/blivet/iscsi.py +index 86451db3..0d063f2a 100644 +--- a/blivet/iscsi.py ++++ b/blivet/iscsi.py +@@ -212,14 +212,23 @@ class iSCSI(object): + @initiator.setter + @storaged_iscsi_required(critical=True, eval_mode=util.EvalMode.onetime) + def initiator(self, val): +- if self.initiator_set and val != self._initiator: +- raise ValueError(_("Unable to change iSCSI initiator name once set")) + if len(val) == 0: + raise ValueError(_("Must provide an iSCSI initiator name")) + ++ active = self._get_active_sessions() ++ if active: ++ raise errors.ISCSIError(_("Cannot change initiator name with an active session")) ++ + log.info("Setting up iSCSI initiator name %s", self.initiator) + args = GLib.Variant("(sa{sv})", (val, None)) + self._call_initiator_method("SetInitiatorName", args) ++ ++ if self.initiator_set and val != self._initiator: ++ log.info("Restarting iscsid after initiator name change") ++ rc = util.run_program(["systemctl", "restart", "iscsid"]) ++ if rc != 0: ++ raise errors.ISCSIError(_("Failed to restart iscsid after initiator name change")) ++ + self._initiator = val + + def active_nodes(self, target=None): +-- +2.40.1 + + +From b71991d65c270c023364b03c499b4bf3e245fbd0 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Mon, 6 Mar 2023 15:10:28 +0100 +Subject: [PATCH 2/5] Add a basic test case for the iscsi module + +Related: rhbz#2083139 +--- + tests/storage_tests/__init__.py | 2 + + tests/storage_tests/iscsi_test.py | 157 +++++++++++++++++++++++++++++ + 3 files changed, 162 insertions(+) + create mode 100644 tests/storage_tests/iscsi_test.py + +diff --git a/tests/storage_tests/__init__.py b/tests/storage_tests/__init__.py +index 3b2a6cc4..e69fcc34 100644 +--- a/tests/storage_tests/__init__.py ++++ b/tests/storage_tests/__init__.py +@@ -3,3 +3,5 @@ from .formats_test import * + + from .partitioning_test import * + from .unsupported_disklabel_test import * ++ ++from .iscsi_test import * +diff --git a/tests/storage_tests/iscsi_test.py b/tests/storage_tests/iscsi_test.py +new file mode 100644 +index 00000000..00cc7c36 +--- /dev/null ++++ b/tests/storage_tests/iscsi_test.py +@@ -0,0 +1,157 @@ ++import glob ++import os ++import re ++import shutil ++import subprocess ++import unittest ++ ++from contextlib import contextmanager ++ ++from .storagetestcase import create_sparse_tempfile ++ ++ ++def read_file(filename, mode="r"): ++ with open(filename, mode) as f: ++ content = f.read() ++ return content ++ ++ ++@contextmanager ++def udev_settle(): ++ try: ++ yield ++ finally: ++ os.system("udevadm settle") ++ ++ ++def _delete_backstore(name): ++ status = subprocess.call(["targetcli", "/backstores/fileio/ delete %s" % name], ++ stdout=subprocess.DEVNULL) ++ if status != 0: ++ raise RuntimeError("Failed to delete the '%s' fileio backstore" % name) ++ ++ ++def delete_iscsi_target(iqn, backstore=None): ++ status = subprocess.call(["targetcli", "/iscsi delete %s" % iqn], ++ stdout=subprocess.DEVNULL) ++ if status != 0: ++ raise RuntimeError("Failed to delete the '%s' iscsi device" % iqn) ++ ++ if backstore is not None: ++ _delete_backstore(backstore) ++ ++ ++def create_iscsi_target(fpath, initiator_name=None): ++ """ ++ Creates a new iSCSI target (using targetcli) on top of the ++ :param:`fpath` backing file. ++ ++ :param str fpath: path of the backing file ++ :returns: iSCSI IQN, backstore name ++ :rtype: tuple of str ++ ++ """ ++ ++ # "register" the backing file as a fileio backstore ++ store_name = os.path.basename(fpath) ++ status = subprocess.call(["targetcli", "/backstores/fileio/ create %s %s" % (store_name, fpath)], stdout=subprocess.DEVNULL) ++ if status != 0: ++ raise RuntimeError("Failed to register '%s' as a fileio backstore" % fpath) ++ ++ out = subprocess.check_output(["targetcli", "/backstores/fileio/%s info" % store_name]) ++ out = out.decode("utf-8") ++ store_wwn = None ++ for line in out.splitlines(): ++ if line.startswith("wwn: "): ++ store_wwn = line[5:] ++ if store_wwn is None: ++ raise RuntimeError("Failed to determine '%s' backstore's wwn" % store_name) ++ ++ # create a new iscsi device ++ out = subprocess.check_output(["targetcli", "/iscsi create"]) ++ out = out.decode("utf-8") ++ match = re.match(r'Created target (.*).', out) ++ if match: ++ iqn = match.groups()[0] ++ else: ++ _delete_backstore(store_name) ++ raise RuntimeError("Failed to create a new iscsi target") ++ ++ if initiator_name: ++ status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/acls create %s" % (iqn, initiator_name)], stdout=subprocess.DEVNULL) ++ if status != 0: ++ delete_iscsi_target(iqn, store_name) ++ raise RuntimeError("Failed to set ACLs for '%s'" % iqn) ++ ++ with udev_settle(): ++ status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/luns create /backstores/fileio/%s" % (iqn, store_name)], stdout=subprocess.DEVNULL) ++ if status != 0: ++ delete_iscsi_target(iqn, store_name) ++ raise RuntimeError("Failed to create a new LUN for '%s' using '%s'" % (iqn, store_name)) ++ ++ status = subprocess.call(["targetcli", "/iscsi/%s/tpg1 set attribute generate_node_acls=1" % iqn], stdout=subprocess.DEVNULL) ++ if status != 0: ++ raise RuntimeError("Failed to set ACLs for '%s'" % iqn) ++ ++ return iqn, store_name ++ ++ ++@unittest.skipUnless(os.geteuid() == 0, "requires root privileges") ++@unittest.skipUnless(os.environ.get("JENKINS_HOME"), "jenkins only test") ++@unittest.skipUnless(shutil.which("iscsiadm"), "iscsiadm not available") ++class ISCSITestCase(unittest.TestCase): ++ ++ _disk_size = 512 * 1024**2 ++ initiator = 'iqn.1994-05.com.redhat:iscsi-test' ++ ++ def setUp(self): ++ self.addCleanup(self._clean_up) ++ ++ self._dev_file = None ++ self.dev = None ++ ++ self._dev_file = create_sparse_tempfile("blivet_test", self._disk_size) ++ try: ++ self.dev, self.backstore = create_iscsi_target(self._dev_file, self.initiator) ++ except RuntimeError as e: ++ raise RuntimeError("Failed to setup targetcli device for testing: %s" % e) ++ ++ def _force_logout(self): ++ subprocess.call(["iscsiadm", "--mode", "node", "--logout", "--name", self.dev], stdout=subprocess.DEVNULL) ++ ++ def _clean_up(self): ++ self._force_logout() ++ delete_iscsi_target(self.dev, self.backstore) ++ os.unlink(self._dev_file) ++ ++ def test_discover_login(self): ++ from blivet.iscsi import iscsi, has_iscsi ++ ++ if not has_iscsi(): ++ self.skipTest("iSCSI not available, skipping") ++ ++ iscsi.initiator = self.initiator ++ nodes = iscsi.discover("127.0.0.1") ++ self.assertTrue(nodes) ++ ++ if len(nodes) > 1: ++ self.skipTest("Discovered more than one iSCSI target on localhost, skipping") ++ ++ self.assertEqual(nodes[0].address, "127.0.0.1") ++ self.assertEqual(nodes[0].port, 3260) ++ self.assertEqual(nodes[0].name, self.dev) ++ ++ # change the initiator name ++ iscsi.initiator = self.initiator + "_1" ++ self.assertEqual(iscsi.initiator, self.initiator + "_1") ++ ++ # try to login ++ ret, err = iscsi.log_into_node(nodes[0]) ++ self.assertTrue(ret, "Login failed: %s" % err) ++ ++ # check the session for initiator name ++ sessions = glob.glob("/sys/class/iscsi_session/*/") ++ self.assertTrue(sessions) ++ self.assertEqual(len(sessions), 1) ++ initiator = read_file(sessions[0] + "initiatorname").strip() ++ self.assertEqual(initiator, iscsi.initiator) +-- +2.40.1 + + +From 65e8150a7404e37dd2740841a88e7f2565836406 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Mon, 6 Mar 2023 15:14:40 +0100 +Subject: [PATCH 3/5] tests: Use blivet-specific prefix for targetcli backing + files + +The code is originally from libblockdev hence the "bd" prefix, we +should use a different prefix for blivet to be able to identify +which test suite failed to clean the files. + +Related: rhbz#2083139 +--- + tests/storage_tests/storagetestcase.py | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tests/storage_tests/storagetestcase.py b/tests/storage_tests/storagetestcase.py +index 35d57ce9..9f859977 100644 +--- a/tests/storage_tests/storagetestcase.py ++++ b/tests/storage_tests/storagetestcase.py +@@ -39,7 +39,7 @@ def create_sparse_tempfile(name, size): + :param size: the file size (in bytes) + :returns: the path to the newly created file + """ +- (fd, path) = tempfile.mkstemp(prefix="bd.", suffix="-%s" % name) ++ (fd, path) = tempfile.mkstemp(prefix="blivet.", suffix="-%s" % name) + os.close(fd) + create_sparse_file(path, size) + return path +-- +2.40.1 + + +From 41278ef1b3f949303fd30fff2ccdde75f713c9f8 Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Wed, 19 Jul 2023 13:57:39 +0200 +Subject: [PATCH 4/5] iscsi: Save firmware initiator name to + /etc/iscsi/initiatorname.iscsi + +Resolves: rhbz#2084043 +--- + blivet/iscsi.py | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/blivet/iscsi.py b/blivet/iscsi.py +index 0d063f2a..8080a671 100644 +--- a/blivet/iscsi.py ++++ b/blivet/iscsi.py +@@ -160,6 +160,11 @@ class iSCSI(object): + self._initiator = initiatorname + except Exception as e: # pylint: disable=broad-except + log.info("failed to get initiator name from iscsi firmware: %s", str(e)) ++ else: ++ # write the firmware initiator to /etc/iscsi/initiatorname.iscsi ++ log.info("Setting up firmware iSCSI initiator name %s", self.initiator) ++ args = GLib.Variant("(sa{sv})", (initiatorname, None)) ++ self._call_initiator_method("SetInitiatorName", args) + + # So that users can write iscsi() to get the singleton instance + def __call__(self): +-- +2.40.1 + + +From fce8b73965d968aab546bc7e0ecb65d1995da46f Mon Sep 17 00:00:00 2001 +From: Vojtech Trefny +Date: Wed, 19 Jul 2023 10:38:45 +0200 +Subject: [PATCH 5/5] tests: Improve iscsi_test.ISCSITestCase + +Changed how we create the initiator name ACLs based on RTT test +case for rhbz#2084043 and also improved the test case itself. + +Related: rhbz#2083139 +--- + tests/storage_tests/iscsi_test.py | 36 +++++++++++++++++++++---------- + 1 file changed, 25 insertions(+), 11 deletions(-) + +diff --git a/tests/storage_tests/iscsi_test.py b/tests/storage_tests/iscsi_test.py +index 00cc7c36..6cc83a59 100644 +--- a/tests/storage_tests/iscsi_test.py ++++ b/tests/storage_tests/iscsi_test.py +@@ -77,21 +77,17 @@ def create_iscsi_target(fpath, initiator_name=None): + _delete_backstore(store_name) + raise RuntimeError("Failed to create a new iscsi target") + +- if initiator_name: +- status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/acls create %s" % (iqn, initiator_name)], stdout=subprocess.DEVNULL) +- if status != 0: +- delete_iscsi_target(iqn, store_name) +- raise RuntimeError("Failed to set ACLs for '%s'" % iqn) +- + with udev_settle(): + status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/luns create /backstores/fileio/%s" % (iqn, store_name)], stdout=subprocess.DEVNULL) + if status != 0: + delete_iscsi_target(iqn, store_name) + raise RuntimeError("Failed to create a new LUN for '%s' using '%s'" % (iqn, store_name)) + +- status = subprocess.call(["targetcli", "/iscsi/%s/tpg1 set attribute generate_node_acls=1" % iqn], stdout=subprocess.DEVNULL) +- if status != 0: +- raise RuntimeError("Failed to set ACLs for '%s'" % iqn) ++ if initiator_name: ++ status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/acls create %s" % (iqn, initiator_name)], stdout=subprocess.DEVNULL) ++ if status != 0: ++ delete_iscsi_target(iqn, store_name) ++ raise RuntimeError("Failed to set ACLs for '%s'" % iqn) + + return iqn, store_name + +@@ -130,6 +126,7 @@ class ISCSITestCase(unittest.TestCase): + if not has_iscsi(): + self.skipTest("iSCSI not available, skipping") + ++ # initially set the initiator to the correct/allowed one + iscsi.initiator = self.initiator + nodes = iscsi.discover("127.0.0.1") + self.assertTrue(nodes) +@@ -141,11 +138,28 @@ class ISCSITestCase(unittest.TestCase): + self.assertEqual(nodes[0].port, 3260) + self.assertEqual(nodes[0].name, self.dev) + +- # change the initiator name ++ # change the initiator name to a wrong one + iscsi.initiator = self.initiator + "_1" + self.assertEqual(iscsi.initiator, self.initiator + "_1") + +- # try to login ++ # check the change made it to /etc/iscsi/initiatorname.iscsi ++ initiator_file = read_file("/etc/iscsi/initiatorname.iscsi").strip() ++ self.assertEqual(initiator_file, "InitiatorName=%s" % self.initiator + "_1") ++ ++ # try to login (should fail) ++ ret, err = iscsi.log_into_node(nodes[0]) ++ self.assertFalse(ret) ++ self.assertIn("authorization failure", err) ++ ++ # change the initiator name back to the correct one ++ iscsi.initiator = self.initiator ++ self.assertEqual(iscsi.initiator, self.initiator) ++ ++ # check the change made it to /etc/iscsi/initiatorname.iscsi ++ initiator_file = read_file("/etc/iscsi/initiatorname.iscsi").strip() ++ self.assertEqual(initiator_file, "InitiatorName=%s" % self.initiator) ++ ++ # try to login (should work now) + ret, err = iscsi.log_into_node(nodes[0]) + self.assertTrue(ret, "Login failed: %s" % err) + +-- +2.40.1 + diff --git a/python-blivet.spec b/python-blivet.spec index aed32ae..95ec1f5 100644 --- a/python-blivet.spec +++ b/python-blivet.spec @@ -23,7 +23,7 @@ Version: 3.6.0 #%%global prerelease .b2 # prerelease, if defined, should be something like .a1, .b1, .b2.dev1, or .c2 -Release: 6%{?prerelease}%{?dist} +Release: 7%{?prerelease}%{?dist} Epoch: 1 License: LGPLv2+ %global realname blivet @@ -45,6 +45,7 @@ Patch11: 0012-Add-support-for-specifying-stripe-size-for-RAID-LVs.patch Patch12: 0013-Fix-setting-kickstart-data.patch Patch13: 0014-Do-not-set-memory-limit-for-LUKS2-when-running-in-FI.patch Patch14: 0015-Add-support-for-filesystem-online-resize.patch +Patch15: 0016-Backport-iSCSI-initiator-name-related-fixes.patch # Versions of required components (done so we make sure the buildrequires # match the requires versions of things). @@ -207,6 +208,19 @@ configuration. %endif %changelog +* Mon Jul 24 2023 Jan Pokorny - 3.6.0-7 +Backport iSCSI initiator name related fixes: +- Allow changing iSCSI initiator name after setting it + Resolves: rhbz#2083139 +- Add a basic test case for the iscsi module + Related: rhbz#2083139 +- tests: Use blivet-specific prefix for targetcli backing files + Related: rhbz#2083139 +- iscsi: Save firmware initiator name to /etc/iscsi/initiatorname.iscsi + Resolves: rhbz#2084043 +- tests: Improve iscsi_test.ISCSITestCase + Related: rhbz#2083139 + * Thu May 18 2023 Vojtech Trefny - 3.6.0-6 - Fix setting kickstart data Resolves: rhbz#2175166