)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":27336,"name":"XiaojueGuan","email":"xiaojueguan@gmail.com","username":"XiaojueGuan"},"change_message_id":"1e7fe6fed00150fd4e7b18d2dc09ec2c80962ec8","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When live migrating instance to invalid host, live migration fails"},{"line_number":10,"context_line":"with host not found and sets instance task state to migrating"},{"line_number":11,"context_line":"This changes resets the instance state to \u0027Active\u0027 in the above"},{"line_number":12,"context_line":"scenario."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3fa0c359_c874126b","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":12},"updated":"2018-02-21 14:17:51.000000000","message":"i can\u0027t grasp what \"this changes\" means, is this a programmer mistake or ?","commit_id":"7c6f5e76f4375ef990460cb19df04b4b4014c471"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"91edb3aec5b7a2d106f7fbb0e5feeb599d503320","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"},{"line_number":16,"context_line":"Partial-bug:1643623"},{"line_number":17,"context_line":"Depends-On: I7a9de211ecfaa7f2816fbf8bcd73ebbdd990643c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df7087c5_48be74ff","line":17,"updated":"2018-03-15 14:04:30.000000000","message":"You don\u0027t need a depends-on when the change you\u0027re depending on is in the same repo. This makes it such that the patch on master can\u0027t merge until the dependent patch backports are also all merged, which is not what you want.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"cafd628337296bb5abb14fa29fd445708f935a36","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"},{"line_number":16,"context_line":"Partial-bug:1643623"},{"line_number":17,"context_line":"Depends-On: I7a9de211ecfaa7f2816fbf8bcd73ebbdd990643c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df7087c5_88a56ca4","line":17,"in_reply_to":"df7087c5_48be74ff","updated":"2018-03-15 14:12:41.000000000","message":"Done","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de746dc744fb80a135b5d03c6b3a1aa421b7d941","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix host validity check for live-migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When live migrating instance to invalid host, live migration fails"},{"line_number":10,"context_line":"with host not found and sets instance task state to migrating."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This change handles host validity in API layer before changing instance"},{"line_number":13,"context_line":"task_state to \u0027MIGRATING\u0027 and raise proper exception on invalid host."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"3f79a3b5_976246d8","line":10,"range":{"start_line":10,"start_character":49,"end_line":10,"end_character":51},"updated":"2018-08-03 17:33:20.000000000","message":"as","commit_id":"ec0e530f515b733c8a556283c6b1fea26d93ddf5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de746dc744fb80a135b5d03c6b3a1aa421b7d941","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix host validity check for live-migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When live migrating instance to invalid host, live migration fails"},{"line_number":10,"context_line":"with host not found and sets instance task state to migrating."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This change handles host validity in API layer before changing instance"},{"line_number":13,"context_line":"task_state to \u0027MIGRATING\u0027 and raise proper exception on invalid host."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"3f79a3b5_f765bae2","line":10,"range":{"start_line":10,"start_character":24,"end_line":10,"end_character":28},"updated":"2018-08-03 17:33:20.000000000","message":"leaves","commit_id":"ec0e530f515b733c8a556283c6b1fea26d93ddf5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de746dc744fb80a135b5d03c6b3a1aa421b7d941","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When live migrating instance to invalid host, live migration fails"},{"line_number":10,"context_line":"with host not found and sets instance task state to migrating."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"This change handles host validity in API layer before changing instance"},{"line_number":13,"context_line":"task_state to \u0027MIGRATING\u0027 and raise proper exception on invalid host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"3f79a3b5_579a6ec5","line":12,"updated":"2018-08-03 17:33:20.000000000","message":"Alternatively, we could start doing something like the @reverts_task_state decorator used in ComputeManager but in the API. Then it would be less whack-a-mole for these types of issues in the API.","commit_id":"ec0e530f515b733c8a556283c6b1fea26d93ddf5"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8459a611902846f1abc1259f3e0797c920243262","unresolved":false,"context_lines":[{"line_number":13,"context_line":"task_state to \u0027MIGRATING\u0027 and raise proper exception on invalid host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"},{"line_number":16,"context_line":"Closes-Bug: #1643623"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"3f79a3b5_f40edfe3","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"updated":"2018-08-07 16:54:09.000000000","message":"This bug is already closed. Perhaps we should use https://bugs.launchpad.net/nova/+bug/1785031 at this point instead?","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9f541445f7e96db75857a4ded1fd71c5d8e13ec6","unresolved":false,"context_lines":[{"line_number":13,"context_line":"task_state to \u0027MIGRATING\u0027 and raise proper exception on invalid host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"},{"line_number":16,"context_line":"Closes-Bug: #1643623"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"3f79a3b5_779b98c8","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_575a1c2a","updated":"2018-08-08 15:02:38.000000000","message":"Done","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c12ca8ef154077d25a8d37e37a4c7b5f328a99d4","unresolved":false,"context_lines":[{"line_number":13,"context_line":"task_state to \u0027MIGRATING\u0027 and raise proper exception on invalid host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I7c5e80298b9adf1bd53cc6c464a3744b5397b7e8"},{"line_number":16,"context_line":"Closes-Bug: #1643623"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"3f79a3b5_575a1c2a","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_f40edfe3","updated":"2018-08-08 15:00:55.000000000","message":"I noticed the same and I\u0027m OK with either duplicating bug 1785031 against 1643623 or just mark this as closing 1785031, *or* do:\n\nRelated-Bug: #1643623\nCloses-Bug: #1785031\n\nThat ^ is maybe my preferred option.","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"}],"nova/compute/api.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f4b35ac4d7a8b754f85be88c10654e9fa13ea380","unresolved":false,"context_lines":[{"line_number":3696,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":3697,"context_line":"                                                                host_name)"},{"line_number":3698,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":3699,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":3700,"context_line":"                    instance.task_state \u003d None"},{"line_number":3701,"context_line":"                    instance.save()"},{"line_number":3702,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_f65f2373","line":3699,"updated":"2016-11-23 13:34:06.000000000","message":"Seems reasonable, and in the API layer we catch that ComputeHostNotFound","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c10e63d6ece5f75c14abd35d6e471d40fbecf3af","unresolved":false,"context_lines":[{"line_number":3696,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":3697,"context_line":"                                                                host_name)"},{"line_number":3698,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":3699,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":3700,"context_line":"                    instance.task_state \u003d None"},{"line_number":3701,"context_line":"                    instance.save()"},{"line_number":3702,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_55d53241","line":3699,"in_reply_to":"5a74a57a_f65f2373","updated":"2017-02-10 14:13:49.000000000","message":"Now, I think this is only a partial fix, as the user will still get a 500 error if they are using the latest microversion that does this async:\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/migrate_server.py#L103\n\nIts tempting to call that another bug though.","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":18339,"name":"Sivasathurappan Radhakrishnan","email":"siva.radhakrishnan@intel.com","username":"sivakrishnan"},"change_message_id":"3be3ba9c1171731a2bee3c42369ed99d62ab38ca","unresolved":false,"context_lines":[{"line_number":3696,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":3697,"context_line":"                                                                host_name)"},{"line_number":3698,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":3699,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":3700,"context_line":"                    instance.task_state \u003d None"},{"line_number":3701,"context_line":"                    instance.save()"},{"line_number":3702,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a25e98e_10b5a9cc","line":3699,"in_reply_to":"fa31d9ce_55d53241","updated":"2017-03-13 21:35:07.000000000","message":"Didn\u0027t notice that response was different based on async. Will fix that in a follow up patch","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c10e63d6ece5f75c14abd35d6e471d40fbecf3af","unresolved":false,"context_lines":[{"line_number":3698,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":3699,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":3700,"context_line":"                    instance.task_state \u003d None"},{"line_number":3701,"context_line":"                    instance.save()"},{"line_number":3702,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"},{"line_number":3703,"context_line":"            host_name \u003d None"},{"line_number":3704,"context_line":"            # FIXME(sbauza): Since only Ironic driver uses more than one"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_f55546c6","line":3701,"updated":"2017-02-10 14:13:49.000000000","message":"Its tempting to add (expected_task_state\u003d[task_states.MIGRATING]) but thats over kill here, so this is correct I believe.","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c10e63d6ece5f75c14abd35d6e471d40fbecf3af","unresolved":false,"context_lines":[{"line_number":3729,"context_line":"                # instance fault to database"},{"line_number":3730,"context_line":"                compute_utils.add_instance_fault_from_exc(context,"},{"line_number":3731,"context_line":"                                                          instance,"},{"line_number":3732,"context_line":"                                                          messaging_timeout)"},{"line_number":3733,"context_line":""},{"line_number":3734,"context_line":"    @check_instance_lock"},{"line_number":3735,"context_line":"    @check_instance_cell"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_156ada80","line":3732,"updated":"2017-02-10 14:13:49.000000000","message":"As a follow up patch, it looks like here we would also leave the server in the migrating state, which would be bad in a very similar way.","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"34456ffba9aa5ceb8737a922b427ab1736da4261","unresolved":false,"context_lines":[{"line_number":3729,"context_line":"                # instance fault to database"},{"line_number":3730,"context_line":"                compute_utils.add_instance_fault_from_exc(context,"},{"line_number":3731,"context_line":"                                                          instance,"},{"line_number":3732,"context_line":"                                                          messaging_timeout)"},{"line_number":3733,"context_line":""},{"line_number":3734,"context_line":"    @check_instance_lock"},{"line_number":3735,"context_line":"    @check_instance_cell"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_d574c2d0","line":3732,"in_reply_to":"fa31d9ce_156ada80","updated":"2017-02-10 14:21:23.000000000","message":"Dam, I remember this debate now, and the comment mentions it, we can\u0027t change the state here as we don\u0027t know if live-migration will complete successfully or not at this point.","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"eeef270d51731b62856e1501874462662bf1a292","unresolved":false,"context_lines":[{"line_number":4156,"context_line":"                                                                host_name)"},{"line_number":4157,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4158,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4159,"context_line":"                    instance.task_state \u003d None"},{"line_number":4160,"context_line":"                    instance.save()"},{"line_number":4161,"context_line":""},{"line_number":4162,"context_line":"            # Unset the host to make sure we call the scheduler"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_aa4c7b9e","line":4159,"range":{"start_line":4159,"start_character":20,"end_line":4159,"end_character":46},"updated":"2018-03-09 10:26:16.000000000","message":"nit - A brief comment highlighting that this will allow the instance to return to ACTIVE here would be useful for future readers IMHO.","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"3aa698db06779f9d65294eed5d4d6b7f8191cda8","unresolved":false,"context_lines":[{"line_number":4156,"context_line":"                                                                host_name)"},{"line_number":4157,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4158,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4159,"context_line":"                    instance.task_state \u003d None"},{"line_number":4160,"context_line":"                    instance.save()"},{"line_number":4161,"context_line":""},{"line_number":4162,"context_line":"            # Unset the host to make sure we call the scheduler"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_b0d2ae5b","line":4159,"range":{"start_line":4159,"start_character":20,"end_line":4159,"end_character":46},"in_reply_to":"df7087c5_aa4c7b9e","updated":"2018-03-09 11:17:55.000000000","message":"Done","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"eeef270d51731b62856e1501874462662bf1a292","unresolved":false,"context_lines":[{"line_number":4157,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4158,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4159,"context_line":"                    instance.task_state \u003d None"},{"line_number":4160,"context_line":"                    instance.save()"},{"line_number":4161,"context_line":""},{"line_number":4162,"context_line":"            # Unset the host to make sure we call the scheduler"},{"line_number":4163,"context_line":"            # from the conductor LiveMigrationTask. Yes this is tightly-coupled"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_6a6d0303","line":4160,"range":{"start_line":4160,"start_character":20,"end_line":4160,"end_character":35},"updated":"2018-03-09 10:26:16.000000000","message":"Should this be instance.save(expected_task_state\u003d[None]) as on L4136?","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b768384003be0bcbed932a6d8f4af32de4385900","unresolved":false,"context_lines":[{"line_number":4157,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4158,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4159,"context_line":"                    instance.task_state \u003d None"},{"line_number":4160,"context_line":"                    instance.save()"},{"line_number":4161,"context_line":""},{"line_number":4162,"context_line":"            # Unset the host to make sure we call the scheduler"},{"line_number":4163,"context_line":"            # from the conductor LiveMigrationTask. Yes this is tightly-coupled"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_950c3c3c","line":4160,"range":{"start_line":4160,"start_character":20,"end_line":4160,"end_character":35},"in_reply_to":"df7087c5_6a6d0303","updated":"2018-03-09 10:30:17.000000000","message":"instance.save(expected_task_state\u003dtask_states.MIGRATING) even.","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"3aa698db06779f9d65294eed5d4d6b7f8191cda8","unresolved":false,"context_lines":[{"line_number":4157,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4158,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4159,"context_line":"                    instance.task_state \u003d None"},{"line_number":4160,"context_line":"                    instance.save()"},{"line_number":4161,"context_line":""},{"line_number":4162,"context_line":"            # Unset the host to make sure we call the scheduler"},{"line_number":4163,"context_line":"            # from the conductor LiveMigrationTask. Yes this is tightly-coupled"}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_d0d5e244","line":4160,"range":{"start_line":4160,"start_character":20,"end_line":4160,"end_character":35},"in_reply_to":"df7087c5_950c3c3c","updated":"2018-03-09 11:17:55.000000000","message":"Done","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a9328ae36e1ec104f57666f19c1d4838700cb53f","unresolved":false,"context_lines":[{"line_number":3388,"context_line":"            if host_name is not None:"},{"line_number":3389,"context_line":"                # If there is no request spec we cannot honor the request"},{"line_number":3390,"context_line":"                # and we need to fail."},{"line_number":3391,"context_line":"                raise exception.CannotMigrateWithTargetHost()"},{"line_number":3392,"context_line":"            request_spec \u003d None"},{"line_number":3393,"context_line":""},{"line_number":3394,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_88fb2cbf","line":3391,"updated":"2018-03-15 14:07:38.000000000","message":"We\u0027d have the same issue for resize here. Separate bug/fix though.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"bec22d06f161e5829d45926e9ee71e0ee13b0770","unresolved":false,"context_lines":[{"line_number":3388,"context_line":"            if host_name is not None:"},{"line_number":3389,"context_line":"                # If there is no request spec we cannot honor the request"},{"line_number":3390,"context_line":"                # and we need to fail."},{"line_number":3391,"context_line":"                raise exception.CannotMigrateWithTargetHost()"},{"line_number":3392,"context_line":"            request_spec \u003d None"},{"line_number":3393,"context_line":""},{"line_number":3394,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_d293b8a2","line":3391,"in_reply_to":"df7087c5_88fb2cbf","updated":"2018-03-20 06:02:48.000000000","message":"I have checked cold-migrate api and found that there is no issue if we try to cold-migrate instance to non-existent host. We get 404 error on console, as expected.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"cafd628337296bb5abb14fa29fd445708f935a36","unresolved":false,"context_lines":[{"line_number":3388,"context_line":"            if host_name is not None:"},{"line_number":3389,"context_line":"                # If there is no request spec we cannot honor the request"},{"line_number":3390,"context_line":"                # and we need to fail."},{"line_number":3391,"context_line":"                raise exception.CannotMigrateWithTargetHost()"},{"line_number":3392,"context_line":"            request_spec \u003d None"},{"line_number":3393,"context_line":""},{"line_number":3394,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_a8a07091","line":3391,"in_reply_to":"df7087c5_88fb2cbf","updated":"2018-03-15 14:12:41.000000000","message":"I will report a separate bug for that and fix that ASAP.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8a60b1abbfcc8016851ffcc505c987cb2a80b121","unresolved":false,"context_lines":[{"line_number":3388,"context_line":"            if host_name is not None:"},{"line_number":3389,"context_line":"                # If there is no request spec we cannot honor the request"},{"line_number":3390,"context_line":"                # and we need to fail."},{"line_number":3391,"context_line":"                raise exception.CannotMigrateWithTargetHost()"},{"line_number":3392,"context_line":"            request_spec \u003d None"},{"line_number":3393,"context_line":""},{"line_number":3394,"context_line":"        # TODO(melwitt): We\u0027re not rechecking for strict quota here to guard"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_942004fa","line":3391,"in_reply_to":"df7087c5_d293b8a2","updated":"2018-08-03 16:54:30.000000000","message":"Yeah because of L3304.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"bc7f9adcdca434b3d3c6202985aa62f1b739f7c4","unresolved":false,"context_lines":[{"line_number":4218,"context_line":"        # NOTE(sbauza): Force is a boolean by the new related API version"},{"line_number":4219,"context_line":"        if force is False and host_name:"},{"line_number":4220,"context_line":"            try:"},{"line_number":4221,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4222,"context_line":"                                                                host_name)"},{"line_number":4223,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4224,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4225,"context_line":"                    # NOTE(ratailor): This will allow the instance to return"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_48abd473","line":4222,"range":{"start_line":4221,"start_character":16,"end_line":4222,"end_character":74},"updated":"2018-03-15 14:12:42.000000000","message":"Or we get this before change the instance task state.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d068174ee7b34b997198d965db0ab0525a740e03","unresolved":false,"context_lines":[{"line_number":4218,"context_line":"        # NOTE(sbauza): Force is a boolean by the new related API version"},{"line_number":4219,"context_line":"        if force is False and host_name:"},{"line_number":4220,"context_line":"            try:"},{"line_number":4221,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4222,"context_line":"                                                                host_name)"},{"line_number":4223,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4224,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4225,"context_line":"                    # NOTE(ratailor): This will allow the instance to return"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_a8ce3057","line":4222,"range":{"start_line":4221,"start_character":16,"end_line":4222,"end_character":74},"in_reply_to":"df7087c5_48abd473","updated":"2018-03-15 14:14:34.000000000","message":"That\u0027s how the resize (cold migrate) method works when specifying a host, so I wouldn\u0027t be opposed to that. Note that we have the same problem for evacuate with a host specified.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8a21126d89659d256aab420140d94d5a97022c60","unresolved":false,"context_lines":[{"line_number":4218,"context_line":"        # NOTE(sbauza): Force is a boolean by the new related API version"},{"line_number":4219,"context_line":"        if force is False and host_name:"},{"line_number":4220,"context_line":"            try:"},{"line_number":4221,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4222,"context_line":"                                                                host_name)"},{"line_number":4223,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4224,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4225,"context_line":"                    # NOTE(ratailor): This will allow the instance to return"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_88580c88","line":4222,"range":{"start_line":4221,"start_character":16,"end_line":4222,"end_character":74},"in_reply_to":"df7087c5_a8ce3057","updated":"2018-03-15 14:20:40.000000000","message":"yea, it should be a API layer check, better to failed early, and simple the  code","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"34967482ea33915eb772e8d9cd671673441fc01e","unresolved":false,"context_lines":[{"line_number":4223,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4224,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4225,"context_line":"                    # NOTE(ratailor): This will allow the instance to return"},{"line_number":4226,"context_line":"                    # to vm_state ACTIVE."},{"line_number":4227,"context_line":"                    instance.task_state \u003d None"},{"line_number":4228,"context_line":"                    instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":4229,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_c8a364ff","line":4226,"range":{"start_line":4226,"start_character":34,"end_line":4226,"end_character":40},"updated":"2018-03-15 14:02:45.000000000","message":"or PAUSED? You can also live migrate paused instances (see above).","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"cafd628337296bb5abb14fa29fd445708f935a36","unresolved":false,"context_lines":[{"line_number":4223,"context_line":"            except exception.ComputeHostNotFound:"},{"line_number":4224,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":4225,"context_line":"                    # NOTE(ratailor): This will allow the instance to return"},{"line_number":4226,"context_line":"                    # to vm_state ACTIVE."},{"line_number":4227,"context_line":"                    instance.task_state \u003d None"},{"line_number":4228,"context_line":"                    instance.save(expected_task_state\u003dtask_states.MIGRATING)"},{"line_number":4229,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_c8828400","line":4226,"range":{"start_line":4226,"start_character":34,"end_line":4226,"end_character":40},"in_reply_to":"df7087c5_c8a364ff","updated":"2018-03-15 14:12:41.000000000","message":"Done","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a9328ae36e1ec104f57666f19c1d4838700cb53f","unresolved":false,"context_lines":[{"line_number":4379,"context_line":""},{"line_number":4380,"context_line":"        # NOTE(sbauza): Force is a boolean by the new related API version"},{"line_number":4381,"context_line":"        if force is False and host:"},{"line_number":4382,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host)"},{"line_number":4383,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"},{"line_number":4384,"context_line":"            host \u003d None"},{"line_number":4385,"context_line":"            # FIXME(sbauza): Since only Ironic driver uses more than one"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_481af416","line":4382,"updated":"2018-03-15 14:07:38.000000000","message":"We have the same issue here as in the live migrate code.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"bec22d06f161e5829d45926e9ee71e0ee13b0770","unresolved":false,"context_lines":[{"line_number":4379,"context_line":""},{"line_number":4380,"context_line":"        # NOTE(sbauza): Force is a boolean by the new related API version"},{"line_number":4381,"context_line":"        if force is False and host:"},{"line_number":4382,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host)"},{"line_number":4383,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"},{"line_number":4384,"context_line":"            host \u003d None"},{"line_number":4385,"context_line":"            # FIXME(sbauza): Since only Ironic driver uses more than one"}],"source_content_type":"text/x-python","patch_set":9,"id":"df7087c5_928d407d","line":4382,"in_reply_to":"df7087c5_481af416","updated":"2018-03-20 06:02:48.000000000","message":"I have tried evacuate api and found that there is no issue, if we try to evacuate instances to non-existent host. We get 404 error on console, as expected.","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8a60b1abbfcc8016851ffcc505c987cb2a80b121","unresolved":false,"context_lines":[{"line_number":4379,"context_line":""},{"line_number":4380,"context_line":"        # NOTE(sbauza): Force is a boolean by the new related API version"},{"line_number":4381,"context_line":"        if force is False and host:"},{"line_number":4382,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host)"},{"line_number":4383,"context_line":"            # NOTE(sbauza): Unset the host to make sure we call the scheduler"},{"line_number":4384,"context_line":"            host \u003d None"},{"line_number":4385,"context_line":"            # FIXME(sbauza): Since only Ironic driver uses more than one"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_f4237804","line":4382,"in_reply_to":"df7087c5_928d407d","updated":"2018-08-03 16:54:30.000000000","message":"Yeah I guess that happens in the controller code (it\u0027s really confusing that spread this validation out everywhere):\n\nhttps://github.com/openstack/nova/blob/69def63ed4c662033eebc418351d7af111b1925a/nova/api/openstack/compute/evacuate.py#L108","commit_id":"14bfb78da9528ed9177eb5c5454dd9597dd8defe"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"70e7991e25c59058a3780b782775d163ca6c141d","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"bf659307_a1aaa46e","line":4201,"range":{"start_line":4201,"start_character":11,"end_line":4201,"end_character":29},"updated":"2018-04-04 17:24:30.000000000","message":"I guess this \"force is False and\" can be unnecessary because the target host existence is not checked if force migration on current code.","commit_id":"17663872709dd6e0bc54b2b39d75ef88fef9dbbd"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"dcf7dcaef30026853a26c5400ec9dcc0d07e6497","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"bf659307_8d86a7db","line":4201,"range":{"start_line":4201,"start_character":11,"end_line":4201,"end_character":29},"in_reply_to":"bf659307_a1aaa46e","updated":"2018-04-05 09:57:02.000000000","message":"I think the same was done earlier, I have just moved the get_all_by_host call before changing the task_state to \u0027migrating\u0027.","commit_id":"17663872709dd6e0bc54b2b39d75ef88fef9dbbd"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"ac6e0d060e474663231105ab418694d38b1d8336","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"},{"line_number":4205,"context_line":"            except exception.HostNotFound:"},{"line_number":4206,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost_name)"},{"line_number":4207,"context_line":""},{"line_number":4208,"context_line":"        instance.task_state \u003d task_states.MIGRATING"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_897e6419","line":4205,"range":{"start_line":4201,"start_character":9,"end_line":4205,"end_character":42},"updated":"2018-07-25 07:46:00.000000000","message":"does this mean if we force migrate to one host, it will skip this test? why is that so?","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1944a739a751a225d48afef226115be5e3d75072","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"},{"line_number":4205,"context_line":"            except exception.HostNotFound:"},{"line_number":4206,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost_name)"},{"line_number":4207,"context_line":""},{"line_number":4208,"context_line":"        instance.task_state \u003d task_states.MIGRATING"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_34601033","line":4205,"range":{"start_line":4201,"start_character":9,"end_line":4205,"end_character":42},"in_reply_to":"3f79a3b5_94efa4e8","updated":"2018-08-03 17:00:27.000000000","message":"ComputeHostNotFound extends HostNotFound, so we\u0027re just catching ComputeHostNotFound and re-raising it. We don\u0027t need the try/except.","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fcc9f433e862e0368466328ae1e277771c549523","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"},{"line_number":4205,"context_line":"            except exception.HostNotFound:"},{"line_number":4206,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost_name)"},{"line_number":4207,"context_line":""},{"line_number":4208,"context_line":"        instance.task_state \u003d task_states.MIGRATING"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_54890c24","line":4205,"range":{"start_line":4201,"start_character":9,"end_line":4205,"end_character":42},"in_reply_to":"5f7c97a3_5acb2c4a","updated":"2018-08-03 16:44:41.000000000","message":"We could probably remove the force condition from this, because if force is True, we really need to know that the host exists, but if force is False, we\u0027ll use the requested host as a selected destination to the scheduler and we\u0027ll just end up failing with NoValidHost in the scheduler. So either way it\u0027s a failure, so we don\u0027t really need the force condition here.","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"456dfc2e9d927e73378c66c8bb4f2b9bfab128ca","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"},{"line_number":4205,"context_line":"            except exception.HostNotFound:"},{"line_number":4206,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost_name)"},{"line_number":4207,"context_line":""},{"line_number":4208,"context_line":"        instance.task_state \u003d task_states.MIGRATING"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_5acb2c4a","line":4205,"range":{"start_line":4201,"start_character":9,"end_line":4205,"end_character":42},"in_reply_to":"5f7c97a3_897e6419","updated":"2018-07-25 09:52:47.000000000","message":"I think the same behavior is same as earlier, the only things is that I moved the check before setting the instance task state to \u0027migrating\u0027.\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/api.py#L4351-L4352","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fcc9f433e862e0368466328ae1e277771c549523","unresolved":false,"context_lines":[{"line_number":4198,"context_line":"        LOG.debug(\"Going to try to live migrate instance to %s\","},{"line_number":4199,"context_line":"                  host_name or \"another host\", instance\u003dinstance)"},{"line_number":4200,"context_line":""},{"line_number":4201,"context_line":"        if force is False and host_name:"},{"line_number":4202,"context_line":"            try:"},{"line_number":4203,"context_line":"                nodes \u003d objects.ComputeNodeList.get_all_by_host(context,"},{"line_number":4204,"context_line":"                        host_name)"},{"line_number":4205,"context_line":"            except exception.HostNotFound:"},{"line_number":4206,"context_line":"                raise exception.ComputeHostNotFound(host\u003dhost_name)"},{"line_number":4207,"context_line":""},{"line_number":4208,"context_line":"        instance.task_state \u003d task_states.MIGRATING"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_94efa4e8","line":4205,"range":{"start_line":4201,"start_character":9,"end_line":4205,"end_character":42},"in_reply_to":"5f7c97a3_897e6419","updated":"2018-08-03 16:44:41.000000000","message":"We\u0027d skip the test here in the API, but we\u0027ll do checks in the live migration task in conductor:\n\nhttps://github.com/openstack/nova/blob/69def63ed4c662033eebc418351d7af111b1925a/nova/conductor/tasks/live_migrate.py#L161\n\nAnd somewhere in there we\u0027d fail with a ComputeHostNotFound. Depending on if we\u0027re doing that asynchronously or not (based on microversion 2.34) that error could come back up to the API.","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8459a611902846f1abc1259f3e0797c920243262","unresolved":false,"context_lines":[{"line_number":4345,"context_line":"        if host_name:"},{"line_number":4346,"context_line":"            # Validate the specified host before changing the instance task"},{"line_number":4347,"context_line":"            # state."},{"line_number":4348,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host_name)"},{"line_number":4349,"context_line":""},{"line_number":4350,"context_line":"        instance.task_state \u003d task_states.MIGRATING"},{"line_number":4351,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_f4dc9f47","line":4348,"range":{"start_line":4348,"start_character":12,"end_line":4348,"end_character":17},"updated":"2018-08-07 16:54:09.000000000","message":"See [1] - is there merit to trapping ComputeHostNotFound and resetting the task state?\n\n[1] https://review.openstack.org/#/c/588512/","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cdfbc4da44da8825ad828f9d359a7e186fb135a1","unresolved":false,"context_lines":[{"line_number":4345,"context_line":"        if host_name:"},{"line_number":4346,"context_line":"            # Validate the specified host before changing the instance task"},{"line_number":4347,"context_line":"            # state."},{"line_number":4348,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host_name)"},{"line_number":4349,"context_line":""},{"line_number":4350,"context_line":"        instance.task_state \u003d task_states.MIGRATING"},{"line_number":4351,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_542bfa2e","line":4348,"range":{"start_line":4348,"start_character":12,"end_line":4348,"end_character":17},"in_reply_to":"3f79a3b5_05c5605b","updated":"2018-08-08 15:01:21.000000000","message":"Sorry, yeah, I see now.","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c12ca8ef154077d25a8d37e37a4c7b5f328a99d4","unresolved":false,"context_lines":[{"line_number":4345,"context_line":"        if host_name:"},{"line_number":4346,"context_line":"            # Validate the specified host before changing the instance task"},{"line_number":4347,"context_line":"            # state."},{"line_number":4348,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host_name)"},{"line_number":4349,"context_line":""},{"line_number":4350,"context_line":"        instance.task_state \u003d task_states.MIGRATING"},{"line_number":4351,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_1737c4e5","line":4348,"range":{"start_line":4348,"start_character":12,"end_line":4348,"end_character":17},"in_reply_to":"3f79a3b5_f4dc9f47","updated":"2018-08-08 15:00:55.000000000","message":"As mentioned in earlier review comments, it seems the API should start using the same @reverts_task_state decorator that we use in the compute manager, because we have quite a few cases like this in the API where we change the task_state, then do some stuff (which could fail), and then cast to compute. But that\u0027s a bigger change which we likely don\u0027t want to backport.","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"c9d438b09933101f9860c2846904bbfdb3bfefed","unresolved":false,"context_lines":[{"line_number":4345,"context_line":"        if host_name:"},{"line_number":4346,"context_line":"            # Validate the specified host before changing the instance task"},{"line_number":4347,"context_line":"            # state."},{"line_number":4348,"context_line":"            nodes \u003d objects.ComputeNodeList.get_all_by_host(context, host_name)"},{"line_number":4349,"context_line":""},{"line_number":4350,"context_line":"        instance.task_state \u003d task_states.MIGRATING"},{"line_number":4351,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_05c5605b","line":4348,"range":{"start_line":4348,"start_character":12,"end_line":4348,"end_character":17},"in_reply_to":"3f79a3b5_f4dc9f47","updated":"2018-08-08 02:04:24.000000000","message":"The task state doesn\u0027t update yet. it is updated at next line.","commit_id":"fab4beb4d86f8e183fd30abe32027ef5599e3881"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"241bc47c6be4549c72ef1825fae9c44399cb86f3","unresolved":false,"context_lines":[{"line_number":2215,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2216,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027)"},{"line_number":2217,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027,"},{"line_number":2218,"context_line":"                       side_effect\u003dexception.ComputeHostNotFound"},{"line_number":2219,"context_line":"                       (\u0027fake_dest_host\u0027))"},{"line_number":2220,"context_line":"    def test_live_migrate_computehost_notfound(self, _nodelist, _save, _action,"},{"line_number":2221,"context_line":"                                               get_spec):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_95b5eace","line":2218,"updated":"2017-02-10 14:18:29.000000000","message":"Nit: I actually prefer defining the side_effects inside the method, as it make the setup, test, assert phase of the unit test clearer.","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":18339,"name":"Sivasathurappan Radhakrishnan","email":"siva.radhakrishnan@intel.com","username":"sivakrishnan"},"change_message_id":"3be3ba9c1171731a2bee3c42369ed99d62ab38ca","unresolved":false,"context_lines":[{"line_number":2215,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2216,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027)"},{"line_number":2217,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027,"},{"line_number":2218,"context_line":"                       side_effect\u003dexception.ComputeHostNotFound"},{"line_number":2219,"context_line":"                       (\u0027fake_dest_host\u0027))"},{"line_number":2220,"context_line":"    def test_live_migrate_computehost_notfound(self, _nodelist, _save, _action,"},{"line_number":2221,"context_line":"                                               get_spec):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a25e98e_909c396b","line":2218,"in_reply_to":"fa31d9ce_95b5eace","updated":"2017-03-13 21:35:07.000000000","message":"Done","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"241bc47c6be4549c72ef1825fae9c44399cb86f3","unresolved":false,"context_lines":[{"line_number":2225,"context_line":"        else:"},{"line_number":2226,"context_line":"            api \u003d conductor.api.ComputeTaskAPI"},{"line_number":2227,"context_line":""},{"line_number":2228,"context_line":"        with mock.patch.object(api, \u0027live_migrate_instance\u0027):"},{"line_number":2229,"context_line":"            self.assertRaises(exception.ComputeHostNotFound,"},{"line_number":2230,"context_line":"                              self.compute_api.live_migrate,"},{"line_number":2231,"context_line":"                              self.context, instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa31d9ce_15d9fa21","line":2228,"updated":"2017-02-10 14:18:29.000000000","message":"Why do you mock this method, it is never called if the exception is raised by get_all_by_host?","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":18339,"name":"Sivasathurappan Radhakrishnan","email":"siva.radhakrishnan@intel.com","username":"sivakrishnan"},"change_message_id":"3be3ba9c1171731a2bee3c42369ed99d62ab38ca","unresolved":false,"context_lines":[{"line_number":2225,"context_line":"        else:"},{"line_number":2226,"context_line":"            api \u003d conductor.api.ComputeTaskAPI"},{"line_number":2227,"context_line":""},{"line_number":2228,"context_line":"        with mock.patch.object(api, \u0027live_migrate_instance\u0027):"},{"line_number":2229,"context_line":"            self.assertRaises(exception.ComputeHostNotFound,"},{"line_number":2230,"context_line":"                              self.compute_api.live_migrate,"},{"line_number":2231,"context_line":"                              self.context, instance,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a25e98e_5092c158","line":2228,"in_reply_to":"fa31d9ce_15d9fa21","updated":"2017-03-13 21:35:07.000000000","message":"Done","commit_id":"891da448d749c30d756f02d0a27d7e3b8a949a0a"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"87bd63b122a74e530b5cc7f54f5830f4aaec1740","unresolved":false,"context_lines":[{"line_number":2360,"context_line":""},{"line_number":2361,"context_line":"    @mock.patch.object(objects.RequestSpec, \u0027get_by_instance_uuid\u0027)"},{"line_number":2362,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2363,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027)"},{"line_number":2364,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2365,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist, mock_save,"},{"line_number":2366,"context_line":"                                               mock_action,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a1ced50_3c65a01d","line":2363,"range":{"start_line":2363,"start_character":42,"end_line":2363,"end_character":46},"updated":"2017-03-21 10:25:37.000000000","message":"we can assert this is called","commit_id":"9ac2e6681e5782fcd6eae5c811d9cc24a197cf22"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1d0faf6626cc008cffdd25c0e0c3f04c75ad7cd0","unresolved":false,"context_lines":[{"line_number":2360,"context_line":""},{"line_number":2361,"context_line":"    @mock.patch.object(objects.RequestSpec, \u0027get_by_instance_uuid\u0027)"},{"line_number":2362,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2363,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027)"},{"line_number":2364,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2365,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist, mock_save,"},{"line_number":2366,"context_line":"                                               mock_action,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ffe62b97_ecfbd444","line":2363,"range":{"start_line":2363,"start_character":42,"end_line":2363,"end_character":46},"in_reply_to":"1a1ced50_3c65a01d","updated":"2017-03-30 12:14:16.000000000","message":"Agreed. Let\u0027s do that","commit_id":"9ac2e6681e5782fcd6eae5c811d9cc24a197cf22"},{"author":{"_account_id":18339,"name":"Sivasathurappan Radhakrishnan","email":"siva.radhakrishnan@intel.com","username":"sivakrishnan"},"change_message_id":"d193ac03f4a430d65aeb91e71866e16e10dceabe","unresolved":false,"context_lines":[{"line_number":2360,"context_line":""},{"line_number":2361,"context_line":"    @mock.patch.object(objects.RequestSpec, \u0027get_by_instance_uuid\u0027)"},{"line_number":2362,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2363,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027)"},{"line_number":2364,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2365,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist, mock_save,"},{"line_number":2366,"context_line":"                                               mock_action,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ffe62b97_6aed598a","line":2363,"range":{"start_line":2363,"start_character":42,"end_line":2363,"end_character":46},"in_reply_to":"ffe62b97_ecfbd444","updated":"2017-03-30 17:42:47.000000000","message":"We can try asserting if its not being called or if it\u0027s causing any exception. I thought it\u0027s better not assert it for positive case scenarios and I have seen test cases following this pattern and few asserting save well. I prefer to do it latter way and just asserting a method to check if its being called doesn\u0027t make much value addition for this scenario. Please correct me I am wrong.","commit_id":"9ac2e6681e5782fcd6eae5c811d9cc24a197cf22"},{"author":{"_account_id":22056,"name":"yushangbin","email":"shangbin.yu@easystack.cn","username":"yushangbin"},"change_message_id":"ae42a51d1d601451fa091f8bd5fa6aa4530755f9","unresolved":false,"context_lines":[{"line_number":2394,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2395,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2396,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist,"},{"line_number":2397,"context_line":"                                               mock_action,"},{"line_number":2398,"context_line":"                                               mock_get_spec):"},{"line_number":2399,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":2400,"context_line":"        with mock.patch.object(instance, \u0027save\u0027) as mock_save:"},{"line_number":2401,"context_line":"            mock_nodelist.side_effect \u003d exception.ComputeHostNotFound("}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_89cf657e","line":2398,"range":{"start_line":2397,"start_character":47,"end_line":2398,"end_character":60},"updated":"2018-03-08 07:50:53.000000000","message":"why not use the same as test_live_migrate_messaging_timeout?","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":20733,"name":"Rajesh Tailor","email":"ratailor@redhat.com","username":"rajesht"},"change_message_id":"bde2daff82996585155d4adbf53e93fa143bb566","unresolved":false,"context_lines":[{"line_number":2394,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2395,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2396,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist,"},{"line_number":2397,"context_line":"                                               mock_action,"},{"line_number":2398,"context_line":"                                               mock_get_spec):"},{"line_number":2399,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":2400,"context_line":"        with mock.patch.object(instance, \u0027save\u0027) as mock_save:"},{"line_number":2401,"context_line":"            mock_nodelist.side_effect \u003d exception.ComputeHostNotFound("}],"source_content_type":"text/x-python","patch_set":8,"id":"df7087c5_a47a9021","line":2398,"range":{"start_line":2397,"start_character":47,"end_line":2398,"end_character":60},"in_reply_to":"df7087c5_89cf657e","updated":"2018-03-08 08:42:41.000000000","message":"There was a comment on PS#4, pl see below:\nhttps://review.openstack.org/#/c/401009/4/nova/tests/unit/compute/test_compute_api.py","commit_id":"d7d5c502b3b7a871e6ddce9dfaecd337ac344b77"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"70e7991e25c59058a3780b782775d163ca6c141d","unresolved":false,"context_lines":[{"line_number":2518,"context_line":"                self.context, instance,"},{"line_number":2519,"context_line":"                host_name\u003d\u0027fake_host\u0027,"},{"line_number":2520,"context_line":"                block_migration\u003dTrue, disk_over_commit\u003dTrue,"},{"line_number":2521,"context_line":"                force\u003dFalse)"},{"line_number":2522,"context_line":""},{"line_number":2523,"context_line":"    @mock.patch.object(objects.RequestSpec, \u0027get_by_instance_uuid\u0027)"},{"line_number":2524,"context_line":"    @mock.patch.object(objects.Instance, \u0027save\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"bf659307_e1ce5c5a","line":2521,"range":{"start_line":2521,"start_character":16,"end_line":2521,"end_character":27},"updated":"2018-04-04 17:24:30.000000000","message":"If the previous comment is valid, it is necessary to add force\u003dTrue case also.","commit_id":"17663872709dd6e0bc54b2b39d75ef88fef9dbbd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fcc9f433e862e0368466328ae1e277771c549523","unresolved":false,"context_lines":[{"line_number":2504,"context_line":"                instance,"},{"line_number":2505,"context_line":"                mock.ANY)"},{"line_number":2506,"context_line":""},{"line_number":2507,"context_line":"    @mock.patch.object(objects.RequestSpec, \u0027get_by_instance_uuid\u0027)"},{"line_number":2508,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2509,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2510,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist,"},{"line_number":2511,"context_line":"                                               mock_action,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_9476c43e","line":2508,"range":{"start_line":2507,"start_character":4,"end_line":2508,"end_character":62},"updated":"2018-08-03 16:44:41.000000000","message":"Don\u0027t need either of these mocks.","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1944a739a751a225d48afef226115be5e3d75072","unresolved":false,"context_lines":[{"line_number":2507,"context_line":"    @mock.patch.object(objects.RequestSpec, \u0027get_by_instance_uuid\u0027)"},{"line_number":2508,"context_line":"    @mock.patch.object(objects.InstanceAction, \u0027action_start\u0027)"},{"line_number":2509,"context_line":"    @mock.patch.object(objects.ComputeNodeList, \u0027get_all_by_host\u0027)"},{"line_number":2510,"context_line":"    def test_live_migrate_computehost_notfound(self, mock_nodelist,"},{"line_number":2511,"context_line":"                                               mock_action,"},{"line_number":2512,"context_line":"                                               mock_get_spec):"},{"line_number":2513,"context_line":"        instance \u003d self._create_instance_obj()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_34d570ba","line":2510,"updated":"2018-08-03 17:00:27.000000000","message":"This test doesn\u0027t assert that we didn\u0027t change the instance.task_state, which is the point of the bug.","commit_id":"543ac3ee6478d20973c708a7af591f0d160d067e"}]}