)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78179fd74b093a65da81cee68783562cbb47bbde","unresolved":false,"context_lines":[{"line_number":17,"context_line":"To fix this we should check the \u0027host\u0027 attribute in instances table before"},{"line_number":18,"context_line":"deleting the VM and delete VM from source only if the host has been updated"},{"line_number":19,"context_line":"in db after rebuild"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: If6a244401c5ac4fe25c7dccbcb61acf68fbe9175"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba5da102_09b9242e","line":20,"updated":"2016-11-03 10:23:58.000000000","message":"You should add :\n\n\nCloses-Bug: #1636825","commit_id":"3464b933b33f4773ed4454895bf2c00bedfb44a2"}],"nova/compute/manager.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78179fd74b093a65da81cee68783562cbb47bbde","unresolved":false,"context_lines":[{"line_number":651,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":652,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":653,"context_line":"        for instance in evacuated:"},{"line_number":654,"context_line":"            # We don\u0027t want to destroy if the migration failed early"},{"line_number":655,"context_line":"            # and the instance is still on the source"},{"line_number":656,"context_line":"            if instance.host \u003d\u003d self.host:"},{"line_number":657,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_e9c23084","line":654,"range":{"start_line":654,"start_character":39,"end_line":654,"end_character":68},"updated":"2016-11-03 10:23:58.000000000","message":"if the migration is failed, then the Migration object should have a status being done, right?\n\nIf so, given we get all the evacuations by the [\u0027accepted\u0027, \u0027done\u0027] filters for the status, I\u0027m not sure a done migration would be deleting the instance in the source if we merge your patch.","commit_id":"3464b933b33f4773ed4454895bf2c00bedfb44a2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"1de8f1b85656fff939e4521aba3de8e51028dc81","unresolved":false,"context_lines":[{"line_number":651,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":652,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":653,"context_line":"        for instance in evacuated:"},{"line_number":654,"context_line":"            # We don\u0027t want to destroy if the migration failed early"},{"line_number":655,"context_line":"            # and the instance is still on the source"},{"line_number":656,"context_line":"            if instance.host \u003d\u003d self.host:"},{"line_number":657,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_2c4bb488","line":654,"range":{"start_line":654,"start_character":39,"end_line":654,"end_character":68},"in_reply_to":"9a629dbe_5148f42d","updated":"2016-11-09 13:48:14.000000000","message":"I\u0027m confused, see my comment in PS2.","commit_id":"3464b933b33f4773ed4454895bf2c00bedfb44a2"},{"author":{"_account_id":10207,"name":"Manas Mandlekar","email":"manas.mandlekar@in.ibm.com","username":"manasmandlekar"},"change_message_id":"35c64d63404abb8fce889ec91da61968f2d09e22","unresolved":false,"context_lines":[{"line_number":651,"context_line":"        evacuated \u003d [inst for inst in local_instances"},{"line_number":652,"context_line":"                     if inst.uuid in evacuations]"},{"line_number":653,"context_line":"        for instance in evacuated:"},{"line_number":654,"context_line":"            # We don\u0027t want to destroy if the migration failed early"},{"line_number":655,"context_line":"            # and the instance is still on the source"},{"line_number":656,"context_line":"            if instance.host \u003d\u003d self.host:"},{"line_number":657,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_5148f42d","line":654,"range":{"start_line":654,"start_character":39,"end_line":654,"end_character":68},"in_reply_to":"ba5da102_e9c23084","updated":"2016-11-07 10:58:16.000000000","message":"This patch tries to address the issues with unsuccessful migrations. When that happens the migration object status is still done but the VM has not successfully moved to destination host. That is what we are trying to address here.\nIn case of a successful migration/evacuate the VM\u0027s instance would get updated with the destination host and then we should be ok to delete the instance from source.","commit_id":"3464b933b33f4773ed4454895bf2c00bedfb44a2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78179fd74b093a65da81cee68783562cbb47bbde","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            # We don\u0027t want to destroy if the migration failed early"},{"line_number":655,"context_line":"            # and the instance is still on the source"},{"line_number":656,"context_line":"            if instance.host \u003d\u003d self.host:"},{"line_number":657,"context_line":"                continue"},{"line_number":658,"context_line":"            migration \u003d evacuations[instance.uuid]"},{"line_number":659,"context_line":"            LOG.info(_LI(\u0027Deleting instance as it has been evacuated from \u0027"},{"line_number":660,"context_line":"                         \u0027this host\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_a97e7867","line":657,"updated":"2016-11-03 10:23:58.000000000","message":"You\u0027re missing some unittest here for checking that.","commit_id":"3464b933b33f4773ed4454895bf2c00bedfb44a2"},{"author":{"_account_id":10207,"name":"Manas Mandlekar","email":"manas.mandlekar@in.ibm.com","username":"manasmandlekar"},"change_message_id":"35c64d63404abb8fce889ec91da61968f2d09e22","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            # We don\u0027t want to destroy if the migration failed early"},{"line_number":655,"context_line":"            # and the instance is still on the source"},{"line_number":656,"context_line":"            if instance.host \u003d\u003d self.host:"},{"line_number":657,"context_line":"                continue"},{"line_number":658,"context_line":"            migration \u003d evacuations[instance.uuid]"},{"line_number":659,"context_line":"            LOG.info(_LI(\u0027Deleting instance as it has been evacuated from \u0027"},{"line_number":660,"context_line":"                         \u0027this host\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a629dbe_d16244ad","line":657,"in_reply_to":"ba5da102_a97e7867","updated":"2016-11-07 10:58:16.000000000","message":"Added tests.","commit_id":"3464b933b33f4773ed4454895bf2c00bedfb44a2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5b3f3faedbbcf6c120f4d5e79044f55a3beab283","unresolved":false,"context_lines":[{"line_number":654,"context_line":"            # We don\u0027t want to destroy if the migration failed early"},{"line_number":655,"context_line":"            # and the instance is still on the source"},{"line_number":656,"context_line":"            if instance.host \u003d\u003d self.host:"},{"line_number":657,"context_line":"                continue"},{"line_number":658,"context_line":"            migration \u003d evacuations[instance.uuid]"},{"line_number":659,"context_line":"            LOG.info(_LI(\u0027Deleting instance as it has been evacuated from \u0027"},{"line_number":660,"context_line":"                         \u0027this host\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a629dbe_cfd94a85","line":657,"updated":"2016-11-09 13:53:35.000000000","message":"Like I said in PS1, we only set the migration object status record to \"done\" only once we update the instance host field too : https://github.com/openstack/nova/blob/20591e1ed685af4f4143882a38375cbc87a55038/nova/compute/manager.py#L2741\n\nThat then means there is no reason for lookup at instance.host for the \u0027done\u0027 status, right?","commit_id":"c6a5505ac279ad05eb2ba99a79d3111c115761c4"}]}