)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8ed8e91a74ed75f32924454b8795f5bfca5ea4bb","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Pawel Koniszewski \u003cpawel.koniszewski@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-10-21 16:18:34 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Change RPC post_live_migration_at_destination from cast to call."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Commit cfa7a261f30b7b3158862aa6ba5916549a904f3d changed call to cast,"},{"line_number":10,"context_line":"but by the time no-one considered that we will need to do some operations"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5da102_60c6d468","line":7,"updated":"2016-10-28 07:22:27.000000000","message":"Hum so you missed the change from cast to call ?","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"71da256d4f4dff95696b384e77f55ad880c02cf3","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Pawel Koniszewski \u003cpawel.koniszewski@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-10-21 16:18:34 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Change RPC post_live_migration_at_destination from cast to call."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Commit cfa7a261f30b7b3158862aa6ba5916549a904f3d changed call to cast,"},{"line_number":10,"context_line":"but by the time no-one considered that we will need to do some operations"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5da102_70ba1fe3","line":7,"in_reply_to":"ba5da102_60c6d468","updated":"2016-10-28 10:05:54.000000000","message":"Yeah, somehow it\u0027s missing...","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"8ad28b9a2ec5519c6efb94ae4d3c95ca91252985","unresolved":false,"context_lines":[{"line_number":14,"context_line":"and post_live_migration_at_destination() will race on updating migration"},{"line_number":15,"context_line":"status resource tracker will not be able to track resources correctly"},{"line_number":16,"context_line":"and might use instance.numa_topology corresponding to the source node"},{"line_number":17,"context_line":"while running resourde audit at destination node."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I687f9ee2f2b7fdfa92a2e9e6fefda2ed4fe6b561"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ba5da102_485444fe","line":17,"updated":"2016-11-01 15:38:34.000000000","message":"/s/resourde/resource","commit_id":"843f9ac0623ccc4076fd64a544432eff4e9d7ae5"}],"nova/compute/manager.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"f3791cd5d5ee008cc679ab01e9ab6288d0c2ffbb","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_17b64bdc","line":5463,"updated":"2016-10-21 18:27:06.000000000","message":"do we need to return the new value of \"instance\" from this routine in order to ensure that we have the new numa_topology/state/host/node/etc?","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"ba1f3006a499c7fd09851e4aa84678ed8265183c","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_92b6d388","line":5463,"in_reply_to":"ba5da102_0f7552f8","updated":"2016-10-28 12:30:12.000000000","message":"My statement was just based on the fact that I saw the migration state being set to \"completed\" before post_live_migration_at_destination() ran.\n\nYou make a good point though, that self.driver.cleanup() will call instance.save() and update the instance object, so I\u0027ll remove my objection.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"994636602b94eccb55ef8a22616146cd8439decd","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_f1d80320","line":5463,"in_reply_to":"ba5da102_83122461","updated":"2016-10-27 15:18:16.000000000","message":"Previously it wasn\u0027t truly parallel.  Before this patch, chronologically we *usually* called the routines on the source, then called the code on the destination to update the host/node in the DB.  (Due to the RPC messaging latency, the destination call was delayed somewhat.) So to an external observer, it all made sense.\n\nNow we\u0027re going to update the host/node in the DB from the destination, and then come back to the source and call routines (with external side effects, like updating the scheduler and sending notifications) with the now-stale instance object that has the wrong host/node in it.\n\nSo an external observer could see the updated entry in the DB, but then get a subsequent notification with an invalid host/node.  Which just seems wrong.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"555418a12c465aa1c494ecd88ed6f0ee8b255429","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_83122461","line":5463,"in_reply_to":"ba5da102_9bad3234","updated":"2016-10-27 13:02:00.000000000","message":"I think that we are safe here. As far as I understand the idea behind objects is that they will not change until we call refresh() explicitly. We never called this before so source node always operated on an instance object pointing to old compute node. Only destination node had correct information. This patch does not change it, source operates on an instance object pointing to source node as refresh() is not called, while destination operates on an instance object pointing to destination node.\n\nThe point of this patch is that it only removes parallel execution of post live migration steps. It does not change anything else.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"71da256d4f4dff95696b384e77f55ad880c02cf3","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_0f7552f8","line":5463,"in_reply_to":"ba5da102_f1d80320","updated":"2016-10-28 10:05:54.000000000","message":"We never could assume that and we never made such assumption that those two are no running in parallel. Operations on source node also involve RPC calls, a lot of them actually. There is also cleanup called which may take long time when, e.g., VM is deployed on NFS and it is heavily used. Cleanup method can take really long time to execute all this stuff - https://github.com/openstack/nova/blob/a9c4baf33a77f99888b95a5d4864b80a3688883d/nova/virt/libvirt/driver.py#L874 - post live migration at destination can end meanwhile.\n\nIt really was a parallel execution. I can\u0027t reproduce a single path of execution when this is a RPC cast, as there are so many remote calls involved.\n\nAnother point is that cleanup method is calling save() on instance object which also fetches latest data from the DB. So once it come to updating resource on source side, instance object will already be up-to-date","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"c1467612603b3b5114c72a636cd845976cc09768","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_a651b810","line":5463,"in_reply_to":"da6895a0_17b64bdc","updated":"2016-10-25 09:46:47.000000000","message":"We don\u0027t, this patch does not aim to fix problems with potential race condition in updating instance which really cannot happen in current code.\n\nAlso I can\u0027t see any place right now that would require to have updated instance. I\u0027d use instance.refresh() instead, but this patch does not require it.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b4eef092ae1ec9ee5d98b7529399332dbc100e63","unresolved":false,"context_lines":[{"line_number":5460,"context_line":"        # Define domain at destination host, without doing it,"},{"line_number":5461,"context_line":"        # pause/suspend/terminate do not work."},{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_9bad3234","line":5463,"in_reply_to":"da6895a0_a651b810","updated":"2016-10-27 09:33:30.000000000","message":"My concern is the scenario where the far end changes things like host/node in the DB, and then the source calls self._update_scheduler_instance_info() and self._notify_about_instance_usage() with a stale \"instance\" variable.\n\nThe way the code works now, these functions are called on the source before the destination updates the DB, so the instance variable is usually accurate at the time they\u0027re called.\n\nWith the change to RPC call, we *know* that the source\u0027s \"instance\" variable is now stale. Do we want to be calling these functions with a known-stale object?","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"3cce693e2b11e9c4a964628dfd92a855b99c3d64","unresolved":false,"context_lines":[{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5467,"context_line":"            # post_live_migration_at_destination() fails as it does not affect"},{"line_number":5468,"context_line":"            # source node."}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_80d3e837","line":5465,"updated":"2016-10-28 07:18:57.000000000","message":"I don\u0027t think this code is going what it is intended to do. It\u0027s an RPC cast, the method is non-bloking and so the exception is going to be be reported anyway outside of that block try..catch...\n\nIf we want to handle such case we should update it to make it as a RPC \u0027call\u0027. We need to think a little bit more about the way the two methods are going to update the same status.\n\n- One of this two methods can have something to only wait for a \u0027failed\u0027 or \u0027completed\u0027  status and take action according them.\n\n- Both of the methods can have a logic to look at the status and pass or take action. (i think it\u0027s the current behavior on the Nikola serie)","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"ba1f3006a499c7fd09851e4aa84678ed8265183c","unresolved":false,"context_lines":[{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5467,"context_line":"            # post_live_migration_at_destination() fails as it does not affect"},{"line_number":5468,"context_line":"            # source node."}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_12aba31e","line":5465,"in_reply_to":"ba5da102_26823195","updated":"2016-10-28 12:30:12.000000000","message":"Oops.  Me too. :)","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"0f738c194d6156c1b0cd29b4b42fe57ad39b44b4","unresolved":false,"context_lines":[{"line_number":5462,"context_line":"        try:"},{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5467,"context_line":"            # post_live_migration_at_destination() fails as it does not affect"},{"line_number":5468,"context_line":"            # source node."}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_26823195","line":5465,"in_reply_to":"ba5da102_80d3e837","updated":"2016-10-28 10:08:08.000000000","message":"Your right, I missed change from cast to call somehow...","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"4984c3fc67865ff4a4b3eeb05bc92e30b137c436","unresolved":false,"context_lines":[{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5467,"context_line":"            # post_live_migration_at_destination() fails as it does not affect"},{"line_number":5468,"context_line":"            # source node."},{"line_number":5469,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_97025b3d","line":5466,"updated":"2016-10-21 18:36:54.000000000","message":"this is tricky, since we don\u0027t know what the far end has done.  I think we need to re-read the instance from the DB and see if it\u0027s been updated by the far end or not.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b4eef092ae1ec9ee5d98b7529399332dbc100e63","unresolved":false,"context_lines":[{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5467,"context_line":"            # post_live_migration_at_destination() fails as it does not affect"},{"line_number":5468,"context_line":"            # source node."},{"line_number":5469,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5da102_969347a3","line":5466,"in_reply_to":"da6895a0_862754b5","updated":"2016-10-27 09:33:30.000000000","message":"okay fair enough.  it would be a separate patch to robustify what happens if post_live_migration_at_destination() failed.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"c1467612603b3b5114c72a636cd845976cc09768","unresolved":false,"context_lines":[{"line_number":5463,"context_line":"            self.compute_rpcapi.post_live_migration_at_destination(ctxt,"},{"line_number":5464,"context_line":"                    instance, block_migration, dest)"},{"line_number":5465,"context_line":"        except Exception as error:"},{"line_number":5466,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5467,"context_line":"            # post_live_migration_at_destination() fails as it does not affect"},{"line_number":5468,"context_line":"            # source node."},{"line_number":5469,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_862754b5","line":5466,"in_reply_to":"da6895a0_97025b3d","updated":"2016-10-25 09:46:47.000000000","message":"In this patch we don\u0027t need to care what happened on the far end. It does not aim to fix any other issue than potential race condition between source and destination.","commit_id":"8fb0121a75bfbee18acaa22324c43b3a044ed28f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d9ebf2984035c4194d8f02579c2dc80be7ccf68","unresolved":false,"context_lines":[{"line_number":5482,"context_line":"                    instance, block_migration, dest)"},{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_cfa7f34d","line":5486,"range":{"start_line":5485,"start_character":57,"end_line":5486,"end_character":45},"updated":"2016-11-07 15:10:36.000000000","message":"If an error occurs during post_live_migration_at_destination() on the destination node, why would we want to continue cleaning up (and thus deleting the VM) on the source node? Wouldn\u0027t that result in a bad situation?","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"6332e04fad083030eaf67224db05cfa571ef64ec","unresolved":false,"context_lines":[{"line_number":5482,"context_line":"                    instance, block_migration, dest)"},{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_229dd465","line":5486,"range":{"start_line":5485,"start_character":57,"end_line":5486,"end_character":45},"in_reply_to":"9a629dbe_16d9f621","updated":"2016-11-07 22:23:47.000000000","message":"I think you\u0027ve got a point that we\u0027re not making anything worse.  I do wonder if it might not make sense as a robustness improvement for the exception case to maybe retry the call once or twice...otherwise the DB will not reflect what is actually happening.","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8bc94930ad5003baf1045bdeba902d5782e86631","unresolved":false,"context_lines":[{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_a189d9cc","line":5486,"in_reply_to":"9a629dbe_229dd465","updated":"2016-11-08 06:54:19.000000000","message":"Jay we can\u0027t rely handle an error at this point. in libvirt driver we just force to define the domain XML but actually I think we should pass the libvirt flag VIR_MIGRATE_PERSIST_DEST. I\u0027m going to push a change.","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"5e749ea16d130d4810f6907fdb498adee15f9b40","unresolved":false,"context_lines":[{"line_number":5482,"context_line":"                    instance, block_migration, dest)"},{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_eba48b58","line":5486,"range":{"start_line":5485,"start_character":57,"end_line":5486,"end_character":45},"in_reply_to":"9a629dbe_229dd465","updated":"2016-11-08 08:29:15.000000000","message":"We actually might break more by calling it more than once. If we for some reason timeout here \nhttps://github.com/openstack/nova/blob/20591e1ed685af4f4143882a38375cbc87a55038/nova/compute/manager.py#L5626\nwe will teardown networking at destination in a subsequent call\nhttps://github.com/openstack/nova/blob/20591e1ed685af4f4143882a38375cbc87a55038/nova/compute/manager.py#L5623\nwhich might cause networking disruption before it is configured again. This is because we update instance.host here\nhttps://github.com/openstack/nova/blob/20591e1ed685af4f4143882a38375cbc87a55038/nova/compute/manager.py#L5615\nand therefore in a subsequent call prev_host will point to destination.\n\nIt\u0027s post_live_migration_at_destination that should care about setting everything up and retrying on fail.","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"0d8b3322197caea50443475715b150384bf6b437","unresolved":false,"context_lines":[{"line_number":5482,"context_line":"                    instance, block_migration, dest)"},{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_16d9f621","line":5486,"range":{"start_line":5485,"start_character":57,"end_line":5486,"end_character":45},"in_reply_to":"9a629dbe_5d41d938","updated":"2016-11-07 20:19:20.000000000","message":"Nova does not undefine domain after live migration, libvirt controls when domain can be undefined, nova does not need to care about domain at source node. Therefore at this point domain is already undefined, so even in case of failure we won\u0027t break anything by performing this cleanup. We can\u0027t make it worse by performing cleanup even if post live migration steps fail at destination. We can at least clean up source node so that operator will not need to jump onto source node and perform cleanup manually.\n\nRoll back is not something we want to do once VM is up and running on destination. LM process is not light-weight, it utilizes a lot of resources and also it can take a lot of time. Such operation would be very complex as we would need to perform all the validation once again, i.e., check hypervisor compatibility (we don\u0027t allow to live migrate to older version of hypervisor) and stuff like that. Also there is no guarantee that such roll back would be successful.\n\nI doubt that we can improve robustness by doing some more stuff at source node if destination fails. We should validate everything before starting live migration and we should be 100% sure that particular live migration will not fail during post live migration steps.","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9d5130b6405f298cd0e937459a9868bafee15187","unresolved":false,"context_lines":[{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_28f6898f","line":5486,"in_reply_to":"9a629dbe_a189d9cc","updated":"2016-11-08 07:52:14.000000000","message":"For reference: https://review.openstack.org/394808","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"67fd084587b6634474d735d5b9c74abe5911437a","unresolved":false,"context_lines":[{"line_number":5482,"context_line":"                    instance, block_migration, dest)"},{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_5d41d938","line":5486,"range":{"start_line":5485,"start_character":57,"end_line":5486,"end_character":45},"in_reply_to":"9a629dbe_cfa7f34d","updated":"2016-11-07 15:27:52.000000000","message":"I raised this same question earlier.  Currently we just unconditionally clean up on the source node, so it\u0027s not like this is making anything *worse*.  But now that we\u0027re making it synchronous we have the opportunity to improve robustness.\n\nThat said, how would we handle an exception here?  Presumably it would require some nuance...if we took an exception in _notify_about_instance_usage() after we\u0027ve already set the new host/node/networking/etc., we probably wouldn\u0027t want to roll back.  If we were unable to set the new host/node, maybe we would want to roll back.","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"e54f749715b7cb4e42b6a802a8d86cf97e55274a","unresolved":false,"context_lines":[{"line_number":5482,"context_line":"                    instance, block_migration, dest)"},{"line_number":5483,"context_line":"        except Exception as error:"},{"line_number":5484,"context_line":"            # We don\u0027t want to break _post_live_migration() if"},{"line_number":5485,"context_line":"            # post_live_migration_at_destination() fails as it should never"},{"line_number":5486,"context_line":"            # affect cleaning up source node."},{"line_number":5487,"context_line":"            LOG.exception(_LE(\"Post live migration at destination %s failed\"),"},{"line_number":5488,"context_line":"                    dest, instance\u003dinstance, error\u003derror)"},{"line_number":5489,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_deb5ac31","line":5486,"range":{"start_line":5485,"start_character":57,"end_line":5486,"end_character":45},"in_reply_to":"9a629dbe_eba48b58","updated":"2016-11-08 15:22:12.000000000","message":"That\u0027s a fair point.  I suppose t\u0027s pretty unlikely (though still possible) that the destination would receive the RPC call and start working on it, then have nova-compute crash.","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6de7986845203278df78d017cb0b974d051b5757","unresolved":false,"context_lines":[{"line_number":5517,"context_line":""},{"line_number":5518,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5519,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5520,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5521,"context_line":"            migrate_data.migration.save()"},{"line_number":5522,"context_line":""},{"line_number":5523,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_c04dc49a","line":5520,"updated":"2016-11-16 10:10:42.000000000","message":"It feels bad marking this as completed if there was an error during post_live_migration_at_destination.","commit_id":"61408211875e38ef997cc9436eed2d7ef75fb4c7"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"957c4242f2cb07893b94b41293eb77b4d6d47f83","unresolved":false,"context_lines":[{"line_number":5517,"context_line":""},{"line_number":5518,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5519,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5520,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5521,"context_line":"            migrate_data.migration.save()"},{"line_number":5522,"context_line":""},{"line_number":5523,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_9ca249ad","line":5520,"in_reply_to":"7a77a97e_c01b047f","updated":"2016-11-16 15:12:37.000000000","message":"I think the Right Thing would be to make post_live_migration_at_destination() idempotent, and then do multiple retries.  If none of them work then we\u0027re well and truly hosed.","commit_id":"61408211875e38ef997cc9436eed2d7ef75fb4c7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9169cbf44708fabb71c06a59441986586933be68","unresolved":false,"context_lines":[{"line_number":5517,"context_line":""},{"line_number":5518,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5519,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5520,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5521,"context_line":"            migrate_data.migration.save()"},{"line_number":5522,"context_line":""},{"line_number":5523,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_c01b047f","line":5520,"in_reply_to":"7a77a97e_c04dc49a","updated":"2016-11-16 10:12:44.000000000","message":"But... what would you do with that information. Just panic? I think this is a step forward, so we should try this out.","commit_id":"61408211875e38ef997cc9436eed2d7ef75fb4c7"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d9ebf2984035c4194d8f02579c2dc80be7ccf68","unresolved":false,"context_lines":[{"line_number":5857,"context_line":"            mig_save.assert_called_once_with()"},{"line_number":5858,"context_line":""},{"line_number":5859,"context_line":"    def test_post_live_migration_exc_on_dest_works_correctly(self):"},{"line_number":5860,"context_line":"        # Confirm post_live_migration() works as expected correctly."},{"line_number":5861,"context_line":"        dest \u003d \u0027desthost\u0027"},{"line_number":5862,"context_line":"        srchost \u003d self.compute.host"},{"line_number":5863,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_7296cc95","line":5860,"updated":"2016-11-07 15:10:36.000000000","message":"This isn\u0027t a particularly useful code comment :) How about something like:\n\n \"\"\"Confirm that post_live_migration() completes successfully\n even after post_live_migration_at_destination() raises an exception.\n \"\"\"","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"0d8b3322197caea50443475715b150384bf6b437","unresolved":false,"context_lines":[{"line_number":5857,"context_line":"            mig_save.assert_called_once_with()"},{"line_number":5858,"context_line":""},{"line_number":5859,"context_line":"    def test_post_live_migration_exc_on_dest_works_correctly(self):"},{"line_number":5860,"context_line":"        # Confirm post_live_migration() works as expected correctly."},{"line_number":5861,"context_line":"        dest \u003d \u0027desthost\u0027"},{"line_number":5862,"context_line":"        srchost \u003d self.compute.host"},{"line_number":5863,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_96034673","line":5860,"in_reply_to":"9a629dbe_7296cc95","updated":"2016-11-07 20:19:20.000000000","message":"Yeah, this isn\u0027t super useful. Missed that comment when copy-pasting","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8bc94930ad5003baf1045bdeba902d5782e86631","unresolved":false,"context_lines":[{"line_number":5857,"context_line":"            mig_save.assert_called_once_with()"},{"line_number":5858,"context_line":""},{"line_number":5859,"context_line":"    def test_post_live_migration_exc_on_dest_works_correctly(self):"},{"line_number":5860,"context_line":"        # Confirm post_live_migration() works as expected correctly."},{"line_number":5861,"context_line":"        dest \u003d \u0027desthost\u0027"},{"line_number":5862,"context_line":"        srchost \u003d self.compute.host"},{"line_number":5863,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_41c1dde3","line":5860,"in_reply_to":"9a629dbe_96034673","updated":"2016-11-08 06:54:19.000000000","message":"Make senses yes","commit_id":"60051f3eb256968af38bfc44aaeecab6441eb130"}]}