)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14a1fab56e505b3e7fbe5864bf9313036e77900c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"which means if something fails on the compute and sets the instance"},{"line_number":11,"context_line":"to ERROR, the Exception block in _cold_migrate in conductor can"},{"line_number":12,"context_line":"blindly reset the vm_state to ACTIVE because it is using a stale"},{"line_number":13,"context_line":"copy of the instance."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change refreshes the instance before calling _set_vm_state_and_notify"},{"line_number":16,"context_line":"so we don\u0027t overwrite the current vm_state. This is not really a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"3fa7e38b_2832e7f3","line":13,"updated":"2019-11-07 15:42:07.000000000","message":"I hit this while adding one of the negative tests later in the series.","commit_id":"462d0d813e504df6b3a28db7655eb57ec2839422"}],"nova/conductor/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73c532a72657bd69768731ee7177b2775b7cb446","unresolved":false,"context_lines":[{"line_number":377,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":378,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":379,"context_line":"                # set after we executed the task."},{"line_number":380,"context_line":"                instance.refresh()"},{"line_number":381,"context_line":"                # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":382,"context_line":"                # set_vm_state_and_notify."},{"line_number":383,"context_line":"                updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3fa7e38b_1a115fad","line":380,"updated":"2019-10-21 19:20:57.000000000","message":"This should probably be wrapped in a try/except for InstanceNotFound in case the instance was deleted during the operation:\n\nhttps://review.opendev.org/#/c/688832/2/nova/conductor/manager.py","commit_id":"1dc15b4f1f09385d10a79119ec718c0628e030e3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"82b22402a572ec02b6fdb2dc7ed6f40b198e3d09","unresolved":false,"context_lines":[{"line_number":377,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":378,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":379,"context_line":"                # set after we executed the task."},{"line_number":380,"context_line":"                instance.refresh()"},{"line_number":381,"context_line":"                # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":382,"context_line":"                # set_vm_state_and_notify."},{"line_number":383,"context_line":"                updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3fa7e38b_bd3925d9","line":380,"in_reply_to":"3fa7e38b_1a115fad","updated":"2019-10-21 19:46:21.000000000","message":"Done","commit_id":"1dc15b4f1f09385d10a79119ec718c0628e030e3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e72d8e8b012c6fcd32bf73972223b4b06d6f888c","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":380,"context_line":"                # set after we executed the task."},{"line_number":381,"context_line":"                try:"},{"line_number":382,"context_line":"                    instance.refresh()"},{"line_number":383,"context_line":"                    # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":384,"context_line":"                    # set_vm_state_and_notify."},{"line_number":385,"context_line":"                    updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_6870df74","line":382,"range":{"start_line":382,"start_character":20,"end_line":382,"end_character":38},"updated":"2019-11-07 15:53:43.000000000","message":"This is a non-free operation, right? You said it\u0027s is not an issue for same-cell resize, but we\u0027re incurring the cost in that case anyway?","commit_id":"462d0d813e504df6b3a28db7655eb57ec2839422"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b31f2a11c4767a6e0a63a0531b568e2b37ca33ec","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":380,"context_line":"                # set after we executed the task."},{"line_number":381,"context_line":"                try:"},{"line_number":382,"context_line":"                    instance.refresh()"},{"line_number":383,"context_line":"                    # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":384,"context_line":"                    # set_vm_state_and_notify."},{"line_number":385,"context_line":"                    updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_88c99b7a","line":382,"range":{"start_line":382,"start_character":20,"end_line":382,"end_character":38},"in_reply_to":"3fa7e38b_6870df74","updated":"2019-11-07 16:02:25.000000000","message":"It\u0027s not free no. However, it\u0027s not entirely cross-cell only if the same-cell migration task blows up *before* casting to the dest host and the instance status is changed somehow. So from the time the task executes to we get here:\n\nhttps://github.com/openstack/nova/blob/f28839b523186121b6481e607321ddbff370cfc5/nova/conductor/tasks/migrate.py#L368\n\nLet\u0027s say we add code at some point that changes the vm_state on the instance and this would blow it away. That\u0027s not an issue now, but this would future proof against that.\n\nIf we really wanted to restrict the refresh to only cross-cell resize we could check to see if the task created the Migration record yet and if so, if Migration.cross_cell_move is True. However, that\u0027s probably also a DB query to get the latest copy of the migration record, so it\u0027s also not free.\n\nAnd finally, even in the same-cell case, if the instance were deleted concurrently while the task was executing, this would also trip the InstanceNotFound so we don\u0027t blow up trying to send a notification on a deleted instance, which is something I found while writing tests for bug 1848343.\n\nSo I don\u0027t think it\u0027s worthwhile to try and make this tricky, especially since we\u0027re already in an Exception block and basically dead in the water for this operation.","commit_id":"462d0d813e504df6b3a28db7655eb57ec2839422"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e0153362f704fdb51dfe5b69bcda744cecfbed8a","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":380,"context_line":"                # set after we executed the task."},{"line_number":381,"context_line":"                try:"},{"line_number":382,"context_line":"                    instance.refresh()"},{"line_number":383,"context_line":"                    # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":384,"context_line":"                    # set_vm_state_and_notify."},{"line_number":385,"context_line":"                    updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_e82a8f2d","line":382,"range":{"start_line":382,"start_character":20,"end_line":382,"end_character":38},"in_reply_to":"3fa7e38b_6870df74","updated":"2019-11-07 15:58:23.000000000","message":"It\u0027s not free, but not super harmful considering all the other DB traffic we do in the process of a single migration. I definitely don\u0027t want to get into the business of checking the same-cell-ness of a migration everywhere to gate stuff like this, especially things related to consistency.","commit_id":"462d0d813e504df6b3a28db7655eb57ec2839422"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"64aa95a44154218f4d23d908db350bf1550b91e3","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":380,"context_line":"                # set after we executed the task."},{"line_number":381,"context_line":"                try:"},{"line_number":382,"context_line":"                    instance.refresh()"},{"line_number":383,"context_line":"                    # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":384,"context_line":"                    # set_vm_state_and_notify."},{"line_number":385,"context_line":"                    updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_48988374","line":382,"range":{"start_line":382,"start_character":20,"end_line":382,"end_character":38},"in_reply_to":"3fa7e38b_88c99b7a","updated":"2019-11-07 16:04:04.000000000","message":"Ack, thanks for the explanation.\n\nI actually missed that we were in a(nother) exception block already :(","commit_id":"462d0d813e504df6b3a28db7655eb57ec2839422"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8300f113bbe47945e6ea725cf9b0b4a46d90c3e","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                # Refresh the instance so we don\u0027t overwrite vm_state changes"},{"line_number":380,"context_line":"                # set after we executed the task."},{"line_number":381,"context_line":"                try:"},{"line_number":382,"context_line":"                    instance.refresh()"},{"line_number":383,"context_line":"                    # Passing vm_state is kind of silly but it\u0027s expected in"},{"line_number":384,"context_line":"                    # set_vm_state_and_notify."},{"line_number":385,"context_line":"                    updates \u003d {\u0027vm_state\u0027: instance.vm_state,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa7e38b_4815e3e4","line":382,"range":{"start_line":382,"start_character":20,"end_line":382,"end_character":38},"in_reply_to":"3fa7e38b_e82a8f2d","updated":"2019-11-07 15:59:37.000000000","message":"Yeah, I figured the alternative (checking same-cell-ness) would be worse. Thanks for confirming my understanding at least.","commit_id":"462d0d813e504df6b3a28db7655eb57ec2839422"}]}