)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6c47ee4eeae05a18ea080bbef4c78fbdf810ebac","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Matt Riedemann \u003cmriedem.os@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-05-15 12:27:17 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Delete allocations even if _confirm_resize raises (part 2)"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The backport 37ac54a42ec91821d62864d63c486c002608491b to fix"},{"line_number":10,"context_line":"bug 1821594 did not account for how the _delete_allocation_after_move"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bfb3d3c7_ef55c31e","line":7,"updated":"2019-05-17 22:07:26.000000000","message":"Maybe it\u0027s just me, but I find the [stable only] label helpful for stable-only changes.","commit_id":"5c4df173baa5c332cca63dbd69ae1ba37f39e0be"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1666e77f24fdcf918f6e064ba8455cd82143693a","unresolved":false,"context_lines":[{"line_number":15,"context_line":"However, if self.driver.confirm_migration raises an exception"},{"line_number":16,"context_line":"we still want to cleanup the allocations held on the source node"},{"line_number":17,"context_line":"and for that we call _delete_allocation_after_move. But because"},{"line_number":18,"context_line":"of that tightly coupling before Stein, we need to temporarily"},{"line_number":19,"context_line":"mutate the migration status to \"confirmed\" to get the cleanup"},{"line_number":20,"context_line":"method to do what we want."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"dfbec78f_03e63de9","line":18,"range":{"start_line":18,"start_character":8,"end_line":18,"end_character":15},"updated":"2019-05-15 16:41:43.000000000","message":"tight","commit_id":"5c4df173baa5c332cca63dbd69ae1ba37f39e0be"}],"nova/compute/manager.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6c47ee4eeae05a18ea080bbef4c78fbdf810ebac","unresolved":false,"context_lines":[{"line_number":4101,"context_line":""},{"line_number":4102,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":4103,"context_line":"            rt.drop_move_claim(context, instance, instance.node)"},{"line_number":4104,"context_line":"            self._delete_allocation_after_move(context, instance, migration,"},{"line_number":4105,"context_line":"                                               instance.flavor,"},{"line_number":4106,"context_line":"                                               instance.node)"},{"line_number":4107,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bfb3d3c7_af502b34","line":4104,"updated":"2019-05-17 22:07:26.000000000","message":"Do we need to update this one too or no?","commit_id":"5c4df173baa5c332cca63dbd69ae1ba37f39e0be"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"481e4d2c89dd995b3a0b4eaff2f817cd2015f047","unresolved":false,"context_lines":[{"line_number":4101,"context_line":""},{"line_number":4102,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":4103,"context_line":"            rt.drop_move_claim(context, instance, instance.node)"},{"line_number":4104,"context_line":"            self._delete_allocation_after_move(context, instance, migration,"},{"line_number":4105,"context_line":"                                               instance.flavor,"},{"line_number":4106,"context_line":"                                               instance.node)"},{"line_number":4107,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bfb3d3c7_0feb2db0","line":4104,"in_reply_to":"bfb3d3c7_af502b34","updated":"2019-05-22 20:12:48.000000000","message":"No, this is running on the dest host and instance.node at this point is pointing at the dest node. Above we\u0027ve already set migration.status\u003d\u0027reverted\u0027 which means we get into the else block in _delete_allocation_after_move here:\n\nhttps://review.opendev.org/#/c/659338/1/nova/compute/manager.py@4007\n\nWhich opts to let finish_revert_resize (on the source) do the allocation swap:\n\nhttps://review.opendev.org/#/c/659338/1/nova/compute/manager.py@4146\n\nMaybe it\u0027s not clear, but before 37ac54a42ec91821d62864d63c486c002608491b the _confirm_resize method would cleanup allocations but only if the migration.status was \u0027confirmed\u0027 which was set in _confirm_resize before calling _delete_allocation_after_move. With 37ac54a42ec91821d62864d63c486c002608491b we moved _delete_allocation_after_move out of _confirm_resize but the tight-coupling on the migration.status still exists on the stable branch here (that was removed in Stein). So anyway, these other callers of _delete_allocation_after_move are \u0027normal\u0027 path and shouldn\u0027t need to be changed in this patch.\n\nNow there could arguably be bugs in the other things that call _delete_allocation_after_move like bug 1821594 where we\u0027re not handling errors as robustly as we could which might lead to leaked allocations, but those would be separate bugs from the one I\u0027m trying to fix here.","commit_id":"5c4df173baa5c332cca63dbd69ae1ba37f39e0be"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6c47ee4eeae05a18ea080bbef4c78fbdf810ebac","unresolved":false,"context_lines":[{"line_number":6811,"context_line":""},{"line_number":6812,"context_line":"        if allocs:"},{"line_number":6813,"context_line":"            # We had a migration-based allocation that we need to handle"},{"line_number":6814,"context_line":"            self._delete_allocation_after_move(ctxt,"},{"line_number":6815,"context_line":"                                               instance,"},{"line_number":6816,"context_line":"                                               migrate_data.migration,"},{"line_number":6817,"context_line":"                                               instance.flavor,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bfb3d3c7_4f45ef6f","line":6814,"updated":"2019-05-17 22:07:26.000000000","message":"What about this one?","commit_id":"5c4df173baa5c332cca63dbd69ae1ba37f39e0be"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"481e4d2c89dd995b3a0b4eaff2f817cd2015f047","unresolved":false,"context_lines":[{"line_number":6811,"context_line":""},{"line_number":6812,"context_line":"        if allocs:"},{"line_number":6813,"context_line":"            # We had a migration-based allocation that we need to handle"},{"line_number":6814,"context_line":"            self._delete_allocation_after_move(ctxt,"},{"line_number":6815,"context_line":"                                               instance,"},{"line_number":6816,"context_line":"                                               migrate_data.migration,"},{"line_number":6817,"context_line":"                                               instance.flavor,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bfb3d3c7_2fbf1162","line":6814,"in_reply_to":"bfb3d3c7_4f45ef6f","updated":"2019-05-22 20:12:48.000000000","message":"Same answer as above, we don\u0027t need to change anything about this in this change. There is an ass-load of stuff that could go wrong in this method which could mean we don\u0027t even get here and we leak allocations, but that\u0027s a separate bug which would also need to be dealt with on master. In general these live migration methods (and cold migrate for that matter) are way too big and as a result we don\u0027t do good error handling or testing of them for faults. That\u0027s why I\u0027m (slowly) trying to refactor some of this live migration code to make it more maintainable and testable for fault injection and error handling, i.e.:\n\nhttps://review.opendev.org/#/c/641453/\n\nand bug 1818873.","commit_id":"5c4df173baa5c332cca63dbd69ae1ba37f39e0be"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"936a01c25d9c50d10ea8f048d03d95feb6cee076","unresolved":false,"context_lines":[{"line_number":3901,"context_line":"                    # coupled to the migration status on the confirm step so"},{"line_number":3902,"context_line":"                    # we unfortunately have to mutate the migration status to"},{"line_number":3903,"context_line":"                    # have _delete_allocation_after_move cleanup the allocation"},{"line_number":3904,"context_line":"                    # held by the migration consumer."},{"line_number":3905,"context_line":"                    with utils.temporary_mutation("},{"line_number":3906,"context_line":"                            migration, status\u003d\u0027confirmed\u0027):"},{"line_number":3907,"context_line":"                        self._delete_allocation_after_move("}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_d5ad7b9b","line":3904,"updated":"2019-05-24 15:31:01.000000000","message":"It\u0027s a single call to the reportclient we\u0027re making, so tbh, I think I would have preferred just copying that here instead of mutating an object, but that\u0027s just MHO.","commit_id":"dac3239e92fc1865cacc17bbfbd2316072a9d26e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"edb3c2177c2ad3f52c70fc39d61432b802eab2a4","unresolved":false,"context_lines":[{"line_number":3901,"context_line":"                    # coupled to the migration status on the confirm step so"},{"line_number":3902,"context_line":"                    # we unfortunately have to mutate the migration status to"},{"line_number":3903,"context_line":"                    # have _delete_allocation_after_move cleanup the allocation"},{"line_number":3904,"context_line":"                    # held by the migration consumer."},{"line_number":3905,"context_line":"                    with utils.temporary_mutation("},{"line_number":3906,"context_line":"                            migration, status\u003d\u0027confirmed\u0027):"},{"line_number":3907,"context_line":"                        self._delete_allocation_after_move("}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_95d943f4","line":3904,"in_reply_to":"bfb3d3c7_d5ad7b9b","updated":"2019-05-24 15:42:42.000000000","message":"The problem with that is _delete_allocation_after_move has the logic to handle a migration-based allocation or not for upgrades, which by Rocky isn\u0027t likely a problem but could be in Queens if we\u0027re using old computes and aren\u0027t doing migration-based allocations, and this fix needs to go to queens as well.","commit_id":"dac3239e92fc1865cacc17bbfbd2316072a9d26e"}]}