)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"016bb9606734da3a8f534aaa96a4bf32295c3d48","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"901a5774_4a8329a7","updated":"2025-01-09 15:30:03.000000000","message":"\u003e We discussed this patch at the 8 January 2025 cinder meeting. It was noted that this patch does not implement FCZM, but the community is OK with that as long as the implementation in the patch could easily be extended to add FCZM support ... so please review with that in mind.\n\nI\u0027m going to withdraw the above comment.  After some offline discussion, it\u0027s not clear that allowing this patch to proceed without FCZM support is a good idea, and therefore I think it requires further public discussion.  This discussion could be the midcycle or a special purpose video meeting scheduled at the convenience of all interested parties.  The cinder PTL can decide what would work best.","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a9d37e62568048a83f30b706ce11e0a495c48f2d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"baaa5ddd_08ec00a5","updated":"2025-01-08 18:57:35.000000000","message":"Few comments inline but the major one being using the fibre channel connector designed for SCSI connections.\nThe approach seems to be hacky and might cause us trouble in the future.\nThe best way is to use the nvme directives in sysfs to fetch the right port and node names.","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"fb883c8d4edf9dbbd6390815a565cf17238243ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8f88f0c8_15d96e48","updated":"2024-07-31 15:51:08.000000000","message":"Forgot to add vote on last comment","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"438d7e576dc8315e462e974f613dbd9d13e5144e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a36cc969_c8ad2b5a","updated":"2024-07-31 15:50:39.000000000","message":"I feel that this patch is actually coping out of a real NVMe-FC integration. Most FC based drivers, support the Fibre Channel Zone Manager, but for some reason the Dell PowerStore driver does not and this is what is driving this patch.\nThese changes take no account of FCZM required items like initiator_target_maps which makes it very difficult, if not impossible, for other vendors who do support FCZM to implement NVMe-FC based on the os-brick changes implemented.\nIt might be worth looking at adding the NVMe-FC connector specifically into the existing FC connector, rather than adding onto the current NVMe connector, which was devised for IP based NVMe. Or even create a apecific NVMe-FC connector.","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"af2cea6de4eb39b7fe973022093fd326b8327fb2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"31df9880_7352f9d9","updated":"2024-11-13 14:06:59.000000000","message":"It appears that you are now adding FCZM support to your driver, but this won\u0027t work for NVMe-FC using this patch to implement NVMe-FC support in os-brick, becuase it doesn\u0027t, as previously mentioned, implemen parameters used by the FCZM","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"85554888a7f582d62ba7312db3ffdb9381195a55","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"70e28a7e_891d12b7","updated":"2025-01-08 14:33:34.000000000","message":"We discussed this patch at the 8 January 2025 cinder meeting.  It was noted that this patch does not implement FCZM, but the community is OK with that as long as the implementation in the patch could easily be extended to add FCZM support ... so please review with that in mind.","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":37437,"name":"Amit Zauber","display_name":"Zaubea","email":"amit.zauber@dell.com","username":"amit.zauber@dell.com"},"change_message_id":"8c83926a354d28389e553d4ed3c40d280be9e92e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6bc92b83_94d6b49d","updated":"2025-07-30 07:14:08.000000000","message":"i would like to follow up on Brian comment\n\"\nI\u0027m going to withdraw the above comment. After some offline discussion, it\u0027s not clear that allowing this patch to proceed without FCZM support is a good idea, and therefore I think it requires further public discussion. This discussion could be the midcycle or a special purpose video meeting scheduled at the convenience of all interested parties. The cinder PTL can decide what would work best.\n\"\nit will be good idea to to schedule a discussion about this topic to discuss 2 major topics\n\nadding Cisco Switches into our qualification process\nadd additional feature - FCZM (dynamic zoning)\ni will add our product team into that special purpose video meeting\n\nsmerrow@redhat.com\n - FYI","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":35759,"name":"Yian Zong","display_name":"Yian Zong","email":"yian.zong@dell.com","username":"yianzong"},"change_message_id":"ea70db99e71efad4c7a58eafa2ec5e23ff0cc4be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a7431e68_555fbf97","updated":"2024-08-22 13:28:07.000000000","message":"run Pure Storage CI","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"69faa8ca40d252840fc0f88b90a8ac680dadc51e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e53bf00d_b1abeb92","updated":"2024-08-12 21:43:09.000000000","message":"run Pure Storage CI","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":36625,"name":"Rick Liu","email":"rick.liu@dell.com"},"change_message_id":"50cd266421be7822ea9103f43c7c223330b06233","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4daf6fa9_6d49744e","updated":"2024-07-11 08:19:02.000000000","message":"run-DellEMC PowerStore CI","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":36625,"name":"Rick Liu","email":"rick.liu@dell.com"},"change_message_id":"8b473d8a62e2dee505d5bf9228bea7b45d3b9f8a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"58bc00a4_89852c36","updated":"2024-07-24 00:50:01.000000000","message":"run-DellEMC PowerStore CI","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":36625,"name":"Rick Liu","email":"rick.liu@dell.com"},"change_message_id":"d14f59ea17fdd93036f5e3c84913ec69914b7030","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c3b7a753_adf25e29","updated":"2024-07-23 05:55:56.000000000","message":"run-DellEMC PowerStore CI","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"d06c31d1faea0274f718d27f81099e9e36366e4d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"420b8bb4_9abb36a3","in_reply_to":"31df9880_7352f9d9","updated":"2024-11-13 14:07:32.000000000","message":"REF: https://review.opendev.org/c/openstack/cinder/+/919422","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":35759,"name":"Yian Zong","display_name":"Yian Zong","email":"yian.zong@dell.com","username":"yianzong"},"change_message_id":"64760366d49d23b1f20be41008f9705bed33722b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"cc10c1a5_5160b542","in_reply_to":"420b8bb4_9abb36a3","updated":"2024-11-19 08:32:21.000000000","message":"Cinder patch 919422 adds FCZM for PowerStore FC connectivity per the customer ask, therefore the major code change is in \u0027FibreChannelAdapter\u0027[1]. \n\nAs to this PowerStore NVMe-FC support, dynamic zoning for NVMe-FC is not part of the scope. So far, no customer asks for that.\n\nIf you want to support FCZM for NVMe-FC, you need to ensure the FCZM driver can support NVMe-FC first. E.g. Cisco NXOS 8.1(1) or later revisions is required by Dell storage.\n\n[1] https://review.opendev.org/c/openstack/cinder/+/919422/6/cinder/volume/drivers/dell_emc/powerstore/adapter.py#1125","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":35759,"name":"Yian Zong","display_name":"Yian Zong","email":"yian.zong@dell.com","username":"yianzong"},"change_message_id":"4eaf842b384d7e756c57018dfbf2db0c3e7fbfcc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"746cfa20_066a172b","in_reply_to":"a36cc969_c8ad2b5a","updated":"2024-08-01 04:45:56.000000000","message":"The patch to support FC zone manager for PowerStore is pending on review[1], please kindly take a look.\n\nWe didn\u0027t plan to support FCZM in NVMe-FC, but it\u0027s considerable for the future.\n\n[1] https://review.opendev.org/c/openstack/cinder/+/919422","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":35759,"name":"Yian Zong","display_name":"Yian Zong","email":"yian.zong@dell.com","username":"yianzong"},"change_message_id":"ac2180582d1120b013e0343c447d48b8e1f9ae11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d1decdcd_82a8daee","in_reply_to":"cc10c1a5_5160b542","updated":"2024-11-19 15:35:26.000000000","message":"I forgot to mention, NVMe-FC as a subtype of NVMeOF, its implementation in PowerStore driver is in \u0027NVMEoFAdapter\u0027[2]\n\n[2] https://review.opendev.org/c/openstack/cinder/+/923346/4/cinder/volume/drivers/dell_emc/powerstore/adapter.py#1225","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"494cf4cc1ef43d4a20373401054903f61439d066","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6e3fccda_9c3bd96e","in_reply_to":"d1decdcd_82a8daee","updated":"2024-11-20 14:48:14.000000000","message":"Just because someone doesn\u0027t ask for a feature you know exists and will, at some point, be asked for, that doesn\u0027t mean you can ignore the feature.\nAdding this into the scope now will make it easier in the long run and also enable other vendors to be able to take advantage of FCZM for NVMe-FC. \nYou have to work with the community of these sorts of core patches and not just do what is right for you at this point in time.","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"}],"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a9d37e62568048a83f30b706ce11e0a495c48f2d","unresolved":true,"context_lines":[{"line_number":865,"context_line":"            root_helper: str, *args, **kwargs) -\u003e list[str]:"},{"line_number":866,"context_line":"        \"\"\"Get Fibre Channel transport address.\"\"\""},{"line_number":867,"context_line":"        host_traddrs \u003d []"},{"line_number":868,"context_line":"        fc_conn \u003d FibreChannelConnector(root_helper,"},{"line_number":869,"context_line":"                                        execute\u003dkwargs.get(\u0027execute\u0027))"},{"line_number":870,"context_line":"        fc_dict \u003d fc_conn.get_connector_properties("},{"line_number":871,"context_line":"            root_helper,"},{"line_number":872,"context_line":"            execute\u003dkwargs.get(\u0027execute\u0027))"},{"line_number":873,"context_line":"        for node, port in zip(fc_dict[\u0027wwnns\u0027], fc_dict[\u0027wwpns\u0027]):"},{"line_number":874,"context_line":"            host_traddrs.append(NVMeOFConnector._format_fc_addr(node, port))"},{"line_number":875,"context_line":"        return host_traddrs"}],"source_content_type":"text/x-python","patch_set":4,"id":"ac45faab_64400563","line":872,"range":{"start_line":868,"start_character":8,"end_line":872,"end_character":42},"updated":"2025-01-08 18:57:35.000000000","message":"This doesn\u0027t look like the right way to do it.\nWe are relying on getting the WWPNs and WWNNs from the sysfs layer designed for SCSI connections and wrapping them up with the NVMe constructs (nn-, pn-)\n\nSince sysfs provides the local and remote ports data[1], we should define a new method in linuxfc like this[2] and create a dict out of all the useful info.\n\n[1]\n/sys/class/scsi_host/host*/nvme_info\nLPORT ...\n\n[2] https://github.com/openstack/os-brick/blob/master/os_brick/initiator/linuxfc.py#L236-L250","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a9d37e62568048a83f30b706ce11e0a495c48f2d","unresolved":true,"context_lines":[{"line_number":949,"context_line":"                    traddr \u003d self._format_fc_addr(portal.address, portal.port)"},{"line_number":950,"context_line":"                    host_traddrs \u003d self._get_fc_host_traddrs(self._root_helper)"},{"line_number":951,"context_line":"                    for host_traddr in host_traddrs:"},{"line_number":952,"context_line":"                        cmds.append([\u0027connect\u0027, \u0027-a\u0027, traddr, \u0027-w\u0027,"},{"line_number":953,"context_line":"                                     host_traddr, \u0027-t\u0027, portal.transport, \u0027-n\u0027,"},{"line_number":954,"context_line":"                                     target.nqn, \u0027-Q\u0027, \u0027128\u0027, \u0027-l\u0027, \u0027-1\u0027])"},{"line_number":955,"context_line":"                else:"},{"line_number":956,"context_line":"                    cmds.append([\u0027connect\u0027, \u0027-a\u0027, portal.address, \u0027-s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"d3decfde_82a7b7a1","line":953,"range":{"start_line":952,"start_character":62,"end_line":953,"end_character":48},"updated":"2025-01-08 18:57:35.000000000","message":"This doesn\u0027t look very clean.\nThe only difference between this command and the one for TCP/RDMA is the \u0027-w\u0027 (host-traddr) vs \u0027-s\u0027 (port).\n\nA better way to write this would be,\n\n    cmd \u003d [\u0027connect\u0027, \u0027-a\u0027, traddr, \u0027-t\u0027, portal.transport, \u0027-n\u0027, target.nqn, \u0027-Q\u0027, \u0027128\u0027, \u0027-l\u0027, \u0027-1\u0027]\n    cmd.extend([\u0027-w\u0027, host_traddr]) if portal.transport \u003d\u003d \u0027fc\u0027 else cmd.extend([\u0027-s\u0027, portal.port])\n    cmds.append(cmd)","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":35759,"name":"Yian Zong","display_name":"Yian Zong","email":"yian.zong@dell.com","username":"yianzong"},"change_message_id":"26067c3e65e6035ab755082ea7883d795447060d","unresolved":true,"context_lines":[{"line_number":945,"context_line":"            for portal in missing_portals:"},{"line_number":946,"context_line":"                cmds \u003d []"},{"line_number":947,"context_line":"                # nvme connect"},{"line_number":948,"context_line":"                if portal.transport \u003d\u003d \u0027fc\u0027:"},{"line_number":949,"context_line":"                    traddr \u003d self._format_fc_addr(portal.address, portal.port)"},{"line_number":950,"context_line":"                    host_traddrs \u003d self._get_fc_host_traddrs(self._root_helper)"},{"line_number":951,"context_line":"                    for host_traddr in host_traddrs:"},{"line_number":952,"context_line":"                        cmds.append([\u0027connect\u0027, \u0027-a\u0027, traddr, \u0027-w\u0027,"},{"line_number":953,"context_line":"                                     host_traddr, \u0027-t\u0027, portal.transport, \u0027-n\u0027,"},{"line_number":954,"context_line":"                                     target.nqn, \u0027-Q\u0027, \u0027128\u0027, \u0027-l\u0027, \u0027-1\u0027])"},{"line_number":955,"context_line":"                else:"},{"line_number":956,"context_line":"                    cmds.append([\u0027connect\u0027, \u0027-a\u0027, portal.address, \u0027-s\u0027,"},{"line_number":957,"context_line":"                                 portal.port, \u0027-t\u0027, portal.transport, \u0027-n\u0027,"},{"line_number":958,"context_line":"                                 target.nqn, \u0027-Q\u0027, \u0027128\u0027, \u0027-l\u0027, \u0027-1\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"c020d040_4eea715d","line":955,"range":{"start_line":948,"start_character":0,"end_line":955,"end_character":0},"updated":"2024-08-22 06:50:32.000000000","message":"Hi @jobernar@redhat.com @geguileo@redhat.com @simon@purestorage.com\n\nThe patch implements NVMe-FC connection by \u0027nvme connect\u0027 CMD with host fc address and storage fc targets.\nE.g.\nnvme connect -a nn-0x58ccf090c9600161:pn-0x58ccf09049680161 -w nn-0x200000620b3eedd5:pn-0x100000620b3eedd5 -t fc -n nqn.1988-11.com.dell:powerstore:00:ea4e57588a310EF6166E -Q 128 -l -1\n\nIt\u0027s simple to add FCZM support here by populating host and target FC addresses from \u0027initiator_target_map\u0027 returned by a volume driver.\n\nThis patch is fully tested with PowerStore NVMe-FC CI and should not be blocked merge while we can add FCZM next cycle.\n\nAlso, the patch to add FCZM support for PowerStore FC connection [1] is pending on review.\nIt\u0027s unlikely we have both FCZM support for FC and NVMe-FC in PowerStore driver in this cycle.\n\n[1] 919422: Dell PowerStore: add FC zone manager support | https://review.opendev.org/c/openstack/cinder/+/919422","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a9d37e62568048a83f30b706ce11e0a495c48f2d","unresolved":true,"context_lines":[{"line_number":956,"context_line":"                    cmds.append([\u0027connect\u0027, \u0027-a\u0027, portal.address, \u0027-s\u0027,"},{"line_number":957,"context_line":"                                 portal.port, \u0027-t\u0027, portal.transport, \u0027-n\u0027,"},{"line_number":958,"context_line":"                                 target.nqn, \u0027-Q\u0027, \u0027128\u0027, \u0027-l\u0027, \u0027-1\u0027])"},{"line_number":959,"context_line":"                if target.host_nqn:"},{"line_number":960,"context_line":"                    for cmd in cmds:"},{"line_number":961,"context_line":"                        cmd.extend([\u0027-q\u0027, target.host_nqn])"},{"line_number":962,"context_line":"                try:"},{"line_number":963,"context_line":"                    for cmd in cmds:"},{"line_number":964,"context_line":"                        self.run_nvme_cli(cmd)"},{"line_number":965,"context_line":"                    connected \u003d True"},{"line_number":966,"context_line":"                except putils.ProcessExecutionError as exc:"},{"line_number":967,"context_line":"                    # In some nvme versions code 70 means target is already"}],"source_content_type":"text/x-python","patch_set":4,"id":"4c106952_50dbba88","line":964,"range":{"start_line":959,"start_character":16,"end_line":964,"end_character":46},"updated":"2025-01-08 18:57:35.000000000","message":"if the above suggested change is made, these modifications won\u0027t be required","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"}],"releasenotes/notes/nvmeof-nvmefc-support-721ed812ca98bdfc.yaml":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3e5142546ab6cb2d86e00368328b827ad76fac62","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    NVMeOF connector: Added NVMe-FC support."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"8ecad320_87cf1e98","line":4,"updated":"2025-01-08 15:10:07.000000000","message":"I think the release note should clarify the current state of the feature does not support FCZM.","commit_id":"d0eea3d2c9db1c7ef642ad97c41340d9744b6928"}]}