iSCSI: don't crash when LUN ID >= 256

[vojtechtrefny]

Backport of #1427

Summary by Sourcery

Support hex-encoded iSCSI LUN values and add tests to handle large LUN IDs without crashing

Bug Fixes:

• Prevent crash when parsing iSCSI LUN identifiers greater than or equal to 256

Enhancements:

• Add support for parsing hex-formatted LUN numbers matching systemd’s format_lun_number

Tests:

• Add unit tests for decimal and hex LUN parsing in iScsiDiskDevice

[sourcery-ai]

Reviewer's Guide

This backport enhances LUN parsing in the iScsiDiskDevice constructor to handle 64-bit hex-encoded LUN strings (preventing crashes when LUN ID ≥ 256) and adds unit tests to verify both decimal and hex LUN inputs.

Class diagram for updated iScsiDiskDevice LUN parsing

classDiagram
    class iScsiDiskDevice {
        +offload
        +name
        +target
        +lun
        __init__(device, **kwargs)
    }
    note for iScsiDiskDevice "LUN parsing in __init__ now supports both decimal and 64-bit hex-encoded string formats."

Loading

Flow diagram for LUN parsing logic in iScsiDiskDevice

flowchart TD
    A["Input LUN value (lun)"] --> B["Try: int(lun)"]
    B -- success --> C["Set self.lun = int(lun)"]
    B -- ValueError --> D["Match hex pattern: 0x([0-9a-fA-F]{4})([0-9a-fA-F]{4})00000000"]
    D -- match --> E["self.lun = int(m.group(1), 16) + (1 << 16) * int(m.group(2), 16)"]
    D -- no match --> F["Log warning, set self.lun = None"]
    B -- TypeError --> F

Loading

File-Level Changes

Change	Details	Files
Enhanced LUN parsing in iScsiDiskDevice to support hex-encoded values and avoid crashes for high LUN IDs	Pop the lun argument once and reuse it for both parsing attempts Wrap int(lun) in a try/except to catch non-decimal values Add regex fallback matching “0xHHHHHHHH00000000” and compute LUN as low + (high << 16) Log a warning on parse failure and default lun to None	blivet/devices/disk.py
Added unit tests for iSCSI LUN parsing, covering decimal and hex formats	Introduce iScsiDiskDeviceTestCase class Test that a decimal string lun yields the correct integer Test that a hex-encoded lun string yields the combined integer result	tests/unit_tests/devices_test/disk_test.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `tests/unit_tests/devices_test/disk_test.py:53-62` </location>
<code_context>
+
+
+class iScsiDiskDeviceTestCase(unittest.TestCase):
+    def test_iscsi_lun(self):
+        kwargs = {"node": "", "ibft": "", "nic": "", "initiator": "",
+                  "offload": False, "name": "", "target": "",
+                  "address": "", "port": "", "iface": "", "id_path": ""}
+
+        disk1 = iScsiDiskDevice("test1", lun="1", **kwargs)
+        self.assertEqual(disk1.lun, 1)
+
+        disk2 = iScsiDiskDevice("test1", lun="0x0101000000000000", **kwargs)
+        self.assertEqual(disk2.lun, 257)
</code_context>

<issue_to_address>
**suggestion (testing):** Missing tests for invalid LUN formats and error conditions.

Please add tests for malformed hex strings, non-numeric values, None, and cases where the regex does not match, to verify that `disk.lun` is set to None as expected.
</issue_to_address>

### Comment 2
<location> `blivet/devices/disk.py:368` </location>
<code_context>
    def __init__(self, device, **kwargs):
        """
            :param name: the device name (generally a device node's basename)
            :type name: str
            :keyword exists: does this device exist?
            :type exists: bool
            :keyword size: the device's size
            :type size: :class:`~.size.Size`
            :keyword parents: a list of parent devices
            :type parents: list of :class:`StorageDevice`
            :keyword format: this device's formatting
            :type format: :class:`~.formats.DeviceFormat` or a subclass of it
            :keyword str wwn: the disk's WWN
            :keyword target: the name of the iscsi target
            :type target: str
            :keyword lun: lun of the target
            :type node: str
            :keyword iface: name of network interface to use for operation
            :type iface: str
            :keyword initiator: initiator name
            :type initiator: str
            :keyword offload: a partial offload device (qla4xxx)
            :type: bool
            :keyword address: ip address of the target
            :type: str
            :keyword port: port of the target
            :type: str
        """
        # Backward compatibility attributes - to be removed
        self.node = kwargs.pop("node")
        self.ibft = kwargs.pop("ibft")
        self.nic = kwargs.pop("nic")

        self.initiator = kwargs.pop("initiator")
        self.offload = kwargs.pop("offload")
        name = kwargs.pop("name")
        self.target = kwargs.pop("target")
        lun = kwargs.pop("lun")
        try:
            self.lun = int(lun)
        except ValueError:
            # See systemd udev function format_lun_number()
            m = re.fullmatch('0x([0-9a-fA-F]{4})([0-9a-fA-F]{4})00000000', lun)
            if m is None:
                log.warning("Failed to extract hex from lun '%s'", lun)
                self.lun = None
            else:
                self.lun = int(m.group(1), 16) + (1 << 16) * int(m.group(2), 16)
        except TypeError as e:
            log.warning("Failed to set lun attribute of iscsi disk: %s", e)
            self.lun = None

        self.address = kwargs.pop("address")
        self.port = kwargs.pop("port")
        self.iface = kwargs.pop("iface")
        self.id_path = kwargs.pop("id_path")
        DiskDevice.__init__(self, device, **kwargs)
        NetworkStorageDevice.__init__(self, host_address=self.address, nic=self.iface)
        log.debug("created new iscsi disk %s from target: %s lun: %s portal: %s:%s interface: %s partial offload: %s)",
                  name, self.target, self.lun, self.address, self.port, self.iface, self.offload)

        self._clear_local_tags()

</code_context>

<issue_to_address>
**suggestion (code-quality):** Replace m.group(x) with m[x] for re.Match objects [×2] ([`use-getitem-for-re-match-groups`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/use-getitem-for-re-match-groups/))

```suggestion
                self.lun = int(m[1], 16) + (1 << 16) * int(m[2], 16)
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (code-quality): Replace m.group(x) with m[x] for re.Match objects [×2] (use-getitem-for-re-match-groups)

Suggested change

	self.lun = int(m.group(1), 16) + (1 << 16) * int(m.group(2), 16)
	self.lun = int(m[1], 16) + (1 << 16) * int(m[2], 16)