)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"0ede74b813f71361fc1c7bdd7efd936d3e087c29","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   tracking"},{"line_number":28,"context_line":" * We can now track live migration records in the resource tracker so we"},{"line_number":29,"context_line":"   remove the check that would skip them when updating resources."},{"line_number":30,"context_line":" * Missing is (in libvirt\u0027s case) is applying the migration context do"},{"line_number":31,"context_line":"   the xml we pass to the migrateToURI2 API call. This makes this no"},{"line_number":32,"context_line":"   more broken than it was before."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"ba63e191_840a62d1","line":30,"range":{"start_line":30,"start_character":34,"end_line":30,"end_character":36},"updated":"2016-01-19 08:48:01.000000000","message":"duplicated ”is“?","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"30974075c02ea6cf1c7534058ada94f6e65a4c19","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   tracking"},{"line_number":28,"context_line":" * We can now track live migration records in the resource tracker so we"},{"line_number":29,"context_line":"   remove the check that would skip them when updating resources."},{"line_number":30,"context_line":" * Missing is (in libvirt\u0027s case) is applying the migration context do"},{"line_number":31,"context_line":"   the xml we pass to the migrateToURI2 API call. This makes this no"},{"line_number":32,"context_line":"   more broken than it was before."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"bae84128_809b2b72","line":30,"range":{"start_line":30,"start_character":34,"end_line":30,"end_character":36},"in_reply_to":"ba63e191_840a62d1","updated":"2016-02-24 20:36:22.000000000","message":"Done","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"59d2c98dbb81b116985bf2351efe242fb0999882","unresolved":false,"context_lines":[{"line_number":32,"context_line":"   more broken than it was before."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: Ie80983bb8ce4c8ad1a5761f29140da8a970a29d3"},{"line_number":35,"context_line":"Partial-bug: 1417667"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a740942_2901efae","line":35,"updated":"2015-12-04 10:58:29.000000000","message":"Does it fix https://bugs.launchpad.net/nova/+bug/1289064 issue?","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9ea28c4de289320c2822d7b92065a229dcc5f8ef","unresolved":false,"context_lines":[{"line_number":32,"context_line":"   more broken than it was before."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: Ie80983bb8ce4c8ad1a5761f29140da8a970a29d3"},{"line_number":35,"context_line":"Partial-bug: 1417667"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a740942_f374e268","line":35,"in_reply_to":"7a740942_2901efae","updated":"2015-12-07 14:02:02.000000000","message":"it does - will add it in the next respin - thanks","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   tracking"},{"line_number":28,"context_line":" * We can now track live migration records in the resource tracker so we"},{"line_number":29,"context_line":"   remove the check that would skip them when updating resources."},{"line_number":30,"context_line":" * Currently missing (in libvirt\u0027s case) is applying the migration context do"},{"line_number":31,"context_line":"   the xml we pass to the migrateToURI2 API call. This makes this no"},{"line_number":32,"context_line":"   more broken than it was before."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"9aed3d3a_140d06e8","line":30,"updated":"2016-02-29 18:05:06.000000000","message":"nits (about which I barely care at all):\ns/do/to/\n70 character line length","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef4c6f592c95fe59c26ac0598630a3d2d31e8d1f","unresolved":false,"context_lines":[{"line_number":27,"context_line":"   tracking"},{"line_number":28,"context_line":" * We can now track live migration records in the resource tracker so we"},{"line_number":29,"context_line":"   remove the check that would skip them when updating resources."},{"line_number":30,"context_line":" * Currently missing (in libvirt\u0027s case) is applying the migration context do"},{"line_number":31,"context_line":"   the xml we pass to the migrateToURI2 API call. This makes this no"},{"line_number":32,"context_line":"   more broken than it was before."},{"line_number":33,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"7af24918_72bb600f","line":30,"in_reply_to":"9aed3d3a_140d06e8","updated":"2016-03-01 11:46:34.000000000","message":"Done","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":19,"context_line":"   worth noting that in case the destination was specified when live"},{"line_number":20,"context_line":"   migration API was called, no claims will happen."},{"line_number":21,"context_line":" * We do not drop the claim at any point - this could be improved in a"},{"line_number":22,"context_line":"   future patch, but was left out to keep this patch easier to review."},{"line_number":23,"context_line":" * Makes sure that we apply the migration context after a successful"},{"line_number":24,"context_line":"   live migration, and that we drop it upon failure."},{"line_number":25,"context_line":" * Makes sure that the migration record status is updated in failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"5aef4532_7aca1051","line":22,"updated":"2016-03-07 12:25:07.000000000","message":"I am a bit worried we \"leak\" claims, especially as we just added a cancel live-migrate function.\n\nMaybe its in a follow up, but we do already have the rollback live-migrate call, that could drop the claim, I would have thought?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":19,"context_line":"   worth noting that in case the destination was specified when live"},{"line_number":20,"context_line":"   migration API was called, no claims will happen."},{"line_number":21,"context_line":" * We do not drop the claim at any point - this could be improved in a"},{"line_number":22,"context_line":"   future patch, but was left out to keep this patch easier to review."},{"line_number":23,"context_line":" * Makes sure that we apply the migration context after a successful"},{"line_number":24,"context_line":"   live migration, and that we drop it upon failure."},{"line_number":25,"context_line":" * Makes sure that the migration record status is updated in failure"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"5aef4532_5e7d4c6d","line":22,"in_reply_to":"5aef4532_7aca1051","updated":"2016-03-08 11:27:34.000000000","message":"So this still makes the situation better since currently there are no claims whatsoever.\n\nWe could drop the claim provided that we successfully fetch the migration record from the DB, however there is an upper bound on how many things I wanted to put in this patch and this seemed like a reasonable thing to leave out for later as \"leaking\" temporarily (by not dropping the claim) seems better than leaking the permanently by not checking claims at all. Maybe I should clarify that this is really making the existing leak potential better.\n\nHowever - see inline. I might be able to fix it now.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"}],"nova/compute/manager.py":[{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"90161e3adb988c60a5dec66fca04a05654021e43","unresolved":false,"context_lines":[{"line_number":666,"context_line":""},{"line_number":667,"context_line":"class ComputeManager(manager.Manager):"},{"line_number":668,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    target \u003d messaging.Target(version\u003d\u00274.5\u0027)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    # How long to wait in seconds before re-issuing a shutdown"}],"source_content_type":"text/x-python","patch_set":4,"id":"da85f559_51b3f597","line":669,"updated":"2015-11-12 19:14:53.000000000","message":"guess, you forgot to change this","commit_id":"c997aa8e530689d6c95da9aec10fff7708fd6cc2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"0846c6b9e33c95b557c46ff6ee0e9fc322d2b923","unresolved":false,"context_lines":[{"line_number":666,"context_line":""},{"line_number":667,"context_line":"class ComputeManager(manager.Manager):"},{"line_number":668,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    target \u003d messaging.Target(version\u003d\u00274.5\u0027)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    # How long to wait in seconds before re-issuing a shutdown"}],"source_content_type":"text/x-python","patch_set":4,"id":"da85f559_53e1af99","line":669,"in_reply_to":"da85f559_51b3f597","updated":"2015-11-13 00:09:03.000000000","message":"Yep... thanks multinode job!","commit_id":"c997aa8e530689d6c95da9aec10fff7708fd6cc2"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"0ede74b813f71361fc1c7bdd7efd936d3e087c29","unresolved":false,"context_lines":[{"line_number":5002,"context_line":"        :param context: security context"},{"line_number":5003,"context_line":"        :param instance: dict of instance data"},{"line_number":5004,"context_line":"        :param block_migration: if true, prepare for block migration"},{"line_number":5005,"context_line":"        :param disk_over_commit: if true, allow disk over commit"},{"line_number":5006,"context_line":"        :returns: a dict containing migration info"},{"line_number":5007,"context_line":"        \"\"\""},{"line_number":5008,"context_line":"        if scheduled_node is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba63e191_e6c9e5ad","line":5005,"range":{"start_line":5005,"start_character":58,"end_line":5005,"end_character":64},"updated":"2016-01-19 08:48:01.000000000","message":"param description for new arguments.","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":19712,"name":"nail-zte","email":"nail@zte.com.cn","username":"nail-zte"},"change_message_id":"b368cfabf8a0fe9fa96ce846516235aad3ac3fbb","unresolved":false,"context_lines":[{"line_number":5005,"context_line":"        :param disk_over_commit: if true, allow disk over commit"},{"line_number":5006,"context_line":"        :returns: a dict containing migration info"},{"line_number":5007,"context_line":"        \"\"\""},{"line_number":5008,"context_line":"        if scheduled_node is not None:"},{"line_number":5009,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5010,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5011,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_01e84ae1","line":5008,"updated":"2015-12-11 04:31:23.000000000","message":"The scheduled_node is None when migrate instance with specified destination host, so it will not allocate resources in the destination, include numa topology. In conductor, self.dest_node should be equal to destination when destination is not None.","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"e43ec5b0fc1b9cb88402573a790763f6152788a6","unresolved":false,"context_lines":[{"line_number":5005,"context_line":"        :param disk_over_commit: if true, allow disk over commit"},{"line_number":5006,"context_line":"        :returns: a dict containing migration info"},{"line_number":5007,"context_line":"        \"\"\""},{"line_number":5008,"context_line":"        if scheduled_node is not None:"},{"line_number":5009,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5010,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5011,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a710552_304666c0","line":5008,"in_reply_to":"7a740942_01e84ae1","updated":"2015-12-14 17:45:18.000000000","message":"Yes - that is by design (sadly).\n\nWe want to assume that the administrator knows what they are doing when they force a host. I believe this is mentioned in the commit message.","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"0ede74b813f71361fc1c7bdd7efd936d3e087c29","unresolved":false,"context_lines":[{"line_number":5012,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5013,"context_line":"        try:"},{"line_number":5014,"context_line":"            with live_mig_claim(ctxt, instance, migration) as claim:"},{"line_number":5015,"context_line":"                migration \u003d claim.migration"},{"line_number":5016,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5017,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"},{"line_number":5018,"context_line":"        except exception.ComputeResourcesUnavailable as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba63e191_d8e4b93c","line":5015,"range":{"start_line":5015,"start_character":28,"end_line":5015,"end_character":33},"updated":"2016-01-19 08:48:01.000000000","message":"IMHO, no need to assign the migration. \n\nI have see line 154 call it. \nhttps://review.openstack.org/#/c/244489/6/nova/conductor/tasks/live_migrate.py","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"30974075c02ea6cf1c7534058ada94f6e65a4c19","unresolved":false,"context_lines":[{"line_number":5012,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5013,"context_line":"        try:"},{"line_number":5014,"context_line":"            with live_mig_claim(ctxt, instance, migration) as claim:"},{"line_number":5015,"context_line":"                migration \u003d claim.migration"},{"line_number":5016,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5017,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"},{"line_number":5018,"context_line":"        except exception.ComputeResourcesUnavailable as e:"}],"source_content_type":"text/x-python","patch_set":6,"id":"bae84128_a002afb2","line":5015,"range":{"start_line":5015,"start_character":28,"end_line":5015,"end_character":33},"in_reply_to":"ba63e191_d8e4b93c","updated":"2016-02-24 20:36:22.000000000","message":"Ok seems like this is not necessary","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":2667,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2668,"context_line":"        \"\"\""},{"line_number":2669,"context_line":"        if migration:"},{"line_number":2670,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2671,"context_line":"                migration.status \u003d status"},{"line_number":2672,"context_line":"                migration.save()"},{"line_number":2673,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_8349d562","line":2670,"updated":"2016-02-29 18:05:06.000000000","message":"Not sure I understand what this is for. The only code path here where unless_status is used, we aren\u0027t clobbering a terminal status unless we\u0027re also racing with something else modifying the migration object. However, if we\u0027re racing with something else modifying the migration object we\u0027re not going to pick it up like this. We\u0027d want something like:\n\nmigration.save(unless_status\u003d\u0027foo\u0027)\n\nWhat have I missed? Unless it\u0027s something really stupid, could you explain it in a comment?","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b8ccd656eb4d43e5fbacfe5e1dab9b0068ee346b","unresolved":false,"context_lines":[{"line_number":2667,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2668,"context_line":"        \"\"\""},{"line_number":2669,"context_line":"        if migration:"},{"line_number":2670,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2671,"context_line":"                migration.status \u003d status"},{"line_number":2672,"context_line":"                migration.save()"},{"line_number":2673,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_bb2ae07c","line":2670,"in_reply_to":"7af24918_d2cfd4b1","updated":"2016-03-01 13:15:01.000000000","message":"I see it now, thanks. I personally think that localising it would be clearer, but this is fine.","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef4c6f592c95fe59c26ac0598630a3d2d31e8d1f","unresolved":false,"context_lines":[{"line_number":2667,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2668,"context_line":"        \"\"\""},{"line_number":2669,"context_line":"        if migration:"},{"line_number":2670,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2671,"context_line":"                migration.status \u003d status"},{"line_number":2672,"context_line":"                migration.save()"},{"line_number":2673,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_d2cfd4b1","line":2670,"in_reply_to":"9aed3d3a_8349d562","updated":"2016-03-01 11:46:34.000000000","message":"The reason this was put in is because we call it from a finally block that should never clobber if \u0027failed\u0027 was set in the except clause.\n\nI thought this was a nicer way to express that than a local status check","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":5063,"context_line":"        :param migration: the migration object that tracks data about this"},{"line_number":5064,"context_line":"                          migration"},{"line_number":5065,"context_line":"        :param scheduled_node: Node chosen by the scheduler"},{"line_number":5066,"context_line":"        :param scheduled_node: objects.Limits instance that the scheduler"},{"line_number":5067,"context_line":"                               returne when this host was chosen"},{"line_number":5068,"context_line":"        :returns: a dict containing migration info"},{"line_number":5069,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_dfd1f758","line":5066,"updated":"2016-02-29 18:05:06.000000000","message":"s/scheduled_node/limits/","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef4c6f592c95fe59c26ac0598630a3d2d31e8d1f","unresolved":false,"context_lines":[{"line_number":5063,"context_line":"        :param migration: the migration object that tracks data about this"},{"line_number":5064,"context_line":"                          migration"},{"line_number":5065,"context_line":"        :param scheduled_node: Node chosen by the scheduler"},{"line_number":5066,"context_line":"        :param scheduled_node: objects.Limits instance that the scheduler"},{"line_number":5067,"context_line":"                               returne when this host was chosen"},{"line_number":5068,"context_line":"        :returns: a dict containing migration info"},{"line_number":5069,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_c7b9d4ee","line":5066,"in_reply_to":"9aed3d3a_dfd1f758","updated":"2016-03-01 11:46:34.000000000","message":"Done","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":5064,"context_line":"                          migration"},{"line_number":5065,"context_line":"        :param scheduled_node: Node chosen by the scheduler"},{"line_number":5066,"context_line":"        :param scheduled_node: objects.Limits instance that the scheduler"},{"line_number":5067,"context_line":"                               returne when this host was chosen"},{"line_number":5068,"context_line":"        :returns: a dict containing migration info"},{"line_number":5069,"context_line":"        \"\"\""},{"line_number":5070,"context_line":"        if scheduled_node is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_3f4aa326","line":5067,"updated":"2016-02-29 18:05:06.000000000","message":"nit: returned","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef4c6f592c95fe59c26ac0598630a3d2d31e8d1f","unresolved":false,"context_lines":[{"line_number":5064,"context_line":"                          migration"},{"line_number":5065,"context_line":"        :param scheduled_node: Node chosen by the scheduler"},{"line_number":5066,"context_line":"        :param scheduled_node: objects.Limits instance that the scheduler"},{"line_number":5067,"context_line":"                               returne when this host was chosen"},{"line_number":5068,"context_line":"        :returns: a dict containing migration info"},{"line_number":5069,"context_line":"        \"\"\""},{"line_number":5070,"context_line":"        if scheduled_node is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_a7be48d2","line":5067,"in_reply_to":"9aed3d3a_3f4aa326","updated":"2016-03-01 11:46:34.000000000","message":"Done","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":5073,"context_line":"        else:"},{"line_number":5074,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5075,"context_line":"        try:"},{"line_number":5076,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5077,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5078,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"},{"line_number":5079,"context_line":"        except exception.ComputeResourcesUnavailable as e:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_43286de2","line":5076,"updated":"2016-02-29 18:05:06.000000000","message":"Aside: This interface to claim is just wrong. It suggests that the claim lives only for the duration of the context, which it doesn\u0027t. Should probably be:\n\nclaim \u003d rt.live_migration_claim(ctxt, instance, migration)\nwith claim.abort_on_error():\n  ...\n\nMeh, for another day, and not a problem in this patch.","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef4c6f592c95fe59c26ac0598630a3d2d31e8d1f","unresolved":false,"context_lines":[{"line_number":5073,"context_line":"        else:"},{"line_number":5074,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5075,"context_line":"        try:"},{"line_number":5076,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5077,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5078,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"},{"line_number":5079,"context_line":"        except exception.ComputeResourcesUnavailable as e:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_47ce6484","line":5076,"in_reply_to":"9aed3d3a_43286de2","updated":"2016-03-01 11:46:34.000000000","message":"Agreed...","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":5507,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":5508,"context_line":"                                                         self.host)"},{"line_number":5509,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5510,"context_line":"        # instead"},{"line_number":5511,"context_line":"        try:"},{"line_number":5512,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5513,"context_line":"                context, instance.uuid, \u0027running\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_4dbffca9","line":5510,"updated":"2016-02-29 18:05:06.000000000","message":"+1 This is probably also racy, although in practise not one we\u0027re likely to hit.","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":2659,"context_line":"        self._notify_about_instance_usage(context, instance, \"restore.end\")"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"    @staticmethod"},{"line_number":2662,"context_line":"    def _set_migration_status(migration, status, unless_status\u003d_SENTINEL):"},{"line_number":2663,"context_line":"        \"\"\"Set the status, and guard against a None being passed in."},{"line_number":2664,"context_line":""},{"line_number":2665,"context_line":"        This is useful as some of the compute RPC calls will not pass"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_da16c4a6","line":2662,"updated":"2016-03-07 12:25:07.000000000","message":"can\u0027t we just use None?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":2659,"context_line":"        self._notify_about_instance_usage(context, instance, \"restore.end\")"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"    @staticmethod"},{"line_number":2662,"context_line":"    def _set_migration_status(migration, status, unless_status\u003d_SENTINEL):"},{"line_number":2663,"context_line":"        \"\"\"Set the status, and guard against a None being passed in."},{"line_number":2664,"context_line":""},{"line_number":2665,"context_line":"        This is useful as some of the compute RPC calls will not pass"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_beba9008","line":2662,"in_reply_to":"5aef4532_da16c4a6","updated":"2016-03-08 11:27:34.000000000","message":"Nope, None is a valid value for migration status (although one that we are unlikely to hit in this part of the business logic unless something went wrong - the code should be as correct as possible)","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2a658e94e83fa684e592bbb4681573a9c4f6b046","unresolved":false,"context_lines":[{"line_number":5052,"context_line":"    @wrap_instance_fault"},{"line_number":5053,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5054,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5055,"context_line":"                                           migration\u003dNone, scheduled_node\u003dNone,"},{"line_number":5056,"context_line":"                                           limits\u003dNone):"},{"line_number":5057,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5058,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_3f99916f","line":5055,"updated":"2016-03-07 14:01:36.000000000","message":"The function is called from: https://review.openstack.org/#/c/244489/11/nova/conductor/tasks/live_migrate.py with a destination parameter.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b9589c7a192cd615d296f88ae7bab6c42583267e","unresolved":false,"context_lines":[{"line_number":5052,"context_line":"    @wrap_instance_fault"},{"line_number":5053,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5054,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5055,"context_line":"                                           migration\u003dNone, scheduled_node\u003dNone,"},{"line_number":5056,"context_line":"                                           limits\u003dNone):"},{"line_number":5057,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5058,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_9ac1d331","line":5055,"in_reply_to":"5aef4532_3f99916f","updated":"2016-03-07 14:15:04.000000000","message":"Thats needed for the RPC api to pick the rabbit queue, its not wanted here.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5053,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5054,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5055,"context_line":"                                           migration\u003dNone, scheduled_node\u003dNone,"},{"line_number":5056,"context_line":"                                           limits\u003dNone):"},{"line_number":5057,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5058,"context_line":""},{"line_number":5059,"context_line":"        This runs checks on the destination host, and then calls"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_90effdaf","line":5056,"updated":"2016-03-07 12:25:07.000000000","message":"we don\u0027t seem to use limits here, is that intentional?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5076,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5077,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5078,"context_line":"        else:"},{"line_number":5079,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5080,"context_line":"        try:"},{"line_number":5081,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5082,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_d0786592","line":5079,"updated":"2016-03-07 12:25:07.000000000","message":"do we need to do this when the migration is None as well?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":5076,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5077,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5078,"context_line":"        else:"},{"line_number":5079,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5080,"context_line":"        try:"},{"line_number":5081,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5082,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_546fedc4","line":5079,"in_reply_to":"5aef4532_9aef13b0","updated":"2016-03-08 11:27:34.000000000","message":"Right - so forced hosts will not be claiming anything - so why not make it explicit by making it a NopClaim","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2a658e94e83fa684e592bbb4681573a9c4f6b046","unresolved":false,"context_lines":[{"line_number":5076,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5077,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5078,"context_line":"        else:"},{"line_number":5079,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5080,"context_line":"        try:"},{"line_number":5081,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5082,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_ff49e98f","line":5079,"in_reply_to":"5aef4532_d0786592","updated":"2016-03-07 14:01:36.000000000","message":"I would think we would always claim wouldn\u0027t we? For force to host it would use no limits. Not sure what this case is for.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b9589c7a192cd615d296f88ae7bab6c42583267e","unresolved":false,"context_lines":[{"line_number":5076,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5077,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5078,"context_line":"        else:"},{"line_number":5079,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5080,"context_line":"        try:"},{"line_number":5081,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5082,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_9aef13b0","line":5079,"in_reply_to":"5aef4532_ff49e98f","updated":"2016-03-07 14:15:04.000000000","message":"its for compatibility when the node is not specified during upgrade.\n\nwe could try and pick a node, if there is only one, but it doesn\u0027t seem worth it here.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5078,"context_line":"        else:"},{"line_number":5079,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5080,"context_line":"        try:"},{"line_number":5081,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5082,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5083,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"},{"line_number":5084,"context_line":"        except exception.ComputeResourcesUnavailable as e:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_105f6d73","line":5081,"updated":"2016-03-07 12:25:07.000000000","message":"should this be taking limits, eventually?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":5078,"context_line":"        else:"},{"line_number":5079,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5080,"context_line":"        try:"},{"line_number":5081,"context_line":"            with live_mig_claim(ctxt, instance, migration):"},{"line_number":5082,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5083,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"},{"line_number":5084,"context_line":"        except exception.ComputeResourcesUnavailable as e:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_1e5f44e6","line":5081,"in_reply_to":"5aef4532_105f6d73","updated":"2016-03-08 11:27:34.000000000","message":"Yes! Nice catch!","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5085,"context_line":"            LOG.debug(\"Could not claim resources for live migrating the \""},{"line_number":5086,"context_line":"                      \"instance to this host. Not enough resources available.\","},{"line_number":5087,"context_line":"                      instance\u003dinstance)"},{"line_number":5088,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":5089,"context_line":"                                                reason\u003de.format_message())"},{"line_number":5090,"context_line":""},{"line_number":5091,"context_line":"    def _do_check_can_live_migrate_destination(self, ctxt, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_90737d4d","line":5088,"updated":"2016-03-07 12:25:07.000000000","message":"This doesn\u0027t feel like a build abort case really.\n\nWould it not be simpler to just re-use MigrationPreCheckError here?\n\nAlthough I do like how this is more specific. hmm.\n\nMaybe create a new one that is a subclass of MigrationPreCheckError?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2a658e94e83fa684e592bbb4681573a9c4f6b046","unresolved":false,"context_lines":[{"line_number":5085,"context_line":"            LOG.debug(\"Could not claim resources for live migrating the \""},{"line_number":5086,"context_line":"                      \"instance to this host. Not enough resources available.\","},{"line_number":5087,"context_line":"                      instance\u003dinstance)"},{"line_number":5088,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":5089,"context_line":"                                                reason\u003de.format_message())"},{"line_number":5090,"context_line":""},{"line_number":5091,"context_line":"    def _do_check_can_live_migrate_destination(self, ctxt, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_94540c55","line":5088,"in_reply_to":"5aef4532_90737d4d","updated":"2016-03-07 14:01:36.000000000","message":"Yes, its not actually building at this point.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":5085,"context_line":"            LOG.debug(\"Could not claim resources for live migrating the \""},{"line_number":5086,"context_line":"                      \"instance to this host. Not enough resources available.\","},{"line_number":5087,"context_line":"                      instance\u003dinstance)"},{"line_number":5088,"context_line":"            raise exception.BuildAbortException(instance_uuid\u003dinstance.uuid,"},{"line_number":5089,"context_line":"                                                reason\u003de.format_message())"},{"line_number":5090,"context_line":""},{"line_number":5091,"context_line":"    def _do_check_can_live_migrate_destination(self, ctxt, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_fe0898db","line":5088,"in_reply_to":"5aef4532_94540c55","updated":"2016-03-08 11:27:34.000000000","message":"Yep - good catch!","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5522,"context_line":"            migration \u003d None"},{"line_number":5523,"context_line":"        self.network_api.migrate_instance_finish("},{"line_number":5524,"context_line":"            context, instance,"},{"line_number":5525,"context_line":"            {\u0027source_compute\u0027: instance.host, \u0027dest_compute\u0027: self.host, })"},{"line_number":5526,"context_line":""},{"line_number":5527,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":5528,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_70b0718c","line":5525,"updated":"2016-03-07 12:25:07.000000000","message":"Nit: its nicer not to change the existing code in this change, but I its less confusing like this, I guess.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":5522,"context_line":"            migration \u003d None"},{"line_number":5523,"context_line":"        self.network_api.migrate_instance_finish("},{"line_number":5524,"context_line":"            context, instance,"},{"line_number":5525,"context_line":"            {\u0027source_compute\u0027: instance.host, \u0027dest_compute\u0027: self.host, })"},{"line_number":5526,"context_line":""},{"line_number":5527,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":5528,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_74ab09d7","line":5525,"in_reply_to":"5aef4532_70b0718c","updated":"2016-03-08 11:27:34.000000000","message":"Well we don\u0027t want to call that (horribly hacked) dict a migration since we now actually have a migration in scope. That\u0027s why I changed it. It just happens to be one above the other. I might re-arrange it to make it clear","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5539,"context_line":"        except Exception:"},{"line_number":5540,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":5541,"context_line":"                instance.vm_state \u003d vm_states.ERROR"},{"line_number":5542,"context_line":"                self._set_migration_status(migration, \u0027failed\u0027)"},{"line_number":5543,"context_line":"                LOG.error(_LE(\u0027Unexpected error during post live migration at \u0027"},{"line_number":5544,"context_line":"                               \u0027destination host.\u0027), instance\u003dinstance)"},{"line_number":5545,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_b00319b8","line":5542,"updated":"2016-03-07 12:25:07.000000000","message":"These feel like the could all be a separate patch, but I guess its only done here, as we have the migration object now.\n\nMaybe thats a worthwhile separate patch, passing the migration object down?","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2a658e94e83fa684e592bbb4681573a9c4f6b046","unresolved":false,"context_lines":[{"line_number":5539,"context_line":"        except Exception:"},{"line_number":5540,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":5541,"context_line":"                instance.vm_state \u003d vm_states.ERROR"},{"line_number":5542,"context_line":"                self._set_migration_status(migration, \u0027failed\u0027)"},{"line_number":5543,"context_line":"                LOG.error(_LE(\u0027Unexpected error during post live migration at \u0027"},{"line_number":5544,"context_line":"                               \u0027destination host.\u0027), instance\u003dinstance)"},{"line_number":5545,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_ff4c4978","line":5542,"in_reply_to":"5aef4532_b00319b8","updated":"2016-03-07 14:01:36.000000000","message":"I thought this would be good to do separately as well.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b9589c7a192cd615d296f88ae7bab6c42583267e","unresolved":false,"context_lines":[{"line_number":5539,"context_line":"        except Exception:"},{"line_number":5540,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":5541,"context_line":"                instance.vm_state \u003d vm_states.ERROR"},{"line_number":5542,"context_line":"                self._set_migration_status(migration, \u0027failed\u0027)"},{"line_number":5543,"context_line":"                LOG.error(_LE(\u0027Unexpected error during post live migration at \u0027"},{"line_number":5544,"context_line":"                               \u0027destination host.\u0027), instance\u003dinstance)"},{"line_number":5545,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_da7d1be9","line":5542,"in_reply_to":"5aef4532_ff4c4978","updated":"2016-03-07 14:15:04.000000000","message":"well such a patch would have to do all the passing down of the migration objects, its quite a big bunch of work.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5676,"context_line":"            # currently \"leaking\" resources, but only until the next run of the"},{"line_number":5677,"context_line":"            # update_available_resource periodic task, since we error the"},{"line_number":5678,"context_line":"            # migration. For that, we need to be passing migration objects over"},{"line_number":5679,"context_line":"            # RPC"},{"line_number":5680,"context_line":"            instance.drop_migration_context()"},{"line_number":5681,"context_line":""},{"line_number":5682,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_d0e185d2","line":5679,"updated":"2016-03-07 12:25:07.000000000","message":"It does feel like a patch before this one to pass down the migrate objects would be helpful here.\n\nAlthough, can\u0027t we just get the migration here, and drop the claim, as needed?\n\nBut as you say, its not a massive deal given that periodic update, and the fact the claims are in memory here.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2a658e94e83fa684e592bbb4681573a9c4f6b046","unresolved":false,"context_lines":[{"line_number":5676,"context_line":"            # currently \"leaking\" resources, but only until the next run of the"},{"line_number":5677,"context_line":"            # update_available_resource periodic task, since we error the"},{"line_number":5678,"context_line":"            # migration. For that, we need to be passing migration objects over"},{"line_number":5679,"context_line":"            # RPC"},{"line_number":5680,"context_line":"            instance.drop_migration_context()"},{"line_number":5681,"context_line":""},{"line_number":5682,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_df8ced37","line":5679,"in_reply_to":"5aef4532_d0e185d2","updated":"2016-03-07 14:01:36.000000000","message":"I think putting the migration context in first would be good.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":5676,"context_line":"            # currently \"leaking\" resources, but only until the next run of the"},{"line_number":5677,"context_line":"            # update_available_resource periodic task, since we error the"},{"line_number":5678,"context_line":"            # migration. For that, we need to be passing migration objects over"},{"line_number":5679,"context_line":"            # RPC"},{"line_number":5680,"context_line":"            instance.drop_migration_context()"},{"line_number":5681,"context_line":""},{"line_number":5682,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_3ed5e0b8","line":5679,"in_reply_to":"5aef4532_df8ced37","updated":"2016-03-08 11:27:34.000000000","message":"Responded on the commit message - I just thought that this could be a separate piece of work.\n\nThe problem here is that there is no easy way to say - \"give me the current migration, yes the one I am in the process of dealing with now\". We can ask for the latest migration and by status - but that opens us up to some very weird timing bugs if you consider how our RPC works.\n\nHowever I see that currently we actually have the migration object stashed in the migrate_data object, so let me grab it from there and if it\u0027s None - drop the claim which will make for accurate resource tracking","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":2666,"context_line":"        a migration object in older versions. The check can be removed when"},{"line_number":2667,"context_line":"        we move past 4.x major version of the RPC API."},{"line_number":2668,"context_line":""},{"line_number":2669,"context_line":"        If unless_status is passed in - it will guard against clobering a"},{"line_number":2670,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2671,"context_line":"        \"\"\""},{"line_number":2672,"context_line":"        if migration:"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_39c0367b","line":2669,"updated":"2016-03-16 23:45:33.000000000","message":"s/clobering/clobbering","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":2666,"context_line":"        a migration object in older versions. The check can be removed when"},{"line_number":2667,"context_line":"        we move past 4.x major version of the RPC API."},{"line_number":2668,"context_line":""},{"line_number":2669,"context_line":"        If unless_status is passed in - it will guard against clobering a"},{"line_number":2670,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2671,"context_line":"        \"\"\""},{"line_number":2672,"context_line":"        if migration:"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_02043c7c","line":2669,"in_reply_to":"1af94dfe_39c0367b","updated":"2016-03-21 12:20:36.000000000","message":"Done","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":2670,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2671,"context_line":"        \"\"\""},{"line_number":2672,"context_line":"        if migration:"},{"line_number":2673,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2674,"context_line":"                migration.status \u003d status"},{"line_number":2675,"context_line":"                migration.save()"},{"line_number":2676,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_19bbba0f","line":2673,"updated":"2016-03-16 23:45:33.000000000","message":"Instead of the _SENTINEL stuff, why not just do:\n\n terminal_states \u003d (\u0027failed\u0027, \u0027finished\u0027)\n if migration.status not in terminal_states:\n     migration.status \u003d status\n     migration.save()","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":2670,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2671,"context_line":"        \"\"\""},{"line_number":2672,"context_line":"        if migration:"},{"line_number":2673,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2674,"context_line":"                migration.status \u003d status"},{"line_number":2675,"context_line":"                migration.save()"},{"line_number":2676,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_0279dcfc","line":2673,"in_reply_to":"1af94dfe_19bbba0f","updated":"2016-03-21 12:20:36.000000000","message":"Well the reason I did not want to do that is because that would be codifying (in an ad-hoc manner) some idea about the state machine.\n\nWe already have some specs and patches dealing with it so I didn\u0027t want to open that particular can of worms here.","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":2672,"context_line":"        if migration:"},{"line_number":2673,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2674,"context_line":"                migration.status \u003d status"},{"line_number":2675,"context_line":"                migration.save()"},{"line_number":2676,"context_line":""},{"line_number":2677,"context_line":"    def _rebuild_default_impl(self, context, instance, image_meta,"},{"line_number":2678,"context_line":"                              injected_files, admin_password, bdms,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_7938fea0","line":2675,"updated":"2016-03-16 23:45:33.000000000","message":"Might be good to have an else: block with a LOG message saying something like \"Attempted to set migration status after migration already in a terminal state.\"","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":2672,"context_line":"        if migration:"},{"line_number":2673,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2674,"context_line":"                migration.status \u003d status"},{"line_number":2675,"context_line":"                migration.save()"},{"line_number":2676,"context_line":""},{"line_number":2677,"context_line":"    def _rebuild_default_impl(self, context, instance, image_meta,"},{"line_number":2678,"context_line":"                              injected_files, admin_password, bdms,"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_4283d4cc","line":2675,"in_reply_to":"1af94dfe_7938fea0","updated":"2016-03-21 12:20:36.000000000","message":"Makes a lot of sense! Done","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":5107,"context_line":"                                 if None, ignore disk usage checking"},{"line_number":5108,"context_line":"        :param migration: the migration object that tracks data about this"},{"line_number":5109,"context_line":"                          migration"},{"line_number":5110,"context_line":"        :param scheduled_node: Node chosen by the scheduler"},{"line_number":5111,"context_line":"        :param limits: objects.Limits instance that the scheduler"},{"line_number":5112,"context_line":"                               returned when this host was chosen"},{"line_number":5113,"context_line":"        :returns: a dict containing migration info"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_593b829c","line":5110,"updated":"2016-03-16 23:45:33.000000000","message":"Isn\u0027t the destination node already in the Migration object?\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/migration.py#L45\n\nwhy do we need to pass it here? On line 5115 you could just as easily compare the migration object\u0027s dest_compute|dest_node and source_compute|source_node attributes to see if there is any actual migration of the instance to a different host (though this pulls into question what they usefulness of live-migrating an instance to the same host actually would be! ;) )","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":5107,"context_line":"                                 if None, ignore disk usage checking"},{"line_number":5108,"context_line":"        :param migration: the migration object that tracks data about this"},{"line_number":5109,"context_line":"                          migration"},{"line_number":5110,"context_line":"        :param scheduled_node: Node chosen by the scheduler"},{"line_number":5111,"context_line":"        :param limits: objects.Limits instance that the scheduler"},{"line_number":5112,"context_line":"                               returned when this host was chosen"},{"line_number":5113,"context_line":"        :returns: a dict containing migration info"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_05cc4334","line":5110,"in_reply_to":"1af94dfe_593b829c","updated":"2016-03-21 12:20:36.000000000","message":"We need it because it only makes sense to assign the destination on the migration object once the claim is successful (and indeed that\u0027s what we do):\n\nhttps://github.com/openstack/nova/blob/b4444de5a1a50276bb484aee7d40bcf960051fa9/nova/compute/resource_tracker.py#L261","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":5551,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":5552,"context_line":"                                                         self.host)"},{"line_number":5553,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5554,"context_line":"        # instead"},{"line_number":5555,"context_line":"        try:"},{"line_number":5556,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5557,"context_line":"                context, instance.uuid, \u0027running\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_b942860e","line":5554,"updated":"2016-03-16 23:45:33.000000000","message":"Since the only place that calls this was changed above on line 5122, why not just pass the migration object now? Why the TODO?","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":5551,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":5552,"context_line":"                                                         self.host)"},{"line_number":5553,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5554,"context_line":"        # instead"},{"line_number":5555,"context_line":"        try:"},{"line_number":5556,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5557,"context_line":"                context, instance.uuid, \u0027running\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_45bbdb72","line":5554,"in_reply_to":"1af94dfe_b942860e","updated":"2016-03-21 12:20:36.000000000","message":"Well ultimately I needed to scope this patch somehow as it was already getting quite big, and this seemed a reasonable thing to cut out.","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":5580,"context_line":"                instance.vm_state \u003d vm_states.ERROR"},{"line_number":5581,"context_line":"                self._set_migration_status(migration, \u0027failed\u0027)"},{"line_number":5582,"context_line":"                LOG.error(_LE(\u0027Unexpected error during post live migration at \u0027"},{"line_number":5583,"context_line":"                               \u0027destination host.\u0027), instance\u003dinstance)"},{"line_number":5584,"context_line":"        finally:"},{"line_number":5585,"context_line":"            # Restore instance state and update host"},{"line_number":5586,"context_line":"            current_power_state \u003d self._get_power_state(context, instance)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_99450a19","line":5583,"updated":"2016-03-16 23:45:33.000000000","message":"any reason to change this line?","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":5580,"context_line":"                instance.vm_state \u003d vm_states.ERROR"},{"line_number":5581,"context_line":"                self._set_migration_status(migration, \u0027failed\u0027)"},{"line_number":5582,"context_line":"                LOG.error(_LE(\u0027Unexpected error during post live migration at \u0027"},{"line_number":5583,"context_line":"                               \u0027destination host.\u0027), instance\u003dinstance)"},{"line_number":5584,"context_line":"        finally:"},{"line_number":5585,"context_line":"            # Restore instance state and update host"},{"line_number":5586,"context_line":"            current_power_state \u003d self._get_power_state(context, instance)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_2882fe52","line":5583,"in_reply_to":"1af94dfe_99450a19","updated":"2016-03-21 12:20:36.000000000","message":"yeah I guess not I should undo this.","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7e8a64f62aeab11cba8475bba5a230737ac4ec64","unresolved":false,"context_lines":[{"line_number":2686,"context_line":"        a migration object in older versions. The check can be removed when"},{"line_number":2687,"context_line":"        we move past 4.x major version of the RPC API."},{"line_number":2688,"context_line":""},{"line_number":2689,"context_line":"        If unless_status is passed in - it will guard against clobering a"},{"line_number":2690,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2691,"context_line":"        \"\"\""},{"line_number":2692,"context_line":"        if migration:"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_f6cff07c","line":2689,"updated":"2016-04-06 03:04:32.000000000","message":"Doesn\u0027t look like you addressed any of my comments from PS13 :)\n\ns/clobering/clobbering/","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"86dd6b2823c2d3ec406528423b99285810c0b9f0","unresolved":false,"context_lines":[{"line_number":2686,"context_line":"        a migration object in older versions. The check can be removed when"},{"line_number":2687,"context_line":"        we move past 4.x major version of the RPC API."},{"line_number":2688,"context_line":""},{"line_number":2689,"context_line":"        If unless_status is passed in - it will guard against clobering a"},{"line_number":2690,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2691,"context_line":"        \"\"\""},{"line_number":2692,"context_line":"        if migration:"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_4f3235a8","line":2689,"in_reply_to":"9a061dce_f6cff07c","updated":"2016-04-07 09:01:20.000000000","message":"Yeah sorry about that Jay, there was quite a lot of changes made so some of the nits I just forgot about.","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7e8a64f62aeab11cba8475bba5a230737ac4ec64","unresolved":false,"context_lines":[{"line_number":2692,"context_line":"        if migration:"},{"line_number":2693,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2694,"context_line":"                migration.status \u003d status"},{"line_number":2695,"context_line":"                migration.save()"},{"line_number":2696,"context_line":""},{"line_number":2697,"context_line":"    def _rebuild_default_impl(self, context, instance, image_meta,"},{"line_number":2698,"context_line":"                              injected_files, admin_password, bdms,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_d6caac6d","line":2695,"updated":"2016-04-06 03:04:32.000000000","message":"In PS13, I wrote:\n\nMight be good to have an else: block with a LOG message saying something like \"Attempted to set migration status after migration already in a terminal state.\"","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"86dd6b2823c2d3ec406528423b99285810c0b9f0","unresolved":false,"context_lines":[{"line_number":2692,"context_line":"        if migration:"},{"line_number":2693,"context_line":"            if unless_status is _SENTINEL or migration.status !\u003d unless_status:"},{"line_number":2694,"context_line":"                migration.status \u003d status"},{"line_number":2695,"context_line":"                migration.save()"},{"line_number":2696,"context_line":""},{"line_number":2697,"context_line":"    def _rebuild_default_impl(self, context, instance, image_meta,"},{"line_number":2698,"context_line":"                              injected_files, admin_password, bdms,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_6fd61929","line":2695,"in_reply_to":"9a061dce_d6caac6d","updated":"2016-04-07 09:01:20.000000000","message":"Yep same as above - missed it due to a lot of changes that needed to be made","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7e8a64f62aeab11cba8475bba5a230737ac4ec64","unresolved":false,"context_lines":[{"line_number":5614,"context_line":"                instance.vm_state \u003d vm_states.ERROR"},{"line_number":5615,"context_line":"                self._set_migration_status(migration, \u0027failed\u0027)"},{"line_number":5616,"context_line":"                LOG.error(_LE(\u0027Unexpected error during post live migration at \u0027"},{"line_number":5617,"context_line":"                               \u0027destination host.\u0027), instance\u003dinstance)"},{"line_number":5618,"context_line":"        finally:"},{"line_number":5619,"context_line":"            # Restore instance state and update host"},{"line_number":5620,"context_line":"            current_power_state \u003d self._get_power_state(context, instance)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_3678482a","line":5617,"updated":"2016-04-06 03:04:32.000000000","message":"Not sure why this line changed...","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":12813,"name":"Rong Han","email":"han.rong3@zte.com.cn","username":"hanrong-zte"},"change_message_id":"e84a8cbb96c2c61ba438145e6994d856176a196b","unresolved":false,"context_lines":[{"line_number":5639,"context_line":"                migrate_data \u003d \\"},{"line_number":5640,"context_line":"                    migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5641,"context_line":"                        migrate_data)"},{"line_number":5642,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5643,"context_line":"                self.driver.rollback_live_migration_at_destination("},{"line_number":5644,"context_line":"                    context, instance, network_info, block_device_info,"},{"line_number":5645,"context_line":"                    destroy_disks\u003ddestroy_disks, migrate_data\u003dmigrate_data)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1aa78d24_7032bc60","line":5642,"range":{"start_line":5642,"start_character":26,"end_line":5642,"end_character":51},"updated":"2016-07-06 08:29:24.000000000","message":"This code will make instance.numa_topology is migration_context.new_numa_topology in database. Because instance.save operation is in driver.rollback_live_migration_at_destination, and after yield statement, instance.numa_topology is set migration_context.old_numa_topology, but instance object is not saved.\nI submitted a patch to modify this question.\nhttps://review.openstack.org/#/c/336771/","commit_id":"8bb13a52605ffad106f2aa5ce1b1ad6bc2fe560b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d691e38badac7b51383b59a311ff2c84bb61e430","unresolved":false,"context_lines":[{"line_number":2557,"context_line":"        a migration object in older versions. The check can be removed when"},{"line_number":2558,"context_line":"        we move past 4.x major version of the RPC API."},{"line_number":2559,"context_line":""},{"line_number":2560,"context_line":"        If unless_status is passed in - it will guard against clobbering a"},{"line_number":2561,"context_line":"        terminal status (for example \u0027failed\u0027 or \u0027finished\u0027)"},{"line_number":2562,"context_line":"        \"\"\""},{"line_number":2563,"context_line":"        if migration:"}],"source_content_type":"text/x-python","patch_set":24,"id":"1aa78d24_5f15ff3d","line":2560,"updated":"2016-07-06 13:00:07.000000000","message":"To be honest I don\u0027t really understand this","commit_id":"cfa687624bee60237212f96dc739e15967192297"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d691e38badac7b51383b59a311ff2c84bb61e430","unresolved":false,"context_lines":[{"line_number":5035,"context_line":"            compute_node \u003d self._get_compute_info(ctxt, self.host)"},{"line_number":5036,"context_line":"            scheduled_node \u003d compute_node.hypervisor_hostname"},{"line_number":5037,"context_line":"        except exception.ComputeHostNotFound:"},{"line_number":5038,"context_line":"            LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5039,"context_line":"                          self.host)"},{"line_number":5040,"context_line":""},{"line_number":5041,"context_line":"        if scheduled_node is not None:"}],"source_content_type":"text/x-python","patch_set":24,"id":"1aa78d24_5f433f42","line":5038,"updated":"2016-07-06 13:00:07.000000000","message":"Please refer the instance context in this log message.","commit_id":"cfa687624bee60237212f96dc739e15967192297"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1d41c6ab8e5906c532fbf4b206c169dffe5ee4dd","unresolved":false,"context_lines":[{"line_number":5243,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5244,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5245,"context_line":"        try:"},{"line_number":5246,"context_line":"            self.driver.live_migration(context, instance, dest,"},{"line_number":5247,"context_line":"                                       self._post_live_migration,"},{"line_number":5248,"context_line":"                                       self._rollback_live_migration,"},{"line_number":5249,"context_line":"                                       block_migration, migrate_data)"}],"source_content_type":"text/x-python","patch_set":27,"id":"dada55a8_e0117f5e","line":5246,"updated":"2016-07-21 10:03:46.000000000","message":"We missed to apply the migration context here. virt layer is expecting an instance with its new NUMA topology to update the domain xml prior the migration.","commit_id":"52b9bbc5c7d6c7dc472f5cc345e19c9334a11620"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2ce8f6f4f93e1f83a4593fbcfe142b59d076eee1","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5076,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5077,"context_line":"        else:"},{"line_number":5078,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5079,"context_line":"        try:"},{"line_number":5080,"context_line":"            with live_mig_claim(ctxt, instance, migration, limits\u003dlimits):"},{"line_number":5081,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_0655c4c4","line":5078,"updated":"2016-08-01 16:25:32.000000000","message":"@Yingxin: The scheduled node is for Ironic baremetal resource nodes, which cannot participate in any live migration actions. Unfortunately, this is currently the only way to grab a resource tracker instance and therefore we need to get the hypervisor_hostname from the compute node object from the _get_compute_info() method and use that to get the resource tracker...","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"46b5befb40e4e3a374170a3f1c4259e1fa576cda","unresolved":false,"context_lines":[{"line_number":5074,"context_line":"        if scheduled_node is not None:"},{"line_number":5075,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5076,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5077,"context_line":"        else:"},{"line_number":5078,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5079,"context_line":"        try:"},{"line_number":5080,"context_line":"            with live_mig_claim(ctxt, instance, migration, limits\u003dlimits):"},{"line_number":5081,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":30,"id":"dada55a8_83b8522c","line":5078,"range":{"start_line":5077,"start_character":8,"end_line":5078,"end_character":44},"updated":"2016-07-22 08:52:28.000000000","message":"Not sure why it is here. I see no unit tests or functional tests will test this branch.\n\nIt is said that NopClaim is for \"forced hosts\" from previous conversations https://review.openstack.org/#/c/244489/11/nova/compute/manager.py@5079  But the implementation is modified start from patchset 14.","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"c0340b9e6777ba87d24ff5fdd5917a19fc9af8e9","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5076,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5077,"context_line":"        else:"},{"line_number":5078,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5079,"context_line":"        try:"},{"line_number":5080,"context_line":"            with live_mig_claim(ctxt, instance, migration, limits\u003dlimits):"},{"line_number":5081,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_86235f55","line":5078,"in_reply_to":"bacf61ea_0655c4c4","updated":"2016-08-02 01:55:04.000000000","message":"I\u0027m not quite familiar with ironic nodes. But if \"scheduled_node is None\" means it is a Ironic baremetal resource which cannot participate in any migration actions. Is it necessary to raise exception.MigrationPreCheckError() here?\n\nSome tests covered here might be better.","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"78347b2e9d9ec543b2cef95a3e75f4a01d5d8add","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5076,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5077,"context_line":"        else:"},{"line_number":5078,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5079,"context_line":"        try:"},{"line_number":5080,"context_line":"            with live_mig_claim(ctxt, instance, migration, limits\u003dlimits):"},{"line_number":5081,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":30,"id":"dada55a8_8ca9917e","line":5078,"in_reply_to":"dada55a8_0180d545","updated":"2016-07-22 14:07:20.000000000","message":"I mean after the change https://review.openstack.org/#/c/244489/13..14/nova/compute/manager.py@5134 , the `scheduled_node` is no longer a RPC argument that can determine whether this host is selected by scheduler or forced by api.","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"6c8d4d8f94dd59c346bf818d27b56489ca07671a","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"            rt \u003d self._get_resource_tracker(scheduled_node)"},{"line_number":5076,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5077,"context_line":"        else:"},{"line_number":5078,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5079,"context_line":"        try:"},{"line_number":5080,"context_line":"            with live_mig_claim(ctxt, instance, migration, limits\u003dlimits):"},{"line_number":5081,"context_line":"                return self._do_check_can_live_migrate_destination("}],"source_content_type":"text/x-python","patch_set":30,"id":"dada55a8_0180d545","line":5078,"in_reply_to":"dada55a8_83b8522c","updated":"2016-07-22 10:49:49.000000000","message":"What do you mean by \"the implementation is modified ?\"","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2ce8f6f4f93e1f83a4593fbcfe142b59d076eee1","unresolved":false,"context_lines":[{"line_number":5567,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5568,"context_line":"                instance.drop_migration_context()"},{"line_number":5569,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5570,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5571,"context_line":""},{"line_number":5572,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"},{"line_number":5573,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_e64fd031","line":5570,"updated":"2016-08-01 16:25:32.000000000","message":"This is just so difficult to follow :( The whole apply_migration_context() then save() then immediately drop_migration_context() is extremely confusing.","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"bf32f85539c51f4106135e9d81e3c898467ce755","unresolved":false,"context_lines":[{"line_number":5610,"context_line":"                instance.progress \u003d 0"},{"line_number":5611,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5612,"context_line":"                instance.drop_migration_context()"},{"line_number":5613,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5614,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5615,"context_line":""},{"line_number":5616,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"}],"source_content_type":"text/x-python","patch_set":33,"id":"7a8ec9b2_3ca8741f","line":5613,"updated":"2016-09-14 15:52:53.000000000","message":"I\u0027m concerned about ordering between setting the status to \"finished\" here and setting it to \"completed\" on the source, when this is an RPC cast.  It seems like we run the risk of this code running after it was set to \"completed\" on the source, which would cause the Migration object to be returned by MigrationList.get_in_progress_by_host_and_node() in the resource audit.\n\nDo we need to make this an RPC call instead?","commit_id":"6b4398f3e365c2bab0da7976aec61040cad4cff4"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"c58ddad4c02c89e50f884d48149a947aa45cbd4f","unresolved":false,"context_lines":[{"line_number":5610,"context_line":"                instance.progress \u003d 0"},{"line_number":5611,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5612,"context_line":"                instance.drop_migration_context()"},{"line_number":5613,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5614,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5615,"context_line":""},{"line_number":5616,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"}],"source_content_type":"text/x-python","patch_set":33,"id":"7a8ec9b2_2ee1fefe","line":5613,"in_reply_to":"7a8ec9b2_0b5b5488","updated":"2016-09-15 00:38:35.000000000","message":"Alternately, we could just drop this code to set the status to \u0027finished\u0027 since nothing seems to care about this status.","commit_id":"6b4398f3e365c2bab0da7976aec61040cad4cff4"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9750dc6de673e4221e298d3766ea67d5e346a428","unresolved":false,"context_lines":[{"line_number":5610,"context_line":"                instance.progress \u003d 0"},{"line_number":5611,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5612,"context_line":"                instance.drop_migration_context()"},{"line_number":5613,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5614,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5615,"context_line":""},{"line_number":5616,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"}],"source_content_type":"text/x-python","patch_set":33,"id":"7a8ec9b2_0b5b5488","line":5613,"in_reply_to":"7a8ec9b2_3ca8741f","updated":"2016-09-15 00:18:20.000000000","message":"I\u0027ve confirmed that with this patch I\u0027m seeing migrations end up the \"finished\" state, presumably because this code runs after the caller sets it to \"completed\".  Instead of using an RPC call, maybe we should consider setting the migration status to \"completed\" here instead of in the caller?  (Does it buy us anything to do it in the caller, especially when the caller doesn\u0027t know if/when this code runs?)","commit_id":"6b4398f3e365c2bab0da7976aec61040cad4cff4"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9b9569c31cf3d5bf5c778cbd47c33bb3020accbe","unresolved":false,"context_lines":[{"line_number":5607,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5608,"context_line":"                instance.drop_migration_context()"},{"line_number":5609,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5610,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5611,"context_line":""},{"line_number":5612,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"},{"line_number":5613,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":34,"id":"1a95cdbc_0b5252dd","line":5610,"range":{"start_line":5610,"start_character":43,"end_line":5610,"end_character":56},"updated":"2016-10-06 17:02:56.000000000","message":"My previous comments still hold true...since this is called via RPC cast, we don\u0027t know when it will run relative to _post_live_migration() on the source node.  In practice I\u0027ve seen _post_live_migration() run first, in which case this overwrites the \"completed\" state and sets it to \"finished\", which causes problems later on in the resource accounting.","commit_id":"b0672090f4800132509d89b410fb943bd263a8a1"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9130dca39bf1553abecd2c32ef1825004012588f","unresolved":false,"context_lines":[{"line_number":5607,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5608,"context_line":"                instance.drop_migration_context()"},{"line_number":5609,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5610,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5611,"context_line":""},{"line_number":5612,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"},{"line_number":5613,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa6399be_bbdc48e5","line":5610,"in_reply_to":"1a95cdbc_0b5252dd","updated":"2016-10-20 12:54:32.000000000","message":"Yes your comment makes sense and Pawel addressed it","commit_id":"b0672090f4800132509d89b410fb943bd263a8a1"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"67612ff2d0bd8c936323c9b992d8de96b8e3c6f3","unresolved":false,"context_lines":[{"line_number":5607,"context_line":"                instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":5608,"context_line":"                instance.drop_migration_context()"},{"line_number":5609,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5610,"context_line":"                                           unless_status\u003d\u0027failed\u0027)"},{"line_number":5611,"context_line":""},{"line_number":5612,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"},{"line_number":5613,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":34,"id":"fa6399be_9b46a479","line":5610,"in_reply_to":"fa6399be_bbdc48e5","updated":"2016-10-20 12:57:45.000000000","message":"Yeah, that was racy. Addressed it in the newest patch set, hope it is enough. Not sure whether there is any other way to address it given that post_live_migration_at_destination is cast from source node.","commit_id":"b0672090f4800132509d89b410fb943bd263a8a1"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e1725097ba7ec9693bc5e51769165a4fd9657515","unresolved":false,"context_lines":[{"line_number":5627,"context_line":"                instance.drop_migration_context()"},{"line_number":5628,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5629,"context_line":"                                           unless_status\u003d[\u0027failed\u0027,"},{"line_number":5630,"context_line":"                                                          \u0027completed\u0027])"},{"line_number":5631,"context_line":""},{"line_number":5632,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"},{"line_number":5633,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa6399be_bb5fe83f","line":5630,"updated":"2016-10-20 12:58:08.000000000","message":"Yes that is resolving the comment of Chris, the first node which finish the post_live_migration is not going to override the status of the one late.","commit_id":"8b9dd40be11032f853a503111be555349d53432a"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"7279b9594b738460d00debba12acbee10f12052d","unresolved":false,"context_lines":[{"line_number":5617,"context_line":"                LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5618,"context_line":"                              self.host)"},{"line_number":5619,"context_line":"            finally:"},{"line_number":5620,"context_line":"                instance.apply_migration_context()"},{"line_number":5621,"context_line":"                instance.host \u003d self.host"},{"line_number":5622,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5623,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":36,"id":"fa6399be_ed6b5b1c","line":5620,"range":{"start_line":5620,"start_character":25,"end_line":5620,"end_character":48},"updated":"2016-10-20 16:13:04.000000000","message":"The fact that we apply the numa topology here on the destination in an async way (via RPC cast) leads to a race with the resource audit on the destination.  We\u0027ve tripped over it in real life.\n\nIf the source node sets the migration to \"complete\" and then the resource audit runs on the destination node before post_live_migration_at_destination(), the instance being migrated will be accounted by _update_usage_from_instances() instead of _update_usage_from_migrations().  However, because this code hasn\u0027t run yet it will be accounted using the instance.numa_topology corresponding to the source node.\n\nThe simplest way to make this robust would be to change post_live_migration_at_destination() to be an RPC call instead of cast.  This is what we\u0027ve done internally.","commit_id":"2883f295eb7adcd87bd61cfb58881d04997f504c"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"9a1ce8e101f7cbf679741627d383005d65edb2fd","unresolved":false,"context_lines":[{"line_number":5617,"context_line":"                LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5618,"context_line":"                              self.host)"},{"line_number":5619,"context_line":"            finally:"},{"line_number":5620,"context_line":"                instance.apply_migration_context()"},{"line_number":5621,"context_line":"                instance.host \u003d self.host"},{"line_number":5622,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5623,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":36,"id":"da6895a0_f073b8b2","line":5620,"range":{"start_line":5620,"start_character":25,"end_line":5620,"end_character":48},"in_reply_to":"fa6399be_ed6b5b1c","updated":"2016-10-21 07:02:22.000000000","message":"Good catch. That \u0027cast\u0027 causes a lot of troubles not only with resource audit, but also with refreshing volume connection. Wonder what\u0027s the gain from having cast here instead of call.","commit_id":"2883f295eb7adcd87bd61cfb58881d04997f504c"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"12bae8a758c3a5462386890f1cc0e0bded87fc88","unresolved":false,"context_lines":[{"line_number":5624,"context_line":"                LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5625,"context_line":"                              self.host)"},{"line_number":5626,"context_line":"            finally:"},{"line_number":5627,"context_line":"                instance.apply_migration_context()"},{"line_number":5628,"context_line":"                instance.host \u003d self.host"},{"line_number":5629,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5630,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":37,"id":"da6895a0_a868dabb","line":5627,"updated":"2016-10-21 20:42:57.000000000","message":"Just as a heads-up, we think that this introduces a race with the resource audit on the source side, due to the fact that instance.save() is not a single atomic DB transaction. Instead, the instance fields which are objects (such as numa_topology) get saved first, then the instance fields which aren\u0027t objects get updated.  Because of this, _update_usage_from_instances() on the source node can see an instance with the new instance.numa_topology but the old instance.host/instance.node values, which can result in it applying the wrong accounting.","commit_id":"deee6621d4ebb11de2e7524e0e19adbd40970fdc"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"605ed09eb2d87380e5ee2178ae77ca414e6de696","unresolved":false,"context_lines":[{"line_number":5624,"context_line":"                LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5625,"context_line":"                              self.host)"},{"line_number":5626,"context_line":"            finally:"},{"line_number":5627,"context_line":"                instance.apply_migration_context()"},{"line_number":5628,"context_line":"                instance.host \u003d self.host"},{"line_number":5629,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5630,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":37,"id":"ba5da102_c0731614","line":5627,"in_reply_to":"da6895a0_a868dabb","updated":"2016-10-28 12:58:03.000000000","message":"The fix we\u0027ve done internally is to add the following in _update_usage_from_instances() right after \"for instance in instances:\":\n\n    if instance.task_state \u003d\u003d task_states.MIGRATING and \\\n           instance.migration_context is not None:\n        LOG.debug(\"Instance %s task state is MIGRATING, \"\n                  \"ignoring in _update_usage_from_instances().\",\n                  instance.uuid)\n        continue","commit_id":"deee6621d4ebb11de2e7524e0e19adbd40970fdc"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"255a60f8f0ce14117ae8cf7f7cfb79cb65811e05","unresolved":false,"context_lines":[{"line_number":5634,"context_line":"                instance.drop_migration_context()"},{"line_number":5635,"context_line":"                self._set_migration_status(migration, \u0027finished\u0027,"},{"line_number":5636,"context_line":"                                           unless_status\u003d[\u0027failed\u0027,"},{"line_number":5637,"context_line":"                                                          \u0027completed\u0027])"},{"line_number":5638,"context_line":""},{"line_number":5639,"context_line":"        # NOTE(tr3buchet): tear down networks on source host"},{"line_number":5640,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":37,"id":"da6895a0_d71db367","line":5637,"range":{"start_line":5637,"start_character":59,"end_line":5637,"end_character":68},"updated":"2016-10-21 18:06:02.000000000","message":"now that this is being called via an rpc call, I don\u0027t think there\u0027s any way for it to be \"completed\" at this point.","commit_id":"deee6621d4ebb11de2e7524e0e19adbd40970fdc"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"2815e75b3bdfeaf32577808e7f18c4af1785cf53","unresolved":false,"context_lines":[{"line_number":5627,"context_line":"                LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5628,"context_line":"                              self.host)"},{"line_number":5629,"context_line":"            finally:"},{"line_number":5630,"context_line":"                instance.apply_migration_context()"},{"line_number":5631,"context_line":"                instance.host \u003d self.host"},{"line_number":5632,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5633,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":41,"id":"9a629dbe_3d46e523","line":5630,"range":{"start_line":5630,"start_character":25,"end_line":5630,"end_character":48},"updated":"2016-11-07 15:31:37.000000000","message":"There is still a race here, see my comments from Oct 21 and 28.","commit_id":"910eca0372f5efccc938553b18b75443857c3a01"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"a316a20b5ff5b53b7666e657e587c08a02ae5679","unresolved":false,"context_lines":[{"line_number":5378,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5379,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5380,"context_line":"        try:"},{"line_number":5381,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5382,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5383,"context_line":"                                           self._post_live_migration,"},{"line_number":5384,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":46,"id":"3a71b18c_79bbed75","line":5381,"range":{"start_line":5381,"start_character":12,"end_line":5381,"end_character":54},"updated":"2016-12-08 13:01:15.000000000","message":"so we need to make sure that method below the \nmutated_migration_context won\u0027t call instance.save() as \nit will save the applied migration context \nhttps://github.com/openstack/nova/blob/77b72a5b107acf130e7404cc080ba721f474db8f/nova/objects/instance.py#L995-L998\n\nwe should really protect it in the  code and not just in a comment (no body read them :)). \nnot relevant to this patch :)","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"a15519a9e7a2f74c8727a1569719a7f1151f6893","unresolved":false,"context_lines":[{"line_number":5378,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5379,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5380,"context_line":"        try:"},{"line_number":5381,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5382,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5383,"context_line":"                                           self._post_live_migration,"},{"line_number":5384,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":46,"id":"1a6eadb0_72c22835","line":5381,"range":{"start_line":5381,"start_character":12,"end_line":5381,"end_character":54},"in_reply_to":"1a6eadb0_4873e1b1","updated":"2016-12-13 13:36:53.000000000","message":"So I was thinking to protect the instance so I it will be read only for when we are in scope of with instance.mutated_migration_context()\n\nIt was just a side comment. I don\u0027t think you need to do anything about it in this patch","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"742573b23dd66cb1fcc33b14e3c0a6cdd5847727","unresolved":false,"context_lines":[{"line_number":5378,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5379,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5380,"context_line":"        try:"},{"line_number":5381,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5382,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5383,"context_line":"                                           self._post_live_migration,"},{"line_number":5384,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":46,"id":"1a6eadb0_4873e1b1","line":5381,"range":{"start_line":5381,"start_character":12,"end_line":5381,"end_character":54},"in_reply_to":"3a71b18c_79bbed75","updated":"2016-12-12 21:27:26.000000000","message":"But \"below\" in this case means everywhere. This context manager changes instance object that is used by all methods thereafter (including post live migration steps and live  migration rollback). Isn\u0027t that an issue then?","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"a316a20b5ff5b53b7666e657e587c08a02ae5679","unresolved":false,"context_lines":[{"line_number":5659,"context_line":"        #                  plug_vifs"},{"line_number":5660,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":5661,"context_line":"                                                         self.host)"},{"line_number":5662,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5663,"context_line":"        # instead"},{"line_number":5664,"context_line":"        try:"},{"line_number":5665,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5666,"context_line":"                context, instance.uuid, \u0027running\u0027)"}],"source_content_type":"text/x-python","patch_set":46,"id":"3a71b18c_79b5adce","line":5663,"range":{"start_line":5662,"start_character":0,"end_line":5663,"end_character":17},"updated":"2016-12-08 13:01:15.000000000","message":"I wish the comment specify why we want to pass migration over RPC. Is the RPC will be between the computes? is this to avoid races with the resource tracking audit.","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"742573b23dd66cb1fcc33b14e3c0a6cdd5847727","unresolved":false,"context_lines":[{"line_number":5659,"context_line":"        #                  plug_vifs"},{"line_number":5660,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":5661,"context_line":"                                                         self.host)"},{"line_number":5662,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5663,"context_line":"        # instead"},{"line_number":5664,"context_line":"        try:"},{"line_number":5665,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5666,"context_line":"                context, instance.uuid, \u0027running\u0027)"}],"source_content_type":"text/x-python","patch_set":46,"id":"1a6eadb0_48aa01d6","line":5663,"range":{"start_line":5662,"start_character":0,"end_line":5663,"end_character":17},"in_reply_to":"3a71b18c_79b5adce","updated":"2016-12-12 21:27:26.000000000","message":"I will investigate that and add appropriate comment","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"a316a20b5ff5b53b7666e657e587c08a02ae5679","unresolved":false,"context_lines":[{"line_number":5708,"context_line":"                # transaction, therefore, resource audit on the source side"},{"line_number":5709,"context_line":"                # might see an instance with updated, e.g., numa_topology, but"},{"line_number":5710,"context_line":"                # the old host/node."},{"line_number":5711,"context_line":"                instance.apply_migration_context()"},{"line_number":5712,"context_line":"                instance.host \u003d self.host"},{"line_number":5713,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5714,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":46,"id":"3a71b18c_1922f9b9","line":5711,"range":{"start_line":5711,"start_character":0,"end_line":5711,"end_character":50},"updated":"2016-12-08 13:01:15.000000000","message":"Don\u0027t you need to apply the migration_context() only when \n migration_status \u003d\u003d \u0027finished\u0027","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"742573b23dd66cb1fcc33b14e3c0a6cdd5847727","unresolved":false,"context_lines":[{"line_number":5708,"context_line":"                # transaction, therefore, resource audit on the source side"},{"line_number":5709,"context_line":"                # might see an instance with updated, e.g., numa_topology, but"},{"line_number":5710,"context_line":"                # the old host/node."},{"line_number":5711,"context_line":"                instance.apply_migration_context()"},{"line_number":5712,"context_line":"                instance.host \u003d self.host"},{"line_number":5713,"context_line":"                instance.power_state \u003d current_power_state"},{"line_number":5714,"context_line":"                instance.task_state \u003d None"}],"source_content_type":"text/x-python","patch_set":46,"id":"1a6eadb0_6896dd95","line":5711,"range":{"start_line":5711,"start_character":0,"end_line":5711,"end_character":50},"in_reply_to":"3a71b18c_1922f9b9","updated":"2016-12-12 21:27:26.000000000","message":"I think it\u0027s okay to apply migration context in both cases as VM is already up and running on destination, so we might want to update numa topologies etc. in the DB.","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"9292bbe0a1be641056f8094bb1495487a8e0da6f","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":48,"id":"1a6eadb0_2f5ccb4a","line":5391,"updated":"2016-12-13 13:21:58.000000000","message":"So I\u0027m trying to understand why we would need this there. Driver does not need to have mutated migration context as it does not even read values from pci_requests, pci_devices and numa_topology from instance object. Those three are required by post live migration steps and by live migration rollback. However, both methods apply/drop migration context and set mutated migration context if needed.","commit_id":"d2ddd7a2bf4706ecdbda498f0c07b1ec7aee084d"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"959e177103a3751d547115fb076c17e09075a9a7","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":48,"id":"1a6eadb0_8b314aa5","line":5391,"in_reply_to":"1a6eadb0_2f5ccb4a","updated":"2016-12-13 15:49:53.000000000","message":"I think it was originally added by Sahid in response to my comment on https://review.openstack.org/#/c/286744/ on June 20th.  The issue was that we want the new xml to represent the newly-claimed pinned CPUs on the destination.","commit_id":"d2ddd7a2bf4706ecdbda498f0c07b1ec7aee084d"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"baa1e025012bf42294dba25edabeaf6c26ef4e2e","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":48,"id":"1a6eadb0_7204e816","line":5391,"in_reply_to":"1a6eadb0_2f5ccb4a","updated":"2016-12-13 13:45:09.000000000","message":"so the method get self._post_live_migration and self._rollback_live_migration as callback and trigger it here \nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6505-L6508\nso it make sense to me","commit_id":"d2ddd7a2bf4706ecdbda498f0c07b1ec7aee084d"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"46708e25e48d1a709b858466d35081635f0b3def","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":48,"id":"1a6eadb0_38e6cfb4","line":5391,"in_reply_to":"1a6eadb0_7204e816","updated":"2016-12-13 14:15:07.000000000","message":"Okay, I will go through once again to see whether those two methods really need to have mutated migration context set. Let\u0027s move forward now.","commit_id":"d2ddd7a2bf4706ecdbda498f0c07b1ec7aee084d"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"0e666e27d234247ee8b85b59d2d7ed6da9af0cc1","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":48,"id":"1a6eadb0_e0c0e27e","line":5391,"in_reply_to":"1a6eadb0_8b314aa5","updated":"2016-12-14 15:43:04.000000000","message":"That\u0027s a good point, thanks!","commit_id":"d2ddd7a2bf4706ecdbda498f0c07b1ec7aee084d"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"e0ba0c5bd9ab5970b0bde24034977691addfd752","unresolved":false,"context_lines":[{"line_number":5183,"context_line":"    @wrap_instance_fault"},{"line_number":5184,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5185,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5186,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":5187,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5188,"context_line":""},{"line_number":5189,"context_line":"        This runs checks on the destination host, and then calls"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_9ba734bc","line":5186,"range":{"start_line":5186,"start_character":43,"end_line":5186,"end_character":70},"updated":"2017-01-31 08:55:01.000000000","message":"rechecked this, please remove default values from here, there are no cases when these 2 defaults are required.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"217354e2e75cc3245173a167e06724908f782b21","unresolved":false,"context_lines":[{"line_number":5183,"context_line":"    @wrap_instance_fault"},{"line_number":5184,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5185,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5186,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":5187,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5188,"context_line":""},{"line_number":5189,"context_line":"        This runs checks on the destination host, and then calls"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_f1cdb766","line":5186,"range":{"start_line":5186,"start_character":43,"end_line":5186,"end_character":70},"in_reply_to":"3a461143_9ba734bc","updated":"2017-01-31 10:07:46.000000000","message":"Well, it will be incompatible with older RPC, so it needs to be there, e.g., if operator has Newton deployment but RPC pinned to 4.0 those two need to have default values.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"0b27743bd3a2d35afcd0cc9e1efebbc3067d1ac7","unresolved":false,"context_lines":[{"line_number":5183,"context_line":"    @wrap_instance_fault"},{"line_number":5184,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5185,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5186,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":5187,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5188,"context_line":""},{"line_number":5189,"context_line":"        This runs checks on the destination host, and then calls"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_c0dc6348","line":5186,"range":{"start_line":5186,"start_character":43,"end_line":5186,"end_character":70},"in_reply_to":"3a461143_e0707fa6","updated":"2017-01-31 11:57:22.000000000","message":"It\u0027s nova assumption that RPC needs to be compatible across all versions.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"101b399b8666c4a9d10a788e85fd3ce99c058453","unresolved":false,"context_lines":[{"line_number":5183,"context_line":"    @wrap_instance_fault"},{"line_number":5184,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5185,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":5186,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":5187,"context_line":"        \"\"\"Check if it is possible to execute live migration."},{"line_number":5188,"context_line":""},{"line_number":5189,"context_line":"        This runs checks on the destination host, and then calls"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_e0707fa6","line":5186,"range":{"start_line":5186,"start_character":43,"end_line":5186,"end_character":70},"in_reply_to":"3a461143_f1cdb766","updated":"2017-01-31 11:48:21.000000000","message":"Could you please provide usecase, when operator decides to do so, during upgrades process the controller\u0027s side will be upgraded first.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"e0ba0c5bd9ab5970b0bde24034977691addfd752","unresolved":false,"context_lines":[{"line_number":5202,"context_line":"        :returns: a dict containing migration info"},{"line_number":5203,"context_line":"        \"\"\""},{"line_number":5204,"context_line":"        scheduled_node \u003d None"},{"line_number":5205,"context_line":"        try:"},{"line_number":5206,"context_line":"            compute_node \u003d self._get_compute_info(ctxt, self.host)"},{"line_number":5207,"context_line":"            scheduled_node \u003d compute_node.hypervisor_hostname"},{"line_number":5208,"context_line":"        except exception.ComputeHostNotFound:"},{"line_number":5209,"context_line":"            LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5210,"context_line":"                          self.host)"},{"line_number":5211,"context_line":""},{"line_number":5212,"context_line":"        if scheduled_node is not None:"},{"line_number":5213,"context_line":"            rt \u003d self._get_resource_tracker()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_3b76403e","line":5210,"range":{"start_line":5205,"start_character":8,"end_line":5210,"end_character":36},"updated":"2017-01-31 08:55:01.000000000","message":"I wonder why not to do it in conductor, rather than a compute - \nit will reduce RPC round trip to fetch compute node info.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"454c8a3765d99f93a6183497974711ee8da38227","unresolved":false,"context_lines":[{"line_number":5202,"context_line":"        :returns: a dict containing migration info"},{"line_number":5203,"context_line":"        \"\"\""},{"line_number":5204,"context_line":"        scheduled_node \u003d None"},{"line_number":5205,"context_line":"        try:"},{"line_number":5206,"context_line":"            compute_node \u003d self._get_compute_info(ctxt, self.host)"},{"line_number":5207,"context_line":"            scheduled_node \u003d compute_node.hypervisor_hostname"},{"line_number":5208,"context_line":"        except exception.ComputeHostNotFound:"},{"line_number":5209,"context_line":"            LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5210,"context_line":"                          self.host)"},{"line_number":5211,"context_line":""},{"line_number":5212,"context_line":"        if scheduled_node is not None:"},{"line_number":5213,"context_line":"            rt \u003d self._get_resource_tracker()"}],"source_content_type":"text/x-python","patch_set":58,"id":"1a430d35_3d4289ae","line":5210,"range":{"start_line":5205,"start_character":8,"end_line":5210,"end_character":36},"in_reply_to":"3a461143_3b76403e","updated":"2017-02-09 10:34:18.000000000","message":"For me this code looks like:\n\n1. we get information from db about host where nova-compute is running.\n2. if no record is in db (why?) we use NopClaim, i.e. not calculating resources.\n\nCan somebody, please, clarify that for me?\n\n@tdurakov I believe there is no RPC trip here, do I miss something?","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"454c8a3765d99f93a6183497974711ee8da38227","unresolved":false,"context_lines":[{"line_number":5223,"context_line":"            LOG.debug(\"Could not claim resources for live migrating the \""},{"line_number":5224,"context_line":"                      \"instance to this host. Not enough resources available.\","},{"line_number":5225,"context_line":"                      instance\u003dinstance)"},{"line_number":5226,"context_line":"            raise exception.MigrationPreCheckError(reason\u003de.format_message())"},{"line_number":5227,"context_line":""},{"line_number":5228,"context_line":"    def _do_check_can_live_migrate_destination(self, ctxt, instance,"},{"line_number":5229,"context_line":"                                               block_migration,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1a430d35_3d61a905","line":5226,"updated":"2017-02-09 10:34:18.000000000","message":"nit: At first glance, it was hard for me to grasp this context manager. Finally, I finished in my head something like:\nwith abort_live_migration_on_error(live_mig_claim):\n...","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"44db781776b98517db0044eb4c4f5e732a7fca6c","unresolved":false,"context_lines":[{"line_number":5312,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5313,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5314,"context_line":""},{"line_number":5315,"context_line":"        with instance.mutated_migration_context():"},{"line_number":5316,"context_line":"            migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5317,"context_line":"                                           instance,"},{"line_number":5318,"context_line":"                                           block_device_info,"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_279b5c67","line":5315,"updated":"2017-01-24 13:55:07.000000000","message":"AFAICT nothing related to migration_context is used at least by the libvirt pre_live_migration. However, this makes good sense to me.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"6a18c8b06c7a11ef0d113a3be263336df228cbc9","unresolved":false,"context_lines":[{"line_number":5312,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5313,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5314,"context_line":""},{"line_number":5315,"context_line":"        with instance.mutated_migration_context():"},{"line_number":5316,"context_line":"            migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5317,"context_line":"                                           instance,"},{"line_number":5318,"context_line":"                                           block_device_info,"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_fb248d60","line":5315,"in_reply_to":"5a3905b3_279b5c67","updated":"2017-01-24 14:29:36.000000000","message":"It does when plugging vifs on destination host. Please refer to Moshe\u0027s comment on patch set #46","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b7765971591a4d76e9188fed13f379162c0f771","unresolved":false,"context_lines":[{"line_number":5312,"context_line":"                     context, instance, \"live_migration.pre.start\","},{"line_number":5313,"context_line":"                     network_info\u003dnetwork_info)"},{"line_number":5314,"context_line":""},{"line_number":5315,"context_line":"        with instance.mutated_migration_context():"},{"line_number":5316,"context_line":"            migrate_data \u003d self.driver.pre_live_migration(context,"},{"line_number":5317,"context_line":"                                           instance,"},{"line_number":5318,"context_line":"                                           block_device_info,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_b4af9d7b","line":5315,"in_reply_to":"5a3905b3_fb248d60","updated":"2017-01-31 10:26:48.000000000","message":"Can you add a comment to this effect?","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"44db781776b98517db0044eb4c4f5e732a7fca6c","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_0a9e6506","line":5391,"updated":"2017-01-24 13:55:07.000000000","message":"This looks like a problem to me if the live migration fails and is rolled back. At this point in the control flow the claim has already been made on the destination, so instance.migration_context has been updated to reflect the destination. In the context of mutated_migration_context(), the instance now reflects the destination. As noted in the mutated_migration_context(), we should be careful not to save the instance here.\n\nHowever, _rollback_live_migration, which is called automatically via callback from the driver on failure, does exactly that.\n\nI believe this means that if we aborted a live migration and rolled back to the source we would still retain the values of numa_topology, pci_requests, and pci_devices which we set on the destination claim. This doesn\u0027t seem right.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"75acc4c93a2203795fa3cacb80e807cfa9d25ffe","unresolved":false,"context_lines":[{"line_number":5388,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":5389,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data)"},{"line_number":5390,"context_line":"        try:"},{"line_number":5391,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5392,"context_line":"                self.driver.live_migration(context, instance, dest,"},{"line_number":5393,"context_line":"                                           self._post_live_migration,"},{"line_number":5394,"context_line":"                                           self._rollback_live_migration,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1a1ced50_659bab18","line":5391,"in_reply_to":"5a3905b3_0a9e6506","updated":"2017-03-22 22:36:26.000000000","message":"I think this was added by Sahid in response to my comment on https://review.openstack.org/#/c/286744/ on July 20th. The issue was that we want the new xml to represent the newly-claimed pinned CPUs on the destination.\n\nThe alternative to this would be to explicitly generate the XML based on instance.migration_context.new_numa_topology, and to maybe also use it for the calls to driver_interface.get_guest_memory_backing_config() and driver_interface.get_guest_cpu_config() as well.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"bb631a00a1bbe6abb5836192c64718447678ba11","unresolved":false,"context_lines":[{"line_number":5629,"context_line":""},{"line_number":5630,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5631,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5632,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5633,"context_line":"            migrate_data.migration.save()"},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_c6045116","line":5632,"updated":"2017-01-24 15:56:39.000000000","message":"This is the \u0027completed\u0027 race.\n\nIdeally you\u0027d delete this because we\u0027re doing it on the destination, but that\u0027s an upgrade issue. We need to know if the destination will mark it complete, which an old compute will not.\n\nAlternatively, I wonder if there\u0027s any way of doing the state update on the source, and perhaps only marking failure on the destination? Either way, I don\u0027t think we want both ends racing to update this: one or the other.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"bec8dda4bf407dac9280c12c6811281fe8f7537b","unresolved":false,"context_lines":[{"line_number":5629,"context_line":""},{"line_number":5630,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5631,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5632,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5633,"context_line":"            migrate_data.migration.save()"},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_e7d3c45d","line":5632,"in_reply_to":"3a461143_e26819bd","updated":"2017-01-26 15:50:14.000000000","message":"It was described in the previous review, I\u0027m really just repeating it. But essentially you\u0027ve got:\n\n_post_live_migration:\n\nSOURCE                                    DEST\n\n... blah ...\n\npost_live_migration_at_destination()\n\n... blah ...                              ... blah ...\n                                          \n                                          ... update instance ...\n\nmigration.status \u003d \u0027completed\u0027            migration.status \u003d \u0027finished\u0027\nmigration.save()                          migration.save()\n\n\nSo depending on the race, migration may be set to either \u0027completed\u0027 or \u0027finished\u0027, and this may happen either before or after instance.host has been updated.\n\nThere was a proposal to make post_live_migration_at_destination() a call rather than a cast. While this would make this deterministic, it is not a good idea as it would cause an error if post live migration took longer than 60 seconds. As there\u0027s no reason it shouldn\u0027t take longer than 60 seconds (it does a bunch of RPC and IO), this isn\u0027t a good fit.\n\nWe should decide that one side or the other is responsible for the completion of the migration. Realistically it should be the destination, but this requires jumping through some upgrade hoops.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8a1ac58a2491338c867709bc573391df2df22aa0","unresolved":false,"context_lines":[{"line_number":5629,"context_line":""},{"line_number":5630,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5631,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5632,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5633,"context_line":"            migrate_data.migration.save()"},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_677354e8","line":5632,"in_reply_to":"3a461143_e7d3c45d","updated":"2017-01-26 16:03:17.000000000","message":"As discussed on IRC, post_live_migration_at_destination *has* since been turned into a call. For the reasons above I don\u0027t think that\u0027s a good idea, but it does mean there\u0027s no race here.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"415b39a8006adea98d75b8a2ab3e56ee87a57755","unresolved":false,"context_lines":[{"line_number":5629,"context_line":""},{"line_number":5630,"context_line":"        self._clean_instance_console_tokens(ctxt, instance)"},{"line_number":5631,"context_line":"        if migrate_data and migrate_data.obj_attr_is_set(\u0027migration\u0027):"},{"line_number":5632,"context_line":"            migrate_data.migration.status \u003d \u0027completed\u0027"},{"line_number":5633,"context_line":"            migrate_data.migration.save()"},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"    def _consoles_enabled(self):"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_e26819bd","line":5632,"in_reply_to":"5a3905b3_c6045116","updated":"2017-01-26 14:01:34.000000000","message":"Not sure what\u0027s racy there. Can you be more explicit there?","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"e0ba0c5bd9ab5970b0bde24034977691addfd752","unresolved":false,"context_lines":[{"line_number":5672,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5673,"context_line":"        # instead to reduce number of DB calls"},{"line_number":5674,"context_line":"        try:"},{"line_number":5675,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5676,"context_line":"                context, instance.uuid, \u0027running\u0027)"},{"line_number":5677,"context_line":"        except exception.MigrationNotFoundByStatus:"},{"line_number":5678,"context_line":"            LOG.warning(_LW(\"No migration record found for this instance \""},{"line_number":5679,"context_line":"                            \"during post_live_migrate routine\"),"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_fbd4b8ec","line":5676,"range":{"start_line":5675,"start_character":12,"end_line":5676,"end_character":50},"updated":"2017-01-31 08:55:01.000000000","message":"there are several bugs that could lead nova to have more then 1 active migration for instance(at least it\u0027s possible to introduce new bug for that:( ), on the other hand, migration_context contains migration_id record, let\u0027s just fetch migration by id here. It will be much safer as for me","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"bde9be42b0df82bc40dcf1fc223e1b113adb887c","unresolved":false,"context_lines":[{"line_number":5672,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5673,"context_line":"        # instead to reduce number of DB calls"},{"line_number":5674,"context_line":"        try:"},{"line_number":5675,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5676,"context_line":"                context, instance.uuid, \u0027running\u0027)"},{"line_number":5677,"context_line":"        except exception.MigrationNotFoundByStatus:"},{"line_number":5678,"context_line":"            LOG.warning(_LW(\"No migration record found for this instance \""},{"line_number":5679,"context_line":"                            \"during post_live_migrate routine\"),"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_803a5b97","line":5676,"range":{"start_line":5675,"start_character":12,"end_line":5676,"end_character":50},"in_reply_to":"3a461143_e53df1dd","updated":"2017-01-31 11:52:24.000000000","message":"p.s. we do the claim on destination node during check_can_live_migrate_destination, so it should be always there for post_live_migration_at_destination(both will be on the same place, no matter during n-1 to n or n to n).","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"101b399b8666c4a9d10a788e85fd3ce99c058453","unresolved":false,"context_lines":[{"line_number":5672,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5673,"context_line":"        # instead to reduce number of DB calls"},{"line_number":5674,"context_line":"        try:"},{"line_number":5675,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5676,"context_line":"                context, instance.uuid, \u0027running\u0027)"},{"line_number":5677,"context_line":"        except exception.MigrationNotFoundByStatus:"},{"line_number":5678,"context_line":"            LOG.warning(_LW(\"No migration record found for this instance \""},{"line_number":5679,"context_line":"                            \"during post_live_migrate routine\"),"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_e53df1dd","line":5676,"range":{"start_line":5675,"start_character":12,"end_line":5676,"end_character":50},"in_reply_to":"3a461143_f170570d","updated":"2017-01-31 11:48:21.000000000","message":"@Pawel, I mean you could get migration.id from instance.migration_context.migration_id. Don\u0027t we have migration_context set here?","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"217354e2e75cc3245173a167e06724908f782b21","unresolved":false,"context_lines":[{"line_number":5672,"context_line":"        # TODO(ndipanov): We should be passing the migration over RPC here"},{"line_number":5673,"context_line":"        # instead to reduce number of DB calls"},{"line_number":5674,"context_line":"        try:"},{"line_number":5675,"context_line":"            migration \u003d objects.Migration.get_by_instance_and_status("},{"line_number":5676,"context_line":"                context, instance.uuid, \u0027running\u0027)"},{"line_number":5677,"context_line":"        except exception.MigrationNotFoundByStatus:"},{"line_number":5678,"context_line":"            LOG.warning(_LW(\"No migration record found for this instance \""},{"line_number":5679,"context_line":"                            \"during post_live_migrate routine\"),"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_f170570d","line":5676,"range":{"start_line":5675,"start_character":12,"end_line":5676,"end_character":50},"in_reply_to":"3a461143_fbd4b8ec","updated":"2017-01-31 10:07:46.000000000","message":"Here we don\u0027t have migration id. We would need to pass it there and conditionally use this logic or logic based on migration id, depends if migration is from N-1 to N or N to N. This would make this code much more complicated than it is right now. This change is hard enough to be reviewed so that I don\u0027t want to add another unnecessary code there.\n\nHowever, I will add TODO here that we might want to use migration ID instead of fetching active migration for instance.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"bb631a00a1bbe6abb5836192c64718447678ba11","unresolved":false,"context_lines":[{"line_number":5717,"context_line":"                # TODO(pkoniszewski): instance.save() is not a single atomic"},{"line_number":5718,"context_line":"                # transaction, therefore, resource audit on the source side"},{"line_number":5719,"context_line":"                # might see an instance with updated, e.g., numa_topology, but"},{"line_number":5720,"context_line":"                # the old host/node."},{"line_number":5721,"context_line":"                instance.apply_migration_context()"},{"line_number":5722,"context_line":"                instance.host \u003d self.host"},{"line_number":5723,"context_line":"                instance.power_state \u003d current_power_state"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_139bfd66","line":5720,"updated":"2017-01-24 15:56:39.000000000","message":"Did you add this comment due to cfriesen\u0027s comment here:\n  https://review.openstack.org/#/c/244489/36/nova/compute/manager.py\n\nIf so, that\u0027s not the race being described there, and I don\u0027t think it\u0027s addressed. The issue is that the source node sets migration status to \u0027completed\u0027 while it\u0027s still in progress on the destination because it\u0027s a cast, not a call. Given the RPC latency and the amount of work to be done in this method, I suspect that the source will almost always win this race. This means that there\u0027s a period in which the migration is marked as \u0027completed\u0027 (by the source), but the instance object hasn\u0027t been updated to reflect this, yet. The issue here is that when resource_tracker on the destination does _update_available_resource it will not find the instance (because it\u0027s host is still the source), or the migration (because it\u0027s \u0027completed\u0027: see db.migration_get_in_progress_by_host_and_node()).\n\ncfriesen suggested making the cast into a call, but I don\u0027t think we want to do that. Calls are themselves problematic when there\u0027s an indeterminate amount of work to do at the remote end because they fail if they take longer than 60 seconds to complete in ways which we rarely handle well. Cast is the right thing here. However, I don\u0027t think the source should be marking the migration \u0027completed\u0027 when it is not.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"415b39a8006adea98d75b8a2ab3e56ee87a57755","unresolved":false,"context_lines":[{"line_number":5717,"context_line":"                # TODO(pkoniszewski): instance.save() is not a single atomic"},{"line_number":5718,"context_line":"                # transaction, therefore, resource audit on the source side"},{"line_number":5719,"context_line":"                # might see an instance with updated, e.g., numa_topology, but"},{"line_number":5720,"context_line":"                # the old host/node."},{"line_number":5721,"context_line":"                instance.apply_migration_context()"},{"line_number":5722,"context_line":"                instance.host \u003d self.host"},{"line_number":5723,"context_line":"                instance.power_state \u003d current_power_state"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_421bad72","line":5720,"in_reply_to":"5a3905b3_139bfd66","updated":"2017-01-26 14:01:34.000000000","message":"RPC has been changed to call\n\nhttps://review.openstack.org/#/c/389687/","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"44db781776b98517db0044eb4c4f5e732a7fca6c","unresolved":false,"context_lines":[{"line_number":5835,"context_line":"            if isinstance(migrate_data, dict):"},{"line_number":5836,"context_line":"                migrate_data \u003d \\"},{"line_number":5837,"context_line":"                    migrate_data_obj.LiveMigrateData.detect_implementation("},{"line_number":5838,"context_line":"                        migrate_data)"},{"line_number":5839,"context_line":"            with instance.mutated_migration_context():"},{"line_number":5840,"context_line":"                self.driver.rollback_live_migration_at_destination("},{"line_number":5841,"context_line":"                    context, instance, network_info, block_device_info,"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_6d1e230d","line":5838,"updated":"2017-01-24 13:55:07.000000000","message":"As above, the libvirt driver saves the instance during rollback_live_migration_at_destination (in cleanup, called via destroy). I think this means we\u0027ll end up persisting the mutated migration context.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e4890c4de13695cfb4fa177aeead2fabc161e95d","unresolved":false,"context_lines":[{"line_number":5222,"context_line":"            LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5223,"context_line":"                          self.host)"},{"line_number":5224,"context_line":""},{"line_number":5225,"context_line":"        if scheduled_node is not None:"},{"line_number":5226,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":5227,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5228,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":59,"id":"da36d5c6_d25c2c4d","line":5225,"updated":"2017-02-24 17:09:55.000000000","message":"add:\n\n and migration is not None:","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d4edb666cb081550e6bbdd58eff67816b0e7f9e1","unresolved":false,"context_lines":[{"line_number":5222,"context_line":"            LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5223,"context_line":"                          self.host)"},{"line_number":5224,"context_line":""},{"line_number":5225,"context_line":"        if scheduled_node is not None:"},{"line_number":5226,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":5227,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5228,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":59,"id":"da36d5c6_7243806c","line":5225,"updated":"2017-02-24 17:08:35.000000000","message":"also migration is not None here.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"891bca349359c3ccdc9948a771cf2103ad9f7092","unresolved":false,"context_lines":[{"line_number":5222,"context_line":"            LOG.exception(_LE(\u0027Failed to get compute_info for %s\u0027),"},{"line_number":5223,"context_line":"                          self.host)"},{"line_number":5224,"context_line":""},{"line_number":5225,"context_line":"        if scheduled_node is not None:"},{"line_number":5226,"context_line":"            rt \u003d self._get_resource_tracker()"},{"line_number":5227,"context_line":"            live_mig_claim \u003d rt.live_migration_claim"},{"line_number":5228,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":59,"id":"ff82abbf_98d67bc3","line":5225,"in_reply_to":"da36d5c6_d25c2c4d","updated":"2017-11-23 15:53:59.000000000","message":"Done","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"117f06988c6d30437a430936995a5285a39ab8ae","unresolved":false,"context_lines":[{"line_number":5228,"context_line":"        else:"},{"line_number":5229,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5230,"context_line":"        try:"},{"line_number":5231,"context_line":"            with live_mig_claim(ctxt, instance, scheduled_node,"},{"line_number":5232,"context_line":"                                migration, limits\u003dlimits):"},{"line_number":5233,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5234,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"}],"source_content_type":"text/x-python","patch_set":59,"id":"da36d5c6_72b3800a","line":5231,"updated":"2017-02-24 16:58:39.000000000","message":"Feels like in the upgrade case we need to NopClaim here, when migration and limits are None.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"891bca349359c3ccdc9948a771cf2103ad9f7092","unresolved":false,"context_lines":[{"line_number":5228,"context_line":"        else:"},{"line_number":5229,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5230,"context_line":"        try:"},{"line_number":5231,"context_line":"            with live_mig_claim(ctxt, instance, scheduled_node,"},{"line_number":5232,"context_line":"                                migration, limits\u003dlimits):"},{"line_number":5233,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5234,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"}],"source_content_type":"text/x-python","patch_set":59,"id":"ff82abbf_58e0f396","line":5231,"in_reply_to":"da36d5c6_127c04b4","updated":"2017-11-23 15:53:59.000000000","message":"Done","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"acfecf5543e2cf0fade3d83a9934a4cb353f29e5","unresolved":false,"context_lines":[{"line_number":5228,"context_line":"        else:"},{"line_number":5229,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5230,"context_line":"        try:"},{"line_number":5231,"context_line":"            with live_mig_claim(ctxt, instance, scheduled_node,"},{"line_number":5232,"context_line":"                                migration, limits\u003dlimits):"},{"line_number":5233,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5234,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"}],"source_content_type":"text/x-python","patch_set":59,"id":"da36d5c6_925c747e","line":5231,"in_reply_to":"da36d5c6_72b3800a","updated":"2017-02-24 17:05:03.000000000","message":"To be clear, I mean when a new node does\u0027t get these optional params (see RPC api doesn\u0027t send this from the old node, or a new node that hasn\u0027t raised the RPC pin yet), this code here tries to create a migration object, but we already did that in the old node already.\n\nI think the best approach here is to use the NopClaim if we don\u0027t get migration and limit objects here. Its tempting to check the min service version of compute nodes here, and only start doing the claim until every has been upgraded.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d4edb666cb081550e6bbdd58eff67816b0e7f9e1","unresolved":false,"context_lines":[{"line_number":5228,"context_line":"        else:"},{"line_number":5229,"context_line":"            live_mig_claim \u003d claims.NopClaim"},{"line_number":5230,"context_line":"        try:"},{"line_number":5231,"context_line":"            with live_mig_claim(ctxt, instance, scheduled_node,"},{"line_number":5232,"context_line":"                                migration, limits\u003dlimits):"},{"line_number":5233,"context_line":"                return self._do_check_can_live_migrate_destination("},{"line_number":5234,"context_line":"                    ctxt, instance, block_migration, disk_over_commit)"}],"source_content_type":"text/x-python","patch_set":59,"id":"da36d5c6_127c04b4","line":5231,"in_reply_to":"da36d5c6_925c747e","updated":"2017-02-24 17:08:35.000000000","message":"So lets just use NopClaim here if the migration is None to avoid this.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"1a6b91d7085042b832d95eccc1fdd01398162e29","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        \"\"\"Create a claim for a live_migration operation.\"\"\""},{"line_number":195,"context_line":"        instance_type \u003d instance.flavor"},{"line_number":196,"context_line":"        image_meta \u003d objects.ImageMeta.from_instance(instance)"},{"line_number":197,"context_line":"        return self._move_claim(context, instance, instance_type,"},{"line_number":198,"context_line":"                                move_type\u003d\u0027live-migration\u0027, limits\u003dlimits,"},{"line_number":199,"context_line":"                                image_meta\u003dimage_meta, migration\u003dmigration)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a68dd71_e8c43460","line":197,"updated":"2016-01-27 06:51:34.000000000","message":"MoveClaim actually calls `nova.objects.InstancePCIRequest.get_by_instance_uuid_and_newness()`[1] to get pci requests and claim pci devices on the destination node. This is because MoveClaim is originally renamed from ResizeClaim, and the resizing process requires a new set of pci requests.\n\n\nHowever it is problematic to apply the same logic to live migration, because LM always needs the same set of resources to migrate a VM. So it is needed to add a logic to duplicate existing pci requests to new pci requests, or to create a new Claim class to get pci requests from `nova.objects.InstancePCIRequest.get_by_instance_uuid()`.\n\n\n[1] https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L298-L300","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"30974075c02ea6cf1c7534058ada94f6e65a4c19","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        \"\"\"Create a claim for a live_migration operation.\"\"\""},{"line_number":195,"context_line":"        instance_type \u003d instance.flavor"},{"line_number":196,"context_line":"        image_meta \u003d objects.ImageMeta.from_instance(instance)"},{"line_number":197,"context_line":"        return self._move_claim(context, instance, instance_type,"},{"line_number":198,"context_line":"                                move_type\u003d\u0027live-migration\u0027, limits\u003dlimits,"},{"line_number":199,"context_line":"                                image_meta\u003dimage_meta, migration\u003dmigration)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bae84128_20b33f0c","line":197,"in_reply_to":"9a68dd71_e8c43460","updated":"2016-02-24 20:36:22.000000000","message":"I don\u0027t think live migration with PCI devices can ever work really - we should just disable it.","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ac540176b6c580ec313a0486238d66afeab5420f","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        else:"},{"line_number":310,"context_line":"            migration.migration_type \u003d migration_obj.determine_migration_type("},{"line_number":311,"context_line":"                migration)"},{"line_number":312,"context_line":"        migration.create()"},{"line_number":313,"context_line":"        return migration"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def _claim_existing_migration(self, migration):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7aa08908_55dba221","line":312,"updated":"2016-06-15 17:05:31.000000000","message":"Wouldn\u0027t the migration.id be set when calling create()?","commit_id":"8e2a4bb0d57949fe79d2440a5adff1a1d873636e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d3f484a5097dbcea7d0590f9c485ecace8c91a11","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        else:"},{"line_number":310,"context_line":"            migration.migration_type \u003d migration_obj.determine_migration_type("},{"line_number":311,"context_line":"                migration)"},{"line_number":312,"context_line":"        migration.create()"},{"line_number":313,"context_line":"        return migration"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def _claim_existing_migration(self, migration):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7aa08908_bb9d7e0c","line":312,"in_reply_to":"7aa08908_55dba221","updated":"2016-06-16 08:02:37.000000000","message":"Yes exactly...","commit_id":"8e2a4bb0d57949fe79d2440a5adff1a1d873636e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d691e38badac7b51383b59a311ff2c84bb61e430","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        # get memory overhead required to build this instance:"},{"line_number":213,"context_line":"        overhead \u003d self.driver.estimate_instance_overhead(new_instance_type)"},{"line_number":214,"context_line":"        LOG.debug(\"Memory overhead for %(flavor)d MB instance; %(overhead)d \""},{"line_number":215,"context_line":"                  \"MB\", {\u0027flavor\u0027: new_instance_type.memory_mb,"},{"line_number":216,"context_line":"                          \u0027overhead\u0027: overhead[\u0027memory_mb\u0027]})"},{"line_number":217,"context_line":"        LOG.debug(\"Disk overhead for %(flavor)d GB instance; %(overhead)d \""}],"source_content_type":"text/x-python","patch_set":24,"id":"1aa78d24_e2837454","side":"PARENT","line":214,"updated":"2016-07-06 13:00:07.000000000","message":"Any reason why did you removed this ?","commit_id":"5a7e2739011085f8d7228b54639485cddf3df08a"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"a316a20b5ff5b53b7666e657e587c08a02ae5679","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        instance_type \u003d instance.flavor"},{"line_number":168,"context_line":"        image_meta \u003d objects.ImageMeta.from_instance(instance)"},{"line_number":169,"context_line":"        return self._move_claim(context, instance, instance_type,"},{"line_number":170,"context_line":"                                move_type\u003d\u0027live-migration\u0027, limits\u003dlimits,"},{"line_number":171,"context_line":"                                image_meta\u003dimage_meta, migration\u003dmigration)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"}],"source_content_type":"text/x-python","patch_set":46,"id":"3a71b18c_9416cba5","line":170,"range":{"start_line":170,"start_character":42,"end_line":170,"end_character":57},"updated":"2016-12-08 13:01:15.000000000","message":"not for this patch but we should move all these move_type to ENUM","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"742573b23dd66cb1fcc33b14e3c0a6cdd5847727","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        instance_type \u003d instance.flavor"},{"line_number":168,"context_line":"        image_meta \u003d objects.ImageMeta.from_instance(instance)"},{"line_number":169,"context_line":"        return self._move_claim(context, instance, instance_type,"},{"line_number":170,"context_line":"                                move_type\u003d\u0027live-migration\u0027, limits\u003dlimits,"},{"line_number":171,"context_line":"                                image_meta\u003dimage_meta, migration\u003dmigration)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"}],"source_content_type":"text/x-python","patch_set":46,"id":"1a6eadb0_c85ff10c","line":170,"range":{"start_line":170,"start_character":42,"end_line":170,"end_character":57},"in_reply_to":"3a71b18c_9416cba5","updated":"2016-12-12 21:27:26.000000000","message":"Agree and agree","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e24acae4a1595d562b8981c1007b19ca58a3c364","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        self._update(elevated, cn)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        return claim"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def live_migration_claim(self, context, instance, nodename, migration\u003dNone,"},{"line_number":173,"context_line":"                             limits\u003dNone):"},{"line_number":174,"context_line":"        \"\"\"Create a claim for a live_migration operation.\"\"\""}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_be7065a5","line":171,"updated":"2017-01-23 17:56:44.000000000","message":"@utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1a86e1a1b0084c8427f7844b1e2595ffd215157a","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        self._update(elevated, cn)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        return claim"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def live_migration_claim(self, context, instance, nodename, migration\u003dNone,"},{"line_number":173,"context_line":"                             limits\u003dNone):"},{"line_number":174,"context_line":"        \"\"\"Create a claim for a live_migration operation.\"\"\""}],"source_content_type":"text/x-python","patch_set":58,"id":"ff82abbf_78ccd769","line":171,"in_reply_to":"3a461143_5bedccb5","updated":"2017-11-23 15:50:27.000000000","message":"Done","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"e0ba0c5bd9ab5970b0bde24034977691addfd752","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        self._update(elevated, cn)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        return claim"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"    def live_migration_claim(self, context, instance, nodename, migration\u003dNone,"},{"line_number":173,"context_line":"                             limits\u003dNone):"},{"line_number":174,"context_line":"        \"\"\"Create a claim for a live_migration operation.\"\"\""}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_5bedccb5","line":171,"in_reply_to":"5a3905b3_be7065a5","updated":"2017-01-31 08:55:01.000000000","message":"++ seems valid","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"117f06988c6d30437a430936995a5285a39ab8ae","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        if migration:"},{"line_number":222,"context_line":"            self._claim_existing_migration(migration, nodename)"},{"line_number":223,"context_line":"        else:"},{"line_number":224,"context_line":"            migration \u003d self._create_migration(context, instance,"},{"line_number":225,"context_line":"                                               new_instance_type,"},{"line_number":226,"context_line":"                                               nodename, move_type)"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":59,"id":"da36d5c6_d2e50c09","line":224,"updated":"2017-02-24 16:58:39.000000000","message":"I think we need to worry about this for upgrades.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"f4f1acacdf1886fd331e59f367299c282d1152d3","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        * ...  - Remove refresh_provider_fw_rules()"},{"line_number":328,"context_line":"        * 4.9  - Add live_migration_force_complete()"},{"line_number":329,"context_line":"        * 4.10  - Add live_migration_abort()"},{"line_number":330,"context_line":"        * 4.11  - Add migration, scheduler_node and limits arguments to"},{"line_number":331,"context_line":"                 check_can_live_migrate_destination"},{"line_number":332,"context_line":"    \u0027\u0027\u0027"},{"line_number":333,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9aed3d3a_3f12a357","line":330,"updated":"2016-02-29 18:05:06.000000000","message":"s/scheduler_node/scheduled_node/","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ef4c6f592c95fe59c26ac0598630a3d2d31e8d1f","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        * ...  - Remove refresh_provider_fw_rules()"},{"line_number":328,"context_line":"        * 4.9  - Add live_migration_force_complete()"},{"line_number":329,"context_line":"        * 4.10  - Add live_migration_abort()"},{"line_number":330,"context_line":"        * 4.11  - Add migration, scheduler_node and limits arguments to"},{"line_number":331,"context_line":"                 check_can_live_migrate_destination"},{"line_number":332,"context_line":"    \u0027\u0027\u0027"},{"line_number":333,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"7af24918_671720ee","line":330,"in_reply_to":"9aed3d3a_3f12a357","updated":"2016-03-01 11:46:34.000000000","message":"Done","commit_id":"b79bdeb4a8a98136a7aee92e7fc748d6ced7b7b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2ce8f6f4f93e1f83a4593fbcfe142b59d076eee1","unresolved":false,"context_lines":[{"line_number":427,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":428,"context_line":"                                           block_migration, disk_over_commit):"},{"line_number":429,"context_line":"        version \u003d \u00274.11\u0027"},{"line_number":430,"context_line":"        cell_client \u003d self.get_cell_client(ctxt)"},{"line_number":431,"context_line":"        if not cell_client.can_send_version(version):"},{"line_number":432,"context_line":"            # NOTE(eliqiao): This is a new feature that is only available"},{"line_number":433,"context_line":"            # once all compute nodes support at least version 4.11."}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_46617ca8","side":"PARENT","line":430,"updated":"2016-08-01 16:25:32.000000000","message":"I don\u0027t know much about the cell-specific RPC client, but is there a reason you removed this in favor of referring to self.client?","commit_id":"eec3a2b9e8df358178eaea7069107fdd7df15ef3"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"b66ab49eec125b707a8416a52adbd9d50a69d0a3","unresolved":false,"context_lines":[{"line_number":455,"context_line":"            # This means the new REST API that supports this needs to handle"},{"line_number":456,"context_line":"            # this exception correctly. This can all be removed when we bump"},{"line_number":457,"context_line":"            # the major version of this RPC API."},{"line_number":458,"context_line":"            if block_migration is None or disk_over_commit is None:"},{"line_number":459,"context_line":"                raise exception.LiveMigrationWithOldNovaNotSupported()"},{"line_number":460,"context_line":"            else:"},{"line_number":461,"context_line":"                version \u003d \u00274.0\u0027"},{"line_number":462,"context_line":"        cctxt \u003d client.prepare(server\u003ddestination, version\u003dversion)"},{"line_number":463,"context_line":"        result \u003d cctxt.call(ctxt, \u0027check_can_live_migrate_destination\u0027, **kw)"},{"line_number":464,"context_line":"        if isinstance(result, migrate_data_obj.LiveMigrateData):"}],"source_content_type":"text/x-python","patch_set":36,"id":"fa6399be_61967af1","line":461,"range":{"start_line":458,"start_character":12,"end_line":461,"end_character":31},"updated":"2016-10-20 14:40:05.000000000","message":"Okay, the reason why this fails in LM+grenade job is that it was nested before and checked only if client couldn\u0027t send version 4.11. Will fix it in a subsequent patch.","commit_id":"2883f295eb7adcd87bd61cfb58881d04997f504c"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"695cb5d962675b98359910e856d9ab04fa7092d1","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":439,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":440,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":441,"context_line":"        version \u003d \u00274.14\u0027"},{"line_number":442,"context_line":"        client \u003d self.router.by_host(ctxt, destination)"},{"line_number":443,"context_line":"        kw \u003d {\u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":50,"id":"1a6eadb0_881207b3","line":440,"range":{"start_line":440,"start_character":43,"end_line":440,"end_character":70},"updated":"2016-12-21 10:57:09.000000000","message":"as we discussed on irc, I think these guards aren\u0027t needed here, as it won\u0027t affect compute nodes during upgrade procedure","commit_id":"272490317885ef27f60e8e63f6bfbe18e531c01e"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"c11e4e7da8f4cedf5e4ed91ced924dd69cbcf23d","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":439,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":440,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":441,"context_line":"        version \u003d \u00274.14\u0027"},{"line_number":442,"context_line":"        client \u003d self.router.by_host(ctxt, destination)"},{"line_number":443,"context_line":"        kw \u003d {\u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":50,"id":"1a6eadb0_5e2cf701","line":440,"range":{"start_line":440,"start_character":43,"end_line":440,"end_character":70},"in_reply_to":"1a6eadb0_881207b3","updated":"2016-12-21 11:54:26.000000000","message":"The question is whether nova-conductor is the only caller of compute/rpcapi or whether nova allows other conductor-like thing to run the environment. E.g. different conductor class value set in nova.conf under manager key in [conductor] section.","commit_id":"272490317885ef27f60e8e63f6bfbe18e531c01e"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"695cb5d962675b98359910e856d9ab04fa7092d1","unresolved":false,"context_lines":[{"line_number":450,"context_line":"            # NOTE(ndipanov): If we are talking to an older compute node that"},{"line_number":451,"context_line":"            # will not do the claim properly, we need to update the migration"},{"line_number":452,"context_line":"            # record in the conductor (that is to say - here)"},{"line_number":453,"context_line":"            if migration:"},{"line_number":454,"context_line":"                migration.dest_compute \u003d destination"},{"line_number":455,"context_line":"                migration.save()"},{"line_number":456,"context_line":"            kw.pop(\u0027migration\u0027)"},{"line_number":457,"context_line":"            kw.pop(\u0027limits\u0027)"},{"line_number":458,"context_line":"        if not client.can_send_version(version):"}],"source_content_type":"text/x-python","patch_set":50,"id":"1a6eadb0_153262f2","line":455,"range":{"start_line":453,"start_character":12,"end_line":455,"end_character":32},"updated":"2016-12-21 10:57:09.000000000","message":"I would prefer to move this save somewhere like:\nhttps://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L153\nThe reason for that is not to spread logic and try to keep rpcapi clean in this context.","commit_id":"272490317885ef27f60e8e63f6bfbe18e531c01e"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"c11e4e7da8f4cedf5e4ed91ced924dd69cbcf23d","unresolved":false,"context_lines":[{"line_number":450,"context_line":"            # NOTE(ndipanov): If we are talking to an older compute node that"},{"line_number":451,"context_line":"            # will not do the claim properly, we need to update the migration"},{"line_number":452,"context_line":"            # record in the conductor (that is to say - here)"},{"line_number":453,"context_line":"            if migration:"},{"line_number":454,"context_line":"                migration.dest_compute \u003d destination"},{"line_number":455,"context_line":"                migration.save()"},{"line_number":456,"context_line":"            kw.pop(\u0027migration\u0027)"},{"line_number":457,"context_line":"            kw.pop(\u0027limits\u0027)"},{"line_number":458,"context_line":"        if not client.can_send_version(version):"}],"source_content_type":"text/x-python","patch_set":50,"id":"1a6eadb0_98c78797","line":455,"range":{"start_line":453,"start_character":12,"end_line":455,"end_character":32},"in_reply_to":"1a6eadb0_153262f2","updated":"2016-12-21 11:54:26.000000000","message":"In find_destination we don\u0027t know yet whether we have to make such DB call. If conductor saves it in find_destination, compute will save it for the second time when doing claim.","commit_id":"272490317885ef27f60e8e63f6bfbe18e531c01e"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"e0ba0c5bd9ab5970b0bde24034977691addfd752","unresolved":false,"context_lines":[{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":441,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":442,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":443,"context_line":"        version \u003d \u00274.14\u0027"},{"line_number":444,"context_line":"        client \u003d self.router.by_host(ctxt, destination)"},{"line_number":445,"context_line":"        kw \u003d {\u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_5b44ecb5","line":442,"range":{"start_line":442,"start_character":43,"end_line":442,"end_character":70},"updated":"2017-01-31 08:55:01.000000000","message":"please don\u0027t make them default, it doesn\u0027t make any sence, because call site and this code both are executed within single service","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"217354e2e75cc3245173a167e06724908f782b21","unresolved":false,"context_lines":[{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":441,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":442,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":443,"context_line":"        version \u003d \u00274.14\u0027"},{"line_number":444,"context_line":"        client \u003d self.router.by_host(ctxt, destination)"},{"line_number":445,"context_line":"        kw \u003d {\u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_913713fe","line":442,"range":{"start_line":442,"start_character":43,"end_line":442,"end_character":70},"in_reply_to":"3a461143_5b44ecb5","updated":"2017-01-31 10:07:46.000000000","message":"I\u0027d say that this is an API and a good practice is to add parameters with defaults values when extending API even if this is an internal call. We can remove those defaults when bumping RPC version.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"101b399b8666c4a9d10a788e85fd3ce99c058453","unresolved":false,"context_lines":[{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":441,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":442,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":443,"context_line":"        version \u003d \u00274.14\u0027"},{"line_number":444,"context_line":"        client \u003d self.router.by_host(ctxt, destination)"},{"line_number":445,"context_line":"        kw \u003d {\u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":58,"id":"3a461143_c01f0359","line":442,"range":{"start_line":442,"start_character":43,"end_line":442,"end_character":70},"in_reply_to":"3a461143_913713fe","updated":"2017-01-31 11:48:21.000000000","message":"it\u0027s really useless here, and I haven\u0027t seen such pattern in nova before...","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"454c8a3765d99f93a6183497974711ee8da38227","unresolved":false,"context_lines":[{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance, destination,"},{"line_number":441,"context_line":"                                           block_migration, disk_over_commit,"},{"line_number":442,"context_line":"                                           migration\u003dNone, limits\u003dNone):"},{"line_number":443,"context_line":"        version \u003d \u00274.14\u0027"},{"line_number":444,"context_line":"        client \u003d self.router.by_host(ctxt, destination)"},{"line_number":445,"context_line":"        kw \u003d {\u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":58,"id":"1a430d35_58492330","line":442,"range":{"start_line":442,"start_character":43,"end_line":442,"end_character":70},"in_reply_to":"3a461143_c01f0359","updated":"2017-02-09 10:34:18.000000000","message":"I think it\u0027s nit, but we really can live without default params here.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"31d447f2c025163eead211f0d30a170b1a7f7d86","unresolved":false,"context_lines":[{"line_number":447,"context_line":"              \u0027disk_over_commit\u0027: disk_over_commit,"},{"line_number":448,"context_line":"              \u0027migration\u0027: migration,"},{"line_number":449,"context_line":"              \u0027limits\u0027: limits}"},{"line_number":450,"context_line":"        if not client.can_send_version(version):"},{"line_number":451,"context_line":"            version \u003d \u00274.11\u0027"},{"line_number":452,"context_line":"            # NOTE(ndipanov): If we are talking to an older compute node that"},{"line_number":453,"context_line":"            # will not do the claim properly, we need to update the migration"}],"source_content_type":"text/x-python","patch_set":59,"id":"ff82abbf_f34d6872","line":450,"updated":"2017-11-23 16:07:19.000000000","message":"We normally put this kind of thing inside compute manager, rather than it being hidden in here.\n\nFeels like this DB work violates the level of abstraction we are expecting here.\n\nWe normally look at service version, but RPC API pin seems OK too I guess.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"de3afaf83f379e9b20c7d31bda1a228b70414cb2","unresolved":false,"context_lines":[{"line_number":447,"context_line":"              \u0027disk_over_commit\u0027: disk_over_commit,"},{"line_number":448,"context_line":"              \u0027migration\u0027: migration,"},{"line_number":449,"context_line":"              \u0027limits\u0027: limits}"},{"line_number":450,"context_line":"        if not client.can_send_version(version):"},{"line_number":451,"context_line":"            version \u003d \u00274.11\u0027"},{"line_number":452,"context_line":"            # NOTE(ndipanov): If we are talking to an older compute node that"},{"line_number":453,"context_line":"            # will not do the claim properly, we need to update the migration"}],"source_content_type":"text/x-python","patch_set":59,"id":"5f93b717_0e4ae5aa","line":450,"in_reply_to":"ff82abbf_f34d6872","updated":"2018-01-31 16:06:52.000000000","message":"I suppose LiveMigrationTask._call_livem_checks_on_host() could call objects.Service.get_minimum_version() and use the result to decide whether or not to save the migration itself.\n\nBut if we did that, wouldn\u0027t we still need a check like this here to decide whether or not to include the \"migration\" and \"limits\" args in the RPC call?  And the other version checks in this file are using the RPC version.","commit_id":"9aded8d70328362cae041a271b0f5f700e744221"}],"nova/conductor/tasks/live_migrate.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"66e9e7a06066a4d8e25532b3803ff77c34610f38","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        attempted_hosts \u003d [self.source]"},{"line_number":163,"context_line":"        image \u003d utils.get_image_from_system_metadata("},{"line_number":164,"context_line":"            self.instance.system_metadata)"},{"line_number":165,"context_line":"        request_spec \u003d scheduler_utils.build_request_spec(self.context, image,"},{"line_number":166,"context_line":"                                                          [self.instance])"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        host \u003d node \u003d limits \u003d None"},{"line_number":169,"context_line":"        while host is None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_f80e013a","line":166,"range":{"start_line":165,"start_character":0,"end_line":166,"end_character":74},"updated":"2015-12-07 13:55:19.000000000","message":"here, that just builds a fresh new request_spec dict that doesn\u0027t take care of the original boot request","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9ea28c4de289320c2822d7b92065a229dcc5f8ef","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        attempted_hosts \u003d [self.source]"},{"line_number":163,"context_line":"        image \u003d utils.get_image_from_system_metadata("},{"line_number":164,"context_line":"            self.instance.system_metadata)"},{"line_number":165,"context_line":"        request_spec \u003d scheduler_utils.build_request_spec(self.context, image,"},{"line_number":166,"context_line":"                                                          [self.instance])"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        host \u003d node \u003d limits \u003d None"},{"line_number":169,"context_line":"        while host is None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_73d2325e","line":166,"range":{"start_line":165,"start_character":0,"end_line":166,"end_character":74},"in_reply_to":"7a740942_f80e013a","updated":"2015-12-07 14:02:02.000000000","message":"yeah it was there already - no idea what you mean tbh","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"66e9e7a06066a4d8e25532b3803ff77c34610f38","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            scheduler_utils.setup_instance_group(self.context, request_spec,"},{"line_number":173,"context_line":"                                                 filter_properties)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"            host_dict \u003d self.scheduler_client.select_destinations(self.context,"},{"line_number":176,"context_line":"                                request_spec, filter_properties)[0]"},{"line_number":177,"context_line":"            host, node, limits \u003d (host_dict[\u0027host\u0027],"},{"line_number":178,"context_line":"                                  host_dict[\u0027nodename\u0027],"},{"line_number":179,"context_line":"                                  host_dict[\u0027limits\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_18250dc0","line":176,"range":{"start_line":175,"start_character":12,"end_line":176,"end_character":67},"updated":"2015-12-07 13:55:19.000000000","message":"here, you\u0027re passing a brand new request dictionary to the scheduler, so the scheduler will pick a destionation without knowing what the user provided when booting.","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9ea28c4de289320c2822d7b92065a229dcc5f8ef","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            scheduler_utils.setup_instance_group(self.context, request_spec,"},{"line_number":173,"context_line":"                                                 filter_properties)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"            host_dict \u003d self.scheduler_client.select_destinations(self.context,"},{"line_number":176,"context_line":"                                request_spec, filter_properties)[0]"},{"line_number":177,"context_line":"            host, node, limits \u003d (host_dict[\u0027host\u0027],"},{"line_number":178,"context_line":"                                  host_dict[\u0027nodename\u0027],"},{"line_number":179,"context_line":"                                  host_dict[\u0027limits\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_b3bc1a07","line":176,"range":{"start_line":175,"start_character":12,"end_line":176,"end_character":67},"in_reply_to":"7a740942_18250dc0","updated":"2015-12-07 14:02:02.000000000","message":"Yes - I didn\u0027t change any of that - why would you -1 this patch for it?","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"3b995a48595f8e194b7a53c6ba555291b68ee236","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            scheduler_utils.setup_instance_group(self.context, request_spec,"},{"line_number":173,"context_line":"                                                 filter_properties)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"            host_dict \u003d self.scheduler_client.select_destinations(self.context,"},{"line_number":176,"context_line":"                                request_spec, filter_properties)[0]"},{"line_number":177,"context_line":"            host, node, limits \u003d (host_dict[\u0027host\u0027],"},{"line_number":178,"context_line":"                                  host_dict[\u0027nodename\u0027],"},{"line_number":179,"context_line":"                                  host_dict[\u0027limits\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_c51b1972","line":176,"range":{"start_line":175,"start_character":12,"end_line":176,"end_character":67},"in_reply_to":"7a740942_b3bc1a07","updated":"2015-12-07 14:10:50.000000000","message":"Nevermind my previous comment, I\u0027m reviewing the change further now because I was confused by the RPC API change which was naming a new argument \"scheduled_node\".","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2a658e94e83fa684e592bbb4681573a9c4f6b046","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                self._call_livem_checks_on_host(host, node\u003dnode, limits\u003dlimits)"},{"line_number":215,"context_line":"            except (exception.Invalid,"},{"line_number":216,"context_line":"                    exception.MigrationPreCheckError,"},{"line_number":217,"context_line":"                    exception.BuildAbortException) as e:"},{"line_number":218,"context_line":"                LOG.debug(\"Skipping host: %(host)s because: %(e)s\","},{"line_number":219,"context_line":"                    {\"host\": host, \"e\": e})"},{"line_number":220,"context_line":"                attempted_hosts.append(host)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_9ae4333c","line":217,"updated":"2016-03-07 14:01:36.000000000","message":"Question around BuildAbortException in https://review.openstack.org/#/c/244489/11/nova/compute/manager.py","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                self._call_livem_checks_on_host(host, node\u003dnode, limits\u003dlimits)"},{"line_number":215,"context_line":"            except (exception.Invalid,"},{"line_number":216,"context_line":"                    exception.MigrationPreCheckError,"},{"line_number":217,"context_line":"                    exception.BuildAbortException) as e:"},{"line_number":218,"context_line":"                LOG.debug(\"Skipping host: %(host)s because: %(e)s\","},{"line_number":219,"context_line":"                    {\"host\": host, \"e\": e})"},{"line_number":220,"context_line":"                attempted_hosts.append(host)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_97733780","line":217,"in_reply_to":"5aef4532_9ae4333c","updated":"2016-03-08 11:27:34.000000000","message":"Yep will fix it. Good catch","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        if source_version \u003e destination_version:"},{"line_number":156,"context_line":"            raise exception.DestinationHypervisorTooOld()"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def _call_livem_checks_on_host(self, destination, node\u003dNone, limits\u003dNone):"},{"line_number":159,"context_line":"        try:"},{"line_number":160,"context_line":"            self.migrate_data \u003d self.compute_rpcapi.\\"},{"line_number":161,"context_line":"                check_can_live_migrate_destination(self.context, self.instance,"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_f94c0e44","line":158,"updated":"2016-03-16 23:45:33.000000000","message":"Nikola, what am I missing here? Why do we need to bother with the node attribute? That is only relevant for bare-metal nodes, and you can\u0027t live-migrate between those. We are trying to get rid of the hypervisor_nodename attribute entirely and I\u0027m concerned with adding anything further to Nova that references that attribute of the compute node object...","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        if source_version \u003e destination_version:"},{"line_number":156,"context_line":"            raise exception.DestinationHypervisorTooOld()"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def _call_livem_checks_on_host(self, destination, node\u003dNone, limits\u003dNone):"},{"line_number":159,"context_line":"        try:"},{"line_number":160,"context_line":"            self.migrate_data \u003d self.compute_rpcapi.\\"},{"line_number":161,"context_line":"                check_can_live_migrate_destination(self.context, self.instance,"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_a8dd6e72","line":158,"in_reply_to":"1af94dfe_f94c0e44","updated":"2016-03-21 12:20:36.000000000","message":"So it is required for us to be able to fish out the correct resource tracker. I could (I guess) do the old trick with getting the first (and only) node out of the list we get back.\n\nI think it\u0027s pretty gross but maybe makes sense for the live migration case?","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                host_dict \u003d self.scheduler_client.select_destinations("},{"line_number":200,"context_line":"                            self.context, request_spec)[0]"},{"line_number":201,"context_line":"                host, node, limits \u003d (host_dict[\u0027host\u0027],"},{"line_number":202,"context_line":"                                      host_dict[\u0027nodename\u0027],"},{"line_number":203,"context_line":"                                      host_dict[\u0027limits\u0027])"},{"line_number":204,"context_line":"            except messaging.RemoteError as ex:"},{"line_number":205,"context_line":"                # TODO(ShaoHe Feng) There maybe multi-scheduler, and the"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_d947921b","line":202,"updated":"2016-03-16 23:45:33.000000000","message":"Again, nodename is only used in bare-metal nodes, so I\u0027m failing to see how this is necessary for live-migration tasks?","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                host_dict \u003d self.scheduler_client.select_destinations("},{"line_number":200,"context_line":"                            self.context, request_spec)[0]"},{"line_number":201,"context_line":"                host, node, limits \u003d (host_dict[\u0027host\u0027],"},{"line_number":202,"context_line":"                                      host_dict[\u0027nodename\u0027],"},{"line_number":203,"context_line":"                                      host_dict[\u0027limits\u0027])"},{"line_number":204,"context_line":"            except messaging.RemoteError as ex:"},{"line_number":205,"context_line":"                # TODO(ShaoHe Feng) There maybe multi-scheduler, and the"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_883cea30","line":202,"in_reply_to":"1af94dfe_d947921b","updated":"2016-03-21 12:20:36.000000000","message":"See above - it\u0027s not entirely unreasonable to remove this...","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":17532,"name":"Tina Kevin","email":"song.ruixia@zte.com.cn","username":"SongRuixia"},"change_message_id":"d3e20a9e20a0a60ca0b9e5405062b23f1e7d515b","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                 servicegroup_api, scheduler_client, request_spec\u003dNone):"},{"line_number":42,"context_line":"        super(LiveMigrationTask, self).__init__(context, instance)"},{"line_number":43,"context_line":"        self.destination \u003d destination"},{"line_number":44,"context_line":"        self.dest_node \u003d None"},{"line_number":45,"context_line":"        self.sched_limits \u003d None"},{"line_number":46,"context_line":"        self.block_migration \u003d block_migration"},{"line_number":47,"context_line":"        self.disk_over_commit \u003d disk_over_commit"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba0121b8_cc2b5dea","line":44,"range":{"start_line":44,"start_character":22,"end_line":44,"end_character":23},"updated":"2016-03-30 03:21:17.000000000","message":"self.dest_node is defined and get value, but it is not used.\nCan be removed it?","commit_id":"9448fd748bbe0d84c18961a1d441cbf514c8d3a5"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"3a4f5b43a45b58f379eb64f99171b96bec2776d8","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        if not self.destination:"},{"line_number":61,"context_line":"            (self.destination,"},{"line_number":62,"context_line":"             self.scheduler_limits) \u003d self._find_destination()"},{"line_number":63,"context_line":"        else:"},{"line_number":64,"context_line":"            self._check_requested_destination()"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"ba0121b8_e15415ff","line":62,"range":{"start_line":62,"start_character":18,"end_line":62,"end_character":34},"updated":"2016-03-31 11:08:45.000000000","message":"there should be \u0027self.sched_limits\u0027. But this won\u0027t affect anything. It is ok with later patch to fix.","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"86dd6b2823c2d3ec406528423b99285810c0b9f0","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        if not self.destination:"},{"line_number":61,"context_line":"            (self.destination,"},{"line_number":62,"context_line":"             self.scheduler_limits) \u003d self._find_destination()"},{"line_number":63,"context_line":"        else:"},{"line_number":64,"context_line":"            self._check_requested_destination()"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_efc7c993","line":62,"range":{"start_line":62,"start_character":18,"end_line":62,"end_character":34},"in_reply_to":"9a061dce_969dd45b","updated":"2016-04-07 09:01:20.000000000","message":"Yep this looks like a bug and it was never properly tested (we always just pass None in the unit tests).","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7e8a64f62aeab11cba8475bba5a230737ac4ec64","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        if not self.destination:"},{"line_number":61,"context_line":"            (self.destination,"},{"line_number":62,"context_line":"             self.scheduler_limits) \u003d self._find_destination()"},{"line_number":63,"context_line":"        else:"},{"line_number":64,"context_line":"            self._check_requested_destination()"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a061dce_969dd45b","line":62,"range":{"start_line":62,"start_character":18,"end_line":62,"end_character":34},"in_reply_to":"ba0121b8_e15415ff","updated":"2016-04-06 03:04:32.000000000","message":"No, I think this is a real problem Alex. self.sched_limits is used below on line 107-108 in the call to _call_livem_checks_on_host(), and as this stands, the limits param will always be None.","commit_id":"f01b9bd3a53a4e2ef235148515d600c521181bef"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1608bc211c6ad164d1ccc337fc7c097ef539982b","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self._check_host_is_up(self.source)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        if not self.destination:"},{"line_number":53,"context_line":"            (self.destination,"},{"line_number":54,"context_line":"             self.sched_limits) \u003d self._find_destination()"},{"line_number":55,"context_line":"        else:"},{"line_number":56,"context_line":"            self._check_requested_destination()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"3a71b18c_2a74bdcd","line":54,"range":{"start_line":53,"start_character":0,"end_line":54,"end_character":58},"updated":"2016-12-08 09:45:21.000000000","message":"nit for future cleanup - this wrapping seems unnecessary","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e24acae4a1595d562b8981c1007b19ca58a3c364","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                 servicegroup_api, scheduler_client, request_spec\u003dNone):"},{"line_number":34,"context_line":"        super(LiveMigrationTask, self).__init__(context, instance)"},{"line_number":35,"context_line":"        self.destination \u003d destination"},{"line_number":36,"context_line":"        self.sched_limits \u003d None"},{"line_number":37,"context_line":"        self.block_migration \u003d block_migration"},{"line_number":38,"context_line":"        self.disk_over_commit \u003d disk_over_commit"},{"line_number":39,"context_line":"        self.migration \u003d migration"}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_f8ae852c","line":36,"updated":"2017-01-23 17:56:44.000000000","message":"This isn\u0027t passed in via initialisation, and is only used within the single lexical scope of _execute(), which is also intuitively correct. IOW, it should be a local variable in _execute. Putting it here obfuscates its scope. It would also make it more obvious in code (rather than just the commit message) that we\u0027re not populating if we were passed a destination. We should also add a comment about that.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"e24acae4a1595d562b8981c1007b19ca58a3c364","unresolved":false,"context_lines":[{"line_number":50,"context_line":"        self._check_host_is_up(self.source)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        if not self.destination:"},{"line_number":53,"context_line":"            self.destination, self.sched_limits \u003d self._find_destination()"},{"line_number":54,"context_line":"        else:"},{"line_number":55,"context_line":"            self._check_requested_destination()"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":58,"id":"5a3905b3_db3d43c5","line":53,"updated":"2017-01-23 17:56:44.000000000","message":"See above. sched_limits should be a local variable here.","commit_id":"1dbd3fe170af7899c0b48b1f1be526eb7ab5df09"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d70830e1859d434e8b70e5f742e8c56dac5af05b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        if not self.destination:"},{"line_number":52,"context_line":"            self.destination \u003d self._find_destination()"},{"line_number":53,"context_line":"            self.migration.dest_compute \u003d self.destination"},{"line_number":54,"context_line":"            self.migration.save()"},{"line_number":55,"context_line":"        else:"},{"line_number":56,"context_line":"            self._check_requested_destination()"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":59,"id":"ff82abbf_b3f3600a","side":"PARENT","line":54,"updated":"2017-11-23 16:14:04.000000000","message":"hmm, upgrade worries here, but I guess its covered by that RPC API...","commit_id":"7b5c3ac11d791ae61d0975a556da18f3333fb164"}],"nova/objects/service.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"b31f46c8d60528d75686e0e4af41a308829a98e1","unresolved":false,"context_lines":[{"line_number":93,"context_line":"    # is deleted before RPC to nova-compute."},{"line_number":94,"context_line":"    {\u0027compute_rpc\u0027: \u00274.13\u0027},"},{"line_number":95,"context_line":"    # Version 16: Changes to check_can_live_migrate_destination signature"},{"line_number":96,"context_line":"    {\u0027compute_rpc\u0027: \u00274.15\u0027},"},{"line_number":97,"context_line":")"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"dada55a8_7e6250d9","line":96,"updated":"2016-07-21 10:22:25.000000000","message":"should be 14","commit_id":"61cd0e17a94cf92b87a0e096d6bf1fef37ab288d"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":5894,"context_line":""},{"line_number":5895,"context_line":"        # start test"},{"line_number":5896,"context_line":"        self.mox.ReplayAll()"},{"line_number":5897,"context_line":"        patch_mutate \u003d mock.patch.object(instance, \u0027mutated_migration_context\u0027)"},{"line_number":5898,"context_line":"        patch_drop \u003d mock.patch.object(instance, \u0027drop_migration_context\u0027)"},{"line_number":5899,"context_line":"        with patch_mutate as mutate_mock, patch_drop as drop_mock:"},{"line_number":5900,"context_line":"            ret \u003d self.compute.rollback_live_migration_at_destination(c,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_8b006cac","line":5897,"updated":"2016-03-07 12:25:07.000000000","message":"I am not sure about mixing mock and mox like this. I can\u0027t remember why/if this was causing problems somewhere.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":5894,"context_line":""},{"line_number":5895,"context_line":"        # start test"},{"line_number":5896,"context_line":"        self.mox.ReplayAll()"},{"line_number":5897,"context_line":"        patch_mutate \u003d mock.patch.object(instance, \u0027mutated_migration_context\u0027)"},{"line_number":5898,"context_line":"        patch_drop \u003d mock.patch.object(instance, \u0027drop_migration_context\u0027)"},{"line_number":5899,"context_line":"        with patch_mutate as mutate_mock, patch_drop as drop_mock:"},{"line_number":5900,"context_line":"            ret \u003d self.compute.rollback_live_migration_at_destination(c,"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_77e30b06","line":5897,"in_reply_to":"5aef4532_8b006cac","updated":"2016-03-08 11:27:34.000000000","message":"Ok fair - I will change this to use mox","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2ce8f6f4f93e1f83a4593fbcfe142b59d076eee1","unresolved":false,"context_lines":[{"line_number":5915,"context_line":"        mock_detect.return_value \u003d \u0027fake-migrate-data\u0027"},{"line_number":5916,"context_line":"        instance \u003d self._create_fake_instance_obj()"},{"line_number":5917,"context_line":"        patch_mutate \u003d mock.patch.object(instance, \u0027mutated_migration_context\u0027)"},{"line_number":5918,"context_line":"        patch_drop \u003d mock.patch.object(instance, \u0027drop_migration_context\u0027)"},{"line_number":5919,"context_line":"        with patch_mutate as mutate_mock, patch_drop as drop_mock:"},{"line_number":5920,"context_line":"            self.assertRaises("},{"line_number":5921,"context_line":"                test.TestingException,"}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_266448b7","line":5918,"updated":"2016-08-01 16:25:32.000000000","message":"Would have preferred it if you just added two @mock.patch decorators to this test method instead of using a with plus indentation.","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"9a0f00d656b5e77567de1196772c732fa000742b","unresolved":false,"context_lines":[{"line_number":1925,"context_line":"    def test_check_can_live_migrate_destination_success(self):"},{"line_number":1926,"context_line":"        self._test_check_can_live_migrate_destination()"},{"line_number":1927,"context_line":""},{"line_number":1928,"context_line":"    def test_check_can_live_migrate_destination_success_mig(self):"},{"line_number":1929,"context_line":"        self._test_check_can_live_migrate_destination(migration\u003dTrue)"},{"line_number":1930,"context_line":""},{"line_number":1931,"context_line":"    def test_check_can_live_migrate_destination_success_mig_node(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_ebd2d033","line":1928,"updated":"2016-03-07 12:25:07.000000000","message":"Hmm, I thought we banned extra mox tests.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"8fa8194c804a0652a464c9eb91814df660296b00","unresolved":false,"context_lines":[{"line_number":1925,"context_line":"    def test_check_can_live_migrate_destination_success(self):"},{"line_number":1926,"context_line":"        self._test_check_can_live_migrate_destination()"},{"line_number":1927,"context_line":""},{"line_number":1928,"context_line":"    def test_check_can_live_migrate_destination_success_mig(self):"},{"line_number":1929,"context_line":"        self._test_check_can_live_migrate_destination(migration\u003dTrue)"},{"line_number":1930,"context_line":""},{"line_number":1931,"context_line":"    def test_check_can_live_migrate_destination_success_mig_node(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5aef4532_771cab13","line":1928,"in_reply_to":"5aef4532_ebd2d033","updated":"2016-03-08 11:27:34.000000000","message":"Well this really just refactors an existing one. Not sure what the win is in adding a separate test that uses mock for mocking when most of the infra for this test is here. If anything it will cause fragmentation and confusion as to what is being tested where.","commit_id":"230c1b0007eccdb287511eb0c27e1ff3ca7b7edb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2ce8f6f4f93e1f83a4593fbcfe142b59d076eee1","unresolved":false,"context_lines":[{"line_number":1990,"context_line":"        dest_info \u003d \u0027dest_info\u0027"},{"line_number":1991,"context_line":"        dest_check_data \u003d dict(foo\u003d\u0027bar\u0027)"},{"line_number":1992,"context_line":"        mig_data \u003d dict(cow\u003d\u0027moo\u0027)"},{"line_number":1993,"context_line":"        expected_result \u003d dict(mig_data)"},{"line_number":1994,"context_line":""},{"line_number":1995,"context_line":"        migration_obj \u003d None"},{"line_number":1996,"context_line":"        if migration:"}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_8657f4c8","line":1993,"updated":"2016-08-01 16:25:32.000000000","message":"I think you meant to do:\n\n expected_result \u003d copy.deepcopy(mig_data)\n\n?\n\ndoing dict(mig_data) will be a shallow copy.","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2ce8f6f4f93e1f83a4593fbcfe142b59d076eee1","unresolved":false,"context_lines":[{"line_number":2011,"context_line":"        compute_node.local_gb_used \u003d 0"},{"line_number":2012,"context_line":"        compute_node.vcpus \u003d 2"},{"line_number":2013,"context_line":"        compute_node.vcpus_used \u003d 0"},{"line_number":2014,"context_line":"        compute_node.get \u003d mock.Mock()"},{"line_number":2015,"context_line":"        compute_node.get.return_value \u003d None"},{"line_number":2016,"context_line":"        compute_node.hypervisor_hostname \u003d \u0027fake-node\u0027"},{"line_number":2017,"context_line":"        fake_rt.compute_node \u003d compute_node"}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_665ac0f3","line":2014,"updated":"2016-08-01 16:25:32.000000000","message":"Say what? Why is this here?","commit_id":"b8d9c02e2c4ed7b585887719f87dff02b27ce8f3"}],"nova/tests/unit/conductor/tasks/test_live_migrate.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0d4e2b154ce16a12b8740bcd853a509f6ec7c07d","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                migrate_data\u003dNone).AndReturn(\"bob\")"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.mox.ReplayAll()"},{"line_number":100,"context_line":"        self.assertEqual(\"bob\", self.task.execute())"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def test_check_instance_is_active_passes_when_paused(self):"},{"line_number":103,"context_line":"        self.task.instance[\u0027power_state\u0027] \u003d power_state.PAUSED"}],"source_content_type":"text/x-python","patch_set":13,"id":"1af94dfe_395796cf","line":100,"updated":"2016-03-16 23:45:33.000000000","message":"Any reason to remove these tests here? Do we not still want to test that the migration object\u0027s status was updated?","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"802312aa50752d2bf8dff4a1547226f495423fd1","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                migrate_data\u003dNone).AndReturn(\"bob\")"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        self.mox.ReplayAll()"},{"line_number":100,"context_line":"        self.assertEqual(\"bob\", self.task.execute())"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def test_check_instance_is_active_passes_when_paused(self):"},{"line_number":103,"context_line":"        self.task.instance[\u0027power_state\u0027] \u003d power_state.PAUSED"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa0719c6_032b9b83","line":100,"in_reply_to":"1af94dfe_395796cf","updated":"2016-03-21 12:20:36.000000000","message":"Right - so we don\u0027t save anymore in the execute() since the destination (the only thing we were updating) is now set on the compute host only after a successful claim.","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":17532,"name":"Tina Kevin","email":"song.ruixia@zte.com.cn","username":"SongRuixia"},"change_message_id":"aeefdb7e30787fcb3879a6a9ee7b18fdef1a06a4","unresolved":false,"context_lines":[{"line_number":5671,"context_line":"                # TODO(ndipanov): Update this method to update the new XML with"},{"line_number":5672,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5673,"context_line":"                # MigrationContext on the Instance"},{"line_number":5674,"context_line":"                new_xml_str \u003d self._update_xml(old_xml_str,"},{"line_number":5675,"context_line":"                                               volume,"},{"line_number":5676,"context_line":"                                               listen_addrs,"},{"line_number":5677,"context_line":"                                               serial_listen_addr)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_8f802e8d","line":5674,"updated":"2015-12-10 02:02:26.000000000","message":"_update_xml method does not update instance numa topology info, the numa topology info in new_xml_str is also the source host, not dest host","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"e7ed6b83caf5ebed2e4e73e307fe89cdb40249f2","unresolved":false,"context_lines":[{"line_number":5671,"context_line":"                # TODO(ndipanov): Update this method to update the new XML with"},{"line_number":5672,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5673,"context_line":"                # MigrationContext on the Instance"},{"line_number":5674,"context_line":"                new_xml_str \u003d self._update_xml(old_xml_str,"},{"line_number":5675,"context_line":"                                               volume,"},{"line_number":5676,"context_line":"                                               listen_addrs,"},{"line_number":5677,"context_line":"                                               serial_listen_addr)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_008ddd7a","line":5674,"in_reply_to":"7a740942_8f802e8d","updated":"2015-12-10 09:36:03.000000000","message":"Yep - that\u0027s basically what the above comment says. It will still not work properly.","commit_id":"41749ec3abd712993db78a8bf5018cddf10c769d"},{"author":{"_account_id":17288,"name":"liuxiuli","email":"liu.lixiu@zte.com.cn","username":"liu-lixiu"},"change_message_id":"4308772d26ca0d426dcc166557da7a26abf57635","unresolved":false,"context_lines":[{"line_number":6001,"context_line":"                # TODO(ndipanov): Update this method to update the new XML with"},{"line_number":6002,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":6003,"context_line":"                # MigrationContext on the Instance"},{"line_number":6004,"context_line":"                new_xml_str \u003d self._update_xml(old_xml_str,"},{"line_number":6005,"context_line":"                                               migrate_data.bdms,"},{"line_number":6006,"context_line":"                                               listen_addrs,"},{"line_number":6007,"context_line":"                                               serial_listen_addr)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7aa08908_00affd2b","line":6004,"updated":"2016-06-16 07:32:48.000000000","message":"You need modify _update_xml function to update numa_topology, but do not.","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"4d343907bdecd0c8d8fa77fc82b7a3bef511a2e8","unresolved":false,"context_lines":[{"line_number":6001,"context_line":"                # TODO(ndipanov): Update this method to update the new XML with"},{"line_number":6002,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":6003,"context_line":"                # MigrationContext on the Instance"},{"line_number":6004,"context_line":"                new_xml_str \u003d self._update_xml(old_xml_str,"},{"line_number":6005,"context_line":"                                               migrate_data.bdms,"},{"line_number":6006,"context_line":"                                               listen_addrs,"},{"line_number":6007,"context_line":"                                               serial_listen_addr)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7aa08908_f6c8bb58","line":6004,"in_reply_to":"7aa08908_00affd2b","updated":"2016-06-16 08:02:02.000000000","message":"Yes it\u0027s something we do in following patches","commit_id":"c5a8c973bf9aa78e57ee3674b68cdc93d7416aea"},{"author":{"_account_id":17532,"name":"Tina Kevin","email":"song.ruixia@zte.com.cn","username":"SongRuixia"},"change_message_id":"d3e20a9e20a0a60ca0b9e5405062b23f1e7d515b","unresolved":false,"context_lines":[{"line_number":6001,"context_line":"                # TODO(ndipanov): Update this method to update the new XML with"},{"line_number":6002,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":6003,"context_line":"                # MigrationContext on the Instance"},{"line_number":6004,"context_line":"                new_xml_str \u003d self._update_xml(old_xml_str,"},{"line_number":6005,"context_line":"                                               migrate_data.bdms,"},{"line_number":6006,"context_line":"                                               listen_addrs,"},{"line_number":6007,"context_line":"                                               serial_listen_addr)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba0121b8_c74df673","line":6004,"range":{"start_line":6004,"start_character":0,"end_line":6004,"end_character":59},"updated":"2016-03-30 03:21:17.000000000","message":"You should add a parameter new_numa_topolopy in the function of _update_xml, new_numa_topolopy is instance numa_topolopy in dest node, if new_numa_topolopy is not empty, you need to update to the new_numa_topology in new_xml_str.\nOtherwise, the numa_topology of the VM is source node numa_topolopy and VM is dest node.","commit_id":"9448fd748bbe0d84c18961a1d441cbf514c8d3a5"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"143dcc2466ffdccc19334ac53ff56e7f7427e0f5","unresolved":false,"context_lines":[{"line_number":5955,"context_line":"            # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5956,"context_line":"            # MigrationContext on the Instance"},{"line_number":5957,"context_line":"            new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":5958,"context_line":"                # TODO(sahid): It\u0027s not a really well idea to pass"},{"line_number":5959,"context_line":"                # the method _get_volume_config and we should to find"},{"line_number":5960,"context_line":"                # a way to avoid this in future."},{"line_number":5961,"context_line":"                guest, migrate_data, self._get_volume_config)"}],"source_content_type":"text/x-python","patch_set":46,"id":"3a71b18c_e6022072","line":5958,"range":{"start_line":5958,"start_character":49,"end_line":5958,"end_character":53},"updated":"2016-12-07 16:56:43.000000000","message":"nit: \"good\" is the correct term here","commit_id":"745faf16c97d30a4b9d4f01200575aedf4bf83fe"}]}