)]}'
{"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"b1729246a19d91cbd33cf788971b6d4febe499f4","unresolved":true,"context_lines":[{"line_number":419,"context_line":"                        \"subnqn %(conn_nqn)s that is not connected.\","},{"line_number":420,"context_line":"                        {\u0027device_path\u0027: device_path, \u0027conn_nqn\u0027: conn_nqn})"},{"line_number":421,"context_line":"            return"},{"line_number":422,"context_line":"        nvme_controller \u003d self._get_nvme_controller(self, conn_nqn)"},{"line_number":423,"context_line":"        if self._is_last_nvme_device(current_nvme_devices,"},{"line_number":424,"context_line":"                                     nvme_controller,"},{"line_number":425,"context_line":"                                     device_path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"24648343_d4438cfd","line":422,"updated":"2021-08-12 11:06:04.000000000","message":"`NVMeOFConnector._get_nvme_controller(executor, nqn)` definition is getting renamed (prefix `_` removed because it is also used outside connector) and redefined to `get_nvme_controller(nqn)` (no executor param needed) in the nvmeof connection agent change https://review.opendev.org/c/openstack/os-brick/+/802691\n\nI am glad you are using this method! Depending on which patch gets merged first, would you be ok to rename this call to `self.get_nvme_controller(conn_nqn)` ? Or I could do that change in the other patch if this one merges first this way.\n\nThank you!","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"c51d9c1593476a3b0bbf817e30f8b1dda8d70c62","unresolved":false,"context_lines":[{"line_number":419,"context_line":"                        \"subnqn %(conn_nqn)s that is not connected.\","},{"line_number":420,"context_line":"                        {\u0027device_path\u0027: device_path, \u0027conn_nqn\u0027: conn_nqn})"},{"line_number":421,"context_line":"            return"},{"line_number":422,"context_line":"        nvme_controller \u003d self._get_nvme_controller(self, conn_nqn)"},{"line_number":423,"context_line":"        if self._is_last_nvme_device(current_nvme_devices,"},{"line_number":424,"context_line":"                                     nvme_controller,"},{"line_number":425,"context_line":"                                     device_path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"f9cac7c0_678d6139","line":422,"in_reply_to":"1f843ee8_39f0ed9e","updated":"2021-08-13 04:24:21.000000000","message":"Sounds good.\n\nI think we should merge this patch first, then I will rebase my patch, resolve merge conflicts, and update the _get_nvme_controller() call in your code.","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"6fcc85be842cef6ba57ba33c5f000ac1f19f65be","unresolved":true,"context_lines":[{"line_number":419,"context_line":"                        \"subnqn %(conn_nqn)s that is not connected.\","},{"line_number":420,"context_line":"                        {\u0027device_path\u0027: device_path, \u0027conn_nqn\u0027: conn_nqn})"},{"line_number":421,"context_line":"            return"},{"line_number":422,"context_line":"        nvme_controller \u003d self._get_nvme_controller(self, conn_nqn)"},{"line_number":423,"context_line":"        if self._is_last_nvme_device(current_nvme_devices,"},{"line_number":424,"context_line":"                                     nvme_controller,"},{"line_number":425,"context_line":"                                     device_path):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f843ee8_39f0ed9e","line":422,"in_reply_to":"24648343_d4438cfd","updated":"2021-08-12 13:07:46.000000000","message":"thanks Zohar, yes we can synchronize the function name any way :)","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"b1729246a19d91cbd33cf788971b6d4febe499f4","unresolved":true,"context_lines":[{"line_number":458,"context_line":"            if not ignore_errors:"},{"line_number":459,"context_line":"                raise exc"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _is_last_nvme_device(self,"},{"line_number":462,"context_line":"                             current_nvme_devices,"},{"line_number":463,"context_line":"                             nvme_controller,"},{"line_number":464,"context_line":"                             nvme_device):"}],"source_content_type":"text/x-python","patch_set":3,"id":"a6462772_8504e7d2","line":461,"updated":"2021-08-12 11:06:04.000000000","message":"Just a discussion note (not asking for changes,) is there a race condition possible here?\n\nCould it happen that during a new volume connetion on the same nvme subsystem (especially by another process, such as between nova and cinder image copy) where it takes some time for the device to appear and be found, then this could trigger the same controller to be disconnected on line 423?","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"c51d9c1593476a3b0bbf817e30f8b1dda8d70c62","unresolved":false,"context_lines":[{"line_number":458,"context_line":"            if not ignore_errors:"},{"line_number":459,"context_line":"                raise exc"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _is_last_nvme_device(self,"},{"line_number":462,"context_line":"                             current_nvme_devices,"},{"line_number":463,"context_line":"                             nvme_controller,"},{"line_number":464,"context_line":"                             nvme_device):"}],"source_content_type":"text/x-python","patch_set":3,"id":"d514480e_554796e2","line":461,"in_reply_to":"3cfe0965_2d400a6c","updated":"2021-08-13 04:24:21.000000000","message":"We ran into such issues with KumoScale, that is why we decided not to do `nvme disconnect` during `disconnect_volume()`\n\nWe also looked at doing it when there were zero (instead of one) devices left in the subsystem on `disconnect_volume()`, but that was also causing the same race condition.\n\nAs you mentioned, the \"unpublish/unmap/unbind\" on the storage system end is asynchronous to this code, so even with inter-process locking (which @lockutils.synchronized supposed to have such a feature, but it did not work for us) - there will still be a race problem.","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"6fcc85be842cef6ba57ba33c5f000ac1f19f65be","unresolved":true,"context_lines":[{"line_number":458,"context_line":"            if not ignore_errors:"},{"line_number":459,"context_line":"                raise exc"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _is_last_nvme_device(self,"},{"line_number":462,"context_line":"                             current_nvme_devices,"},{"line_number":463,"context_line":"                             nvme_controller,"},{"line_number":464,"context_line":"                             nvme_device):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3cfe0965_2d400a6c","line":461,"in_reply_to":"a6462772_8504e7d2","updated":"2021-08-12 13:07:46.000000000","message":"yes, this is a good question. Volume disconnect happens not here but via \"unmap/unbind\" commands on storage system side. Here we try to disconnect a subsystem which is not really necessary, especially if it can be in race with other processes. May be it will be less harm to keep subsystem connection active.","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"748739712f69efffcd089024748d3fab3fde5f35","unresolved":true,"context_lines":[{"line_number":271,"context_line":"        return nvme_devices_filtered"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    @utils.retry(exception.NotFound, retries\u003d5)"},{"line_number":274,"context_line":"    def _is_nvme_available(self, nvme_name):"},{"line_number":275,"context_line":"        current_nvme_devices \u003d self._get_nvme_devices()"},{"line_number":276,"context_line":"        if self._filter_nvme_devices(current_nvme_devices, nvme_name):"},{"line_number":277,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":4,"id":"fdac1d26_20da9e3c","line":274,"range":{"start_line":274,"start_character":8,"end_line":274,"end_character":26},"updated":"2021-08-13 17:36:18.000000000","message":"not strictly your problem, but looking at the code coverage report, this function doesn\u0027t have coverage, and the two functions it calls don\u0027t have coverage either, and it\u0027s called by _wait_for_blk(), that also doesn\u0027t have any test coverage.  So you should look these all over carefully and add tests if you see anything weird or easily breakable in refactoring.","commit_id":"85a8a207c889d97372e095b7646cc09dbf71cf87"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"748739712f69efffcd089024748d3fab3fde5f35","unresolved":true,"context_lines":[{"line_number":405,"context_line":"                        {\u0027device_path\u0027: device_path, \u0027conn_nqn\u0027: conn_nqn})"},{"line_number":406,"context_line":"            return"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"        exc \u003d exception.ExceptionChainer()"},{"line_number":409,"context_line":"        with exc.context(force, \u0027Flushing %s failed\u0027, device_path):"},{"line_number":410,"context_line":"            self._linuxscsi.flush_device_io(device_path)"},{"line_number":411,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d299e91d_07017295","line":408,"range":{"start_line":408,"start_character":8,"end_line":408,"end_character":42},"updated":"2021-08-13 17:36:18.000000000","message":"you don\u0027t need the exception chainer any more now that you\u0027re no longer trying multiple operations, and you\u0027re not checking ignore_errors to decide whether to raise or not.  I suggest going back to the pattern \n\n  try:\n      flush_device_io()\n  except putils.ProcessExecutionError:\n      # don\u0027t need to log, flush_device_io() will log\n      if not ignore_errors:\n          raise\n\n(the exception chaining was introduced in change Ie6c8bdf1f3a)","commit_id":"85a8a207c889d97372e095b7646cc09dbf71cf87"},{"author":{"_account_id":34550,"name":"Gili Buzaglo","email":"gili.buzaglo@kioxia.com","username":"gilib"},"change_message_id":"607e334bb360f9cd3f7bdd65eff095e8de9fe5df","unresolved":true,"context_lines":[{"line_number":340,"context_line":"        port \u003d connection_properties[\u0027target_port\u0027]"},{"line_number":341,"context_line":"        nvme_transport_type \u003d connection_properties[\u0027transport_type\u0027]"},{"line_number":342,"context_line":"        host_nqn \u003d connection_properties.get(\u0027host_nqn\u0027)"},{"line_number":343,"context_line":"        device_nguid \u003d connection_properties.get(\u0027volume_nguid\u0027)"},{"line_number":344,"context_line":"        cmd \u003d ["},{"line_number":345,"context_line":"            \u0027nvme\u0027, \u0027connect\u0027,"},{"line_number":346,"context_line":"            \u0027-t\u0027, nvme_transport_type,"}],"source_content_type":"text/x-python","patch_set":7,"id":"807476d6_1a683c4c","line":343,"updated":"2022-02-16 15:23:26.000000000","message":"Its not good that each vendor invents its own property name for the volume UUID.\nKIOXIA calls it vol_uuid, and here Dell added volume_nguid.\nCore maintainers should decide on a single property for that.","commit_id":"4edb7c641ee84d6080d0a011903b0cdab1bb203a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"a57f4139d33de636e8049080be37c4012559c68b","unresolved":true,"context_lines":[{"line_number":361,"context_line":"        if device_nguid:"},{"line_number":362,"context_line":"            path \u003d self._get_device_path_by_nguid(device_nguid)"},{"line_number":363,"context_line":"        else:"},{"line_number":364,"context_line":"            path \u003d self._get_device_path(current_nvme_devices)"},{"line_number":365,"context_line":"        device_info[\u0027path\u0027] \u003d path"},{"line_number":366,"context_line":"        LOG.debug(\"NVMe device to be connected to is %(path)s\","},{"line_number":367,"context_line":"                  {\u0027path\u0027: path})"}],"source_content_type":"text/x-python","patch_set":7,"id":"e9268290_d35afa5a","line":364,"range":{"start_line":364,"start_character":12,"end_line":364,"end_character":62},"updated":"2021-09-28 16:18:09.000000000","message":"This is a regression, _get_device_path has always returned a list with only the first item being returned in device_info previously. With this change now _all_ paths are returned.","commit_id":"4edb7c641ee84d6080d0a011903b0cdab1bb203a"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"326dac77f308666dbc9798d0909a007f0330d88c","unresolved":false,"context_lines":[{"line_number":362,"context_line":"            path \u003d self._get_device_path_by_nguid(device_nguid)"},{"line_number":363,"context_line":"        else:"},{"line_number":364,"context_line":"            path \u003d self._get_device_path(current_nvme_devices)"},{"line_number":365,"context_line":"        device_info[\u0027path\u0027] \u003d path"},{"line_number":366,"context_line":"        LOG.debug(\"NVMe device to be connected to is %(path)s\","},{"line_number":367,"context_line":"                  {\u0027path\u0027: path})"},{"line_number":368,"context_line":"        return device_info"}],"source_content_type":"text/x-python","patch_set":7,"id":"cad28a64_2f4afa6c","line":365,"updated":"2021-09-30 09:46:43.000000000","message":"https://review.opendev.org/c/openstack/os-brick/+/811944","commit_id":"4edb7c641ee84d6080d0a011903b0cdab1bb203a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"208836ee17fbc33b5ff4b2d5701c98e0f53665b3","unresolved":false,"context_lines":[{"line_number":362,"context_line":"            path \u003d self._get_device_path_by_nguid(device_nguid)"},{"line_number":363,"context_line":"        else:"},{"line_number":364,"context_line":"            path \u003d self._get_device_path(current_nvme_devices)"},{"line_number":365,"context_line":"        device_info[\u0027path\u0027] \u003d path"},{"line_number":366,"context_line":"        LOG.debug(\"NVMe device to be connected to is %(path)s\","},{"line_number":367,"context_line":"                  {\u0027path\u0027: path})"},{"line_number":368,"context_line":"        return device_info"}],"source_content_type":"text/x-python","patch_set":7,"id":"dd395e53_86fd5213","line":365,"in_reply_to":"cad28a64_2f4afa6c","updated":"2021-09-30 13:59:35.000000000","message":"Also https://review.opendev.org/c/openstack/os-brick/+/811886","commit_id":"4edb7c641ee84d6080d0a011903b0cdab1bb203a"}],"releasenotes/notes/nvmeof-multiple-volumes-within-subsystem-support-05879c1c3bdf52c9.yaml":[{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"12fb46d973dfb72d18cf71929d1b5530b33098b4","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    NVMe-OF connector: Added support for storage systems presenting"},{"line_number":5,"context_line":"    multiple volumes within one NVMe subsystem."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"4b2a7760_2c786528","line":5,"updated":"2021-08-12 13:33:52.000000000","message":"nit. Missing newline","commit_id":"55e52144e0fc647f7c604bf487506b8b5ac042b7"}]}
