)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e1ed6101cba8a8bdba9f42cedd090826d419fe3c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b7480d43_8d29938f","updated":"2022-07-22 07:11:41.000000000","message":"Few comments inline but in general, I don\u0027t think creating lock + volume files is a good approach to handle this case","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":32927,"name":"mitya-eremeev-2","display_name":"Mitya Eremeev","email":"mitossvyaz@mail.ru","username":"mitya-eremeev-2"},"change_message_id":"010b537d36dfcbf7d096b35164eaec213c814e85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"70868454_0c46b648","in_reply_to":"b7480d43_8d29938f","updated":"2022-07-22 09:54:14.000000000","message":"It\u0027s standard openstack approach for process locking (oslo_concurrency).\nIf you have better concrete detailed solution - please share.","commit_id":"28857ced946a94749959d04b670326de07ba96fe"}],"glance_store/common/attachments_for_processes.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e1ed6101cba8a8bdba9f42cedd090826d419fe3c","unresolved":true,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"HOST \u003d socket.gethostname()"},{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"attachments_folder \u003d os.path.join(get_lock_path(CONF), \u0027attachments\u0027)"},{"line_number":34,"context_line":"lock_path \u003d os.path.join(attachments_folder, \u0027locks\u0027)"},{"line_number":35,"context_line":"state_path \u003d os.path.join(attachments_folder, \u0027state\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"@contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":4,"id":"6b20aa40_08f23c92","line":35,"range":{"start_line":32,"start_character":0,"end_line":35,"end_character":54},"updated":"2022-07-22 07:11:41.000000000","message":"I don\u0027t these directories already exist, where are we creating them?","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":32927,"name":"mitya-eremeev-2","display_name":"Mitya Eremeev","email":"mitossvyaz@mail.ru","username":"mitya-eremeev-2"},"change_message_id":"010b537d36dfcbf7d096b35164eaec213c814e85","unresolved":true,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"HOST \u003d socket.gethostname()"},{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"attachments_folder \u003d os.path.join(get_lock_path(CONF), \u0027attachments\u0027)"},{"line_number":34,"context_line":"lock_path \u003d os.path.join(attachments_folder, \u0027locks\u0027)"},{"line_number":35,"context_line":"state_path \u003d os.path.join(attachments_folder, \u0027state\u0027)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"@contextlib.contextmanager"}],"source_content_type":"text/x-python","patch_set":4,"id":"3834a3a3_8751564b","line":35,"range":{"start_line":32,"start_character":0,"end_line":35,"end_character":54},"in_reply_to":"6b20aa40_08f23c92","updated":"2022-07-22 09:54:14.000000000","message":"Directory for lock path is created here by oslo concurrency lib:\nwith external_lock(volume, lock_file_prefix\u003d_prefix, lock_path\u003dlock_path)\n \nand state folder is created here:\nos.makedirs(state_path, exist_ok\u003dTrue)","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e1ed6101cba8a8bdba9f42cedd090826d419fe3c","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            yield"},{"line_number":45,"context_line":"        finally:"},{"line_number":46,"context_line":"            remove_external_lock_file(volume, lock_file_prefix\u003d_prefix,"},{"line_number":47,"context_line":"                                      lock_path\u003dlock_path)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _attach(client, volume_id, host, mode\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"621e77db_6aa1009c","line":47,"range":{"start_line":46,"start_character":12,"end_line":47,"end_character":58},"updated":"2022-07-22 07:11:41.000000000","message":"what happens if the glace process dies during the operation and this is never executed? are we accounting for lock files cleanup when the service initializes?\nIf not, we will end up having stale lock files","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":32927,"name":"mitya-eremeev-2","display_name":"Mitya Eremeev","email":"mitossvyaz@mail.ru","username":"mitya-eremeev-2"},"change_message_id":"010b537d36dfcbf7d096b35164eaec213c814e85","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            yield"},{"line_number":45,"context_line":"        finally:"},{"line_number":46,"context_line":"            remove_external_lock_file(volume, lock_file_prefix\u003d_prefix,"},{"line_number":47,"context_line":"                                      lock_path\u003dlock_path)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _attach(client, volume_id, host, mode\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"1fe84f35_d7de4a9e","line":47,"range":{"start_line":46,"start_character":12,"end_line":47,"end_character":58},"in_reply_to":"621e77db_6aa1009c","updated":"2022-07-22 09:54:14.000000000","message":"Yes, if process dies we have stale lock files.\nPlease share your concrete solution for the case.","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e1ed6101cba8a8bdba9f42cedd090826d419fe3c","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            LOG.exception(_LE(\u0027Error attaching volume %(volume_id)s\u0027,"},{"line_number":68,"context_line":"                          {\u0027volume_id\u0027: volume_id}))"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":"        volume_state_path \u003d os.path.join(state_path, volume_id)"},{"line_number":71,"context_line":"        try:"},{"line_number":72,"context_line":"            with open(volume_state_path) as volume_state_file:"},{"line_number":73,"context_line":"                volume_state \u003d json.load(volume_state_file)"},{"line_number":74,"context_line":"        except FileNotFoundError:"},{"line_number":75,"context_line":"            os.makedirs(state_path, exist_ok\u003dTrue)"},{"line_number":76,"context_line":"            volume_state \u003d {\"attachments\": []}"},{"line_number":77,"context_line":"        volume_state[\"attachments\"].append(attachment[\"id\"])"},{"line_number":78,"context_line":"        with open(volume_state_path, \u0027w\u0027) as volume_state_file:"},{"line_number":79,"context_line":"            json.dump(volume_state, volume_state_file)"},{"line_number":80,"context_line":"    LOG.info("},{"line_number":81,"context_line":"        \u0027Attached volume %(volume_id)s to host %(host)s with mode %(mode)s \u0027"},{"line_number":82,"context_line":"        \u0027successfully. Attachment_id\u003d%(attachment_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9493fc27_2d96af62","line":79,"range":{"start_line":70,"start_character":0,"end_line":79,"end_character":54},"updated":"2022-07-22 07:11:41.000000000","message":"so for every volume, we are creating/modifying a file\nThis doesn\u0027t seem like a good approach in general","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":32927,"name":"mitya-eremeev-2","display_name":"Mitya Eremeev","email":"mitossvyaz@mail.ru","username":"mitya-eremeev-2"},"change_message_id":"010b537d36dfcbf7d096b35164eaec213c814e85","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            LOG.exception(_LE(\u0027Error attaching volume %(volume_id)s\u0027,"},{"line_number":68,"context_line":"                          {\u0027volume_id\u0027: volume_id}))"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":"        volume_state_path \u003d os.path.join(state_path, volume_id)"},{"line_number":71,"context_line":"        try:"},{"line_number":72,"context_line":"            with open(volume_state_path) as volume_state_file:"},{"line_number":73,"context_line":"                volume_state \u003d json.load(volume_state_file)"},{"line_number":74,"context_line":"        except FileNotFoundError:"},{"line_number":75,"context_line":"            os.makedirs(state_path, exist_ok\u003dTrue)"},{"line_number":76,"context_line":"            volume_state \u003d {\"attachments\": []}"},{"line_number":77,"context_line":"        volume_state[\"attachments\"].append(attachment[\"id\"])"},{"line_number":78,"context_line":"        with open(volume_state_path, \u0027w\u0027) as volume_state_file:"},{"line_number":79,"context_line":"            json.dump(volume_state, volume_state_file)"},{"line_number":80,"context_line":"    LOG.info("},{"line_number":81,"context_line":"        \u0027Attached volume %(volume_id)s to host %(host)s with mode %(mode)s \u0027"},{"line_number":82,"context_line":"        \u0027successfully. Attachment_id\u003d%(attachment_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"327b6017_1afbb9c4","line":79,"range":{"start_line":70,"start_character":0,"end_line":79,"end_character":54},"in_reply_to":"9493fc27_2d96af62","updated":"2022-07-22 09:54:14.000000000","message":"Please share your solution","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e1ed6101cba8a8bdba9f42cedd090826d419fe3c","unresolved":true,"context_lines":[{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            with open(volume_state_path) as volume_state_file:"},{"line_number":121,"context_line":"                volume_state \u003d json.load(volume_state_file)"},{"line_number":122,"context_line":"        except FileNotFoundError:"},{"line_number":123,"context_line":"            LOG.error(_LE("},{"line_number":124,"context_line":"                \u0027Volume state file %(volume_state_path)s was not found \u0027"},{"line_number":125,"context_line":"                \u0027during detaching volume %(volume_id)s from host %(host)s \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3791227c_92577745","line":122,"range":{"start_line":122,"start_character":8,"end_line":122,"end_character":33},"updated":"2022-07-22 07:11:41.000000000","message":"if the file is not found, we are not going to disconnect the volume? this could leave stale devices in the host system","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":32927,"name":"mitya-eremeev-2","display_name":"Mitya Eremeev","email":"mitossvyaz@mail.ru","username":"mitya-eremeev-2"},"change_message_id":"010b537d36dfcbf7d096b35164eaec213c814e85","unresolved":true,"context_lines":[{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            with open(volume_state_path) as volume_state_file:"},{"line_number":121,"context_line":"                volume_state \u003d json.load(volume_state_file)"},{"line_number":122,"context_line":"        except FileNotFoundError:"},{"line_number":123,"context_line":"            LOG.error(_LE("},{"line_number":124,"context_line":"                \u0027Volume state file %(volume_state_path)s was not found \u0027"},{"line_number":125,"context_line":"                \u0027during detaching volume %(volume_id)s from host %(host)s \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"ebcab6a2_b1ce597b","line":122,"range":{"start_line":122,"start_character":8,"end_line":122,"end_character":33},"in_reply_to":"3791227c_92577745","updated":"2022-07-22 09:54:14.000000000","message":"If we don\u0027t find volume state file during detachment - it\u0027s unusual unexpected error, because file was created during attachment.\nThis check was added just to log some weird behavior. So operator can check lock folder and it\u0027s permissions and so on. In general this error never happens.","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"e1ed6101cba8a8bdba9f42cedd090826d419fe3c","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                {\u0027volume_id\u0027: volume_id, \u0027host\u0027: host,"},{"line_number":116,"context_line":"                 \u0027attachment_id\u0027: attachment_id}))"},{"line_number":117,"context_line":"            raise"},{"line_number":118,"context_line":"        volume_state_path \u003d os.path.join(state_path, volume_id)"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            with open(volume_state_path) as volume_state_file:"},{"line_number":121,"context_line":"                volume_state \u003d json.load(volume_state_file)"},{"line_number":122,"context_line":"        except FileNotFoundError:"},{"line_number":123,"context_line":"            LOG.error(_LE("},{"line_number":124,"context_line":"                \u0027Volume state file %(volume_state_path)s was not found \u0027"},{"line_number":125,"context_line":"                \u0027during detaching volume %(volume_id)s from host %(host)s \u0027"},{"line_number":126,"context_line":"                \u0027with %(attachment_id)s\u0027,"},{"line_number":127,"context_line":"                {\u0027volume_state_path\u0027: volume_state_path,"},{"line_number":128,"context_line":"                 \u0027volume_id\u0027: volume_id, \u0027host\u0027: host,"},{"line_number":129,"context_line":"                 \u0027attachment_id\u0027: attachment_id}))"},{"line_number":130,"context_line":"        else:"},{"line_number":131,"context_line":"            try:"},{"line_number":132,"context_line":"                volume_state[\"attachments\"].remove(attachment_id)"},{"line_number":133,"context_line":"            except ValueError:"},{"line_number":134,"context_line":"                LOG.error(_LE("},{"line_number":135,"context_line":"                    \u0027Attachment id %(attachment_id)s was not found in volume \u0027"},{"line_number":136,"context_line":"                    \u0027state file (%(volume_state_path)s during detaching \u0027"},{"line_number":137,"context_line":"                    \u0027volume %(volume_id)s from host %(host)s.\u0027,"},{"line_number":138,"context_line":"                    {\u0027volume_state_path\u0027: volume_state_path,"},{"line_number":139,"context_line":"                     \u0027volume_id\u0027: volume_id, \u0027host\u0027: host,"},{"line_number":140,"context_line":"                     \u0027attachment_id\u0027: attachment_id}))"},{"line_number":141,"context_line":"            else:"},{"line_number":142,"context_line":"                if volume_state[\"attachments\"]:"},{"line_number":143,"context_line":"                    with open(volume_state_path, \u0027w\u0027) as volume_state_file:"},{"line_number":144,"context_line":"                        json.dump(volume_state, volume_state_file)"},{"line_number":145,"context_line":"                else:"},{"line_number":146,"context_line":"                    connector.disconnect_volume(connection_info, device)"},{"line_number":147,"context_line":"                    os.remove(volume_state_path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"c8b86412_ef6fd012","line":144,"range":{"start_line":118,"start_character":0,"end_line":144,"end_character":66},"updated":"2022-07-22 07:11:41.000000000","message":"A lot of file handling is going on here which has potential to go wrong in some or the other way.\nAlso we\u0027ve the issue when the operation fails before detach and then we\u0027ve a stale volume file that is never cleaned up.","commit_id":"28857ced946a94749959d04b670326de07ba96fe"},{"author":{"_account_id":32927,"name":"mitya-eremeev-2","display_name":"Mitya Eremeev","email":"mitossvyaz@mail.ru","username":"mitya-eremeev-2"},"change_message_id":"010b537d36dfcbf7d096b35164eaec213c814e85","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                {\u0027volume_id\u0027: volume_id, \u0027host\u0027: host,"},{"line_number":116,"context_line":"                 \u0027attachment_id\u0027: attachment_id}))"},{"line_number":117,"context_line":"            raise"},{"line_number":118,"context_line":"        volume_state_path \u003d os.path.join(state_path, volume_id)"},{"line_number":119,"context_line":"        try:"},{"line_number":120,"context_line":"            with open(volume_state_path) as volume_state_file:"},{"line_number":121,"context_line":"                volume_state \u003d json.load(volume_state_file)"},{"line_number":122,"context_line":"        except FileNotFoundError:"},{"line_number":123,"context_line":"            LOG.error(_LE("},{"line_number":124,"context_line":"                \u0027Volume state file %(volume_state_path)s was not found \u0027"},{"line_number":125,"context_line":"                \u0027during detaching volume %(volume_id)s from host %(host)s \u0027"},{"line_number":126,"context_line":"                \u0027with %(attachment_id)s\u0027,"},{"line_number":127,"context_line":"                {\u0027volume_state_path\u0027: volume_state_path,"},{"line_number":128,"context_line":"                 \u0027volume_id\u0027: volume_id, \u0027host\u0027: host,"},{"line_number":129,"context_line":"                 \u0027attachment_id\u0027: attachment_id}))"},{"line_number":130,"context_line":"        else:"},{"line_number":131,"context_line":"            try:"},{"line_number":132,"context_line":"                volume_state[\"attachments\"].remove(attachment_id)"},{"line_number":133,"context_line":"            except ValueError:"},{"line_number":134,"context_line":"                LOG.error(_LE("},{"line_number":135,"context_line":"                    \u0027Attachment id %(attachment_id)s was not found in volume \u0027"},{"line_number":136,"context_line":"                    \u0027state file (%(volume_state_path)s during detaching \u0027"},{"line_number":137,"context_line":"                    \u0027volume %(volume_id)s from host %(host)s.\u0027,"},{"line_number":138,"context_line":"                    {\u0027volume_state_path\u0027: volume_state_path,"},{"line_number":139,"context_line":"                     \u0027volume_id\u0027: volume_id, \u0027host\u0027: host,"},{"line_number":140,"context_line":"                     \u0027attachment_id\u0027: attachment_id}))"},{"line_number":141,"context_line":"            else:"},{"line_number":142,"context_line":"                if volume_state[\"attachments\"]:"},{"line_number":143,"context_line":"                    with open(volume_state_path, \u0027w\u0027) as volume_state_file:"},{"line_number":144,"context_line":"                        json.dump(volume_state, volume_state_file)"},{"line_number":145,"context_line":"                else:"},{"line_number":146,"context_line":"                    connector.disconnect_volume(connection_info, device)"},{"line_number":147,"context_line":"                    os.remove(volume_state_path)"}],"source_content_type":"text/x-python","patch_set":4,"id":"0db0f3b4_c4aae393","line":144,"range":{"start_line":118,"start_character":0,"end_line":144,"end_character":66},"in_reply_to":"c8b86412_ef6fd012","updated":"2022-07-22 09:54:14.000000000","message":"Yes, a lot of file handling, please share your solution.\nAlso what do you mean by \"the operation fails before detach\" ?\nWhat operation ?","commit_id":"28857ced946a94749959d04b670326de07ba96fe"}]}