)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8d4f062e663b565674b4564c35c9c8800458f7e8","unresolved":false,"context_lines":[{"line_number":52,"context_line":"with direct physical ports, it will have both kinds of events."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"For same host resize reverts we do not update the binding host as the"},{"line_number":55,"context_line":"host does not change, as such for same host resize we do not recive"},{"line_number":56,"context_line":"bind time events. For same host revert we therefore do not wait for"},{"line_number":57,"context_line":"bind time events in the compute manager."},{"line_number":58,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_40b9d412","line":55,"range":{"start_line":55,"start_character":61,"end_line":55,"end_character":67},"updated":"2019-06-24 19:16:35.000000000","message":"receive","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0d3baab5869640a4a5eb277457f534dc39b77f1","unresolved":false,"context_lines":[{"line_number":63,"context_line":"and/or in libvirt, when plugging the VIFs, for plug-time events."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Change-Id: I51673e58fc8d5f051df911630f6d7a928d123a5b"},{"line_number":66,"context_line":"Closes-bug: 1832028"},{"line_number":67,"context_line":"Co-Authored-By: Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":68,"context_line":"Co-Authored-By: Artom Lifshitz \u003califshit@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_60ddf86e","line":66,"updated":"2019-06-24 19:15:11.000000000","message":"Add Closes-Bug: #1833902 in addition. That\u0027s the same-host resize bug.","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"12cc0f895ef4c72f74b3d3c11e0934fcfa72775a","unresolved":false,"context_lines":[{"line_number":63,"context_line":"and/or in libvirt, when plugging the VIFs, for plug-time events."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Change-Id: I51673e58fc8d5f051df911630f6d7a928d123a5b"},{"line_number":66,"context_line":"Closes-bug: 1832028"},{"line_number":67,"context_line":"Co-Authored-By: Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":68,"context_line":"Co-Authored-By: Artom Lifshitz \u003califshit@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_00c2fc59","line":66,"in_reply_to":"9fb8cfa7_60ddf86e","updated":"2019-06-24 19:21:39.000000000","message":"ack will do. its still in the queue so ill do that now.","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":65,"context_line":"Closes-bug: #1832028"},{"line_number":66,"context_line":"Closes-Bug: #1833902"},{"line_number":67,"context_line":"Co-Authored-By: Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":68,"context_line":"Co-Authored-By: Artom Lifshitz \u003califshit@redhat.com\u003e"},{"line_number":69,"context_line":"Change-Id: I51673e58fc8d5f051df911630f6d7a928d123a5b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7faddb67_f18ef32c","line":68,"updated":"2019-07-05 21:16:39.000000000","message":"nit: don\u0027t really need this when you are the author.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":65,"context_line":"Closes-bug: #1832028"},{"line_number":66,"context_line":"Closes-Bug: #1833902"},{"line_number":67,"context_line":"Co-Authored-By: Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":68,"context_line":"Co-Authored-By: Artom Lifshitz \u003califshit@redhat.com\u003e"},{"line_number":69,"context_line":"Change-Id: I51673e58fc8d5f051df911630f6d7a928d123a5b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7faddb67_7222cc02","line":68,"in_reply_to":"7faddb67_f18ef32c","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"12eba0f88644dfec61c92f11b56958d375c25729","unresolved":false,"context_lines":[{"line_number":4182,"context_line":"        events \u003d []"},{"line_number":4183,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4184,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4185,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4186,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4187,"context_line":"            if events:"},{"line_number":4188,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_ddf04b6a","line":4185,"range":{"start_line":4185,"start_character":60,"end_line":4185,"end_character":77},"updated":"2019-06-24 18:51:35.000000000","message":"Looks like this is the main difference since the revert, right?\n\nThe tricky thing about this is it\u0027s making an assumption for *all* vif types that could be returned from get_bind_time_events, right?\n\nWould it be better to pass the same_host (or migration) value to get_bind_time_events so that method can determine what is appropriate per vif type?","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"abaf221fb0007adf9f902b4e91ceadd070159d90","unresolved":false,"context_lines":[{"line_number":4182,"context_line":"        events \u003d []"},{"line_number":4183,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4184,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4185,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4186,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4187,"context_line":"            if events:"},{"line_number":4188,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_fd518f37","line":4185,"range":{"start_line":4185,"start_character":60,"end_line":4185,"end_character":77},"in_reply_to":"9fb8cfa7_ddf04b6a","updated":"2019-06-24 18:59:02.000000000","message":"yes this is the main change\n\ni could pass it yes, i do not know if we would have to handle this differently per vif. i believe the behaviour shoudl be the same for all vifs for same host revert. \n\nSince the host_id is not change when we update the port in this case it will not trigger the vif to  be rebound and therefore we should not get bind time events.","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8d4f062e663b565674b4564c35c9c8800458f7e8","unresolved":false,"context_lines":[{"line_number":4182,"context_line":"        events \u003d []"},{"line_number":4183,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4184,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4185,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4186,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4187,"context_line":"            if events:"},{"line_number":4188,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_c0cdc4b7","line":4185,"range":{"start_line":4185,"start_character":60,"end_line":4185,"end_character":77},"in_reply_to":"9fb8cfa7_fd518f37","updated":"2019-06-24 19:16:35.000000000","message":"OK I guess we could pass something to get_bind_time_events later if needed.","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"010d9f27d49c98bddd74f6903458a6877e6e8fc0","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"                                       network_info,"},{"line_number":4283,"context_line":"                                       block_device_info, power_on, migration)"},{"line_number":4284,"context_line":""},{"line_number":4285,"context_line":"            instance.drop_migration_context()"},{"line_number":4286,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"},{"line_number":4287,"context_line":"            instance.save(expected_task_state\u003dtask_states.RESIZE_REVERTING)"},{"line_number":4288,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_363838d1","line":4285,"range":{"start_line":4285,"start_character":12,"end_line":4285,"end_character":45},"updated":"2019-07-01 19:29:14.000000000","message":"The instance should still have the migration context on it so you can access the Migration via that rather than pass it through if you want. Some might argue that the explicit kwarg on the driver method is cleaner (I\u0027d agree with that) but if you want to avoid the driver interface change for backports, do the migration_context thing with a TODO to remove it in a master-only change with the driver interface change (which won\u0027t get backported).","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8fd3d3a2e9a6f911710d5e00cf37a14fdbff7044","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"                                       network_info,"},{"line_number":4283,"context_line":"                                       block_device_info, power_on, migration)"},{"line_number":4284,"context_line":""},{"line_number":4285,"context_line":"            instance.drop_migration_context()"},{"line_number":4286,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"},{"line_number":4287,"context_line":"            instance.save(expected_task_state\u003dtask_states.RESIZE_REVERTING)"},{"line_number":4288,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_d978e8c1","line":4285,"range":{"start_line":4285,"start_character":12,"end_line":4285,"end_character":45},"in_reply_to":"9fb8cfa7_363838d1","updated":"2019-07-02 08:17:23.000000000","message":"Actually I think I\u0027d rather just do the TypeError try/except instead of inserting the extra DB query.","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"26fa1cf7c23e41d2b0c0dfe1ccb6dd6a1e5586c9","unresolved":false,"context_lines":[{"line_number":4278,"context_line":"                    context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":4279,"context_line":""},{"line_number":4280,"context_line":"            power_on \u003d old_vm_state !\u003d vm_states.STOPPED"},{"line_number":4281,"context_line":"            try:"},{"line_number":4282,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4283,"context_line":"                    context, instance, network_info, block_device_info,"},{"line_number":4284,"context_line":"                    power_on, migration)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_e5ddff6f","line":4281,"range":{"start_line":4281,"start_character":12,"end_line":4281,"end_character":16},"updated":"2019-07-02 11:48:17.000000000","message":"ok so we can have a follow up patch to drop this on master or we can leave a todo to do it in U to give people a release to update there out of tree drivers.\nlater\nah i see you have the remvoeal in the follow up patch\nhttps://review.opendev.org/#/c/668631/1\n+1","commit_id":"6e9c0ad8c0fd4f62a1110b9d2b839123a1444d90"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"18829e685eaa52c4c40e4654c59cf3ce2b1db4e1","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4283,"context_line":"                    context, instance, network_info, block_device_info,"},{"line_number":4284,"context_line":"                    power_on, migration)"},{"line_number":4285,"context_line":"            except TypeError:"},{"line_number":4286,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4287,"context_line":"                            \"\u0027migration\u0027 parameter to the \""},{"line_number":4288,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_eede25d9","line":4285,"updated":"2019-07-03 15:26:52.000000000","message":"I don\u0027t like this for the backports if we have a way to avoid it, which we discussed before (look up the migration using the instance.migration_context.migration_id and then drop that on master with the interface change which doesn\u0027t get backported.","commit_id":"6e9c0ad8c0fd4f62a1110b9d2b839123a1444d90"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"81dcf60510d31592afb2629c71224ab37e1b1a8f","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4283,"context_line":"                    context, instance, network_info, block_device_info,"},{"line_number":4284,"context_line":"                    power_on, migration)"},{"line_number":4285,"context_line":"            except TypeError:"},{"line_number":4286,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4287,"context_line":"                            \"\u0027migration\u0027 parameter to the \""},{"line_number":4288,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_4e6ad1ed","line":4285,"in_reply_to":"9fb8cfa7_eede25d9","updated":"2019-07-03 15:30:17.000000000","message":"I don\u0027t really care about the change for backports since this is all monolithic code inside the nova-compute binary. What I do care about is below, and thus I\u0027m fine with Matt\u0027s suggestion if it matters to him. Avoid the change, then make the change, but don\u0027t half-do it at any point.","commit_id":"6e9c0ad8c0fd4f62a1110b9d2b839123a1444d90"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"81dcf60510d31592afb2629c71224ab37e1b1a8f","unresolved":false,"context_lines":[{"line_number":4286,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4287,"context_line":"                            \"\u0027migration\u0027 parameter to the \""},{"line_number":4288,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""},{"line_number":4289,"context_line":"                            \"please update your virt driver.\")"},{"line_number":4290,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4291,"context_line":"                    context, instance, network_info, block_device_info,"},{"line_number":4292,"context_line":"                    power_on)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_aec02de2","line":4289,"updated":"2019-07-03 15:30:17.000000000","message":"I\u0027m not okay with this. Virt drivers are in-tree, when we make a change to the signature we should modify them all. I\u0027m not even in favor of the obligatory heads-up to the mailing list, really, but that\u0027s my issue. However, I do *not* think we should have a bunch of in-tree code to handle changes in the unversioned unstable virt driver API.","commit_id":"6e9c0ad8c0fd4f62a1110b9d2b839123a1444d90"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":998,"context_line":"                block_dev_info \u003d self._get_instance_block_device_info(context,"},{"line_number":999,"context_line":"                                                                      instance)"},{"line_number":1000,"context_line":""},{"line_number":1001,"context_line":"                self.driver.finish_revert_migration(context,"},{"line_number":1002,"context_line":"                    instance, net_info, block_dev_info, power_on)"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_7173e3e8","line":1001,"updated":"2019-07-05 21:16:39.000000000","message":"Note that we could call the method here as well on restart of the compute service while an instance was being migrated off the source host. In this case the migration_context should still be set on the instance.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":4183,"context_line":"        network_info \u003d instance.get_network_info()"},{"line_number":4184,"context_line":"        events \u003d []"},{"line_number":4185,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4186,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4187,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4188,"context_line":"            events \u003d network_info.get_bind_time_events(migration)"},{"line_number":4189,"context_line":"            if events:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_f10a7388","line":4186,"range":{"start_line":4186,"start_character":20,"end_line":4186,"end_character":70},"updated":"2019-07-05 21:16:39.000000000","message":"Why not use migration.is_same_host() here?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":4183,"context_line":"        network_info \u003d instance.get_network_info()"},{"line_number":4184,"context_line":"        events \u003d []"},{"line_number":4185,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4186,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4187,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4188,"context_line":"            events \u003d network_info.get_bind_time_events(migration)"},{"line_number":4189,"context_line":"            if events:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_1287d8d9","line":4186,"range":{"start_line":4186,"start_character":20,"end_line":4186,"end_character":70},"in_reply_to":"7faddb67_f10a7388","updated":"2019-07-10 16:01:10.000000000","message":"Err, this is left over from Sean\u0027s original patch, it shouldn\u0027t be here, as the migration.is_same_host() checking happens in the network model.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":4184,"context_line":"        events \u003d []"},{"line_number":4185,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4186,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4187,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4188,"context_line":"            events \u003d network_info.get_bind_time_events(migration)"},{"line_number":4189,"context_line":"            if events:"},{"line_number":4190,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_31a9eb7a","line":4187,"range":{"start_line":4187,"start_character":60,"end_line":4187,"end_character":77},"updated":"2019-07-05 21:16:39.000000000","message":"OK this...","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":4184,"context_line":"        events \u003d []"},{"line_number":4185,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4186,"context_line":"        same_host \u003d migration.source_compute \u003d\u003d migration.dest_compute"},{"line_number":4187,"context_line":"        if deadline and utils.is_neutron() and network_info and not same_host:"},{"line_number":4188,"context_line":"            events \u003d network_info.get_bind_time_events(migration)"},{"line_number":4189,"context_line":"            if events:"},{"line_number":4190,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_924bc8ae","line":4187,"range":{"start_line":4187,"start_character":60,"end_line":4187,"end_character":77},"in_reply_to":"7faddb67_31a9eb7a","updated":"2019-07-10 16:01:10.000000000","message":"Removed, see above.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":4188,"context_line":"            events \u003d network_info.get_bind_time_events(migration)"},{"line_number":4189,"context_line":"            if events:"},{"line_number":4190,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"},{"line_number":4191,"context_line":"        elif same_host:"},{"line_number":4192,"context_line":"            LOG.debug(\u0027Same host resize revert for instance %s, \u0027"},{"line_number":4193,"context_line":"                      \u0027not waiting for neutron vif plug events.\u0027,"},{"line_number":4194,"context_line":"                      instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_91b1bfe4","line":4191,"updated":"2019-07-05 21:16:39.000000000","message":"...and this are the main things for this version of the change compared to https://review.opendev.org/#/c/644881/37/nova/compute/manager.py.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":4211,"context_line":"                                                             migration)"},{"line_number":4212,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4213,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4214,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events: %s\u0027, events)"},{"line_number":4215,"context_line":""},{"line_number":4216,"context_line":"    @wrap_exception()"},{"line_number":4217,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_31748b35","line":4214,"updated":"2019-07-05 21:16:39.000000000","message":"You dropped the instance\u003dinstance kwarg here for some reason:\n\nhttps://review.opendev.org/#/c/644881/37/nova/compute/manager.py@4209","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0341dfeab55eb426a4854f696315c32cbc5e4f12","unresolved":false,"context_lines":[{"line_number":4211,"context_line":"                                                             migration)"},{"line_number":4212,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4213,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4214,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events: %s\u0027, events)"},{"line_number":4215,"context_line":""},{"line_number":4216,"context_line":"    @wrap_exception()"},{"line_number":4217,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_5fbfc2fa","line":4214,"in_reply_to":"7faddb67_14485dca","updated":"2019-07-08 14:06:27.000000000","message":"Instance is special and will cause oslo.log to print the instance uuid in brackets.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":4211,"context_line":"                                                             migration)"},{"line_number":4212,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4213,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4214,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events: %s\u0027, events)"},{"line_number":4215,"context_line":""},{"line_number":4216,"context_line":"    @wrap_exception()"},{"line_number":4217,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_d22060b3","line":4214,"in_reply_to":"7faddb67_31748b35","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0a2518db63b67277df633095955322d692d507a3","unresolved":false,"context_lines":[{"line_number":4211,"context_line":"                                                             migration)"},{"line_number":4212,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4213,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4214,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events: %s\u0027, events)"},{"line_number":4215,"context_line":""},{"line_number":4216,"context_line":"    @wrap_exception()"},{"line_number":4217,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_14485dca","line":4214,"in_reply_to":"7faddb67_31748b35","updated":"2019-07-05 22:56:23.000000000","message":"i drop that in teh previous version as it was not used by the error message.  unless i missunderdstand how LOG.error works i dont think the instance would have been printed before?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"051c80a06b82558500e940290b0cd532b3ba4ce9","unresolved":false,"context_lines":[{"line_number":4211,"context_line":"                                                             migration)"},{"line_number":4212,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4213,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4214,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events: %s\u0027, events)"},{"line_number":4215,"context_line":""},{"line_number":4216,"context_line":"    @wrap_exception()"},{"line_number":4217,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_629ea164","line":4214,"in_reply_to":"7faddb67_5fbfc2fa","updated":"2019-07-08 14:59:15.000000000","message":"yar: https://github.com/openstack/oslo.log/blob/3.44.0/oslo_log/formatters.py#L426","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4c83e807a5e55edf76d7277bd3f89e5d822042f9","unresolved":false,"context_lines":[{"line_number":4279,"context_line":""},{"line_number":4280,"context_line":"            power_on \u003d old_vm_state !\u003d vm_states.STOPPED"},{"line_number":4281,"context_line":"            self.driver.finish_revert_migration(context, instance,"},{"line_number":4282,"context_line":"                                         network_info,"},{"line_number":4283,"context_line":"                                         block_device_info, power_on)"},{"line_number":4284,"context_line":""},{"line_number":4285,"context_line":"            instance.drop_migration_context()"},{"line_number":4286,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_3321f6fc","line":4283,"range":{"start_line":4282,"start_character":1,"end_line":4283,"end_character":69},"updated":"2019-07-05 16:10:30.000000000","message":"nit: unrelated but im guessing this is a sidefect of reverting the api change.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8b642384cee707f02e86a791aad1688fab4aa41a","unresolved":false,"context_lines":[{"line_number":4279,"context_line":""},{"line_number":4280,"context_line":"            power_on \u003d old_vm_state !\u003d vm_states.STOPPED"},{"line_number":4281,"context_line":"            self.driver.finish_revert_migration(context, instance,"},{"line_number":4282,"context_line":"                                         network_info,"},{"line_number":4283,"context_line":"                                         block_device_info, power_on)"},{"line_number":4284,"context_line":""},{"line_number":4285,"context_line":"            instance.drop_migration_context()"},{"line_number":4286,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_9e0a3124","line":4283,"range":{"start_line":4282,"start_character":1,"end_line":4283,"end_character":69},"in_reply_to":"7faddb67_3321f6fc","updated":"2019-07-05 16:54:29.000000000","message":"Yeah we should fix this especially since we\u0027re backporting.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":4279,"context_line":""},{"line_number":4280,"context_line":"            power_on \u003d old_vm_state !\u003d vm_states.STOPPED"},{"line_number":4281,"context_line":"            self.driver.finish_revert_migration(context, instance,"},{"line_number":4282,"context_line":"                                         network_info,"},{"line_number":4283,"context_line":"                                         block_device_info, power_on)"},{"line_number":4284,"context_line":""},{"line_number":4285,"context_line":"            instance.drop_migration_context()"},{"line_number":4286,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_22dd0914","line":4283,"range":{"start_line":4282,"start_character":1,"end_line":4283,"end_character":69},"in_reply_to":"7faddb67_9e0a3124","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":4573,"context_line":"            migration.status \u003d \u0027migrating\u0027"},{"line_number":4574,"context_line":"            migration.save()"},{"line_number":4575,"context_line":""},{"line_number":4576,"context_line":"            instance.task_state \u003d task_states.RESIZE_MIGRATING"},{"line_number":4577,"context_line":"            instance.save(expected_task_state\u003dtask_states.RESIZE_PREP)"},{"line_number":4578,"context_line":""},{"line_number":4579,"context_line":"            bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":4580,"context_line":"                    context, instance.uuid)"},{"line_number":4581,"context_line":"            self._send_resize_instance_notifications("},{"line_number":4582,"context_line":"                context, instance, bdms, network_info,"},{"line_number":4583,"context_line":"                fields.NotificationPhase.START)"},{"line_number":4584,"context_line":""},{"line_number":4585,"context_line":"            block_device_info \u003d self._get_instance_block_device_info("},{"line_number":4586,"context_line":"                                context, instance, bdms\u003dbdms)"},{"line_number":4587,"context_line":""},{"line_number":4588,"context_line":"            timeout, retry_interval \u003d self._get_power_off_values(context,"},{"line_number":4589,"context_line":"                                            instance, clean_shutdown)"},{"line_number":4590,"context_line":"            disk_info \u003d self.driver.migrate_disk_and_power_off("},{"line_number":4591,"context_line":"                    context, instance, migration.dest_host,"},{"line_number":4592,"context_line":"                    instance_type, network_info,"},{"line_number":4593,"context_line":"                    block_device_info,"},{"line_number":4594,"context_line":"                    timeout, retry_interval)"},{"line_number":4595,"context_line":""},{"line_number":4596,"context_line":"            self._terminate_volume_connections(context, instance, bdms)"},{"line_number":4597,"context_line":""},{"line_number":4598,"context_line":"            migration_p \u003d obj_base.obj_to_primitive(migration)"},{"line_number":4599,"context_line":"            self.network_api.migrate_instance_start(context,"},{"line_number":4600,"context_line":"                                                    instance,"},{"line_number":4601,"context_line":"                                                    migration_p)"},{"line_number":4602,"context_line":""},{"line_number":4603,"context_line":"            migration.status \u003d \u0027post-migrating\u0027"},{"line_number":4604,"context_line":"            migration.save()"},{"line_number":4605,"context_line":""},{"line_number":4606,"context_line":"            instance.host \u003d migration.dest_compute"},{"line_number":4607,"context_line":"            instance.node \u003d migration.dest_node"},{"line_number":4608,"context_line":"            instance.task_state \u003d task_states.RESIZE_MIGRATED"},{"line_number":4609,"context_line":"            instance.save(expected_task_state\u003dtask_states.RESIZE_MIGRATING)"},{"line_number":4610,"context_line":""},{"line_number":4611,"context_line":"            # RPC cast to the destination host to finish the resize/migration."},{"line_number":4612,"context_line":"            self.compute_rpcapi.finish_resize(context, instance,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_f167d324","line":4609,"range":{"start_line":4576,"start_character":12,"end_line":4609,"end_character":75},"updated":"2019-07-05 21:16:39.000000000","message":"Note to self: between here the instance task_state is RESIZE_MIGRATING and if the service crashed, we\u0027ll try to revert the migration on restart of the compute service so we should still have the migration_context set on the instance but the migration status itself might be \"error\" because of the errors_out_migration_ctxt context manager we\u0027re in.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"}],"nova/network/model.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba878b0ae497d2132aff7e784db88b521cbc3034","unresolved":false,"context_lines":[{"line_number":465,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":466,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":467,"context_line":"        \"\"\""},{"line_number":468,"context_line":"        if migration:"},{"line_number":469,"context_line":"            return (self.is_hybrid_plug_enabled() and not"},{"line_number":470,"context_line":"                    migration.is_same_host())"},{"line_number":471,"context_line":"        return False"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"    def is_hybrid_plug_enabled(self):"},{"line_number":474,"context_line":"        return self[\u0027details\u0027].get(VIF_DETAILS_OVS_HYBRID_PLUG, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_506a94ec","line":471,"range":{"start_line":468,"start_character":8,"end_line":471,"end_character":20},"updated":"2019-07-01 18:11:45.000000000","message":"the migration is not optional so im not sure why you are doing an if.\n\ndid you intend migration ot be kwarg that default to None?","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8fd3d3a2e9a6f911710d5e00cf37a14fdbff7044","unresolved":false,"context_lines":[{"line_number":465,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":466,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":467,"context_line":"        \"\"\""},{"line_number":468,"context_line":"        if migration:"},{"line_number":469,"context_line":"            return (self.is_hybrid_plug_enabled() and not"},{"line_number":470,"context_line":"                    migration.is_same_host())"},{"line_number":471,"context_line":"        return False"},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"    def is_hybrid_plug_enabled(self):"},{"line_number":474,"context_line":"        return self[\u0027details\u0027].get(VIF_DETAILS_OVS_HYBRID_PLUG, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_79853c59","line":471,"range":{"start_line":468,"start_character":8,"end_line":471,"end_character":20},"in_reply_to":"9fb8cfa7_506a94ec","updated":"2019-07-02 08:17:23.000000000","message":"It\u0027s because the migration, as passed to finish_revert_migration in the driver [1], can be None if the latter was called from _init_instance [2] in the compute manager.\n\nBut I\u0027ll make it clearer by making it a kwarg here to indicate that it could be None.\n\n[1] https://review.opendev.org/#/c/667177/5/nova/virt/libvirt/driver.py@9141\n[2] https://review.opendev.org/#/c/667177/5/nova/compute/manager.py@1001","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba878b0ae497d2132aff7e784db88b521cbc3034","unresolved":false,"context_lines":[{"line_number":527,"context_line":"    def json(self):"},{"line_number":528,"context_line":"        return jsonutils.dumps(self)"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    def get_bind_time_events(self, migration):"},{"line_number":531,"context_line":"        \"\"\"network-vif-plugged events can be sent as soon as the port binding"},{"line_number":532,"context_line":"        is updated in the following case(s):"},{"line_number":533,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_f0514831","line":530,"range":{"start_line":530,"start_character":34,"end_line":530,"end_character":44},"updated":"2019-07-01 18:11:45.000000000","message":"as above the migration is required here so if we expect ti to be used optionally this should be a kwarg if its required then has_bind_time_event shoudl not treat it as optional interally","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba878b0ae497d2132aff7e784db88b521cbc3034","unresolved":false,"context_lines":[{"line_number":537,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":538,"context_line":"                for vif in self if vif.has_bind_time_event(migration)]"},{"line_number":539,"context_line":""},{"line_number":540,"context_line":"    def get_plug_time_events(self, migration):"},{"line_number":541,"context_line":"        \"\"\"Complementary to get_bind_time_events(), any event that does not"},{"line_number":542,"context_line":"        fall in that category is a bind-time event."},{"line_number":543,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_503854ec","line":540,"range":{"start_line":540,"start_character":9,"end_line":540,"end_character":46},"updated":"2019-07-01 18:11:45.000000000","message":"same as above","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0341dfeab55eb426a4854f696315c32cbc5e4f12","unresolved":false,"context_lines":[{"line_number":461,"context_line":"    def has_bind_time_event(self, migration\u003dNone):"},{"line_number":462,"context_line":"        \"\"\"network-vif-plugged events can be sent as soon as the port binding"},{"line_number":463,"context_line":"        is updated in the following case(s):"},{"line_number":464,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":465,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":466,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":467,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_5f1b421a","line":464,"range":{"start_line":464,"start_character":8,"end_line":464,"end_character":9},"updated":"2019-07-08 14:06:27.000000000","message":"A single-item bullet list looks weird here. If you\u0027re going to respin this comment, maybe just squash this?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":461,"context_line":"    def has_bind_time_event(self, migration\u003dNone):"},{"line_number":462,"context_line":"        \"\"\"network-vif-plugged events can be sent as soon as the port binding"},{"line_number":463,"context_line":"        is updated in the following case(s):"},{"line_number":464,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":465,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":466,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":467,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_35299e89","line":464,"range":{"start_line":464,"start_character":8,"end_line":464,"end_character":9},"in_reply_to":"7faddb67_5f1b421a","updated":"2019-07-10 16:01:10.000000000","message":"There\u0027s more coming in a subsequent patch (SRIOV direct-physical ports), but yeah, let\u0027s make the list change in there and leave it as a sentence for now.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":463,"context_line":"        is updated in the following case(s):"},{"line_number":464,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":465,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":466,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":467,"context_line":"        \"\"\""},{"line_number":468,"context_line":"        if migration:"},{"line_number":469,"context_line":"            return (self.is_hybrid_plug_enabled() and not"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_11daaf16","line":466,"range":{"start_line":466,"start_character":37,"end_line":466,"end_character":39},"updated":"2019-07-05 21:16:39.000000000","message":"for?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":463,"context_line":"        is updated in the following case(s):"},{"line_number":464,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":465,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":466,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":467,"context_line":"        \"\"\""},{"line_number":468,"context_line":"        if migration:"},{"line_number":469,"context_line":"            return (self.is_hybrid_plug_enabled() and not"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_155b422e","line":466,"range":{"start_line":466,"start_character":37,"end_line":466,"end_character":39},"in_reply_to":"7faddb67_11daaf16","updated":"2019-07-10 16:01:10.000000000","message":"No, but I tried to make it clearer.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0341dfeab55eb426a4854f696315c32cbc5e4f12","unresolved":false,"context_lines":[{"line_number":530,"context_line":"    def get_bind_time_events(self, migration\u003dNone):"},{"line_number":531,"context_line":"        \"\"\"network-vif-plugged events can be sent as soon as the port binding"},{"line_number":532,"context_line":"        is updated in the following case(s):"},{"line_number":533,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":534,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":535,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":536,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_7f187e1c","line":533,"range":{"start_line":533,"start_character":8,"end_line":533,"end_character":9},"updated":"2019-07-08 14:06:27.000000000","message":"same here","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":530,"context_line":"    def get_bind_time_events(self, migration\u003dNone):"},{"line_number":531,"context_line":"        \"\"\"network-vif-plugged events can be sent as soon as the port binding"},{"line_number":532,"context_line":"        is updated in the following case(s):"},{"line_number":533,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":534,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":535,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":536,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_35d3de62","line":533,"range":{"start_line":533,"start_character":8,"end_line":533,"end_character":9},"in_reply_to":"7faddb67_7f187e1c","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":532,"context_line":"        is updated in the following case(s):"},{"line_number":533,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":534,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":535,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":536,"context_line":"        \"\"\""},{"line_number":537,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":538,"context_line":"                for vif in self if vif.has_bind_time_event(migration)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_91ff7f66","line":535,"range":{"start_line":535,"start_character":37,"end_line":535,"end_character":39},"updated":"2019-07-05 21:16:39.000000000","message":"for?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":532,"context_line":"        is updated in the following case(s):"},{"line_number":533,"context_line":"        - updating the binding of an OVS hybrid plug port to a host that"},{"line_number":534,"context_line":"          already has the instance in a shutoff state. In practice, this means"},{"line_number":535,"context_line":"          reverting a cold migration or non-same-host resize"},{"line_number":536,"context_line":"        \"\"\""},{"line_number":537,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":538,"context_line":"                for vif in self if vif.has_bind_time_event(migration)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_55d69a51","line":535,"range":{"start_line":535,"start_character":37,"end_line":535,"end_character":39},"in_reply_to":"7faddb67_91ff7f66","updated":"2019-07-10 16:01:10.000000000","message":"Just rewrote the whole docstring.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":539,"context_line":""},{"line_number":540,"context_line":"    def get_plug_time_events(self, migration\u003dNone):"},{"line_number":541,"context_line":"        \"\"\"Complementary to get_bind_time_events(), any event that does not"},{"line_number":542,"context_line":"        fall in that category is a bind-time event."},{"line_number":543,"context_line":"        \"\"\""},{"line_number":544,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":545,"context_line":"                for vif in self if not vif.has_bind_time_event(migration)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_51f9077e","line":542,"range":{"start_line":542,"start_character":35,"end_line":542,"end_character":39},"updated":"2019-07-05 21:16:39.000000000","message":"plug?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":539,"context_line":""},{"line_number":540,"context_line":"    def get_plug_time_events(self, migration\u003dNone):"},{"line_number":541,"context_line":"        \"\"\"Complementary to get_bind_time_events(), any event that does not"},{"line_number":542,"context_line":"        fall in that category is a bind-time event."},{"line_number":543,"context_line":"        \"\"\""},{"line_number":544,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":545,"context_line":"                for vif in self if not vif.has_bind_time_event(migration)]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_f5cc66bd","line":542,"range":{"start_line":542,"start_character":35,"end_line":542,"end_character":39},"in_reply_to":"7faddb67_51f9077e","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"}],"nova/objects/migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba878b0ae497d2132aff7e784db88b521cbc3034","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    def instance(self, instance):"},{"line_number":186,"context_line":"        self._cached_instance \u003d instance"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def is_same_host(self):"},{"line_number":189,"context_line":"        return self.source_compute \u003d\u003d self.dest_compute"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"@base.NovaObjectRegistry.register"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_3085a0ad","line":189,"range":{"start_line":188,"start_character":5,"end_line":189,"end_character":55},"updated":"2019-07-01 18:11:45.000000000","message":"sure this makes sense to have on the migration object\n\nyou could also make it a property\n\ne.g. \n@property\ndef same_host(self):\n    retrun  self.source_compute \u003d\u003d self.dest_compute\n\nbut i think this is fine","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":5084,"context_line":"            self.assertEqual(migration.source_compute, \u0027fake-source\u0027)"},{"line_number":5085,"context_line":"            # NOTE(artom) This looks weird, but it\u0027s checking that the"},{"line_number":5086,"context_line":"            # temporaty_mutation() context manager did its job."},{"line_number":5087,"context_line":"            self.assertEqual(migration.dest_compute, \u0027fake-source\u0027)"},{"line_number":5088,"context_line":""},{"line_number":5089,"context_line":"        with test.nested("},{"line_number":5090,"context_line":"            mock.patch.object(self.compute.virtapi,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_f12fd3f3","line":5087,"range":{"start_line":5087,"start_character":53,"end_line":5087,"end_character":66},"updated":"2019-07-05 21:16:39.000000000","message":"nit: Since you\u0027re allowing callers to pass a migration, this would be better as migration.source_compute in case the caller isn\u0027t using the same thing you default to above. That probably means removing the assertEqual above as well since self.assertEqual(migration.source_compute, migration.source_compute) would be pointless.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":5084,"context_line":"            self.assertEqual(migration.source_compute, \u0027fake-source\u0027)"},{"line_number":5085,"context_line":"            # NOTE(artom) This looks weird, but it\u0027s checking that the"},{"line_number":5086,"context_line":"            # temporaty_mutation() context manager did its job."},{"line_number":5087,"context_line":"            self.assertEqual(migration.dest_compute, \u0027fake-source\u0027)"},{"line_number":5088,"context_line":""},{"line_number":5089,"context_line":"        with test.nested("},{"line_number":5090,"context_line":"            mock.patch.object(self.compute.virtapi,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_b5282ee2","line":5087,"range":{"start_line":5087,"start_character":53,"end_line":5087,"end_character":66},"in_reply_to":"7faddb67_f12fd3f3","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0a2518db63b67277df633095955322d692d507a3","unresolved":false,"context_lines":[{"line_number":5084,"context_line":"            self.assertEqual(migration.source_compute, \u0027fake-source\u0027)"},{"line_number":5085,"context_line":"            # NOTE(artom) This looks weird, but it\u0027s checking that the"},{"line_number":5086,"context_line":"            # temporaty_mutation() context manager did its job."},{"line_number":5087,"context_line":"            self.assertEqual(migration.dest_compute, \u0027fake-source\u0027)"},{"line_number":5088,"context_line":""},{"line_number":5089,"context_line":"        with test.nested("},{"line_number":5090,"context_line":"            mock.patch.object(self.compute.virtapi,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_546c9535","line":5087,"range":{"start_line":5087,"start_character":53,"end_line":5087,"end_character":66},"in_reply_to":"7faddb67_f12fd3f3","updated":"2019-07-05 22:56:23.000000000","message":"this is an inner function that wont be called directly except via the mock on line 5092 but ya we are using the migration that is passed in _test_finish_revert_resize_network_migrate_finish and by hardcoding this we are forcing a precondition on the caller of _test_finish_revert_resize_network_migrate_finish\n\nas a result of hardcoding this value so ya i guess this makes sense to just\nelf.assertEqual(migration.source_compute, migration.dest_compute)\n\nand remove the first check.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"}],"nova/tests/unit/network/test_network_info.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":871,"context_line":"                                      dest_compute\u003d\u0027fake-host\u0027)"},{"line_number":872,"context_line":"        diff_host \u003d objects.Migration(source_compute\u003d\u0027fake-host1\u0027,"},{"line_number":873,"context_line":"                                      dest_compute\u003d\u0027fake-host2\u0027)"},{"line_number":874,"context_line":"        self.assertItemsEqual("},{"line_number":875,"context_line":"            [(\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":876,"context_line":"             (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)],"},{"line_number":877,"context_line":"            network_info.get_plug_time_events(same_host))"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_311ecbba","line":874,"updated":"2019-07-05 21:16:39.000000000","message":"This is a bit tricky, but it\u0027s just saying we wait for the event at plug-time in both cases b/c it\u0027s a same host resize. Comments in the test could help per case you\u0027re testing.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":871,"context_line":"                                      dest_compute\u003d\u0027fake-host\u0027)"},{"line_number":872,"context_line":"        diff_host \u003d objects.Migration(source_compute\u003d\u0027fake-host1\u0027,"},{"line_number":873,"context_line":"                                      dest_compute\u003d\u0027fake-host2\u0027)"},{"line_number":874,"context_line":"        self.assertItemsEqual("},{"line_number":875,"context_line":"            [(\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":876,"context_line":"             (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)],"},{"line_number":877,"context_line":"            network_info.get_plug_time_events(same_host))"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_5be1ebcf","line":874,"in_reply_to":"7faddb67_311ecbba","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b24d7d67f6634e36b231a0891000cdf844400e54","unresolved":false,"context_lines":[{"line_number":876,"context_line":"            [(\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":877,"context_line":"             (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)],"},{"line_number":878,"context_line":"            network_info.get_plug_time_events(same_host))"},{"line_number":879,"context_line":"        # Same host migration will have no plug-time events."},{"line_number":880,"context_line":"        self.assertEqual([], network_info.get_bind_time_events(same_host))"},{"line_number":881,"context_line":"        # Diff-host migration + OVS hybrid plug \u003d bind-time events"},{"line_number":882,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_2cc5ba5b","line":879,"range":{"start_line":879,"start_character":43,"end_line":879,"end_character":47},"updated":"2019-07-11 20:01:28.000000000","message":"bind","commit_id":"7a7a223602ca5aa0aca8f65a6ab143f1d8f8ec1b"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":19661,"context_line":"            self.flags(instances_path\u003dtmpdir)"},{"line_number":19662,"context_line":"            ins_ref \u003d self._create_instance()"},{"line_number":19663,"context_line":"            ins_ref.migration_context \u003d objects.MigrationContext("},{"line_number":19664,"context_line":"                migration_id\u003d42)"},{"line_number":19665,"context_line":"            os.mkdir(os.path.join(tmpdir, ins_ref[\u0027name\u0027]))"},{"line_number":19666,"context_line":"            libvirt_xml_path \u003d os.path.join(tmpdir,"},{"line_number":19667,"context_line":"                                            ins_ref[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_51da67e9","line":19664,"range":{"start_line":19664,"start_character":29,"end_line":19664,"end_character":31},"updated":"2019-07-05 21:16:39.000000000","message":"This should be migration.id.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":19661,"context_line":"            self.flags(instances_path\u003dtmpdir)"},{"line_number":19662,"context_line":"            ins_ref \u003d self._create_instance()"},{"line_number":19663,"context_line":"            ins_ref.migration_context \u003d objects.MigrationContext("},{"line_number":19664,"context_line":"                migration_id\u003d42)"},{"line_number":19665,"context_line":"            os.mkdir(os.path.join(tmpdir, ins_ref[\u0027name\u0027]))"},{"line_number":19666,"context_line":"            libvirt_xml_path \u003d os.path.join(tmpdir,"},{"line_number":19667,"context_line":"                                            ins_ref[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_c2d5fce6","line":19664,"range":{"start_line":19664,"start_character":29,"end_line":19664,"end_character":31},"in_reply_to":"7faddb67_51da67e9","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4c83e807a5e55edf76d7277bd3f89e5d822042f9","unresolved":false,"context_lines":[{"line_number":19680,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":19681,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)]"},{"line_number":19682,"context_line":"                else:"},{"line_number":19683,"context_line":"                    # Diff host is only normal"},{"line_number":19684,"context_line":"                    self.events_passed_to_fake_create \u003d ["},{"line_number":19685,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.normal_vif)]"},{"line_number":19686,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_93d78ac0","line":19683,"range":{"start_line":19683,"start_character":20,"end_line":19683,"end_character":46},"updated":"2019-07-05 16:10:30.000000000","message":"nit: # for different host hybrid plugged vifs emit bind time events.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8b642384cee707f02e86a791aad1688fab4aa41a","unresolved":false,"context_lines":[{"line_number":19680,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":19681,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)]"},{"line_number":19682,"context_line":"                else:"},{"line_number":19683,"context_line":"                    # Diff host is only normal"},{"line_number":19684,"context_line":"                    self.events_passed_to_fake_create \u003d ["},{"line_number":19685,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.normal_vif)]"},{"line_number":19686,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_19041b15","line":19683,"range":{"start_line":19683,"start_character":20,"end_line":19683,"end_character":46},"in_reply_to":"7faddb67_93d78ac0","updated":"2019-07-05 16:54:29.000000000","message":"++","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":19680,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":19681,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)]"},{"line_number":19682,"context_line":"                else:"},{"line_number":19683,"context_line":"                    # Diff host is only normal"},{"line_number":19684,"context_line":"                    self.events_passed_to_fake_create \u003d ["},{"line_number":19685,"context_line":"                        (\u0027network-vif-plugged\u0027, uuids.normal_vif)]"},{"line_number":19686,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_22b2d0e7","line":19683,"range":{"start_line":19683,"start_character":20,"end_line":19683,"end_character":46},"in_reply_to":"7faddb67_93d78ac0","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4c83e807a5e55edf76d7277bd3f89e5d822042f9","unresolved":false,"context_lines":[{"line_number":19689,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":19690,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)]"},{"line_number":19691,"context_line":""},{"line_number":19692,"context_line":"            with mock.patch.object(objects.Migration, \u0027get_by_id_and_instance\u0027,"},{"line_number":19693,"context_line":"                                   return_value\u003dmigration) as mock_get_mig:"},{"line_number":19694,"context_line":"                self.drvr.finish_revert_migration("},{"line_number":19695,"context_line":"                    context.get_admin_context(), ins_ref,"},{"line_number":19696,"context_line":"                    network_info, None, power_on)"},{"line_number":19697,"context_line":"                mock_get_mig.assert_called_with(mock.ANY, 42, ins_ref.uuid)"},{"line_number":19698,"context_line":""},{"line_number":19699,"context_line":"            self.assertTrue(self.fake_create_domain_called)"},{"line_number":19700,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_5356f22b","line":19697,"range":{"start_line":19692,"start_character":11,"end_line":19697,"end_character":75},"updated":"2019-07-05 16:10:30.000000000","message":"ok so this is asserting the db lookup behaviour for\nhttps://review.opendev.org/#/c/667177/8/nova/virt/libvirt/driver.py@9169","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":19689,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":19690,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)]"},{"line_number":19691,"context_line":""},{"line_number":19692,"context_line":"            with mock.patch.object(objects.Migration, \u0027get_by_id_and_instance\u0027,"},{"line_number":19693,"context_line":"                                   return_value\u003dmigration) as mock_get_mig:"},{"line_number":19694,"context_line":"                self.drvr.finish_revert_migration("},{"line_number":19695,"context_line":"                    context.get_admin_context(), ins_ref,"},{"line_number":19696,"context_line":"                    network_info, None, power_on)"},{"line_number":19697,"context_line":"                mock_get_mig.assert_called_with(mock.ANY, 42, ins_ref.uuid)"},{"line_number":19698,"context_line":""},{"line_number":19699,"context_line":"            self.assertTrue(self.fake_create_domain_called)"},{"line_number":19700,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_b1ee3b86","line":19697,"range":{"start_line":19692,"start_character":11,"end_line":19697,"end_character":75},"in_reply_to":"7faddb67_5356f22b","updated":"2019-07-05 21:16:39.000000000","message":"This is confusing to me when the method takes migration as an optional kwarg. I would think that if migration\u003dNone we would assert that we don\u0027t try to look it up.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":19689,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.normal_vif),"},{"line_number":19690,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.hybrid_vif)]"},{"line_number":19691,"context_line":""},{"line_number":19692,"context_line":"            with mock.patch.object(objects.Migration, \u0027get_by_id_and_instance\u0027,"},{"line_number":19693,"context_line":"                                   return_value\u003dmigration) as mock_get_mig:"},{"line_number":19694,"context_line":"                self.drvr.finish_revert_migration("},{"line_number":19695,"context_line":"                    context.get_admin_context(), ins_ref,"},{"line_number":19696,"context_line":"                    network_info, None, power_on)"},{"line_number":19697,"context_line":"                mock_get_mig.assert_called_with(mock.ANY, 42, ins_ref.uuid)"},{"line_number":19698,"context_line":""},{"line_number":19699,"context_line":"            self.assertTrue(self.fake_create_domain_called)"},{"line_number":19700,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_1b88531d","line":19697,"range":{"start_line":19692,"start_character":11,"end_line":19697,"end_character":75},"in_reply_to":"7faddb67_b1ee3b86","updated":"2019-07-10 16:01:10.000000000","message":"Based on [1] we\u0027re always going to look up a migration, so I\u0027ve adjusted the test to match this.\n\n[1] https://review.opendev.org/#/c/667177/8/nova/virt/libvirt/driver.py@9175","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":19707,"context_line":"    def test_finish_revert_migration_same_host(self):"},{"line_number":19708,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host\u0027,"},{"line_number":19709,"context_line":"                                      dest_compute\u003d\u0027fake-host\u0027)"},{"line_number":19710,"context_line":"        self._test_finish_revert_migration(True, migration)"},{"line_number":19711,"context_line":""},{"line_number":19712,"context_line":"    def test_finish_revert_migration_diff_host(self):"},{"line_number":19713,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host1\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_919e9f05","line":19710,"range":{"start_line":19710,"start_character":43,"end_line":19710,"end_character":47},"updated":"2019-07-05 21:16:39.000000000","message":"nit: use power_on\u003dTrue here and below (I dislike having to look at the method being called to figure out what this variable is for).","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"051c80a06b82558500e940290b0cd532b3ba4ce9","unresolved":false,"context_lines":[{"line_number":19707,"context_line":"    def test_finish_revert_migration_same_host(self):"},{"line_number":19708,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host\u0027,"},{"line_number":19709,"context_line":"                                      dest_compute\u003d\u0027fake-host\u0027)"},{"line_number":19710,"context_line":"        self._test_finish_revert_migration(True, migration)"},{"line_number":19711,"context_line":""},{"line_number":19712,"context_line":"    def test_finish_revert_migration_diff_host(self):"},{"line_number":19713,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host1\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_c2c8b556","line":19710,"range":{"start_line":19710,"start_character":43,"end_line":19710,"end_character":47},"in_reply_to":"7faddb67_748511ad","updated":"2019-07-08 14:59:15.000000000","message":"\u003e useing arg names for positional args is much safter than passing keyword args as postionals so i guess we could do that to improve readability but it would imply that power_on is optional and it is not. i would prefer to make power_on a kwarg in also _test_finish_revert_migration and both should then use the kwarg style\n\nDoesn\u0027t really matter what you do (kwarg or just have a variable in here named power_on and pass that in as positional) as long as you\u0027re not just passing True/False so I have to figure out what that means.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":19707,"context_line":"    def test_finish_revert_migration_same_host(self):"},{"line_number":19708,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host\u0027,"},{"line_number":19709,"context_line":"                                      dest_compute\u003d\u0027fake-host\u0027)"},{"line_number":19710,"context_line":"        self._test_finish_revert_migration(True, migration)"},{"line_number":19711,"context_line":""},{"line_number":19712,"context_line":"    def test_finish_revert_migration_diff_host(self):"},{"line_number":19713,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host1\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_bba33f9e","line":19710,"range":{"start_line":19710,"start_character":43,"end_line":19710,"end_character":47},"in_reply_to":"7faddb67_919e9f05","updated":"2019-07-10 16:01:10.000000000","message":"Done","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0a2518db63b67277df633095955322d692d507a3","unresolved":false,"context_lines":[{"line_number":19707,"context_line":"    def test_finish_revert_migration_same_host(self):"},{"line_number":19708,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host\u0027,"},{"line_number":19709,"context_line":"                                      dest_compute\u003d\u0027fake-host\u0027)"},{"line_number":19710,"context_line":"        self._test_finish_revert_migration(True, migration)"},{"line_number":19711,"context_line":""},{"line_number":19712,"context_line":"    def test_finish_revert_migration_diff_host(self):"},{"line_number":19713,"context_line":"        migration \u003d objects.Migration(source_compute\u003d\u0027fake-host1\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_748511ad","line":19710,"range":{"start_line":19710,"start_character":43,"end_line":19710,"end_character":47},"in_reply_to":"7faddb67_919e9f05","updated":"2019-07-05 22:56:23.000000000","message":"its technically not a keyword argument so while you can still refer to ti by name its not ususally considered a good pratice that said i also had to look this up when reviewing to figure out what True ment.\n\nuseing arg names for positional args is much safter than passing keyword args  as postionals so i guess we could do that to improve readability but it would imply that power_on is optional and it is not. i would prefer to make power_on a kwarg in also _test_finish_revert_migration and both should then use the kwarg style","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b24d7d67f6634e36b231a0891000cdf844400e54","unresolved":false,"context_lines":[{"line_number":19674,"context_line":"                self.drvr.finish_revert_migration("},{"line_number":19675,"context_line":"                    context.get_admin_context(), ins_ref,"},{"line_number":19676,"context_line":"                    network_info, None, power_on)"},{"line_number":19677,"context_line":"                mock_get_mig.assert_called_with(mock.ANY, migration.id,"},{"line_number":19678,"context_line":"                                                ins_ref.uuid)"},{"line_number":19679,"context_line":""},{"line_number":19680,"context_line":"            self.assertTrue(self.fake_create_domain_called)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_0c78fe27","line":19677,"range":{"start_line":19677,"start_character":48,"end_line":19677,"end_character":56},"updated":"2019-07-11 20:01:28.000000000","message":"nit: if you moved the context.get_admin_context() to a variable and passed that to finish_revert_migration then you could assert it\u0027s the one used here as well.","commit_id":"7a7a223602ca5aa0aca8f65a6ab143f1d8f8ec1b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b24d7d67f6634e36b231a0891000cdf844400e54","unresolved":false,"context_lines":[{"line_number":19730,"context_line":"                                                  \u0027test exception\u0027)"},{"line_number":19731,"context_line":"            drvr.finish_revert_migration(context, ins_ref,"},{"line_number":19732,"context_line":"                                         network_model.NetworkInfo())"},{"line_number":19733,"context_line":"            mock_get_mig.assert_called_with(mock.ANY, 42, ins_ref.uuid)"},{"line_number":19734,"context_line":"            if backup_made:"},{"line_number":19735,"context_line":"                mock_rename.assert_called_once_with(\u0027/fake/foo_resize\u0027,"},{"line_number":19736,"context_line":"                                                    \u0027/fake/foo\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_ac876a0f","line":19733,"range":{"start_line":19733,"start_character":44,"end_line":19733,"end_character":52},"updated":"2019-07-11 20:01:28.000000000","message":"I would think this could be the context variable.","commit_id":"7a7a223602ca5aa0aca8f65a6ab143f1d8f8ec1b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b24d7d67f6634e36b231a0891000cdf844400e54","unresolved":false,"context_lines":[{"line_number":19776,"context_line":"                mock.patch.object(objects.Migration, \u0027get_by_id_and_instance\u0027,"},{"line_number":19777,"context_line":"                                  return_value\u003dmigration)"},{"line_number":19778,"context_line":"        ) as (mock_img_bkend, mock_cdan, mock_gifsm, mock_ggxml, mock_get_mig):"},{"line_number":19779,"context_line":"            drvr.finish_revert_migration(\u0027\u0027, instance,"},{"line_number":19780,"context_line":"                                         network_model.NetworkInfo(),"},{"line_number":19781,"context_line":"                                         power_on\u003dFalse)"},{"line_number":19782,"context_line":"            mock_get_mig.assert_called_with(mock.ANY, 42, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_2c737a4f","line":19779,"range":{"start_line":19779,"start_character":41,"end_line":19779,"end_character":43},"updated":"2019-07-11 20:01:28.000000000","message":"unrelated but boo, lazy","commit_id":"7a7a223602ca5aa0aca8f65a6ab143f1d8f8ec1b"}],"nova/virt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba878b0ae497d2132aff7e784db88b521cbc3034","unresolved":false,"context_lines":[{"line_number":737,"context_line":"        raise NotImplementedError()"},{"line_number":738,"context_line":""},{"line_number":739,"context_line":"    def finish_revert_migration(self, context, instance, network_info,"},{"line_number":740,"context_line":"                                block_device_info\u003dNone, power_on\u003dTrue,"},{"line_number":741,"context_line":"                                migration\u003dNone):"},{"line_number":742,"context_line":"        \"\"\"Finish reverting a resize/migration."},{"line_number":743,"context_line":""},{"line_number":744,"context_line":"        :param context: the context for the finish_revert_migration"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_108adcbd","line":741,"range":{"start_line":740,"start_character":70,"end_line":741,"end_character":46},"updated":"2019-07-01 18:11:45.000000000","message":"so this unfortunetly reintoduced a driver api change\n\nso we need to handel that here\nhttps://review.opendev.org/#/c/667177/5/nova/compute/manager.py@4281\nsuch that we dont break out of tree driver otherwise we wont be able to backport this.","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"60a16c75308bbd7a5e0e60136fccdca442cb7f8a","unresolved":false,"context_lines":[{"line_number":737,"context_line":"        raise NotImplementedError()"},{"line_number":738,"context_line":""},{"line_number":739,"context_line":"    def finish_revert_migration(self, context, instance, network_info,"},{"line_number":740,"context_line":"                                block_device_info\u003dNone, power_on\u003dTrue,"},{"line_number":741,"context_line":"                                migration\u003dNone):"},{"line_number":742,"context_line":"        \"\"\"Finish reverting a resize/migration."},{"line_number":743,"context_line":""},{"line_number":744,"context_line":"        :param context: the context for the finish_revert_migration"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_16141443","line":741,"range":{"start_line":740,"start_character":70,"end_line":741,"end_character":46},"in_reply_to":"9fb8cfa7_108adcbd","updated":"2019-07-01 19:27:22.000000000","message":"You might be able to avoid this - does the instance still have a migration_context set on it at this point? If so, you could pull the Migration record using that migration_context:\n\nmigration \u003d objects.Migration.get_by_id(\n    context, instance.migration_context.migration_id)","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"abaf221fb0007adf9f902b4e91ceadd070159d90","unresolved":false,"context_lines":[{"line_number":9135,"context_line":"        # _finish_revert_resize_network_migrate_finish(), right after updating"},{"line_number":9136,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9137,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9138,"context_line":"        events \u003d network_info.get_plug_time_events()"},{"line_number":9139,"context_line":"        if events:"},{"line_number":9140,"context_line":"            LOG.debug(\u0027Instance is using plug-time events: %s\u0027, events,"},{"line_number":9141,"context_line":"                      instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_5d045b4a","line":9138,"range":{"start_line":9138,"start_character":8,"end_line":9138,"end_character":52},"updated":"2019-06-24 18:59:02.000000000","message":"We don\u0027t need to check for for same host revert resize  here because plug time event should be sent regardless of if its a same host or different host revert.\n\nThis is a good thing because we don\u0027t have a migration object here so trying to figure out if this was a same host resize would be a pain if we actully needed to do it.","commit_id":"4db2c21e0adccb96b19a0c03b83e49fa81eb1bce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a3eb6dcf5d220604b917daa5bc39d9bf5ab46b58","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_b1607b34","line":9175,"updated":"2019-07-05 21:16:39.000000000","message":"First, a style nit:\n\n  if \u0027migration_context\u0027 in instance:\n\nSecond, the migration_context should definitely be set at this point because we don\u0027t drop it until after calling this driver method in the compute manager:\n\nhttps://review.opendev.org/#/c/667177/8/nova/compute/manager.py@4285\n\nGiven that, if for some reason we didn\u0027t have the migration_context set on the instance and hit this code, we\u0027d not do the wait properly which would be a bug.\n\nI\u0027m guessing you\u0027re just doing this so you don\u0027t have to impact existing tests for this method, but you\u0027re going to have to update those anyway when you change the interface to pass in the migration object, so I think it would probably be better to expect instance.migration_context is set and not have a condition for it.\n\nA TODO about looking up the migration record here is only temporary and will get cleaned up separately might also be nice.\n\n(later)\n\nActually, it looks like we could call this method when the compute service starts:\n\nhttps://review.opendev.org/#/c/667177/8/nova/compute/manager.py@1001\n\nLooking at when we could hit that window the instance.migration_context should still be set (and we actually don\u0027t drop the migration context if that works, which is maybe an unrelated bug).","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9809ec3b2a212835fde19e1b8ff8a95c2c891d77","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_56576459","line":9175,"in_reply_to":"7faddb67_4c2596c1","updated":"2019-07-11 21:18:59.000000000","message":"Here is the functional test for that crash scenario:\n\nhttps://review.opendev.org/#/c/670393/","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b0b7d1298e4575272ec8229513adc9fbc44e34d9","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_dbc15b82","line":9175,"in_reply_to":"7faddb67_b1607b34","updated":"2019-07-10 16:01:10.000000000","message":"Notes to self:\n\nWe rollback in _init_instance() if the task state is RESIZE_MIGRATING. This means we\u0027ve crashed anywhere between [1] and [2]. This is indeed a fairly small window, and we only apply the migration context way later in _finish_resize().\n\nConclusion: yeah, we should always expect a migration context to be set on the instance here.\n\n[1] https://review.opendev.org/#/c/667177/8/nova/compute/manager.py@4573\n[2] https://review.opendev.org/#/c/667177/8/nova/compute/manager.py@4605\n[2] https://review.opendev.org/#/c/667177/8/nova/compute/manager.py@4727\n\nPS: looks like we never clear the migration context in _init_instance() (either with apply() or drop()) - is that a bug?\n\n\u003clater\u003e Ah I see you noticed that too in your last para.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0a2518db63b67277df633095955322d692d507a3","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_b44aa97d","line":9175,"in_reply_to":"7faddb67_b1607b34","updated":"2019-07-05 22:56:23.000000000","message":"so if i am parsing that correctly we can unconditionally assume the migration_context is set as it would be an error if it wasnt because at best we would not wait for the events which would be a bug.\n\nthis is one of those case were i feel like it would be nice to have an assert at the top of the function that enforces that precondition but sicne i only found one assert not in tests https://github.com/openstack/nova/blob/5c528df45de7f638c4d167fe6bce432552126c48/nova/virt/xenapi/fake.py#L698\n\nim not going to advocate breaking with the rest of the coding style and adding it.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"051c80a06b82558500e940290b0cd532b3ba4ce9","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_6248e1cb","line":9175,"in_reply_to":"7faddb67_b44aa97d","updated":"2019-07-08 14:59:15.000000000","message":"For the revert resize case yes the migration_context will be set.\n\nFor the compute service restart case I linked, I think the migration context would still be set but I doubt we have anything testing that functionally (unit tests are just going to mock out the driver call from the compute manager). Frankly I don\u0027t even know if that code to revert the failed resize on restart of the compute service even works anymore - it\u0027s best effort anyway. Someone would have to test it in devstack or something to see if it still works (maybe stub out migrate_disk_and_power_off to sleep indefinitely and then kill the compute service while it\u0027s sleeping in the driver method). At the very least I could add a functional test to hit that code but it would just be using the fake driver so I\u0027m not sure how much help that is (probably better than unit tests though).\n\nAnyway, I think right now we can assume the instance has a migration_context set by just looking at the code. A functional test for the restart + crashed behavior would maybe also help - but that would probably be a separate patch. If the migration_context isn\u0027t set in here I think we should be logging an error and likely not waiting for events since we don\u0027t really know if we should wait, right?","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b24d7d67f6634e36b231a0891000cdf844400e54","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_ecbb22c1","line":9175,"in_reply_to":"7faddb67_dbc15b82","updated":"2019-07-11 20:01:28.000000000","message":"\u003e This is indeed a fairly small window, and we only apply the migration context way later in _finish_resize().\n\nI\u0027m not sure why this matters. finish_resize happens on the dest host when, well, finishing the initial resize operation (it goes prep_resize (dest) -\u003e resize_instance (source) -\u003e finish_resize (dest)) and then the instance is in VERIFY_RESIZE status. What you care about is the revert resize flow and when the migration context is *dropped* and that happens on the source host after calling finish_revert_migration on the driver.\n\nThe migration_context is set on the instance object during the resize_claim in the resource tracker (which is called from prep_resize):\n\nhttps://github.com/openstack/nova/blob/ff0feed25d56c8ccd2298d5b5b82e636880fa986/nova/compute/manager.py#L4391\n\nhttps://github.com/openstack/nova/blob/ff0feed25d56c8ccd2298d5b5b82e636880fa986/nova/compute/resource_tracker.py#L346\n\n\u003e PS: looks like we never clear the migration context in _init_instance() (either with apply() or drop()) - is that a bug?\n\nYeah, I\u0027m not sure. The instance migration context after that might not hurt anything but I don\u0027t think it\u0027s used for anything either - maybe it would screw up routing network events in the API? There are likely other things we\u0027re not cleaning up there either, like the old vm_state stashed in the system_metadata or the new_flavor (both of which get set in prep_resize):\n\nhttps://github.com/openstack/nova/blob/ff0feed25d56c8ccd2298d5b5b82e636880fa986/nova/compute/manager.py#L4388\n\nRegardless of all that, I think it would be good to at least have a functional test hit that recovery scenario since it\u0027s fairly complicated. I think I\u0027ll throw a follow up patch on top of this to add that test.","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6460d623c9bb310c9122461e094a7265b5ebd4c2","unresolved":false,"context_lines":[{"line_number":9172,"context_line":"        # the port binding. For any ports not covered by those \"bind-time\""},{"line_number":9173,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9174,"context_line":"        migration \u003d None"},{"line_number":9175,"context_line":"        if instance.obj_attr_is_set(\u0027migration_context\u0027):"},{"line_number":9176,"context_line":"            migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9177,"context_line":"                context, instance.migration_context.migration_id,"},{"line_number":9178,"context_line":"                instance.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_4c2596c1","line":9175,"in_reply_to":"7faddb67_ecbb22c1","updated":"2019-07-11 20:20:56.000000000","message":"\u003e There are likely other things we\u0027re not cleaning up there either, like the old vm_state stashed in the system_metadata or the new_flavor (both of which get set in prep_resize):\n\nOh and likely the allocations (swapped by conductor, created by scheduler) are getting leaked in placement...","commit_id":"ee7880c1d91ca559b5fbdebdeac05f01cc044b1c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b24d7d67f6634e36b231a0891000cdf844400e54","unresolved":false,"context_lines":[{"line_number":9186,"context_line":"        # events, we wait for \"plug-time\" events here."},{"line_number":9187,"context_line":"        # TODO(artom) This DB lookup is done for backportability. A subsequent"},{"line_number":9188,"context_line":"        # patch will remove it and change the finish_revert_migration() method"},{"line_number":9189,"context_line":"        # signature to pass is the migration object."},{"line_number":9190,"context_line":"        migration \u003d objects.Migration.get_by_id_and_instance("},{"line_number":9191,"context_line":"            context, instance.migration_context.migration_id, instance.uuid)"},{"line_number":9192,"context_line":"        events \u003d network_info.get_plug_time_events(migration)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7faddb67_6cc73247","line":9189,"range":{"start_line":9189,"start_character":28,"end_line":9189,"end_character":30},"updated":"2019-07-11 20:01:28.000000000","message":"in","commit_id":"7a7a223602ca5aa0aca8f65a6ab143f1d8f8ec1b"}],"releasenotes/notes/finish-revert-migration-method-signature-c7ebf4a878757238.yaml":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ba878b0ae497d2132aff7e784db88b521cbc3034","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    Virt drivers\u0027s \u0027finish_revert_migration\u0027 method signature now accepts a new"},{"line_number":5,"context_line":"    keyword argument, \u0027migration\u0027. This is a \u0027Migration\u0027 object that the"},{"line_number":6,"context_line":"    libvirt driver uses as part of its decision on what external events it"},{"line_number":7,"context_line":"    needs to wait for. If you have out of tree virt drivers, please update"},{"line_number":8,"context_line":"    them."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"9fb8cfa7_b0c2f0e7","line":8,"range":{"start_line":7,"start_character":23,"end_line":8,"end_character":9},"updated":"2019-07-01 18:11:45.000000000","message":"so we will need to mail people on the list assuming we proceed with this to let them know but we also should handel this gracefully in the manager and not pass the migration if they dont support it.","commit_id":"6d5832c65e5784394bbcc2d38164af45c965082e"}]}