)]}'
{"zun/volume/cinder_workflow.py":[{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"6dd2a27b5e18f2faed0c1fca270f72c77fd56113","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                                    six.text_type(e))"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        conn_info \u003d jsonutils.loads(volume.connection_info)"},{"line_number":161,"context_line":"        if not self._volume_connection_keep(context, volume_id):"},{"line_number":162,"context_line":"            try:"},{"line_number":163,"context_line":"                self._disconnect_volume(conn_info)"},{"line_number":164,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_cb1a6b6d","line":161,"range":{"start_line":161,"start_character":20,"end_line":161,"end_character":43},"updated":"2018-08-16 03:28:06.000000000","message":"@weikeyou,\n\nI have another patch (https://review.openstack.org/#/c/592250/) which might fix the same problem. I compared it with this patch and it looks the major difference is that you disconnect the volume conditionally and I always disconnect the volume.\n\nI am not an expert of this volume staff so want to get a better understanding of this difference. Is there any reason you want to disconnect the volume based on this condition. If we remove this condition, anything will break.\n\nIf it turns out it is necessary to do this, I can abandon my patch and merge this patch.","commit_id":"8073756f612d0d8414a399f25a952dbee6f6b5be"},{"author":{"_account_id":23829,"name":"weikeyou","email":"wei.keyou@zte.com.cn","username":"weikeyou"},"change_message_id":"fa70d389480c73c87476d1170aea255b5c90fce1","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                                    six.text_type(e))"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        conn_info \u003d jsonutils.loads(volume.connection_info)"},{"line_number":161,"context_line":"        if not self._volume_connection_keep(context, volume_id):"},{"line_number":162,"context_line":"            try:"},{"line_number":163,"context_line":"                self._disconnect_volume(conn_info)"},{"line_number":164,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_eb4167a7","line":161,"range":{"start_line":161,"start_character":20,"end_line":161,"end_character":43},"in_reply_to":"3f79a3b5_cb1a6b6d","updated":"2018-08-16 04:18:09.000000000","message":"I have look up this patch (https://review.openstack.org/#/c/592250/), The following case might be considered.\nIf a volume is attached to two container that is on the same node, they share device path and LUN group in Disk Array. In this case, \"_disconnect_volume\"(clean up device path) and \"terminate_connection\"(remove LUN group in Disk Array) should not be called, and just \"cinder_api.detach\"(clean up db in Cinder) should be called.","commit_id":"8073756f612d0d8414a399f25a952dbee6f6b5be"},{"author":{"_account_id":11536,"name":"hongbin","email":"hongbin034@gmail.com","username":"hongbin"},"change_message_id":"db0d60391e5b27f95e23b60f759b571a1ca3d870","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                                    six.text_type(e))"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        conn_info \u003d jsonutils.loads(volume.connection_info)"},{"line_number":161,"context_line":"        if not self._volume_connection_keep(context, volume_id):"},{"line_number":162,"context_line":"            try:"},{"line_number":163,"context_line":"                self._disconnect_volume(conn_info)"},{"line_number":164,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_d8e76646","line":161,"range":{"start_line":161,"start_character":20,"end_line":161,"end_character":43},"in_reply_to":"3f79a3b5_eb4167a7","updated":"2018-08-18 00:48:00.000000000","message":"I see. I agree with you that this approach is correct. I will approve this patch after resolving the merge conflict.","commit_id":"8073756f612d0d8414a399f25a952dbee6f6b5be"}]}
