)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"87dec261fa8149e314a53c93da06c730b73c2570","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7c669113_7b807e7a","updated":"2022-08-19 15:07:15.000000000","message":"Question inline.","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"cb699cb6b4b150b97339a26692acbe834b26dce1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"eb252740_1b7fef47","updated":"2024-01-19 15:11:06.000000000","message":"The comments are valid but this looks better than before, so +1.","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"}],"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"23042a39d768c03df55ca9f4dd7ab1e91b70e7f9","unresolved":true,"context_lines":[{"line_number":380,"context_line":"            return"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        hostnqn: Optional[str] \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":383,"context_line":"        if hostnqn is None:"},{"line_number":384,"context_line":"            raise exception.BrickException(\u0027hostnqn is None\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # List of portal addresses and transports for this target"}],"source_content_type":"text/x-python","patch_set":1,"id":"8b0f4839_8a1569a8","line":383,"updated":"2022-08-29 13:52:13.000000000","message":"nit:\n\n  if not hostnqn:","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"12a0473a53556857b3fc892c5f44ebdd47c6ac90","unresolved":true,"context_lines":[{"line_number":380,"context_line":"            return"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        hostnqn: Optional[str] \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":383,"context_line":"        if hostnqn is None:"},{"line_number":384,"context_line":"            raise exception.BrickException(\u0027hostnqn is None\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # List of portal addresses and transports for this target"}],"source_content_type":"text/x-python","patch_set":1,"id":"d57b8c6d_f74e30a0","line":383,"in_reply_to":"8b0f4839_8a1569a8","updated":"2022-09-14 14:57:08.000000000","message":"So we want it to fail on \"\", too?","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"87dec261fa8149e314a53c93da06c730b73c2570","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        hostnqn: Optional[str] \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":383,"context_line":"        if hostnqn is None:"},{"line_number":384,"context_line":"            raise exception.BrickException(\u0027hostnqn is None\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # List of portal addresses and transports for this target"},{"line_number":387,"context_line":"        # Unlike \"nvme list-subsys -o json\" sysfs addr is separated by a comma"}],"source_content_type":"text/x-python","patch_set":1,"id":"f12d41c3_5fe4f45d","line":384,"updated":"2022-08-19 15:07:15.000000000","message":"I wonder if we really want to raise here.  utils.get_host_nqn() may return an empty string (when priv_nvme.create_hostnqn() swallows an Exception), which isn\u0027t much more useful than None.  On the other hand, that could be an argument for also checking here for \u0027\u0027 and raising.  I think we need to check with Gorka (see line 414 below, which makes me think a None hostnqn shouldn\u0027t be fatal, but I don\u0027t understand this part of the code well enough, he may be talking about something else there).","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"241ce63f93f12fab60333d13a10c9f0a7e490649","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        hostnqn: Optional[str] \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":383,"context_line":"        if hostnqn is None:"},{"line_number":384,"context_line":"            raise exception.BrickException(\u0027hostnqn is None\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # List of portal addresses and transports for this target"},{"line_number":387,"context_line":"        # Unlike \"nvme list-subsys -o json\" sysfs addr is separated by a comma"}],"source_content_type":"text/x-python","patch_set":1,"id":"750016a3_5b0b7dc0","line":384,"range":{"start_line":384,"start_character":44,"end_line":384,"end_character":60},"updated":"2022-08-19 14:54:03.000000000","message":"Maybe \"NVMeoF: no hostnqn found.\"\n\nhttps://opendev.org/openstack/os-brick/src/branch/master/os_brick/initiator/connectors/lightos.py#L84","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"984c5e41f312857365c086d411a47f72e3802a86","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        hostnqn: Optional[str] \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":383,"context_line":"        if hostnqn is None:"},{"line_number":384,"context_line":"            raise exception.BrickException(\u0027hostnqn is None\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # List of portal addresses and transports for this target"},{"line_number":387,"context_line":"        # Unlike \"nvme list-subsys -o json\" sysfs addr is separated by a comma"}],"source_content_type":"text/x-python","patch_set":1,"id":"98452f93_c820f4de","line":384,"in_reply_to":"01eeb4da_ec180cb6","updated":"2022-12-16 14:23:37.000000000","message":"I agree with Gorka that this is a reasonable patch. \nThe message should be something like \"NVMe: hostnqn not found\". Slightly different to Sofia\u0027s suggestion as this is a genetic NVMe message, not NVMeOF which is slightly different. The semantics of this protocol are very complex and confusing so we need to be sematically correct.","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"23042a39d768c03df55ca9f4dd7ab1e91b70e7f9","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        hostnqn: Optional[str] \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":383,"context_line":"        if hostnqn is None:"},{"line_number":384,"context_line":"            raise exception.BrickException(\u0027hostnqn is None\u0027)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        # List of portal addresses and transports for this target"},{"line_number":387,"context_line":"        # Unlike \"nvme list-subsys -o json\" sysfs addr is separated by a comma"}],"source_content_type":"text/x-python","patch_set":1,"id":"01eeb4da_ec180cb6","line":384,"in_reply_to":"f12d41c3_5fe4f45d","updated":"2022-08-29 13:52:13.000000000","message":"I believe that if host_nqn is not returned by get_host_nqn NVMe connections will not work, so this patch seems right to me.  In some cases this will fail on the Cinder driver side if they do host nqn based ACLs.\n\nThe hostnqn from L414-415 is referring to a different, but related, one. It\u0027s the one in sysfs for a specific connection, whereas this on is the \"global default\" one that will be used if one is not forced by the Cinder driver.","commit_id":"95ace3cdfeb9a083287e4f27bdae5b3d71c72585"}]}