)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"nvmeof.py: Disconnect NVMe driver in disconnect_volume()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"disconnect_volume() needs to be done by storage system host. Disconnecting controller and then removing desired namespace from a subsystem on storage system side will propagate to client. The only thing left for client to do is a controller removal. Nvme disconnect with \u0027-d\u0027 flag removes device and it listeners not removing the subsystem itself if it is not empty yet."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Signed-off-by: mionsz \u003cmilosz.linkiewicz@intel.com\u003e"},{"line_number":12,"context_line":"Change-Id: I99bb0aa937bdde04cf8a5fbb3c447319a7da27f2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"b53c2794_ac559f7e","line":9,"updated":"2022-02-23 21:56:29.000000000","message":"-1: Please wrap on 72 characters as described in the documentation: https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"disconnect_volume() needs to be done by storage system host. Disconnecting controller and then removing desired namespace from a subsystem on storage system side will propagate to client. The only thing left for client to do is a controller removal. Nvme disconnect with \u0027-d\u0027 flag removes device and it listeners not removing the subsystem itself if it is not empty yet."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Signed-off-by: mionsz \u003cmilosz.linkiewicz@intel.com\u003e"},{"line_number":12,"context_line":"Change-Id: I99bb0aa937bdde04cf8a5fbb3c447319a7da27f2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8023b9a1_30f9eaef","line":11,"updated":"2022-02-23 21:56:29.000000000","message":"-1: Please add the footer:\n\nCloses-Bug: #1961102\n\nAnd also add a release note mentioning the fix.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":30921,"name":"Abdallah Yasin","email":"abdallahyas@mellanox.com","username":"abdallahyas"},"change_message_id":"a728589d3daf139460f1ca611eefc77a0927d17f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cc2256e2_f5bc93f3","updated":"2022-01-19 14:15:31.000000000","message":"Thank you for the patch,\nthis seems to fix a race condition we had in the SPDK CI.\n","commit_id":"a0e122c36f0947e36cc04b5d4a1a3268a5957a42"},{"author":{"_account_id":34444,"name":"Miłosz Linkiewicz","email":"milosz.linkiewicz@intel.com","username":"Mionsz"},"change_message_id":"968b76e48116afd943d4f64a2dc5e3be85640b27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1ea2ad52_c06f507b","in_reply_to":"cc2256e2_f5bc93f3","updated":"2022-01-19 19:26:00.000000000","message":"No problem, nice to hear that :).","commit_id":"a0e122c36f0947e36cc04b5d4a1a3268a5957a42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"124ddede_7719bdab","updated":"2022-02-23 21:56:29.000000000","message":"Thank you very much for working on this fix.\n\nThere are some things that need improving, since right now it would break cinder drivers that use the same subsystem to serve multiple volumes.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"}],"os_brick/initiator/connectors/nvmeof.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":230,"context_line":"            raise"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    def _get_nvme_subsys(self):"},{"line_number":233,"context_line":"        # Example output:"},{"line_number":234,"context_line":"        # {"},{"line_number":235,"context_line":"        #   \u0027Subsystems\u0027 : ["},{"line_number":236,"context_line":"        #     {"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa5a0b29_f7f11317","line":233,"updated":"2022-02-23 21:56:29.000000000","message":"nit: All these PEP8 issues are introduced in the previous patch in the series, so they should be fixed there.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":375,"context_line":"                               target_portal, port)"},{"line_number":376,"context_line":"        except exception.NotFound:"},{"line_number":377,"context_line":"            LOG.error(\"Waiting for nvme failed\")"},{"line_number":378,"context_line":"            raise exception.NotFound(message\u003d\"nvme connect: NVMe device \""},{"line_number":379,"context_line":"                                             \"not found\")"},{"line_number":380,"context_line":"        if device_nguid:"},{"line_number":381,"context_line":"            path \u003d self._get_device_path_by_nguid(device_nguid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"967cac64_37542235","line":378,"updated":"2022-02-23 21:56:29.000000000","message":"-1: A disconnect should also happen here, so it\u0027s probably best to use a common method for both cases.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":390,"context_line":"    @synchronized(\u0027connect_volume\u0027, external\u003dTrue)"},{"line_number":391,"context_line":"    def disconnect_volume(self, connection_properties, device_info,"},{"line_number":392,"context_line":"                          force\u003dFalse, ignore_errors\u003dFalse):"},{"line_number":393,"context_line":"        \"\"\"Flush the volume."},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"        Disconnect of volumes happens on storage system side. Deletion of"},{"line_number":396,"context_line":"        namespace from a subsystem should propagate from storage system up."}],"source_content_type":"text/x-python","patch_set":2,"id":"2d494336_3f80f9a0","line":393,"range":{"start_line":393,"start_character":11,"end_line":393,"end_character":16},"updated":"2022-02-23 21:56:29.000000000","message":"nit: Mention the disconnect part, since we are no longer doing just the flushing.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":392,"context_line":"                          force\u003dFalse, ignore_errors\u003dFalse):"},{"line_number":393,"context_line":"        \"\"\"Flush the volume."},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"        Disconnect of volumes happens on storage system side. Deletion of"},{"line_number":396,"context_line":"        namespace from a subsystem should propagate from storage system up."},{"line_number":397,"context_line":"        The only thing left is flushing or disassembly of a correspondng"},{"line_number":398,"context_line":"        RAID device."},{"line_number":399,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2b205236_cd3a787f","line":396,"range":{"start_line":395,"start_character":8,"end_line":396,"end_character":75},"updated":"2022-02-23 21:56:29.000000000","message":"nit: Same thing here, mention the disconnect.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":424,"context_line":"                        {\u0027device_path\u0027: device_path, \u0027conn_nqn\u0027: conn_nqn})"},{"line_number":425,"context_line":"            return"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        device \u003d self._get_nvme_controller(self, conn_nqn, anystate\u003dTrue)"},{"line_number":428,"context_line":"        if device is not None:"},{"line_number":429,"context_line":"            cmd \u003d [\u0027nvme\u0027, \u0027disconnect\u0027, \u0027-d\u0027, \u0027/dev/\u0027 + str(device)]"},{"line_number":430,"context_line":"            self._execute(*cmd,"}],"source_content_type":"text/x-python","patch_set":2,"id":"37b34679_383862da","line":427,"updated":"2022-02-23 21:56:29.000000000","message":"-1: We should reintroduce the old exception.ExceptionChainer that was used before change I5f73ec4287f6ab2a0a5ae83748947fa56a0eeb9d [1] removed it.\n\n[1]: https://review.opendev.org/c/openstack/os-brick/+/800014","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        device \u003d self._get_nvme_controller(self, conn_nqn, anystate\u003dTrue)"},{"line_number":428,"context_line":"        if device is not None:"},{"line_number":429,"context_line":"            cmd \u003d [\u0027nvme\u0027, \u0027disconnect\u0027, \u0027-d\u0027, \u0027/dev/\u0027 + str(device)]"},{"line_number":430,"context_line":"            self._execute(*cmd,"},{"line_number":431,"context_line":"                          root_helper\u003dself._root_helper,"},{"line_number":432,"context_line":"                          run_as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"4ae53b22_a826304d","line":429,"range":{"start_line":429,"start_character":18,"end_line":429,"end_character":69},"updated":"2022-02-23 21:56:29.000000000","message":"-1: We cannot do this, because this introduces a regression of what change I5f73ec4287f6ab2a0a5ae83748947fa56a0eeb9d [1] was trying to fix, which is that NVMe drivers that share the subsystem for multiple namespaces get them all disconnected when we detach a single volume.\n\nTo know if we can disconnect we need to know if there are additional devices using that subsystem.\n\n[1]: https://review.opendev.org/c/openstack/os-brick/+/800014","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"695f1afe9fc2c2e5ad405c174e1f8d7b24db0f08","unresolved":true,"context_lines":[{"line_number":427,"context_line":"        device \u003d self._get_nvme_controller(self, conn_nqn, anystate\u003dTrue)"},{"line_number":428,"context_line":"        if device is not None:"},{"line_number":429,"context_line":"            cmd \u003d [\u0027nvme\u0027, \u0027disconnect\u0027, \u0027-d\u0027, \u0027/dev/\u0027 + str(device)]"},{"line_number":430,"context_line":"            self._execute(*cmd,"},{"line_number":431,"context_line":"                          root_helper\u003dself._root_helper,"},{"line_number":432,"context_line":"                          run_as_root\u003dTrue)"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":"        try:"},{"line_number":435,"context_line":"            self._linuxscsi.flush_device_io(device_path)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b279e84b_3b3db8f9","line":432,"range":{"start_line":430,"start_character":0,"end_line":432,"end_character":43},"updated":"2022-02-23 21:56:29.000000000","message":"-1: Disconnect should happen *after* we have flushed the device.","commit_id":"550ef04ea03755374c13afe7193e8165ce197774"}]}