From 4f11fc025960d4b4ca6863c10559671e1cbb4aa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Sl=C3=A1vik?= Date: Wed, 9 Nov 2022 11:54:00 +0100 Subject: [PATCH] Add a NVMe-FC tab to the Advanced Storage screen Resolves: rhbz#2107346 (cherry picked from commit bd0c061c57149a89788740d0dfe657ec43304290) --- .../modules/common/structures/storage.py | 9 + .../modules/storage/devicetree/viewer.py | 34 ++ .../ui/gui/spokes/advanced_storage.glade | 516 +++++++++++++++++- pyanaconda/ui/gui/spokes/advanced_storage.py | 125 ++++- .../storage/test_module_device_tree.py | 97 +++- 5 files changed, 753 insertions(+), 28 deletions(-) diff --git a/pyanaconda/modules/common/structures/storage.py b/pyanaconda/modules/common/structures/storage.py index 96de552d8ccb..f65e9d649183 100644 --- a/pyanaconda/modules/common/structures/storage.py +++ b/pyanaconda/modules/common/structures/storage.py @@ -173,6 +173,15 @@ def attrs(self) -> Dict[Str, Str]: namespace path-id + Attributes for NVMe Fabrics: + nsid + eui64 + nguid + controllers-id + transports-type + transports-address + subsystems-nqn + Attributes for ZFCP: fcp-lun wwpn diff --git a/pyanaconda/modules/storage/devicetree/viewer.py b/pyanaconda/modules/storage/devicetree/viewer.py index cb08da7070bd..1e31d092a200 100644 --- a/pyanaconda/modules/storage/devicetree/viewer.py +++ b/pyanaconda/modules/storage/devicetree/viewer.py @@ -18,6 +18,7 @@ # Red Hat, Inc. # from abc import abstractmethod, ABC +from functools import partial from blivet.formats import get_format from blivet.size import Size @@ -104,6 +105,8 @@ def get_device_data(self, name): self._set_device_data_iscsi(device, data) elif device.type == "nvdimm": self._set_device_data_nvdimm(device, data) + elif device.type == "nvme-fabrics": + self._set_device_data_nvme_fabrics(device, data) elif device.type == "zfcp": self._set_device_data_zfcp(device, data) @@ -155,6 +158,18 @@ def _set_device_data_nvdimm(self, device, data): data.attrs["namespace"] = self._get_attribute(device, "devname") data.attrs["path-id"] = self._get_attribute(device, "id_path") + def _set_device_data_nvme_fabrics(self, device, data): + """Set data for an NVMe Fabrics device.""" + data.attrs["nsid"] = self._get_attribute(device, "nsid") + data.attrs["eui64"] = self._get_attribute(device, "eui64") + data.attrs["nguid"] = self._get_attribute(device, "nguid") + + get_attrs = partial(self._get_attribute_list, device.controllers) + data.attrs["controllers-id"] = get_attrs("id") + data.attrs["transports-type"] = get_attrs("transport") + data.attrs["transports-address"] = get_attrs("transport_address") + data.attrs["subsystems-nqn"] = get_attrs("subsysnqn") + def _set_device_data_zfcp(self, device, data): """Set data for a ZFCP device.""" data.attrs["fcp-lun"] = self._get_attribute(device, "fcp_lun") @@ -273,6 +288,25 @@ def _get_attribute(self, obj, name): return str(value) + def _get_attribute_list(self, iterable, name): + """Get a list of attributes of the given objects. + + Create a comma-separated list of sorted unique attribute values. + See the _get_attribute method for more info. + + :param iterable: a list of objects + :param name: an attribute name + :return: a string or None + """ + # Collect values. + values = [self._get_attribute(obj, name) for obj in iterable] + + # Skip duplicates and unset values. + values = set(filter(None, values)) + + # Format sorted values if any. + return ", ".join(sorted(values)) or None + def _prune_attributes(self, attrs): """Prune the unset values of attributes. diff --git a/pyanaconda/ui/gui/spokes/advanced_storage.glade b/pyanaconda/ui/gui/spokes/advanced_storage.glade index ec38c829a31f..5c8cc2479345 100644 --- a/pyanaconda/ui/gui/spokes/advanced_storage.glade +++ b/pyanaconda/ui/gui/spokes/advanced_storage.glade @@ -1,7 +1,7 @@ - + @@ -43,6 +43,16 @@ + + + + + + + + + + @@ -51,6 +61,9 @@ diskStore + + diskStore + diskStore @@ -536,6 +549,76 @@ + + + Controllers + True + + + + 2 + 0 + 19 + + + + + + + Transport + True + + + + 2 + 0 + 20 + + + + + + + Transport address + True + + + + 2 + 0 + 21 + + + + + + + Subsystem NQN + True + + + + 2 + 0 + 22 + + + + + + + Namespace ID + True + + + + 2 + 0 + 23 + + + + @@ -1502,12 +1585,432 @@ False + + + True + False + 12 + vertical + 6 + + + True + False + 6 + + + True + False + Filter B_y: + True + nvmefTypeCombo + 0 + + + False + True + 0 + + + + + True + False + + None + Controller + Transport + Subsystem NQN + Namespace ID + + + + + False + True + 1 + + + + + True + False + True + True + False + False + + + True + False + True + + + + + True + False + 6 + + + True + False + _Controller ID: + True + nvmefControllerEntry + + + False + True + 0 + + + + + True + True + True + edit-clear-symbolic + + + + + True + True + 1 + + + + + 1 + + + + + True + False + 6 + + + True + False + _Type: + True + nvmefTransportCombo + + + False + True + 0 + + + + + True + False + + + + False + True + 1 + + + + + True + False + _Address: + True + nvmefTransportAddressEntry + + + False + True + 2 + + + + + True + True + True + edit-clear-symbolic + + + + + True + True + 3 + + + + + 2 + + + + + True + False + 6 + + + True + False + _Subsystem NQN: + True + nvmefSubsystemNqnEntry + + + False + True + 0 + + + + + True + True + True + edit-clear-symbolic + + + + + True + True + 1 + + + + + 3 + + + + + True + False + 6 + + + True + False + _EUI-64 / NGUID / UUID: + True + nvmefNamespaceIdEntry + + + False + True + 0 + + + + + True + True + True + edit-clear-symbolic + + + + + True + True + 1 + + + + + 4 + + + + + + + + + + + True + True + 2 + + + + + False + True + 0 + + + + + True + True + True + True + in + + + True + True + True + True + nvmefModel + True + False + 0 + True + + + + + + + + + + + 2 + 0 + 1 + + + + + + + Namespace + True + + + + 2 + 0 + 17 + + + + + + + Capacity + True + + + + 2 + 0 + 6 + + + + + + + Controllers + True + + + + 2 + 0 + 19 + + + + + + + Transport + True + + + + 2 + 0 + 20 + + + + + + + Transport address + True + + + + 2 + 0 + 21 + + + + + + + Subsystem NQN + True + + + + 2 + 0 + 22 + + + + + + + Namespace ID + True + + + + 2 + 0 + 23 + + + + + + + + + True + True + 1 + + + + + 4 + + + + + True + False + NVMe _Fabrics Devices + True + + + 4 + False + + True False - 6 - 6 12 vertical 6 @@ -1725,9 +2228,10 @@ zModel True False + 0 True - + @@ -1852,7 +2356,7 @@ - 4 + 5 @@ -1863,7 +2367,7 @@ True - 4 + 5 False diff --git a/pyanaconda/ui/gui/spokes/advanced_storage.py b/pyanaconda/ui/gui/spokes/advanced_storage.py index 54b446d8fea2..eba1567ca83c 100644 --- a/pyanaconda/ui/gui/spokes/advanced_storage.py +++ b/pyanaconda/ui/gui/spokes/advanced_storage.py @@ -51,14 +51,19 @@ PAGE_MULTIPATH = 1 PAGE_OTHER = 2 PAGE_NVDIMM = 3 -PAGE_Z = 4 +PAGE_NVMEFABRICS = 4 +PAGE_Z = 5 +# The Z page must be last = highest number, because it is dynamically removed, which would reorder +# the items and invalidate the indices hardcoded here. DiskStoreRow = namedtuple("DiskStoreRow", [ "visible", "selected", "mutable", "name", "type", "model", "capacity", "vendor", "interconnect", "serial", "wwid", "paths", "port", "target", - "lun", "ccw", "wwpn", "namespace", "mode" + "lun", "ccw", "wwpn", "namespace", "mode", + "controllers", "transport", "transport_address", + "subsystem_nqn", "namespace_id" ]) @@ -70,26 +75,40 @@ def create_row(device_data, selected, mutable): :param mutable: False if the device is protected, otherwise True :return: an instance of DiskStoreRow """ + device = device_data + attrs = device_data.attrs + + controller_ids = attrs.get("controllers-id", "").split(", ") + transports_type = attrs.get("transports-type", "").split(", ") + transports_address = attrs.get("transports-address", "").split(", ") + subsystems_nqn = attrs.get("subsystems-nqn", "").split(", ") + namespace_ids = list(filter(None, map(attrs.get, ["eui64", "nguid", "uuid"]))) + return DiskStoreRow( visible=True, selected=selected, - mutable=mutable and not device_data.protected, - name=device_data.name, - type=device_data.type, - model=device_data.attrs.get("model", ""), - capacity=str(Size(device_data.size)), - vendor=device_data.attrs.get("vendor", ""), - interconnect=device_data.attrs.get("bus", ""), - serial=device_data.attrs.get("serial", ""), - wwid=device_data.attrs.get("path-id", ""), - paths="\n".join(device_data.parents), - port=device_data.attrs.get("port", ""), - target=device_data.attrs.get("target", ""), - lun=device_data.attrs.get("lun", "") or device_data.attrs.get("fcp-lun", ""), - ccw=device_data.attrs.get("hba-id", ""), - wwpn=device_data.attrs.get("wwpn", ""), - namespace=device_data.attrs.get("namespace", ""), - mode=device_data.attrs.get("mode", "") + mutable=mutable and not device.protected, + name=device.name, + type=device.type, + model=attrs.get("model", ""), + capacity=str(Size(device.size)), + vendor=attrs.get("vendor", ""), + interconnect=attrs.get("bus", ""), + serial=attrs.get("serial", ""), + wwid=attrs.get("path-id", ""), + paths="\n".join(device.parents), + port=attrs.get("port", ""), + target=attrs.get("target", ""), + lun=attrs.get("lun", "") or attrs.get("fcp-lun", ""), + ccw=attrs.get("hba-id", ""), + wwpn=attrs.get("wwpn", ""), + namespace=attrs.get("namespace", "") or attrs.get("nsid", ""), + mode=attrs.get("mode", ""), + controllers="\n".join(controller_ids), + transport="\n".join(transports_type), + transport_address="\n".join(transports_address), + subsystem_nqn="\n".join(subsystems_nqn), + namespace_id="\n".join(namespace_ids), ) @@ -487,13 +506,74 @@ def get_selected_namespaces(self): return namespaces +class NVMeFabricsPage(FilterPage): + # Match these to nvmefTypeCombo ids in glade + SEARCH_TYPE_CONTROLLER = 'Controller' + SEARCH_TYPE_TRANSPORT = 'Transport' + SEARCH_TYPE_SUBSYSTEM_NQN = 'Subsystem NQN' + SEARCH_TYPE_NAMESPACE_ID = 'Namespace ID' + + def __init__(self, builder): + super().__init__(builder, "nvmefModel", "nvmefTypeCombo") + self._controller_entry = self._builder.get_object("nvmefControllerEntry") + self._transport_combo = self._builder.get_object("nvmefTransportCombo") + self._address_entry = self._builder.get_object("nvmefTransportAddressEntry") + self._subsystem_nqn_entry = self._builder.get_object("nvmefSubsystemNqnEntry") + self._namespace_id_entry = self._builder.get_object("nvmefNamespaceIdEntry") + + def is_member(self, device_type): + return device_type == "nvme-fabrics" + + def setup(self, store, disks, selected_names, protected_names): + transports = set() + + for device_data in disks: + row = create_row( + device_data, + device_data.name in selected_names, + device_data.name not in protected_names, + ) + store.append([*row]) + transports.update(row.transport.split("\n")) + + self._setup_combo(self._transport_combo, transports) + self._transport_combo.set_active(0) + self._setup_search_type() + + def clear(self): + self._controller_entry.set_text("") + self._transport_combo.set_active(0) + self._address_entry.set_text("") + self._subsystem_nqn_entry.set_text("") + self._namespace_id_entry.set_text("") + + def _filter_func(self, filter_by, row): + if filter_by == self.SEARCH_TYPE_CONTROLLER: + return self._controller_entry.get_text().strip() in row.controllers + + if filter_by == self.SEARCH_TYPE_TRANSPORT: + transports = [""] + row.transport.split("\n") + + return self._transport_combo.get_active_text() in transports \ + and self._address_entry.get_text().strip() in row.transport_address + + if filter_by == self.SEARCH_TYPE_SUBSYSTEM_NQN: + return self._subsystem_nqn_entry.get_text().strip() in row.subsystem_nqn + + if filter_by == self.SEARCH_TYPE_NAMESPACE_ID: + return self._namespace_id_entry.get_text().strip() in row.namespace_id + + return False + + class FilterSpoke(NormalSpoke): """ .. inheritance-diagram:: FilterSpoke :parts: 3 """ builderObjects = ["diskStore", "filterWindow", - "searchModel", "multipathModel", "otherModel", "zModel", "nvdimmModel"] + "searchModel", "multipathModel", "otherModel", "zModel", "nvdimmModel", + "nvmefModel"] mainWidgetName = "filterWindow" uiFile = "spokes/advanced_storage.glade" category = SystemCategory @@ -543,6 +623,7 @@ def initialize(self): PAGE_MULTIPATH: MultipathPage(self.builder), PAGE_OTHER: OtherPage(self.builder), PAGE_NVDIMM: NvdimmPage(self.builder), + PAGE_NVMEFABRICS: NVMeFabricsPage(self.builder), PAGE_Z: ZPage(self.builder), } @@ -750,6 +831,10 @@ def on_z_type_combo_changed(self, combo): self._set_notebook_page("zTypeNotebook", combo.get_active()) self._refilter_current_page() + def on_nvmef_type_combo_changed(self, combo): + self._set_notebook_page("nvmefTypeNotebook", combo.get_active()) + self._refilter_current_page() + def _set_notebook_page(self, notebook_name, page_index): notebook = self.builder.get_object(notebook_name) notebook.set_current_page(page_index) diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py b/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py index 13823eb674fb..88f091454b51 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py @@ -22,11 +22,12 @@ import pytest from unittest.mock import patch, Mock, PropertyMock - from tests.unit_tests.pyanaconda_tests import patch_dbus_publish_object, check_task_creation from blivet.devices import StorageDevice, DiskDevice, DASDDevice, ZFCPDiskDevice, PartitionDevice, \ - LUKSDevice, iScsiDiskDevice, NVDIMMNamespaceDevice, FcoeDiskDevice, OpticalDevice + LUKSDevice, iScsiDiskDevice, NVDIMMNamespaceDevice, FcoeDiskDevice, OpticalDevice, \ + NVMeFabricsNamespaceDevice +from blivet.devices.disk import NVMeController from blivet.errors import StorageError, FSError from blivet.formats import get_format, device_formats, DeviceFormat from blivet.formats.fs import FS, Iso9660FS @@ -68,6 +69,46 @@ def _add_device(self, device): """Add a device to the device tree.""" self.storage.devicetree._add_device(device) + def test_get_attribute(self): + """Test the _get_attribute method.""" + value = None + assert self.module._get_attribute(value, "x") is None + + value = Mock(x=None) + assert self.module._get_attribute(value, "x") is None + + value = Mock(x="") + assert self.module._get_attribute(value, "x") is None + + value = Mock(x=0) + assert self.module._get_attribute(value, "x") == "0" + + value = Mock(x=1) + assert self.module._get_attribute(value, "x") == "1" + + value = Mock(x="test") + assert self.module._get_attribute(value, "x") == "test" + + def test_get_attribute_list(self): + """Test the _get_attribute_list method.""" + values = [] + assert self.module._get_attribute_list(values, "x") is None + + values = [None, Mock(x=None), Mock(x="")] + assert self.module._get_attribute_list(values, "x") is None + + values = [Mock(x=0)] + assert self.module._get_attribute_list(values, "x") == "0" + + values = [Mock(x=1), Mock(x=0)] + assert self.module._get_attribute_list(values, "x") == "0, 1" + + values = [Mock(x=1), Mock(x=0), Mock(x=2)] + assert self.module._get_attribute_list(values, "x") == "0, 1, 2" + + values = [Mock(x=1), Mock(x=0), Mock(x=2), Mock(x=0), Mock(x=1)] + assert self.module._get_attribute_list(values, "x") == "0, 1, 2" + def test_get_root_device(self): """Test GetRootDevice.""" assert self.interface.GetRootDevice() == "" @@ -245,6 +286,58 @@ def test_get_nvdimm_device_data(self): "path-id": "pci-0000:00:00.0-bla-1" }) + def test_get_nvme_fabrics_device_data(self): + """Test GetDeviceData for NVMe Fabrics.""" + dev = NVMeFabricsNamespaceDevice( + "nvme4n1", + fmt=get_format("ext4"), + size=Size("10 GiB"), + nsid=5, + eui64="eui.f04xxxxxxxxx0000000100000001", + nguid="0xf04xxxxxxxxx0000000100000001" + ) + ctrl1 = NVMeController( + id=124, + name='nvme1', + nvme_ver='1.0', + serial='80BgLFM7xMoBLABLABLA', + transport='fc', + transport_address='0x10000000', + subsysnqn='nqn.1992-08.com.example:blabla:subsystem.nvme_4', + ) + ctrl2 = NVMeController( + id=123, + name='nvme2', + nvme_ver='1.0', + serial='80BgLFM7xMoBLABLABLA', + transport='fc', + transport_address='0x20000000', + subsysnqn='nqn.1992-08.com.example:blabla:subsystem.nvme_4', + ) + ctrl3 = NVMeController( + id=126, + name='nvme3', + nvme_ver='1.0', + serial='80BgLFM7xMoBLABLABLA', + transport='fc', + transport_address='0x30000000', + subsysnqn='nqn.1992-08.com.example:blabla:subsystem.nvme_4', + ) + dev._controllers = [ctrl1, ctrl2, ctrl3] + self._add_device(dev) + + data = self.interface.GetDeviceData("nvme4n1") + assert data['type'] == get_variant(Str, 'nvme-fabrics') + assert data['attrs'] == get_variant(Dict[Str, Str], { + "nsid": "5", + "eui64": "eui.f04xxxxxxxxx0000000100000001", + "nguid": "0xf04xxxxxxxxx0000000100000001", + "controllers-id": "123, 124, 126", + "transports-type": "fc", + "transports-address": "0x10000000, 0x20000000, 0x30000000", + "subsystems-nqn": "nqn.1992-08.com.example:blabla:subsystem.nvme_4", + }) + def test_get_zfcp_device_data(self): """Test GetDeviceData for zFCP.""" self._add_device(ZFCPDiskDevice(