)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"416efbbf53eb03c738f379e2a9032ad719e3f895","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f305f55a_041da94b","updated":"2026-07-27 18:57:15.000000000","message":"I need to do a deeper review of the parent patch, but this one lgtm. The duplication was worse than I thought, it seems that multiple drivers had identical copies of the get_pci_devices","commit_id":"1648d6f7e20608eba243979ada88aaf7b7c531e5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f716c68a_954b894d","updated":"2026-07-28 16:54:21.000000000","message":"this looks good\n\nsome more comment in line but for the most part this is correct directionally","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d46f56ce_09fa6c1a","in_reply_to":"f716c68a_954b894d","updated":"2026-07-29 14:40:37.000000000","message":"Thanks for the detailed review. Applied all the inline comments in this revision.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"844565a4_7cd8a00e","updated":"2026-07-29 14:40:37.000000000","message":"On the PCI enumeration rework mentioned in the review: while migrating I found one more duplicate. `pci/utils.py` `get_pci_devices()` is another copy of the same lspci call with its own `VENDOR_MAPS`. I kept it out of this series since it belongs to that area. Happy to fold it into the rework, or send a follow-up.","commit_id":"a57289ead2cac55060551d07da12558e51a85052"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"961a4b438d88a62b2544740b1c0ebdcac974fd9c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"71c30eb3_263b9759","updated":"2026-08-01 11:20:39.000000000","message":"recheck","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"}],"cyborg/accelerator/common/utils.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148386d4af9528da0b8cc94e17a9b69962fb26f5","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-nn\u0027, \u0027-D\u0027)[0]"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@cyborg.privsep.sys_admin_pctxt.entrypoint"},{"line_number":165,"context_line":"def pci_details(device):"},{"line_number":166,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device)[0]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"b7bb8f3a_a7d06254","line":164,"updated":"2026-08-01 11:31:20.000000000","message":"The pci_details(device) privsep helper introduced in common/utils.py has no unit test. The closely related lspci_privileged, parse_lspci_line, and get_pci_devices helpers all received tests in test_utils.py, but pci_details was added without any test coverage.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: The pci_details helper has zero test coverage and its only consumer (_get_pf_type) is also untested in any invocation path, meaning regressions in the lspci -k -s parsing or privsep decorator wiring would not be caught by CI.\n\n**Recommendation**:\nAdd a unit test in test_utils.py that mocks processutils.execute (or the privsep entrypoint) and verifies pci_details returns the stdout string for a given device address, mirroring how lspci_privileged is tested indirectly via test_get_pci_devices.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"}],"cyborg/accelerator/drivers/aichip/huawei/ascend.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":66,"context_line":"        driver_dep.driver_name \u003d self.VENDOR"},{"line_number":67,"context_line":"        return [driver_dep]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    # TODO(yikun): can be extracted into PCIDeviceDriver"},{"line_number":70,"context_line":"    def _get_pci_lines(self, keywords\u003d()):"},{"line_number":71,"context_line":"        pci_lines \u003d []"},{"line_number":72,"context_line":"        if keywords:"},{"line_number":73,"context_line":"            lspci_out \u003d utils.lspci_privileged().split(\u0027\\n\u0027)"},{"line_number":74,"context_line":"            for i in range(len(lspci_out)):"},{"line_number":75,"context_line":"                # filter out pci devices info that contains all keywords"},{"line_number":76,"context_line":"                if all([k in (lspci_out[i]) for k in keywords]):"},{"line_number":77,"context_line":"                    pci_lines.append(lspci_out[i])"},{"line_number":78,"context_line":"        return pci_lines"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def discover(self):"},{"line_number":81,"context_line":"        \"\"\"The PCI line would be matched as:"}],"source_content_type":"text/x-python","patch_set":3,"id":"a84c35c3_3e3abf98","line":78,"range":{"start_line":69,"start_character":1,"end_line":78,"end_character":24},"updated":"2026-07-28 16:54:21.000000000","message":"so this appares to eb only used in one palce","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        driver_dep.driver_name \u003d self.VENDOR"},{"line_number":67,"context_line":"        return [driver_dep]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    # TODO(yikun): can be extracted into PCIDeviceDriver"},{"line_number":70,"context_line":"    def _get_pci_lines(self, keywords\u003d()):"},{"line_number":71,"context_line":"        pci_lines \u003d []"},{"line_number":72,"context_line":"        if keywords:"},{"line_number":73,"context_line":"            lspci_out \u003d utils.lspci_privileged().split(\u0027\\n\u0027)"},{"line_number":74,"context_line":"            for i in range(len(lspci_out)):"},{"line_number":75,"context_line":"                # filter out pci devices info that contains all keywords"},{"line_number":76,"context_line":"                if all([k in (lspci_out[i]) for k in keywords]):"},{"line_number":77,"context_line":"                    pci_lines.append(lspci_out[i])"},{"line_number":78,"context_line":"        return pci_lines"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def discover(self):"},{"line_number":81,"context_line":"        \"\"\"The PCI line would be matched as:"}],"source_content_type":"text/x-python","patch_set":3,"id":"406eea44_07afccb0","line":78,"range":{"start_line":69,"start_character":1,"end_line":78,"end_character":24},"in_reply_to":"a84c35c3_3e3abf98","updated":"2026-07-29 14:40:37.000000000","message":"Removed. Inlined it into `discover()`.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":91,"context_line":"          \u0027revision\u0027: \u002720\u0027                     # Revision number"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":"        \"\"\""},{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ae3e188a_0e4dc8d8","line":94,"updated":"2026-07-28 16:54:21.000000000","message":"and you can replace this with \n\n```\n        ascends \u003d list(utils.get_pci_devices(lambda line: \u0027d100\u0027 in line))\n```\nor slightly shorter\n\n```\n     ascends \u003d list(utils.get_pci_devices(utils.has_flags([\u0027d100\u0027])))\n```\n\nalthough since we are just looping over the device we dont actully need to conver this to a list\n\nwe can also inlien the regex as a predicate\n\n\n```suggestion\n        ascends \u003d utils.get_pci_devices(\n            lambda line: \u0027d100\u0027 in line, # 1. Fast string check\n            PCI_INFO_PATTERN.search, # 2. Slower regex check)\n```","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":91,"context_line":"          \u0027revision\u0027: \u002720\u0027                     # Revision number"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":"        \"\"\""},{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"}],"source_content_type":"text/x-python","patch_set":3,"id":"d52a50e9_927661a9","line":94,"in_reply_to":"ae3e188a_0e4dc8d8","updated":"2026-07-29 14:40:37.000000000","message":"`discover()` now calls `get_pci_devices()` with the \u0027d100\u0027 keyword and the regex as predicates.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"},{"line_number":98,"context_line":"            if m:"},{"line_number":99,"context_line":"                pci_dict \u003d m.groupdict()"},{"line_number":100,"context_line":"                pci_dict[\"slot_json\"] \u003d utils.pci_str_to_json(pci_dict[\"slot\"])"},{"line_number":101,"context_line":"                device \u003d driver_device.DriverDevice()"},{"line_number":102,"context_line":"                device.stub \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"b369cc17_8f39f9ae","line":99,"range":{"start_line":97,"start_character":10,"end_line":99,"end_character":40},"updated":"2026-07-28 16:54:21.000000000","message":"if we do the regex match above we still need to do it here to get the groups but we can remove the if since we know it will match and dedet the rest of the loop body\n\n```\n        npu_list \u003d []\n        for ascend in ascends:\n           m \u003d PCI_INFO_PATTERN.search(ascend)\n           pci_dict \u003d m.groupdict()\n           ...\n```","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"},{"line_number":98,"context_line":"            if m:"},{"line_number":99,"context_line":"                pci_dict \u003d m.groupdict()"},{"line_number":100,"context_line":"                pci_dict[\"slot_json\"] \u003d utils.pci_str_to_json(pci_dict[\"slot\"])"},{"line_number":101,"context_line":"                device \u003d driver_device.DriverDevice()"},{"line_number":102,"context_line":"                device.stub \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"58595270_c691db39","line":99,"range":{"start_line":97,"start_character":10,"end_line":99,"end_character":40},"in_reply_to":"b369cc17_8f39f9ae","updated":"2026-07-29 14:40:37.000000000","message":"Dropped the match check since the regex predicate guarantees a match, and dedented the loop body.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"0091abbcedf1b96f255c19c24f2ee3d3490f54c3","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"48588ecd_6ceaecf7","line":82,"updated":"2026-08-01 08:02:15.000000000","message":"The ascend discover() method sets device.model from ascend.get(\u0027model\u0027, \u0027\u0027), but the unified parsed lspci dict from parse_lspci_line() contains \u0027device_name\u0027, not \u0027model\u0027. This key never exists in the dict, so model is always an empty string. All four other drivers (GPU, SSD, Inspur FPGA, Xilinx...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Ascend AI chip devices will always report model as empty string, while all other accelerator types (GPU, SSD, FPGA) correctly populate the model with the device name. This is a behavioral inconsistency across driver types in the same codebase after unification.\n\n**Recommendation**:\nChange `ascend.get(\u0027model\u0027, \u0027\u0027)` to `ascend.get(\u0027device_name\u0027, \u0027\u0027)` on line 82 of ascend.py, matching the pattern used by all other drivers. This will correctly populate the model field with the parsed device name.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"}],"cyborg/accelerator/drivers/fpga/inspur/sysinfo.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":68,"context_line":"    fpgas \u003d utils.get_pci_devices("},{"line_number":69,"context_line":"        utils.has_flags(INSPUR_FPGA_FLAGS), utils.has_vendor(VENDOR_ID)"},{"line_number":70,"context_line":"    )"},{"line_number":71,"context_line":"    for fpga in fpgas:"},{"line_number":72,"context_line":"        m \u003d INSPUR_FPGA_INFO_PATTERN.match(fpga)"},{"line_number":73,"context_line":"        if m:"},{"line_number":74,"context_line":"            fpga_dict \u003d m.groupdict()"},{"line_number":75,"context_line":"            # generate traits info"},{"line_number":76,"context_line":"            traits \u003d get_traits("},{"line_number":77,"context_line":"                fpga_dict[\"vendor_id\"], fpga_dict[\"product_id\"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"58325111_3c6aac43","line":74,"range":{"start_line":71,"start_character":4,"end_line":74,"end_character":37},"updated":"2026-07-28 16:54:21.000000000","message":"since we need to do that anyway im actuly not sure if includignt  the regex in the predicates acly is beter of not\n```suggestion\n    for fpga in fpgas:\n        m \u003d INSPUR_FPGA_INFO_PATTERN.match(fpga)\n        if m is None:\n          continue\n       \n        fpga_dict \u003d m.groupdict()\n```\nis a better way to dedent the loop body.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    fpgas \u003d utils.get_pci_devices("},{"line_number":69,"context_line":"        utils.has_flags(INSPUR_FPGA_FLAGS), utils.has_vendor(VENDOR_ID)"},{"line_number":70,"context_line":"    )"},{"line_number":71,"context_line":"    for fpga in fpgas:"},{"line_number":72,"context_line":"        m \u003d INSPUR_FPGA_INFO_PATTERN.match(fpga)"},{"line_number":73,"context_line":"        if m:"},{"line_number":74,"context_line":"            fpga_dict \u003d m.groupdict()"},{"line_number":75,"context_line":"            # generate traits info"},{"line_number":76,"context_line":"            traits \u003d get_traits("},{"line_number":77,"context_line":"                fpga_dict[\"vendor_id\"], fpga_dict[\"product_id\"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1243ed97_3e9ad5f4","line":74,"range":{"start_line":71,"start_character":4,"end_line":74,"end_character":37},"in_reply_to":"58325111_3c6aac43","updated":"2026-07-29 14:40:37.000000000","message":"dedented the loop with an early continue.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"}],"cyborg/accelerator/drivers/fpga/xilinx/sysinfo.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def _get_pf_type(device):"},{"line_number":65,"context_line":"    cmd \u003d [\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device]"},{"line_number":66,"context_line":"    result \u003d utils.lspci_privileged(cmd)"},{"line_number":67,"context_line":"    for k, v in XILINX_PF_MAPS.items():"},{"line_number":68,"context_line":"        if v in result:"},{"line_number":69,"context_line":"            return k"}],"source_content_type":"text/x-python","patch_set":3,"id":"abe9b946_eaa1f101","line":66,"range":{"start_line":66,"start_character":0,"end_line":66,"end_character":2},"updated":"2026-07-28 16:54:21.000000000","message":"-1 we do not allow passing  arrays of in generic command args to functions decoreted with privsep that is an explicit any pattern. \n\n\n you can pass fixed arguments like the device address but we want to prevent arbaity command execution by having very narrow contract for the privileged functions.\n\ni would create a dedicated pci_details function in utils that does \n\n` cmd \u003d [\u0027lspci\u0027, \u0027-kvv\u0027, \u0027-s\u0027, device]`\nyou do not need vv currenly so we do not need to add that but we may want that later.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def _get_pf_type(device):"},{"line_number":65,"context_line":"    cmd \u003d [\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device]"},{"line_number":66,"context_line":"    result \u003d utils.lspci_privileged(cmd)"},{"line_number":67,"context_line":"    for k, v in XILINX_PF_MAPS.items():"},{"line_number":68,"context_line":"        if v in result:"},{"line_number":69,"context_line":"            return k"}],"source_content_type":"text/x-python","patch_set":3,"id":"1b4d16dd_2719dbee","line":66,"range":{"start_line":66,"start_character":0,"end_line":66,"end_character":2},"in_reply_to":"abe9b946_eaa1f101","updated":"2026-07-29 14:40:37.000000000","message":"Added a narrow `pci_details(device)` in common utils that runs a fixed \u0027lspci -k -s \u003cdevice\u003e\u0027. Left out -vv since it is not needed yet. xilinx `_get_pf_type()` now calls it.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"}]}