)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"832183d6_587576b5","updated":"2022-05-24 10:52:12.000000000","message":"I had a chance to take a look once again.\nGreat work, I believe the changes are going into the right direction.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f0fdbb4c_a1a6c0db","updated":"2022-06-08 16:48:50.000000000","message":"Bartosz thank you very much for the review and the great catch on the bug.","commit_id":"fe80af78322f2dbb0d2c6da6e1c840b909da7ba7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bb3093f96c1c6c6d4300998bfccc197dffa3184b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d8d17673_80b7fd35","updated":"2022-06-10 14:14:06.000000000","message":"recheck - Nova failure on resize test test_resize_server_from_auto_to_manual","commit_id":"8df5abdae207ea8e5b0d58fd105c2cd4b3dc66f4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88d4a97e1db71e6a8c33ff89018115f5d3882caa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"677c0f02_7b2704a8","updated":"2022-07-14 17:51:16.000000000","message":"Have to look at a number of things, so setting WIP for now","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"33a61ce5631d71bdf55c5f2c8c0494fb0a63a70b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"7681d230_ad620e10","updated":"2022-06-29 14:02:09.000000000","message":"Still got some issue. Perhaps it\u0027s only related to my setup (bionic + kernel 5.4)?","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"dcc0b2f219bac7c793a6535f7fe9128aaf5b4f94","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"dcb77698_d5d7a218","updated":"2022-07-25 15:37:09.000000000","message":"Code and tests look good.  I hope that vendors using this connector are good about doing continual testing in updated environments, because this code is kind of brittle (no fault of Gorka\u0027s, that\u0027s just the environment we\u0027re working in) due to the reliance on command line tools that change output and error codes (sometimes due to bugfixes, but still, they change).","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1b692c01da0f40d5342798097e27ed43e3114452","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"cd0b9f98_bf78ce57","updated":"2022-07-25 17:55:32.000000000","message":"Since this is merged, addressed the review comments in a separate patch. https://review.opendev.org/c/openstack/os-brick/+/850937","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"e916163d_9b1c323a","updated":"2022-07-25 10:58:14.000000000","message":"nits and queries inline, nothing major. LGTM.","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"}],"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"1edfcc490234a8af2f0ca7c8515d949d8d5997f8","unresolved":true,"context_lines":[{"line_number":69,"context_line":"# #########################################################"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"def _adapt_connection_properties(func):"},{"line_number":73,"context_line":"    \"\"\"Decorator to adapt the different connector properties to a common format"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    There is an old and a newer connection properties format, which result"}],"source_content_type":"text/x-python","patch_set":2,"id":"4f34e8bf_cfd05b05","line":72,"updated":"2022-04-14 07:35:05.000000000","message":"Hey,\nHave you considered using a class for storing connection properties and using them inside the os-brick code.\n\nIt would be possible to do \n`class NvmeofConnectionProperties(dict)`\n\nSince it would still be a dict, the probability of breaking something is small.\nThanks to the class you could add methods to convert older formats to the unified one (e.g. when building the instance). Then it could have properties so it\u0027s no longer needed to access dict values directly.\n\nThis class would be a standarized interface for connection properties and any future drivers/connectors should respect it.","commit_id":"a3626005275d34637fe611560ff22f5be592dad9"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":false,"context_lines":[{"line_number":69,"context_line":"# #########################################################"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"def _adapt_connection_properties(func):"},{"line_number":73,"context_line":"    \"\"\"Decorator to adapt the different connector properties to a common format"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    There is an old and a newer connection properties format, which result"}],"source_content_type":"text/x-python","patch_set":2,"id":"5d2d90e5_84b1de12","line":72,"in_reply_to":"27aab060_4fe66a5a","updated":"2022-05-24 10:52:12.000000000","message":"Organizing the code into classes looks really good. Thanks!","commit_id":"a3626005275d34637fe611560ff22f5be592dad9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ae27beda2149da13e98b2ec373f267dbce6d2426","unresolved":false,"context_lines":[{"line_number":69,"context_line":"# #########################################################"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"def _adapt_connection_properties(func):"},{"line_number":73,"context_line":"    \"\"\"Decorator to adapt the different connector properties to a common format"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    There is an old and a newer connection properties format, which result"}],"source_content_type":"text/x-python","patch_set":2,"id":"27aab060_4fe66a5a","line":72,"in_reply_to":"4f34e8bf_cfd05b05","updated":"2022-05-12 16:15:48.000000000","message":"We discussed this on IRC but I forgot to update the comment here.\n\nWe agreed that, while this would probably be the way to go in a new implementation, doing it in the current code would be a considerable effort, since all os-brick connectors would need to be changed, all cinder drivers, cinder API, cinder RPC, and even nova would need to be changed.\n\nSince I really consider it to be a great suggestion (one I had considered in the past) I decided to do it at least for this specific connector, so the connection properties dictionary is converted to an instance of the new NVMeOFConnProps class so the dictionary is no longer used internally.","commit_id":"a3626005275d34637fe611560ff22f5be592dad9"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":true,"context_lines":[{"line_number":154,"context_line":"        if transport in (\u0027RoCEv2\u0027, \u0027rdma\u0027):"},{"line_number":155,"context_line":"            self.transport \u003d \u0027rdma\u0027"},{"line_number":156,"context_line":"        else:"},{"line_number":157,"context_line":"            self.transport \u003d \u0027tcp\u0027"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    @property"},{"line_number":160,"context_line":"    def is_live(self) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":4,"id":"b6a9370a_2ff39508","line":157,"updated":"2022-05-24 10:52:12.000000000","message":"Just to throw an error in case of misuse. I\u0027d do\n\n\n  if transport in (\u0027RoCEv2\u0027, \u0027rdma\u0027):\n      self.transport \u003d \u0027rdma\u0027\n  elif transport \u003d\u003d \u0027tcp\u0027:\n      self.transport \u003d \u0027tcp\u0027\n  else:\n      raise NotImplementedError(\"Transport not supported\")","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        if transport in (\u0027RoCEv2\u0027, \u0027rdma\u0027):"},{"line_number":155,"context_line":"            self.transport \u003d \u0027rdma\u0027"},{"line_number":156,"context_line":"        else:"},{"line_number":157,"context_line":"            self.transport \u003d \u0027tcp\u0027"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    @property"},{"line_number":160,"context_line":"    def is_live(self) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":4,"id":"d8df2cfd_ac025036","line":157,"in_reply_to":"b6a9370a_2ff39508","updated":"2022-06-08 16:48:50.000000000","message":"I would do that as well, but unfortunately one of our current code paths doesn\u0027t do that [1], so we should preserve the behavior behavior (at least for now).\n\n[1]: https://github.com/openstack/os-brick/blob/7f747456dce8845a9e4982586729ade99d2bb14e/os_brick/initiator/connectors/nvmeof.py#L577-L580","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":true,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        # Look under the controller, where we will have normal devices and ANA"},{"line_number":224,"context_line":"        # channel devices. For nvme1 we could find nvme1n1 or nvme0c1n1)"},{"line_number":225,"context_line":"        paths \u003d glob.glob(f\u0027{NVME_CTRL_SYSFS_PATH}{self.controller}/nvme*\u0027)"},{"line_number":226,"context_line":"        for path in paths:"},{"line_number":227,"context_line":"            prop_value \u003d sysfs_property(prop_name, path)"},{"line_number":228,"context_line":"            if prop_value \u003d\u003d value:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ec45a7d0_b18d66ba","line":225,"updated":"2022-05-24 10:52:12.000000000","message":"Not sure what are Python version constraints. The better tool for that would be pathlib, although glob is used everywhere here so I guess it\u0027s better to stick to one lib.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"cd0d3a5abfee7c0ad7a427b15470a5aeda20ef88","unresolved":false,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        # Look under the controller, where we will have normal devices and ANA"},{"line_number":224,"context_line":"        # channel devices. For nvme1 we could find nvme1n1 or nvme0c1n1)"},{"line_number":225,"context_line":"        paths \u003d glob.glob(f\u0027{NVME_CTRL_SYSFS_PATH}{self.controller}/nvme*\u0027)"},{"line_number":226,"context_line":"        for path in paths:"},{"line_number":227,"context_line":"            prop_value \u003d sysfs_property(prop_name, path)"},{"line_number":228,"context_line":"            if prop_value \u003d\u003d value:"}],"source_content_type":"text/x-python","patch_set":4,"id":"859b8f4b_29d8af76","line":225,"in_reply_to":"44763bac_e045d369","updated":"2022-06-13 11:22:24.000000000","message":"I don\u0027t see any clear benefit at the moment. It\u0027s just Pathlib is made exactly for working on paths and makes path modifications much simpler and readable.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":true,"context_lines":[{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        # Look under the controller, where we will have normal devices and ANA"},{"line_number":224,"context_line":"        # channel devices. For nvme1 we could find nvme1n1 or nvme0c1n1)"},{"line_number":225,"context_line":"        paths \u003d glob.glob(f\u0027{NVME_CTRL_SYSFS_PATH}{self.controller}/nvme*\u0027)"},{"line_number":226,"context_line":"        for path in paths:"},{"line_number":227,"context_line":"            prop_value \u003d sysfs_property(prop_name, path)"},{"line_number":228,"context_line":"            if prop_value \u003d\u003d value:"}],"source_content_type":"text/x-python","patch_set":4,"id":"44763bac_e045d369","line":225,"in_reply_to":"ec45a7d0_b18d66ba","updated":"2022-06-08 16:48:50.000000000","message":"Currently 3.8 is the oldest supported version, so using pathlib wouldn\u0027t be an issue.\n\nIn this case I\u0027m not sure if there would be much benefit from using it, or at least I\u0027m not seeing it.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":true,"context_lines":[{"line_number":227,"context_line":"            prop_value \u003d sysfs_property(prop_name, path)"},{"line_number":228,"context_line":"            if prop_value \u003d\u003d value:"},{"line_number":229,"context_line":"                # Convert path to the namespace device name"},{"line_number":230,"context_line":"                result \u003d DEV_SEARCH_PATH + nvme_basename(path)"},{"line_number":231,"context_line":"                LOG.debug(\u0027Device found at %s, using %s\u0027, path, result)"},{"line_number":232,"context_line":"                return result"},{"line_number":233,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"94e5fc90_c9bcc39d","line":230,"updated":"2022-05-24 10:52:12.000000000","message":"Why not to use f-string here as well?","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            prop_value \u003d sysfs_property(prop_name, path)"},{"line_number":228,"context_line":"            if prop_value \u003d\u003d value:"},{"line_number":229,"context_line":"                # Convert path to the namespace device name"},{"line_number":230,"context_line":"                result \u003d DEV_SEARCH_PATH + nvme_basename(path)"},{"line_number":231,"context_line":"                LOG.debug(\u0027Device found at %s, using %s\u0027, path, result)"},{"line_number":232,"context_line":"                return result"},{"line_number":233,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"2d34e94c_34642dc3","line":230,"in_reply_to":"94e5fc90_c9bcc39d","updated":"2022-06-08 16:48:50.000000000","message":"I believe in this case it\u0027s clearer and marginally more efficient.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":true,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"103d204b_5acc4183","line":280,"updated":"2022-05-24 10:52:12.000000000","message":"host_nqn is a str as well.\nActually it is not used in the constructor.\n\nI don\u0027t know what\u0027s host_nqn according to nvmeof protocol spec but I need to pass specific host_nqn to connect to my SDS.\n\nPrevious implementation did \"nvme connect ... -q \u003chost-nqn\u003e\" and that something I need.\n\nAt the moment it seems not implemented\n\n cmd \u003d (\u0027connect\u0027, \u0027-a\u0027, portal.address, \u0027-s\u0027, portal.port,\n        \u0027-t\u0027, portal.transport, \u0027-n\u0027, target.nqn, \u0027-Q\u0027, \u0027128\u0027,\n        \u0027-l\u0027, \u0027-1\u0027)","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"35f916db25c337defd73d466abb9cf2ae7d4760a","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"94978c46_427a157e","line":280,"in_reply_to":"0b303137_66c906ce","updated":"2022-06-14 10:31:31.000000000","message":"I\u0027ll update the patch to include the host_nqn support, since it is a regression not supporting it (it is currently supported).","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":true,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"adf5bca8_3d5d8dd1","line":280,"in_reply_to":"103d204b_5acc4183","updated":"2022-06-08 16:48:50.000000000","message":"host_nqn is not currently needed, and afaik you wouldn\u0027t need it either unless you are doing things in an unconventional way.\n\nThe Cinder driver will receive the NQN from the host that wants to connect and that\u0027s the one that should be used by the storage solution to allow access.\n\nHere on the os-brick side the connect command will use the value of /etc/nvme/hostnqn (which is the one reported to Cinder) by default, and that\u0027s why we don\u0027t need to provide it.\n\nDo you have a case where that behavior is not good enough?","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"303825f5915579b019faccd215c75f5da93b2934","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"a9426f5a_cd34c621","line":280,"in_reply_to":"88150bb2_b92ec2c7","updated":"2022-07-25 11:28:36.000000000","message":"Thx. I\u0027m happy as long as I can pass an arbitrary host_nqn.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"33a61ce5631d71bdf55c5f2c8c0494fb0a63a70b","unresolved":true,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d4d737e5_f41ab0e0","line":280,"in_reply_to":"94978c46_427a157e","updated":"2022-06-29 14:02:09.000000000","message":"Thanks for re-implementing this.\n\nThe specification \nhttps://nvmexpress.org/wp-content/uploads/NVM-Express-Base-Specification-2.0b-2021.12.18-Ratified.pdf\n\nSection 4.5\n\"NVMe Qualified Names (NQNs) are used to uniquely describe a host or NVM subsystem for the purposes of identification and authentication\"\n\nAs I understand NQNs are for identification and authentication.\nIf we use hostname as host nqn then what happens if you reboot a VM on a different host?\n\nYou boot VM1 on HOST1, attach NVME1 with subsys_nqn: snqn1 and host_nqn: host1.\nThen you move the VM (e.g. migration, whatever) and it lands on HOST2.\nSo it wants to attach NVME1 it would use subsys_nqn: snqn1 and host_nqn: host2?","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"cd0d3a5abfee7c0ad7a427b15470a5aeda20ef88","unresolved":true,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0b303137_66c906ce","line":280,"in_reply_to":"adf5bca8_3d5d8dd1","updated":"2022-06-13 11:22:24.000000000","message":"In case of the SDS I\u0027m using host_nqn is arbitrarily assigned by the SDS side. Each target has unique host_nqn. So for my case /etc/nvme/hostnqn is not much usable as I need to pass host_nqn which is fetched by cinder driver through SDS API.\n\nI\u0027ll try to find out why it\u0027s like that.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27090495963c12f7cf351280407f4f6787d3a099","unresolved":true,"context_lines":[{"line_number":277,"context_line":"                 uuid: str \u003d None,"},{"line_number":278,"context_line":"                 nguid: str \u003d None,"},{"line_number":279,"context_line":"                 ns_id: str \u003d None,"},{"line_number":280,"context_line":"                 host_nqn\u003dNone,"},{"line_number":281,"context_line":"                 find_controllers\u003dFalse) -\u003e None:"},{"line_number":282,"context_line":"        \"\"\"Initialize instance."},{"line_number":283,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"88150bb2_b92ec2c7","line":280,"in_reply_to":"d4d737e5_f41ab0e0","updated":"2022-07-12 11:21:13.000000000","message":"If you do a VM migration, then Nova knows that connection information for the source and target host will be different.\n\nSo Nova asks Cinder to allow the target host access to that same volume, then moves the VM, and finally Nova asks Cinder to remove access to the volume from the source host.\n\nThat\u0027s why it\u0027s not necessary to set the host_nqn, because VMs are not moved to other locations and expected to have access to the storage.  Just imagine if that was the case and storage is FC with Zoning, the FC switches would not allow traffic to reach the target host.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"ab547800e83075667ed57bfa09cad6e1e64efdd2","unresolved":true,"context_lines":[{"line_number":363,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":364,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport)"},{"line_number":365,"context_line":"            try:"},{"line_number":366,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"},{"line_number":367,"context_line":"                LOG.debug(\u0027Found a valid portal at %s\u0027, ctrl_portal)"},{"line_number":368,"context_line":"                # Update the portal with the found controller name"},{"line_number":369,"context_line":"                self.portals[index].controller \u003d ctrl_name"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f7872d8_598f0d15","line":366,"updated":"2022-05-25 20:20:04.000000000","message":"I think the implementation here is broken.\nObserved on real system - I got 3 portals, but only 2 got controller attribute set.\n\nPlease take a look, I hope it\u0027s readable.\n\n  self.portals \u003d [\u0027p1\u0027, \u0027p2\u0027, \u0027p3\u0027]\n  sysfs_portals \u003d [\u0027p1\u0027, \u0027p2\u0027, \u0027p3\u0027]\n  ctrl_paths \u003d [\u0027p1\u0027, \u0027p2\u0027, \u0027p3\u0027]\n\n  # first loop\n  ctrl_portal \u003d \u0027p1\u0027\n  index \u003d sysfs_portals.index(ctrl_portal)  # index\u003d0\n  self.portals[index].controller \u003d \u0027c1\u0027  # \u0027p1\u0027 got \u0027c1\u0027\n  del sysfs_portals[index]  # remaining sysfs_portals \u003d [\u0027p2\u0027, \u0027p3\u0027]\n  # second loop\n  ctrl_portal \u003d \u0027p2\u0027\n  index \u003d sysfs_portals.index(ctrl_portal)  # index\u003d0 \u003c\u003d\u003d\u003d again the index is 0\n  self.portals[index].controller \u003d \u0027c2\u0027  # \u0027p1\u0027 got \u0027c2\u0027!!!\n\nSince you depend on item index from sysfs_portals and you delete items from it you overwrite controller attribute from self.portals.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":false,"context_lines":[{"line_number":363,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":364,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport)"},{"line_number":365,"context_line":"            try:"},{"line_number":366,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"},{"line_number":367,"context_line":"                LOG.debug(\u0027Found a valid portal at %s\u0027, ctrl_portal)"},{"line_number":368,"context_line":"                # Update the portal with the found controller name"},{"line_number":369,"context_line":"                self.portals[index].controller \u003d ctrl_name"}],"source_content_type":"text/x-python","patch_set":4,"id":"558cfb34_c3d8d84f","line":366,"in_reply_to":"3f7872d8_598f0d15","updated":"2022-06-08 16:48:50.000000000","message":"GREAT CATCH!!\n\nI don\u0027t know in which of my local changes I broke it...\nAnd there\u0027s an additional case were it would break, and that is when a controller name is already present in one of the portals.\n\nFixing it now and adding those 2 missing unit test cases.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"33a61ce5631d71bdf55c5f2c8c0494fb0a63a70b","unresolved":true,"context_lines":[{"line_number":620,"context_line":""},{"line_number":621,"context_line":"    def __init__(self,"},{"line_number":622,"context_line":"                 root_helper: str,"},{"line_number":623,"context_line":"                 driver: Optional \u003d None,"},{"line_number":624,"context_line":"                 use_multipath: bool \u003d False,"},{"line_number":625,"context_line":"                 device_scan_attempts: int \u003d DEVICE_SCAN_ATTEMPTS_DEFAULT,"},{"line_number":626,"context_line":"                 *args, **kwargs) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"0eba88ed_fb850261","line":623,"updated":"2022-06-29 14:02:09.000000000","message":"vscode says: Expected one type argument after \"Optional\" Pylance","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27090495963c12f7cf351280407f4f6787d3a099","unresolved":false,"context_lines":[{"line_number":620,"context_line":""},{"line_number":621,"context_line":"    def __init__(self,"},{"line_number":622,"context_line":"                 root_helper: str,"},{"line_number":623,"context_line":"                 driver: Optional \u003d None,"},{"line_number":624,"context_line":"                 use_multipath: bool \u003d False,"},{"line_number":625,"context_line":"                 device_scan_attempts: int \u003d DEVICE_SCAN_ATTEMPTS_DEFAULT,"},{"line_number":626,"context_line":"                 *args, **kwargs) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9ae41b1b_492feae9","line":623,"in_reply_to":"0eba88ed_fb850261","updated":"2022-07-12 11:21:13.000000000","message":"vscode is right   :-)","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":true,"context_lines":[{"line_number":820,"context_line":"                    LOG.warning(\u0027Race condition with some other application \u0027"},{"line_number":821,"context_line":"                                \u0027when connecting to %s, please check your \u0027"},{"line_number":822,"context_line":"                                \u0027system configuration.\u0027, portal)"},{"line_number":823,"context_line":"                break  # We are connected"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":"            else:  # Could not connect to any of the portals"},{"line_number":826,"context_line":"                raise exception.VolumeDeviceNotFound(device\u003dtarget.nqn)"}],"source_content_type":"text/x-python","patch_set":4,"id":"0ab95f45_073100cb","line":823,"updated":"2022-05-24 10:52:12.000000000","message":"So... If I\u0027m reading this correctly the loop breaks when 1 connection established, regardless of number of portals? There is no ANA multipath yet, right?","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":false,"context_lines":[{"line_number":820,"context_line":"                    LOG.warning(\u0027Race condition with some other application \u0027"},{"line_number":821,"context_line":"                                \u0027when connecting to %s, please check your \u0027"},{"line_number":822,"context_line":"                                \u0027system configuration.\u0027, portal)"},{"line_number":823,"context_line":"                break  # We are connected"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":"            else:  # Could not connect to any of the portals"},{"line_number":826,"context_line":"                raise exception.VolumeDeviceNotFound(device\u003dtarget.nqn)"}],"source_content_type":"text/x-python","patch_set":4,"id":"6356756a_7dd15be6","line":823,"in_reply_to":"0ab95f45_073100cb","updated":"2022-06-08 16:48:50.000000000","message":"Now we do have ANA support, so the logic is different in the latest patch.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"df3c4f714269bc69c652ca68aa13c46ada0ba000","unresolved":true,"context_lines":[{"line_number":867,"context_line":""},{"line_number":868,"context_line":"        return device_path"},{"line_number":869,"context_line":""},{"line_number":870,"context_line":"    def _handle_replicated_volume(self,"},{"line_number":871,"context_line":"                                  host_device_paths: List[str],"},{"line_number":872,"context_line":"                                  conn_props: NVMeOFConnProps) -\u003e str:"},{"line_number":873,"context_line":"        \"\"\"Assemble the raid from found devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"e5b67c3d_78b65b2d","line":870,"updated":"2022-05-24 10:52:12.000000000","message":"Since you\u0027re doing cleanup. I think I\u0027d consider moving RAID support (local replication) code into a separate class. Not sure if that\u0027s a good/worth the effort idea though.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e6f92ac7467a5fea72014dbbc199bbe4d8f12f10","unresolved":false,"context_lines":[{"line_number":867,"context_line":""},{"line_number":868,"context_line":"        return device_path"},{"line_number":869,"context_line":""},{"line_number":870,"context_line":"    def _handle_replicated_volume(self,"},{"line_number":871,"context_line":"                                  host_device_paths: List[str],"},{"line_number":872,"context_line":"                                  conn_props: NVMeOFConnProps) -\u003e str:"},{"line_number":873,"context_line":"        \"\"\"Assemble the raid from found devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"5f22b128_3975759d","line":870,"in_reply_to":"e5b67c3d_78b65b2d","updated":"2022-06-08 16:48:50.000000000","message":"I think it would make the code a bit more readable, though I don\u0027t think that change belongs in this patch and would require a specific patch.","commit_id":"87c15b50c974082a1800b8f03dcf70937d4bb241"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"33a61ce5631d71bdf55c5f2c8c0494fb0a63a70b","unresolved":true,"context_lines":[{"line_number":365,"context_line":"            # The right subsystem, but must also be the right portal"},{"line_number":366,"context_line":"            ctrl_transport \u003d sysfs_property(\u0027transport\u0027, ctrl_path)"},{"line_number":367,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":368,"context_line":"            ctrl_hostnqn \u003d sysfs_property(\u0027hostnqn\u0027, ctrl_path)"},{"line_number":369,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport, ctrl_hostnqn)"},{"line_number":370,"context_line":"            try:"},{"line_number":371,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8851d8ba_9ad6c788","line":368,"updated":"2022-06-29 14:02:09.000000000","message":"So in my tests `set_portals_controllers` method doesn\u0027t work at all due to the fact that `hostnqn` property in sysfs is not present.\n\n```\n(nova) [DEV][brabiega][root /sys/class/nvme-fabrics/ctl/nvme2] ls -la\ntotal 0\ndrwxr-xr-x  4 root root    0 Jun 28 15:52 .\ndrwxr-xr-x  4 root root    0 Jun 21 20:35 ..\n-r--r--r--  1 root root 4096 Jun 28 15:52 address\n-r--r--r--  1 root root 4096 Jun 29 15:51 cntlid\n--w-------  1 root root 4096 Jun 29 15:51 delete_controller\n-r--r--r--  1 root root 4096 Jun 29 15:51 dev\nlrwxrwxrwx  1 root root    0 Jun 28 15:54 device -\u003e ../../ctl\n-r--r--r--  1 root root 4096 Jun 29 15:51 firmware_rev\n-r--r--r--  1 root root 4096 Jun 28 16:45 model\n-r--r--r--  1 root root 4096 Jun 29 15:51 numa_node\ndrwxr-xr-x 10 root root    0 Jun 28 15:52 nvme2c2n1\ndrwxr-xr-x  2 root root    0 Jun 29 15:51 power\n-r--r--r--  1 root root 4096 Jun 29 15:51 queue_count\n--w-------  1 root root 4096 Jun 29 15:51 rescan_controller\n--w-------  1 root root 4096 Jun 29 15:51 reset_controller\n-r--r--r--  1 root root 4096 Jun 29 15:51 serial\n-r--r--r--  1 root root 4096 Jun 29 15:51 sqsize\n-r--r--r--  1 root root 4096 Jun 28 15:54 state\n-r--r--r--  1 root root 4096 Jun 28 15:52 subsysnqn\nlrwxrwxrwx  1 root root    0 Jun 28 15:52 subsystem -\u003e ../../../../../class/nvme\n-r--r--r--  1 root root 4096 Jun 28 15:52 transport\n-rw-r--r--  1 root root 4096 Jun 28 15:52 uevent\n```\n\n```\n# nvme list-subsys\nnvme-subsys2 - NQN\u003dnqn.2014-08.org.nvmexpress:uuid:6e22edbd-9fb2-4e24-a496-51550172eea9\n\\\n +- nvme2 tcp traddr\u003d1.2.3.4 trsvcid\u003d4420 live \n```\n\nUsing Ubuntu Bionic, kernel 5.4.0-64-generic","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aa58a4bbdcc3ff0a9075ba0ee464694a55d670dd","unresolved":false,"context_lines":[{"line_number":365,"context_line":"            # The right subsystem, but must also be the right portal"},{"line_number":366,"context_line":"            ctrl_transport \u003d sysfs_property(\u0027transport\u0027, ctrl_path)"},{"line_number":367,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":368,"context_line":"            ctrl_hostnqn \u003d sysfs_property(\u0027hostnqn\u0027, ctrl_path)"},{"line_number":369,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport, ctrl_hostnqn)"},{"line_number":370,"context_line":"            try:"},{"line_number":371,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"}],"source_content_type":"text/x-python","patch_set":8,"id":"aef40b37_c11825bc","line":368,"in_reply_to":"64c6b90a_3b4af4ec","updated":"2022-07-21 08:54:12.000000000","message":"1. I have tested the latest code with Focal, and it is true that hostnqn is not present, it must be because the Linux NVMe code in Ubuntu is old.  I had assumed that it would always be present, which is incorrect.\n\nI have modified the latest code to support the hostnqn not existing, though it is possible to have problems attaching/detaching volumes if it doesn\u0027t exist in sysfs, so I wouldn\u0027t recommend using arbitrary uuid for host_nqn in your driver.\n\n2. Regarding the /etc/nvme directory, do you have the required nvme packages installed?\n\nHave you tried to attach a volume in OpenStack?  Because the directory is automatically created by os-brick: https://github.com/openstack/os-brick/blob/a944ffc48a307231026a323e62adbfdc3ca33e8a/os_brick/privileged/nvmeof.py#L40","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"0f22f7563e62e0068c1404780d4f3c26f53749b5","unresolved":true,"context_lines":[{"line_number":365,"context_line":"            # The right subsystem, but must also be the right portal"},{"line_number":366,"context_line":"            ctrl_transport \u003d sysfs_property(\u0027transport\u0027, ctrl_path)"},{"line_number":367,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":368,"context_line":"            ctrl_hostnqn \u003d sysfs_property(\u0027hostnqn\u0027, ctrl_path)"},{"line_number":369,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport, ctrl_hostnqn)"},{"line_number":370,"context_line":"            try:"},{"line_number":371,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"}],"source_content_type":"text/x-python","patch_set":8,"id":"64c6b90a_3b4af4ec","line":368,"in_reply_to":"6baa402e_25b9749d","updated":"2022-07-13 21:31:12.000000000","message":"There is no such file in my system. Not even /etc/nvme dir.\nI\u0027m passing host_nqn (per volume arbitrary uuid) from the driver.","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27090495963c12f7cf351280407f4f6787d3a099","unresolved":true,"context_lines":[{"line_number":365,"context_line":"            # The right subsystem, but must also be the right portal"},{"line_number":366,"context_line":"            ctrl_transport \u003d sysfs_property(\u0027transport\u0027, ctrl_path)"},{"line_number":367,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":368,"context_line":"            ctrl_hostnqn \u003d sysfs_property(\u0027hostnqn\u0027, ctrl_path)"},{"line_number":369,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport, ctrl_hostnqn)"},{"line_number":370,"context_line":"            try:"},{"line_number":371,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"}],"source_content_type":"text/x-python","patch_set":8,"id":"6baa402e_25b9749d","line":368,"in_reply_to":"8851d8ba_9ad6c788","updated":"2022-07-12 11:21:13.000000000","message":"I\u0027ll check with Bionic, because it should be there.\n\nWhat are the contents of /etc/nvme/hostnqn ?","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"303825f5915579b019faccd215c75f5da93b2934","unresolved":false,"context_lines":[{"line_number":365,"context_line":"            # The right subsystem, but must also be the right portal"},{"line_number":366,"context_line":"            ctrl_transport \u003d sysfs_property(\u0027transport\u0027, ctrl_path)"},{"line_number":367,"context_line":"            ctrl_addr \u003d sysfs_property(\u0027address\u0027, ctrl_path)"},{"line_number":368,"context_line":"            ctrl_hostnqn \u003d sysfs_property(\u0027hostnqn\u0027, ctrl_path)"},{"line_number":369,"context_line":"            ctrl_portal \u003d (ctrl_addr, ctrl_transport, ctrl_hostnqn)"},{"line_number":370,"context_line":"            try:"},{"line_number":371,"context_line":"                index \u003d sysfs_portals.index(ctrl_portal)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ced8b819_f0a045a0","line":368,"in_reply_to":"aef40b37_c11825bc","updated":"2022-07-25 11:28:36.000000000","message":"I\u0027ve been using this patch ported to Stein, thus probably missing the directory creation.","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"071a95713f3508331d5e622ef69a564bde7cef48","unresolved":true,"context_lines":[{"line_number":783,"context_line":"    @NVMeOFConnProps.from_dictionary_parameter"},{"line_number":784,"context_line":"    def connect_volume("},{"line_number":785,"context_line":"            self, connection_properties: NVMeOFConnProps) -\u003e Dict[str, str]:"},{"line_number":786,"context_line":"        \"\"\"Attach and discover the volume.\"\"\""},{"line_number":787,"context_line":"        if connection_properties.is_replicated is False:"},{"line_number":788,"context_line":"            LOG.debug(\u0027Starting non replicated connection\u0027)"},{"line_number":789,"context_line":"            path \u003d self._connect_target(connection_properties.targets[0])"}],"source_content_type":"text/x-python","patch_set":8,"id":"45c93b7b_23a1cf4c","line":786,"updated":"2022-07-07 20:17:52.000000000","message":"You have to discover the target device before you can attach any presented volume, so this the wrong way around...","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27090495963c12f7cf351280407f4f6787d3a099","unresolved":true,"context_lines":[{"line_number":783,"context_line":"    @NVMeOFConnProps.from_dictionary_parameter"},{"line_number":784,"context_line":"    def connect_volume("},{"line_number":785,"context_line":"            self, connection_properties: NVMeOFConnProps) -\u003e Dict[str, str]:"},{"line_number":786,"context_line":"        \"\"\"Attach and discover the volume.\"\"\""},{"line_number":787,"context_line":"        if connection_properties.is_replicated is False:"},{"line_number":788,"context_line":"            LOG.debug(\u0027Starting non replicated connection\u0027)"},{"line_number":789,"context_line":"            path \u003d self._connect_target(connection_properties.targets[0])"}],"source_content_type":"text/x-python","patch_set":8,"id":"ed86b1b6_5853e043","line":786,"in_reply_to":"45c93b7b_23a1cf4c","updated":"2022-07-12 11:21:13.000000000","message":"This is referring to attaching the volume using the nvme client and then discovering the device within the Linux host.\n\nDiscovery is not required to connect to an nvme subsystem.","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"071a95713f3508331d5e622ef69a564bde7cef48","unresolved":true,"context_lines":[{"line_number":807,"context_line":"        connection."},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"        This method assumes that the controllers for the portals have already"},{"line_number":810,"context_line":"        been set.  For example using the from_dictionary_parameter decorator"},{"line_number":811,"context_line":"        in the NVMeOFConnProps class."},{"line_number":812,"context_line":""},{"line_number":813,"context_line":"        Returns the path of the connected device."}],"source_content_type":"text/x-python","patch_set":8,"id":"f5b44288_173d4d18","line":810,"updated":"2022-07-07 20:17:52.000000000","message":"There is nowhere in the code where an `nvme discover` is performed. If this isn\u0027t done then the `nvme connect` will not succeed, specifically in the case of an external array connecting over RDMA.\nYou need to have the following run (doesn\u0027t matter if it is rerun - makes no difference)  for each target portal\n\nnvme discover -t portal.transport-a portal.address -s portal.port","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"27090495963c12f7cf351280407f4f6787d3a099","unresolved":true,"context_lines":[{"line_number":807,"context_line":"        connection."},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"        This method assumes that the controllers for the portals have already"},{"line_number":810,"context_line":"        been set.  For example using the from_dictionary_parameter decorator"},{"line_number":811,"context_line":"        in the NVMeOFConnProps class."},{"line_number":812,"context_line":""},{"line_number":813,"context_line":"        Returns the path of the connected device."}],"source_content_type":"text/x-python","patch_set":8,"id":"fb63c381_d5d17c54","line":810,"in_reply_to":"f5b44288_173d4d18","updated":"2022-07-12 11:21:13.000000000","message":"From the point of view of the NVMe-oF os-brick connector, the discovery of NVMe subsystems contained in NVMe Targets is a new feature, since os-brick currently doesn\u0027t support it.\n\nDiscovery is not necessary with the current implementation because Cinder drivers are required to send the \"target_nqn\" in the connection information.  So why would os-brick want to do the discovery to look for the information that is already provided in the connection information?\n\nI tested this a while back with RDMA (using the devstack patch that uses Soft-RoCE) and it worked just fine.  You can try it if with https://review.opendev.org/c/openstack/devstack/+/814193.","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"ee4b7929559e27f23dd05e432af81ae318eda1fb","unresolved":false,"context_lines":[{"line_number":807,"context_line":"        connection."},{"line_number":808,"context_line":""},{"line_number":809,"context_line":"        This method assumes that the controllers for the portals have already"},{"line_number":810,"context_line":"        been set.  For example using the from_dictionary_parameter decorator"},{"line_number":811,"context_line":"        in the NVMeOFConnProps class."},{"line_number":812,"context_line":""},{"line_number":813,"context_line":"        Returns the path of the connected device."}],"source_content_type":"text/x-python","patch_set":8,"id":"ed7dcab1_31f39c98","line":810,"in_reply_to":"fb63c381_d5d17c54","updated":"2022-07-13 21:50:42.000000000","message":"Ack","commit_id":"f384b55090bcb3395f97e7a32adb86fcaa6d21f5"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":56,"context_line":"PORTAL \u003d \u0027target_portal\u0027"},{"line_number":57,"context_line":"PORT \u003d \u0027target_port\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"# These were only present in the old connection info, but now we\u0027ll allowed"},{"line_number":60,"context_line":"# both in the old format, the new format, and as part of a volume_replicas"},{"line_number":61,"context_line":"# element."},{"line_number":62,"context_line":"NGUID \u003d \u0027volume_nguid\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"9dc512ed_347c7ff9","line":59,"range":{"start_line":59,"start_character":68,"end_line":59,"end_character":75},"updated":"2022-07-25 10:58:14.000000000","message":"nit: allow","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":56,"context_line":"PORTAL \u003d \u0027target_portal\u0027"},{"line_number":57,"context_line":"PORT \u003d \u0027target_port\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"# These were only present in the old connection info, but now we\u0027ll allowed"},{"line_number":60,"context_line":"# both in the old format, the new format, and as part of a volume_replicas"},{"line_number":61,"context_line":"# element."},{"line_number":62,"context_line":"NGUID \u003d \u0027volume_nguid\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"9173ab18_6a1d16bc","line":59,"range":{"start_line":59,"start_character":2,"end_line":59,"end_character":52},"updated":"2022-07-25 10:58:14.000000000","message":"was there any reason for not doing that or just it was missed while implementing the new path?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1b692c01da0f40d5342798097e27ed43e3114452","unresolved":false,"context_lines":[{"line_number":56,"context_line":"PORTAL \u003d \u0027target_portal\u0027"},{"line_number":57,"context_line":"PORT \u003d \u0027target_port\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"# These were only present in the old connection info, but now we\u0027ll allowed"},{"line_number":60,"context_line":"# both in the old format, the new format, and as part of a volume_replicas"},{"line_number":61,"context_line":"# element."},{"line_number":62,"context_line":"NGUID \u003d \u0027volume_nguid\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"e39985d9_5d08e9a1","line":59,"range":{"start_line":59,"start_character":68,"end_line":59,"end_character":75},"in_reply_to":"9dc512ed_347c7ff9","updated":"2022-07-25 17:55:32.000000000","message":"Done","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":78,"context_line":"# #########################################################"},{"line_number":79,"context_line":"# UTILITY METHODS start"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def ctrl_property(prop_name: str, ctrl_name: str) -\u003e Optional[str]:"},{"line_number":82,"context_line":"    \"\"\"Get a sysfs property of an nvme controller.\"\"\""},{"line_number":83,"context_line":"    return sysfs_property(prop_name, NVME_CTRL_SYSFS_PATH + ctrl_name)"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"399a2ad8_1ef5f2eb","line":81,"range":{"start_line":81,"start_character":4,"end_line":81,"end_character":17},"updated":"2022-07-25 10:58:14.000000000","message":"Just a suggestion: I was confused seeing this method used below, maybe would\u0027ve been better as,\n\nget_ctrl_property","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":83,"context_line":"    return sysfs_property(prop_name, NVME_CTRL_SYSFS_PATH + ctrl_name)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"def blk_property(prop_name: str, blk_name: str) -\u003e Optional[str]:"},{"line_number":87,"context_line":"    \"\"\"Get a sysfs property of a block device.\"\"\""},{"line_number":88,"context_line":"    return sysfs_property(prop_name, BLK_SYSFS_PATH + blk_name)"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"279c62db_8c56f031","line":86,"range":{"start_line":86,"start_character":4,"end_line":86,"end_character":16},"updated":"2022-07-25 10:58:14.000000000","message":"same suggestion, get_blk_property","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":88,"context_line":"    return sysfs_property(prop_name, BLK_SYSFS_PATH + blk_name)"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"def sysfs_property(prop_name: str, path_or_name: str) -\u003e Optional[str]:"},{"line_number":92,"context_line":"    \"\"\"Get sysfs property by path returning None if not possible.\"\"\""},{"line_number":93,"context_line":"    filename \u003d os.path.join(path_or_name, prop_name)"},{"line_number":94,"context_line":"    LOG.debug(\u0027Checking property at %s\u0027, filename)"}],"source_content_type":"text/x-python","patch_set":11,"id":"433bdb5a_9e0d88ba","line":91,"range":{"start_line":91,"start_character":4,"end_line":91,"end_character":18},"updated":"2022-07-25 10:58:14.000000000","message":"same suggestion, get_sysfs_property","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        return basename"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    # nvme0c1n10 \u003d\u003d\u003e nvme0n10"},{"line_number":120,"context_line":"    ctrl, rest \u003d basename.split(\u0027c\u0027, 1)"},{"line_number":121,"context_line":"    ns \u003d rest[rest.index(\u0027n\u0027):]"},{"line_number":122,"context_line":"    return ctrl + ns"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"# UTILITY METHODS end"},{"line_number":125,"context_line":"# #########################################################"}],"source_content_type":"text/x-python","patch_set":11,"id":"232c4af6_60e61f01","line":122,"range":{"start_line":120,"start_character":4,"end_line":122,"end_character":20},"updated":"2022-07-25 10:58:14.000000000","message":"I thought of this way but anyone is good\n\n    return basename.split(\u0027c\u0027)[0] + \u0027n\u0027 + basename.rsplit(\u0027n\u0027, 1)[1]","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":141,"context_line":"    __repr__ \u003d __str__"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def __init__(self,"},{"line_number":144,"context_line":"                 parent_target: \u0027Target\u0027,"},{"line_number":145,"context_line":"                 address: str,"},{"line_number":146,"context_line":"                 port: Union[str, int],"},{"line_number":147,"context_line":"                 transport: str) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":11,"id":"037c5ff7_233a20f8","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"updated":"2022-07-25 10:58:14.000000000","message":"is this a way to specify class as data type in mypy?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"dcc0b2f219bac7c793a6535f7fe9128aaf5b4f94","unresolved":true,"context_lines":[{"line_number":141,"context_line":"    __repr__ \u003d __str__"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def __init__(self,"},{"line_number":144,"context_line":"                 parent_target: \u0027Target\u0027,"},{"line_number":145,"context_line":"                 address: str,"},{"line_number":146,"context_line":"                 port: Union[str, int],"},{"line_number":147,"context_line":"                 transport: str) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":11,"id":"05035ff3_fc8c0702","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"in_reply_to":"037c5ff7_233a20f8","updated":"2022-07-25 15:37:09.000000000","message":"You used to have to do this with mypy because it\u0027s a forward reference to a type that is defined later in this file; not sure if you still need to do that or not.","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46a9ff5414269f7ebdd6bb203975f7316a3d5e02","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    __repr__ \u003d __str__"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def __init__(self,"},{"line_number":144,"context_line":"                 parent_target: \u0027Target\u0027,"},{"line_number":145,"context_line":"                 address: str,"},{"line_number":146,"context_line":"                 port: Union[str, int],"},{"line_number":147,"context_line":"                 transport: str) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":11,"id":"a9a15274_6a67468b","line":144,"range":{"start_line":144,"start_character":32,"end_line":144,"end_character":40},"in_reply_to":"05035ff3_fc8c0702","updated":"2022-07-25 17:56:24.000000000","message":"Ack","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":173,"context_line":"        # Does not automatically search for the controller"},{"line_number":174,"context_line":"        if self.controller:"},{"line_number":175,"context_line":"            return ctrl_property(\u0027state\u0027, self.controller)"},{"line_number":176,"context_line":"        return None"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    def get_device(self) -\u003e Optional[str]:"},{"line_number":179,"context_line":"        \"\"\"Get a device path using available volume identification markers."}],"source_content_type":"text/x-python","patch_set":11,"id":"0d574b74_a7995ae3","line":176,"range":{"start_line":176,"start_character":8,"end_line":176,"end_character":19},"updated":"2022-07-25 10:58:14.000000000","message":"nit: python methods return None by default so no need of this\nbut don\u0027t mind being explicit about it","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        Returns None if device is not found."},{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        LOG.debug(\u0027Looking for device where %s\u003d%s on controller %s\u0027,"},{"line_number":220,"context_line":"                  prop_name, value, self.controller)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # Look under the controller, where we will have normal devices and ANA"}],"source_content_type":"text/x-python","patch_set":11,"id":"bcc87ca8_0b12bda8","line":219,"range":{"start_line":219,"start_character":44,"end_line":219,"end_character":66},"updated":"2022-07-25 10:58:14.000000000","message":"no fstring here? :)","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46a9ff5414269f7ebdd6bb203975f7316a3d5e02","unresolved":false,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        Returns None if device is not found."},{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        LOG.debug(\u0027Looking for device where %s\u003d%s on controller %s\u0027,"},{"line_number":220,"context_line":"                  prop_name, value, self.controller)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # Look under the controller, where we will have normal devices and ANA"}],"source_content_type":"text/x-python","patch_set":11,"id":"f670cf78_21982039","line":219,"range":{"start_line":219,"start_character":44,"end_line":219,"end_character":66},"in_reply_to":"9173df8b_f066a18f","updated":"2022-07-25 17:56:24.000000000","message":"Ack","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":31022,"name":"Bartosz Rabiega","email":"bartosz.rabiega@ovhcloud.com","username":"Bartoszer"},"change_message_id":"303825f5915579b019faccd215c75f5da93b2934","unresolved":true,"context_lines":[{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        Returns None if device is not found."},{"line_number":218,"context_line":"        \"\"\""},{"line_number":219,"context_line":"        LOG.debug(\u0027Looking for device where %s\u003d%s on controller %s\u0027,"},{"line_number":220,"context_line":"                  prop_name, value, self.controller)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        # Look under the controller, where we will have normal devices and ANA"}],"source_content_type":"text/x-python","patch_set":11,"id":"9173df8b_f066a18f","line":219,"range":{"start_line":219,"start_character":44,"end_line":219,"end_character":66},"in_reply_to":"bcc87ca8_0b12bda8","updated":"2022-07-25 11:28:36.000000000","message":"F-strings in logs doesn\u0027t are discouraged. If debug messages are not displayed (due to logging level set to e.g. warn) the message string isn\u0027t constructed at all (saves some cpu cycles). If you\u0027d use f-string then message would be always constructed even if not emitted to logging handler.","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":230,"context_line":"                LOG.debug(\u0027Device found at %s, using %s\u0027, path, result)"},{"line_number":231,"context_line":"                return result"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"            LOG.debug(\u0027Block %s is not the one we look for (%s !\u003d %s)\u0027,"},{"line_number":234,"context_line":"                      path, prop_value, value)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        LOG.debug(\u0027No device Found on controller %s\u0027, self.controller)"}],"source_content_type":"text/x-python","patch_set":11,"id":"67b7a0cf_37e8778d","line":233,"range":{"start_line":233,"start_character":47,"end_line":233,"end_character":54},"updated":"2022-07-25 10:58:14.000000000","message":"nit: we are looking for?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1b692c01da0f40d5342798097e27ed43e3114452","unresolved":false,"context_lines":[{"line_number":230,"context_line":"                LOG.debug(\u0027Device found at %s, using %s\u0027, path, result)"},{"line_number":231,"context_line":"                return result"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"            LOG.debug(\u0027Block %s is not the one we look for (%s !\u003d %s)\u0027,"},{"line_number":234,"context_line":"                      path, prop_value, value)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        LOG.debug(\u0027No device Found on controller %s\u0027, self.controller)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fba820fe_3c4a7508","line":233,"range":{"start_line":233,"start_character":47,"end_line":233,"end_character":54},"in_reply_to":"67b7a0cf_37e8778d","updated":"2022-07-25 17:55:32.000000000","message":"Done","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":340,"context_line":"        hostnqn: str \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        # List of portal addresses and transports for this target"},{"line_number":343,"context_line":"        # Unlike \"nvme list-subsys -o json\" sysfs addr is separated by a coma"},{"line_number":344,"context_line":"        sysfs_portals: List[Tuple[Optional[str],"},{"line_number":345,"context_line":"                                  Optional[str],"},{"line_number":346,"context_line":"                                  Optional[Union[str, utils.Anything]]]] \u003d ["}],"source_content_type":"text/x-python","patch_set":11,"id":"4d2d0264_98f39f3a","line":343,"range":{"start_line":343,"start_character":73,"end_line":343,"end_character":77},"updated":"2022-07-25 10:58:14.000000000","message":"nit: comma\n\ncoma would be fatal!","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1b692c01da0f40d5342798097e27ed43e3114452","unresolved":false,"context_lines":[{"line_number":340,"context_line":"        hostnqn: str \u003d self.host_nqn or utils.get_host_nqn()"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        # List of portal addresses and transports for this target"},{"line_number":343,"context_line":"        # Unlike \"nvme list-subsys -o json\" sysfs addr is separated by a coma"},{"line_number":344,"context_line":"        sysfs_portals: List[Tuple[Optional[str],"},{"line_number":345,"context_line":"                                  Optional[str],"},{"line_number":346,"context_line":"                                  Optional[Union[str, utils.Anything]]]] \u003d ["}],"source_content_type":"text/x-python","patch_set":11,"id":"57a45c66_571b7443","line":343,"range":{"start_line":343,"start_character":73,"end_line":343,"end_character":77},"in_reply_to":"4d2d0264_98f39f3a","updated":"2022-07-25 17:55:32.000000000","message":"Done","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":704,"context_line":""},{"line_number":705,"context_line":"    @classmethod"},{"line_number":706,"context_line":"    def nvme_present(cls: type) -\u003e bool:"},{"line_number":707,"context_line":"        \"\"\"Check if the nvme command is present.\"\"\""},{"line_number":708,"context_line":"        try:"},{"line_number":709,"context_line":"            priv_rootwrap.custom_execute(\u0027nvme\u0027, \u0027version\u0027)"},{"line_number":710,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":11,"id":"9d827b85_0acd924a","line":707,"range":{"start_line":707,"start_character":29,"end_line":707,"end_character":36},"updated":"2022-07-25 10:58:14.000000000","message":"nit: tool/lib/package?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1b692c01da0f40d5342798097e27ed43e3114452","unresolved":false,"context_lines":[{"line_number":704,"context_line":""},{"line_number":705,"context_line":"    @classmethod"},{"line_number":706,"context_line":"    def nvme_present(cls: type) -\u003e bool:"},{"line_number":707,"context_line":"        \"\"\"Check if the nvme command is present.\"\"\""},{"line_number":708,"context_line":"        try:"},{"line_number":709,"context_line":"            priv_rootwrap.custom_execute(\u0027nvme\u0027, \u0027version\u0027)"},{"line_number":710,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":11,"id":"ad1c0b13_8b19b119","line":707,"range":{"start_line":707,"start_character":29,"end_line":707,"end_character":36},"in_reply_to":"9d827b85_0acd924a","updated":"2022-07-25 17:55:32.000000000","message":"Done","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":722,"context_line":"        execute \u003d kwargs.get(\u0027execute\u0027) or priv_rootwrap.execute"},{"line_number":723,"context_line":"        nvmf \u003d NVMeOFConnector(root_helper\u003droot_helper, execute\u003dexecute)"},{"line_number":724,"context_line":"        ret \u003d {}"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"        nqn \u003d None"},{"line_number":727,"context_line":"        uuid \u003d nvmf._get_host_uuid()"},{"line_number":728,"context_line":"        suuid \u003d nvmf._get_system_uuid()"}],"source_content_type":"text/x-python","patch_set":11,"id":"049902aa_110f4a97","line":725,"updated":"2022-07-25 10:58:14.000000000","message":"nit: unrelated change?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"}],"os_brick/tests/initiator/connectors/test_nvmeof.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        mock_property.return_value \u003d \u0027result\u0027"},{"line_number":193,"context_line":"        self.target.uuid \u003d None"},{"line_number":194,"context_line":"        self.target.nguid \u003d None"},{"line_number":195,"context_line":"        self.target.ns_id \u003d \u0027ns_id_value\u0027  # will be ignored"},{"line_number":196,"context_line":"        res \u003d self.portal.get_device()"},{"line_number":197,"context_line":"        self.assertEqual(\u0027result\u0027, res)"},{"line_number":198,"context_line":"        mock_property.assert_called_once_with(\u0027nsid\u0027, \u0027ns_id_value\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"748bbc4b_70ad3477","line":195,"range":{"start_line":195,"start_character":43,"end_line":195,"end_character":60},"updated":"2022-07-25 10:58:14.000000000","message":"nit: ns_id_value will not be ignored, bad copy paste?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1b692c01da0f40d5342798097e27ed43e3114452","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        mock_property.return_value \u003d \u0027result\u0027"},{"line_number":193,"context_line":"        self.target.uuid \u003d None"},{"line_number":194,"context_line":"        self.target.nguid \u003d None"},{"line_number":195,"context_line":"        self.target.ns_id \u003d \u0027ns_id_value\u0027  # will be ignored"},{"line_number":196,"context_line":"        res \u003d self.portal.get_device()"},{"line_number":197,"context_line":"        self.assertEqual(\u0027result\u0027, res)"},{"line_number":198,"context_line":"        mock_property.assert_called_once_with(\u0027nsid\u0027, \u0027ns_id_value\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1d318b58_b3ceb9c9","line":195,"range":{"start_line":195,"start_character":43,"end_line":195,"end_character":60},"in_reply_to":"748bbc4b_70ad3477","updated":"2022-07-25 17:55:32.000000000","message":"Done","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"55180f9026642487f12bbe18728f258e68f41ef8","unresolved":true,"context_lines":[{"line_number":799,"context_line":"                      \u0027portals\u0027: [(\u0027portal1\u0027, \u0027port_value\u0027, \u0027RoCEv2\u0027),"},{"line_number":800,"context_line":"                                  (\u0027portal2\u0027, \u0027port_value\u0027, \u0027anything\u0027)]}"},{"line_number":801,"context_line":"        res \u003d conn.connect_volume(conn_props)"},{"line_number":802,"context_line":""},{"line_number":803,"context_line":"        self.assertEqual(\u0027result\u0027, res)"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"993b926f_e764b34d","line":802,"updated":"2022-07-25 10:58:14.000000000","message":"should we also assert that the dict was successfully converted into NVMeOFConnProps object?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"}],"os_brick/utils.py":[{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"9bdf0bb931754a3100505c629ddbb09762a35a73","unresolved":true,"context_lines":[{"line_number":408,"context_line":"        return None"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":""},{"line_number":411,"context_line":"class Anything(object):"},{"line_number":412,"context_line":"    \"\"\"Object equal to everything.\"\"\""},{"line_number":413,"context_line":"    def __eq__(self, other):"},{"line_number":414,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f75b79d_4db767a7","line":411,"updated":"2022-07-19 12:28:59.000000000","message":"curios why this is needed?","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aa58a4bbdcc3ff0a9075ba0ee464694a55d670dd","unresolved":false,"context_lines":[{"line_number":408,"context_line":"        return None"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":""},{"line_number":411,"context_line":"class Anything(object):"},{"line_number":412,"context_line":"    \"\"\"Object equal to everything.\"\"\""},{"line_number":413,"context_line":"    def __eq__(self, other):"},{"line_number":414,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":11,"id":"bf40deaa_aa956398","line":411,"in_reply_to":"9f75b79d_4db767a7","updated":"2022-07-21 08:54:12.000000000","message":"This is needed because systems may be missing the hostnqn in sysfs, so we cannot use that information to filter NVMe controllers to detect the controller for a target.  So we use an Anything instance to say we don\u0027t care: https://review.opendev.org/c/openstack/os-brick/+/836060/11/os_brick/initiator/connectors/nvmeof.py#371","commit_id":"4c21b40df20ef497c7a3c82ffcf32fe21621034c"}]}