)]}'
{"cinder/tests/test_volume.py":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"54c2f5cd19446c2f9b6c925fabc72f12702400d5","unresolved":false,"context_lines":[{"line_number":2836,"context_line":"        self.assertEqual(volume[\u0027status\u0027], status)"},{"line_number":2837,"context_line":"        self.assertEqual(volume[\u0027attached_host\u0027], attached_host)"},{"line_number":2838,"context_line":"        self.assertEqual(volume[\u0027instance_uuid\u0027], instance_uuid)"},{"line_number":2839,"context_line":"        self.assertEqual(status \u003d\u003d \u0027in-use\u0027, detach.called)"},{"line_number":2840,"context_line":""},{"line_number":2841,"context_line":"    def test_migrate_volume_completion_retype_available(self):"},{"line_number":2842,"context_line":"        self._test_migrate_volume_completion(\u0027available\u0027, retyping\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_eca0e743","line":2839,"updated":"2014-12-23 18:41:46.000000000","message":"I think it might be better to use a \u0027stronger\u0027 test case to demonstrate you have fixed the bug.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"d3f8626055ee95cc7f503dfff3a2e05976e4cc65","unresolved":false,"context_lines":[{"line_number":2836,"context_line":"        self.assertEqual(volume[\u0027status\u0027], status)"},{"line_number":2837,"context_line":"        self.assertEqual(volume[\u0027attached_host\u0027], attached_host)"},{"line_number":2838,"context_line":"        self.assertEqual(volume[\u0027instance_uuid\u0027], instance_uuid)"},{"line_number":2839,"context_line":"        self.assertEqual(status \u003d\u003d \u0027in-use\u0027, detach.called)"},{"line_number":2840,"context_line":""},{"line_number":2841,"context_line":"    def test_migrate_volume_completion_retype_available(self):"},{"line_number":2842,"context_line":"        self._test_migrate_volume_completion(\u0027available\u0027, retyping\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_3244811a","line":2839,"in_reply_to":"3a961159_31172d56","updated":"2015-01-04 06:01:08.000000000","message":"The original test cases didn\u0027t fail as there was not a condition to check whether the detach call was being made. The additions to the test cases test the detach call and without the fix this test fails. I agree that the test cases and the code itself should be made more modular, but this patch is not supposed to refactor the code. The purpose of this patch is to address a specific bug in relation to a detach call not being made when the attached status is in the \u0027in-use\u0027 state.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"2f9077603d42584fff0601bddc1a61686ceb365a","unresolved":false,"context_lines":[{"line_number":2836,"context_line":"        self.assertEqual(volume[\u0027status\u0027], status)"},{"line_number":2837,"context_line":"        self.assertEqual(volume[\u0027attached_host\u0027], attached_host)"},{"line_number":2838,"context_line":"        self.assertEqual(volume[\u0027instance_uuid\u0027], instance_uuid)"},{"line_number":2839,"context_line":"        self.assertEqual(status \u003d\u003d \u0027in-use\u0027, detach.called)"},{"line_number":2840,"context_line":""},{"line_number":2841,"context_line":"    def test_migrate_volume_completion_retype_available(self):"},{"line_number":2842,"context_line":"        self._test_migrate_volume_completion(\u0027available\u0027, retyping\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_58dae04b","line":2839,"in_reply_to":"3a961159_3244811a","updated":"2015-01-06 10:15:50.000000000","message":"Thank you for the detailed explanation! I can see that this is a good fix and the test case is working.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"0ac1629dfb2905aa45ab2344d946fb93f094e8f7","unresolved":false,"context_lines":[{"line_number":2836,"context_line":"        self.assertEqual(volume[\u0027status\u0027], status)"},{"line_number":2837,"context_line":"        self.assertEqual(volume[\u0027attached_host\u0027], attached_host)"},{"line_number":2838,"context_line":"        self.assertEqual(volume[\u0027instance_uuid\u0027], instance_uuid)"},{"line_number":2839,"context_line":"        self.assertEqual(status \u003d\u003d \u0027in-use\u0027, detach.called)"},{"line_number":2840,"context_line":""},{"line_number":2841,"context_line":"    def test_migrate_volume_completion_retype_available(self):"},{"line_number":2842,"context_line":"        self._test_migrate_volume_completion(\u0027available\u0027, retyping\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_31172d56","line":2839,"in_reply_to":"3a961159_e7580033","updated":"2015-01-02 12:36:51.000000000","message":"I mean a test case to first create the condition where the bug is triggered, then ensure the bug doesn\u0027t occur. But this added line only reuses the original existing test cases conditions. If the original existing test cases have already covered the bug condition, the test case should have failed when the bug code was committed and prevented it from merging.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"0ac04849627493209fa01775e6b11eafb8bb9c56","unresolved":false,"context_lines":[{"line_number":2836,"context_line":"        self.assertEqual(volume[\u0027status\u0027], status)"},{"line_number":2837,"context_line":"        self.assertEqual(volume[\u0027attached_host\u0027], attached_host)"},{"line_number":2838,"context_line":"        self.assertEqual(volume[\u0027instance_uuid\u0027], instance_uuid)"},{"line_number":2839,"context_line":"        self.assertEqual(status \u003d\u003d \u0027in-use\u0027, detach.called)"},{"line_number":2840,"context_line":""},{"line_number":2841,"context_line":"    def test_migrate_volume_completion_retype_available(self):"},{"line_number":2842,"context_line":"        self._test_migrate_volume_completion(\u0027available\u0027, retyping\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_e7580033","line":2839,"in_reply_to":"3a961159_eca0e743","updated":"2014-12-23 18:52:37.000000000","message":"Without the fix, this test fails. What do you mean by a stronger test case? detach_volume was not being called before and hence this test fails. This test passes now as detach_volume is being called.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"}],"cinder/volume/manager.py":[{"author":{"_account_id":13900,"name":"Accela Zhao","email":"accelazh@gmail.com","username":"accelazh"},"change_message_id":"54c2f5cd19446c2f9b6c925fabc72f12702400d5","unresolved":false,"context_lines":[{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"        # Delete the source volume (if it fails, don\u0027t fail the migration)"},{"line_number":1117,"context_line":"        try:"},{"line_number":1118,"context_line":"            status_update \u003d {\u0027status\u0027: self._get_original_status(volume)}"},{"line_number":1119,"context_line":"            if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1120,"context_line":"                self.detach_volume(ctxt, volume_id)"},{"line_number":1121,"context_line":"            self.delete_volume(ctxt, volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_0c045348","line":1118,"updated":"2014-12-23 18:41:46.000000000","message":"Line 1097 wants to says \u0027status_update\u0027 should be assigned when \u0027retyping\u0027, and remains \u0027{}\u0027 otherwise. But here you just assigned it anyway. I think what you want is to get original status rather than modify status_update. Be careful with the side effect.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"0ac04849627493209fa01775e6b11eafb8bb9c56","unresolved":false,"context_lines":[{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"        # Delete the source volume (if it fails, don\u0027t fail the migration)"},{"line_number":1117,"context_line":"        try:"},{"line_number":1118,"context_line":"            status_update \u003d {\u0027status\u0027: self._get_original_status(volume)}"},{"line_number":1119,"context_line":"            if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1120,"context_line":"                self.detach_volume(ctxt, volume_id)"},{"line_number":1121,"context_line":"            self.delete_volume(ctxt, volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_a7d51844","line":1118,"in_reply_to":"3a961159_0c045348","updated":"2014-12-23 18:52:37.000000000","message":"Applying Avishay\u0027s change will fix this issue. Refer back to his comment to see the context. I will apply that and push the next patchset once gate gets fixed.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"d34012f9fb2aa23f11dba3bcf8933da4a790f754","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"        # Delete the source volume (if it fails, don\u0027t fail the migration)"},{"line_number":1117,"context_line":"        try:"},{"line_number":1118,"context_line":"            status_update \u003d {\u0027status\u0027: self._get_original_status(volume)}"},{"line_number":1119,"context_line":"            if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1120,"context_line":"                self.detach_volume(ctxt, volume_id)"},{"line_number":1121,"context_line":"            self.delete_volume(ctxt, volume_id)"},{"line_number":1122,"context_line":"        except Exception as ex:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_be461ebe","line":1119,"updated":"2015-01-07 19:35:57.000000000","message":"You shouldn\u0027t be changing the status in all cases, because the status should already be \u0027in-use\u0027 in the case of migration without retype.  So then this line should be:\n\n if \u0027in-use\u0027 in (status_update.get(\u0027status\u0027), volume[\u0027status\u0027]):\n\n(Similar to line 1141)","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"32c44ecef50fd9445cd7c616fd41a9b923be4779","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"        # Delete the source volume (if it fails, don\u0027t fail the migration)"},{"line_number":1117,"context_line":"        try:"},{"line_number":1118,"context_line":"            status_update \u003d {\u0027status\u0027: self._get_original_status(volume)}"},{"line_number":1119,"context_line":"            if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1120,"context_line":"                self.detach_volume(ctxt, volume_id)"},{"line_number":1121,"context_line":"            self.delete_volume(ctxt, volume_id)"},{"line_number":1122,"context_line":"        except Exception as ex:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_8f6e8e2d","line":1119,"in_reply_to":"3a961159_be461ebe","updated":"2015-01-07 20:54:01.000000000","message":"In the case without retype, the status is not \u0027in-use\u0027 and that is why line 1118 was added. I have detailed the reasoning behind this as a comment on the bug https://bugs.launchpad.net/cinder/+bug/1391172. With the fix that you suggested, the migration in the non-retype fails for an attached volume.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"d34012f9fb2aa23f11dba3bcf8933da4a790f754","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"                                      new_volume)"},{"line_number":1132,"context_line":"        self.db.finish_volume_migration(ctxt, volume_id, new_volume_id)"},{"line_number":1133,"context_line":"        self.db.volume_destroy(ctxt, new_volume_id)"},{"line_number":1134,"context_line":"        if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1135,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027}"},{"line_number":1136,"context_line":"            updates.update(status_update)"},{"line_number":1137,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_1152c312","line":1134,"updated":"2015-01-07 19:35:57.000000000","message":"I\u0027m not sure why we\u0027re setting this to \u0027completing\u0027 here, because we already did in line 1114.  Any idea?","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"32c44ecef50fd9445cd7c616fd41a9b923be4779","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"                                      new_volume)"},{"line_number":1132,"context_line":"        self.db.finish_volume_migration(ctxt, volume_id, new_volume_id)"},{"line_number":1133,"context_line":"        self.db.volume_destroy(ctxt, new_volume_id)"},{"line_number":1134,"context_line":"        if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1135,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027}"},{"line_number":1136,"context_line":"            updates.update(status_update)"},{"line_number":1137,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_8f55ae51","line":1134,"in_reply_to":"3a961159_1152c312","updated":"2015-01-07 20:54:01.000000000","message":"Refer to the comment above and reasoning in https://bugs.launchpad.net/cinder/+bug/1391172.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"d34012f9fb2aa23f11dba3bcf8933da4a790f754","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        self.db.volume_update(ctxt, volume_id, updates)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"        if \u0027in-use\u0027 in (status_update.get(\u0027status\u0027), volume[\u0027status\u0027]):"},{"line_number":1142,"context_line":"            # NOTE(jdg): if we\u0027re passing the ref here, why are we"},{"line_number":1143,"context_line":"            # also passing in the various fields from that ref?"},{"line_number":1144,"context_line":"            rpcapi.attach_volume(ctxt,"},{"line_number":1145,"context_line":"                                 volume,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_dea20a3e","line":1142,"updated":"2015-01-07 19:35:57.000000000","message":"Can remove this comment.  The reason is that other calls to attach_volume won\u0027t necessarily have this information in the volume dict.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"32c44ecef50fd9445cd7c616fd41a9b923be4779","unresolved":false,"context_lines":[{"line_number":1139,"context_line":"        self.db.volume_update(ctxt, volume_id, updates)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"        if \u0027in-use\u0027 in (status_update.get(\u0027status\u0027), volume[\u0027status\u0027]):"},{"line_number":1142,"context_line":"            # NOTE(jdg): if we\u0027re passing the ref here, why are we"},{"line_number":1143,"context_line":"            # also passing in the various fields from that ref?"},{"line_number":1144,"context_line":"            rpcapi.attach_volume(ctxt,"},{"line_number":1145,"context_line":"                                 volume,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_2fccba34","line":1142,"in_reply_to":"3a961159_dea20a3e","updated":"2015-01-07 20:54:01.000000000","message":"Will remove this comment in the next patchset.","commit_id":"cc5ee0b14732d716a051354369c1347de18f4ece"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"ad5e0d87910c98d354d80c7d8ee851821078c6f9","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"        rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1095,"context_line":""},{"line_number":1096,"context_line":"        status_update \u003d {}"},{"line_number":1097,"context_line":"        if volume[\u0027status\u0027] \u003d\u003d \u0027retyping\u0027:"},{"line_number":1098,"context_line":"            status_update \u003d {\u0027status\u0027: self._get_original_status(volume)}"},{"line_number":1099,"context_line":""},{"line_number":1100,"context_line":"        if error:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_f0323c5d","line":1097,"updated":"2015-01-12 06:53:37.000000000","message":"Instead of these 3 lines:\n\n orig_volume_status \u003d self._get_original_status(volume)","commit_id":"58b073afc0a138a650be644e25cd289138b9a61f"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"ad5e0d87910c98d354d80c7d8ee851821078c6f9","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"            LOG.info(msg % {\u0027vol1\u0027: volume[\u0027id\u0027],"},{"line_number":1104,"context_line":"                            \u0027vol2\u0027: new_volume[\u0027id\u0027]})"},{"line_number":1105,"context_line":"            new_volume[\u0027migration_status\u0027] \u003d None"},{"line_number":1106,"context_line":"            rpcapi.delete_volume(ctxt, new_volume)"},{"line_number":1107,"context_line":"            updates \u003d {\u0027migration_status\u0027: None}"},{"line_number":1108,"context_line":"            if status_update:"},{"line_number":1109,"context_line":"                updates.update(status_update)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_5039503e","line":1106,"updated":"2015-01-12 06:53:37.000000000","message":"updates \u003d {\u0027migration_status\u0027: None, \u0027status\u0027: orig_volume_status }\n\nThen delete the following two lines.","commit_id":"58b073afc0a138a650be644e25cd289138b9a61f"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"ad5e0d87910c98d354d80c7d8ee851821078c6f9","unresolved":false,"context_lines":[{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"        # Delete the source volume (if it fails, don\u0027t fail the migration)"},{"line_number":1117,"context_line":"        try:"},{"line_number":1118,"context_line":"            status_update \u003d {\u0027status\u0027: self._get_original_status(volume)}"},{"line_number":1119,"context_line":"            if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1120,"context_line":"                self.detach_volume(ctxt, volume_id)"},{"line_number":1121,"context_line":"            self.delete_volume(ctxt, volume_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_70464cc2","line":1118,"updated":"2015-01-12 06:53:37.000000000","message":"Here the \u0027if\u0027 statement should be:\n\n if orig_volume_status \u003d\u003d \u0027in-use\u0027:","commit_id":"58b073afc0a138a650be644e25cd289138b9a61f"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"ad5e0d87910c98d354d80c7d8ee851821078c6f9","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"                                      new_volume)"},{"line_number":1132,"context_line":"        self.db.finish_volume_migration(ctxt, volume_id, new_volume_id)"},{"line_number":1133,"context_line":"        self.db.volume_destroy(ctxt, new_volume_id)"},{"line_number":1134,"context_line":"        if status_update.get(\u0027status\u0027) \u003d\u003d \u0027in-use\u0027:"},{"line_number":1135,"context_line":"            updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027}"},{"line_number":1136,"context_line":"            updates.update(status_update)"},{"line_number":1137,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_3023e407","line":1134,"updated":"2015-01-12 06:53:37.000000000","message":"Here the code should be:\n\n if orig_volume_status \u003d\u003d \u0027in-use\u0027:\n     updates \u003d {\u0027migration_status\u0027: \u0027completing\u0027, \u0027status\u0027: orig_volume_status}\n else:\n     updates \u003d {\u0027migration_status\u0027: None}","commit_id":"58b073afc0a138a650be644e25cd289138b9a61f"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"ad5e0d87910c98d354d80c7d8ee851821078c6f9","unresolved":false,"context_lines":[{"line_number":1138,"context_line":"            updates \u003d {\u0027migration_status\u0027: None}"},{"line_number":1139,"context_line":"        self.db.volume_update(ctxt, volume_id, updates)"},{"line_number":1140,"context_line":""},{"line_number":1141,"context_line":"        if \u0027in-use\u0027 in (status_update.get(\u0027status\u0027), volume[\u0027status\u0027]):"},{"line_number":1142,"context_line":"            rpcapi.attach_volume(ctxt,"},{"line_number":1143,"context_line":"                                 volume,"},{"line_number":1144,"context_line":"                                 volume[\u0027instance_uuid\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_502070f8","line":1141,"updated":"2015-01-12 06:53:37.000000000","message":"Here the \u0027if\u0027 statement should be:\n\n if orig_volume_status \u003d\u003d \u0027in-use\u0027:","commit_id":"58b073afc0a138a650be644e25cd289138b9a61f"}]}