)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"fdb9a81f2639c8bd943d804e46b66fe13b106420","unresolved":false,"context_lines":[{"line_number":12,"context_line":"status."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"In order to avoid these problems, I think we should only allow volumes"},{"line_number":15,"context_line":"in \u0027available\u0027, \u0027in-use\u0027, \u0027downloading\u0027, \u0027reserved\u0027 status to create"},{"line_number":16,"context_line":"attachments."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ie59f609d38290147a67a936c61e0164fe278da99"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fdfeff1_53ed34b5","line":15,"range":{"start_line":15,"start_character":2,"end_line":15,"end_character":51},"updated":"2019-02-12 03:15:25.000000000","message":"it should also be updated.","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"4d1fb682d6ee22e2ab1a310d014ffcffcfd48283","unresolved":false,"context_lines":[{"line_number":12,"context_line":"status."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"In order to avoid these problems, I think we should only allow volumes"},{"line_number":15,"context_line":"in \u0027available\u0027, \u0027in-use\u0027, \u0027downloading\u0027, \u0027reserved\u0027 status to create"},{"line_number":16,"context_line":"attachments."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ie59f609d38290147a67a936c61e0164fe278da99"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fdfeff1_20cc9d68","line":15,"range":{"start_line":15,"start_character":2,"end_line":15,"end_character":51},"in_reply_to":"9fdfeff1_53ed34b5","updated":"2019-02-14 09:48:28.000000000","message":"Done","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"d15ffe07995a2012e0de61a031a9529906e525a4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently the new cinder attachment_create method allows volumes in"},{"line_number":10,"context_line":"all status to be performed for single attachment. This may cause"},{"line_number":11,"context_line":"problems if volume-attach operation is taken on volumes in creating"},{"line_number":12,"context_line":"status. (Related-bug: #1694530)"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"In order to avoid these problems, we should only allow volumes"},{"line_number":15,"context_line":"in \u0027in-use\u0027, \u0027reserved\u0027 status to check whether this attachment"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fdfeff1_31456cb5","line":12,"range":{"start_line":12,"start_character":9,"end_line":12,"end_character":16},"updated":"2019-03-04 18:06:56.000000000","message":"For future reference, these should be on their own line so they get parsed and linked in the bug report.","commit_id":"dabf26c99f3e7142f966b43dd04025474b13793b"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"3e152c5907cb3a6e5b3ffdba7f0ce1f84f0e12d5","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently the new cinder attachment_create method allows volumes in"},{"line_number":10,"context_line":"all status to be performed for single attachment. This may cause"},{"line_number":11,"context_line":"problems if volume-attach operation is taken on volumes in creating"},{"line_number":12,"context_line":"status. (Related-bug: #1694530)"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"In order to avoid these problems, we should only allow volumes"},{"line_number":15,"context_line":"in \u0027in-use\u0027, \u0027reserved\u0027 status to check whether this attachment"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fdfeff1_aae8feb2","line":12,"range":{"start_line":12,"start_character":9,"end_line":12,"end_character":16},"in_reply_to":"9fdfeff1_31456cb5","updated":"2019-03-05 01:28:31.000000000","message":"Done","commit_id":"dabf26c99f3e7142f966b43dd04025474b13793b"}],"cinder/volume/api.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3163443348214951cfb70d6d54b076a153bee966","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"                        override \u003d True"},{"line_number":2090,"context_line":"                        break"},{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if (not override or vref.status not in"},{"line_number":2093,"context_line":"                    (\u0027available\u0027, \u0027in-use\u0027, \u0027downloading\u0027, \u0027reserved\u0027)):"},{"line_number":2094,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2095,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2096,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_16d3d5d6","line":2093,"range":{"start_line":2092,"start_character":29,"end_line":2093,"end_character":71},"updated":"2019-01-31 07:09:48.000000000","message":"This condition is already checked on L#2074","commit_id":"6ab4bfcce5fed0ae154f235fde5b044b1e6c832d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d4d808c9fcd821bb5b344a11678e876240db18cb","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"                        override \u003d True"},{"line_number":2090,"context_line":"                        break"},{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if (not override or vref.status not in"},{"line_number":2093,"context_line":"                    (\u0027available\u0027, \u0027in-use\u0027, \u0027downloading\u0027, \u0027reserved\u0027)):"},{"line_number":2094,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2095,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2096,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_181a1716","line":2093,"range":{"start_line":2092,"start_character":29,"end_line":2093,"end_character":71},"in_reply_to":"9fdfeff1_0a6f26c5","updated":"2019-01-31 15:30:38.000000000","message":"Ok, so you\u0027re describing the scenario when we want to attach and the code works for cases when the volume status isn\u0027t a valid one(like error)\nSounds like a good catch.\ni think we can remove the \u0027available\u0027 and \u0027downloading\u0027 status in this check. after the conditional_update check, looks redundant to me.","commit_id":"6ab4bfcce5fed0ae154f235fde5b044b1e6c832d"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"4d11654989944a533564acd4f11ec7ecf246ea9b","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"                        override \u003d True"},{"line_number":2090,"context_line":"                        break"},{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if (not override or vref.status not in"},{"line_number":2093,"context_line":"                    (\u0027available\u0027, \u0027in-use\u0027, \u0027downloading\u0027, \u0027reserved\u0027)):"},{"line_number":2094,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2095,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2096,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_0a6f26c5","line":2093,"range":{"start_line":2092,"start_character":29,"end_line":2093,"end_character":71},"in_reply_to":"9fdfeff1_16d3d5d6","updated":"2019-01-31 09:45:00.000000000","message":"conditional_update function will return None if the status is not in expected, so this \"if not result:\" code block is meant to deal will this situation.\nSee: https://review.openstack.org/#/c/469252/","commit_id":"6ab4bfcce5fed0ae154f235fde5b044b1e6c832d"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"8f9d6db23933d71240d910a005ec751e66f4d8a9","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"                        override \u003d True"},{"line_number":2090,"context_line":"                        break"},{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if (not override or vref.status not in"},{"line_number":2093,"context_line":"                    (\u0027available\u0027, \u0027in-use\u0027, \u0027downloading\u0027, \u0027reserved\u0027)):"},{"line_number":2094,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2095,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2096,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_7326dc6a","line":2093,"range":{"start_line":2092,"start_character":29,"end_line":2093,"end_character":71},"in_reply_to":"9fdfeff1_181a1716","updated":"2019-02-01 01:44:17.000000000","message":"Done\nOh, you\u0027re right, available and downloading won\u0027t be reached. Thank you for your reviewing.","commit_id":"6ab4bfcce5fed0ae154f235fde5b044b1e6c832d"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"fdb9a81f2639c8bd943d804e46b66fe13b106420","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"                        override \u003d True"},{"line_number":2090,"context_line":"                        break"},{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if not override or vref.status not in (\u0027in-use\u0027, \u0027reserved\u0027):"},{"line_number":2093,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2094,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2095,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_93ce3c58","line":2092,"range":{"start_line":2092,"start_character":50,"end_line":2092,"end_character":72},"updated":"2019-02-12 03:15:25.000000000","message":"-1, I\u0027m not sure why \u0027in-use\u0027 in here? IMO, if we reach here, that means the volume has been marked as \u0027reserved\u0027 by other concurrent request, right? So, I think only \u0027reserved\u0027 should be in here (even it\u0027s a multiattach volume).","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"4d1fb682d6ee22e2ab1a310d014ffcffcfd48283","unresolved":false,"context_lines":[{"line_number":2089,"context_line":"                        override \u003d True"},{"line_number":2090,"context_line":"                        break"},{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if not override or vref.status not in (\u0027in-use\u0027, \u0027reserved\u0027):"},{"line_number":2093,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2094,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2095,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_6005c50f","line":2092,"range":{"start_line":2092,"start_character":50,"end_line":2092,"end_character":72},"in_reply_to":"9fdfeff1_93ce3c58","updated":"2019-02-14 09:48:28.000000000","message":"The \u0027in-use\u0027 status is necessary here, because a non-multiattach in-use volume will reach these codes, and we have to check whether this is an override action.\nMaybe it\u0027s a bit confusing to put this here. I\u0027ll try to put this check where the override flag is set. We only need to check whether this is an override (creating attachment for the same instance) when the volume is in \u0027in-use\u0027 or \u0027reserved\u0027 status.","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"fdb9a81f2639c8bd943d804e46b66fe13b106420","unresolved":false,"context_lines":[{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if not override or vref.status not in (\u0027in-use\u0027, \u0027reserved\u0027):"},{"line_number":2093,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2094,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2095,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"},{"line_number":2096,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":2097,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_33ff50ac","line":2094,"updated":"2019-02-12 03:15:25.000000000","message":"nit: It\u0027s better to change the log to something like \"Volume status must be %(expected)s to reserve, but the status is %(current)s.\"","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"4d1fb682d6ee22e2ab1a310d014ffcffcfd48283","unresolved":false,"context_lines":[{"line_number":2091,"context_line":""},{"line_number":2092,"context_line":"            if not override or vref.status not in (\u0027in-use\u0027, \u0027reserved\u0027):"},{"line_number":2093,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2094,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2095,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"},{"line_number":2096,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":2097,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_802309a7","line":2094,"in_reply_to":"9fdfeff1_33ff50ac","updated":"2019-02-14 09:48:28.000000000","message":"Done","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":20722,"name":"Yikun Jiang","email":"yikunkero@gmail.com","username":"yikunkero"},"change_message_id":"fdb9a81f2639c8bd943d804e46b66fe13b106420","unresolved":false,"context_lines":[{"line_number":2092,"context_line":"            if not override or vref.status not in (\u0027in-use\u0027, \u0027reserved\u0027):"},{"line_number":2093,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2094,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2095,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"},{"line_number":2096,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":2097,"context_line":""},{"line_number":2098,"context_line":"        values \u003d {\u0027volume_id\u0027: vref.id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_d301246c","line":2095,"range":{"start_line":2095,"start_character":55,"end_line":2095,"end_character":73},"updated":"2019-02-12 03:15:25.000000000","message":"-1, If we meet the requirement of \u0027vref.status not in (xxx)\u0027, this expected status is already not right, otherwise we will get back a confusing wrong log.","commit_id":"d5a7033f022213ea736e23165268854707f898a7"},{"author":{"_account_id":13945,"name":"ZhangYang","email":"wosunoozzy@gmail.com","username":"WosunOoO"},"change_message_id":"4d1fb682d6ee22e2ab1a310d014ffcffcfd48283","unresolved":false,"context_lines":[{"line_number":2092,"context_line":"            if not override or vref.status not in (\u0027in-use\u0027, \u0027reserved\u0027):"},{"line_number":2093,"context_line":"                msg \u003d (_(\u0027Volume %(vol_id)s status must be %(statuses)s\u0027) %"},{"line_number":2094,"context_line":"                       {\u0027vol_id\u0027: vref.id,"},{"line_number":2095,"context_line":"                        \u0027statuses\u0027: utils.build_or_str(expected[\u0027status\u0027])})"},{"line_number":2096,"context_line":"                raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":2097,"context_line":""},{"line_number":2098,"context_line":"        values \u003d {\u0027volume_id\u0027: vref.id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_40920159","line":2095,"range":{"start_line":2095,"start_character":55,"end_line":2095,"end_character":73},"in_reply_to":"9fdfeff1_d301246c","updated":"2019-02-14 09:48:28.000000000","message":"The override scene is a cheat to nova\u0027s live-migrate or similar scenario, maybe we don\u0027t need to add this info here. Should we add \u0027in-use\u0027 and \u0027reserved\u0027 to expected status here?","commit_id":"d5a7033f022213ea736e23165268854707f898a7"}]}
