)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Revert resize: wait for events according to hybrid plug"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when reverting a resized instance back to the source host,"},{"line_number":10,"context_line":"the libvirt driver waits for vif-plugged events when spawning the"},{"line_number":11,"context_line":"instance. If Neutron is configured to use OVS hybrid plug, it will"},{"line_number":12,"context_line":"send the vif-plugged event as soon as the Nova compute manager updates"},{"line_number":13,"context_line":"the port binding to point to the source host. This causes the wait in"},{"line_number":14,"context_line":"the libvirt driver to time out because the event is received before"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"9fb8cfa7_870e398b","line":11,"range":{"start_line":10,"start_character":0,"end_line":11,"end_character":9},"updated":"2019-06-05 19:21:15.000000000","message":"It would help if you were clear about which driver method is doing this (finish_revert_migration called from finish_revert_resize on the source host - and that wait behavior is new as of Id515137747a4b76e9b7057c95f80c8ae74017519). Referencing the history on this will help with loading context for the review and potentially eventual revert when we find out this breaks some exotic neutron backend - see the revert story on https://review.opendev.org/#/c/553035/ as an example of how bad we are at this.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1654a2c9b8a0d5621f134c0f48583cee495784c7","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Revert resize: wait for events according to hybrid plug"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when reverting a resized instance back to the source host,"},{"line_number":10,"context_line":"the libvirt driver waits for vif-plugged events when spawning the"},{"line_number":11,"context_line":"instance. If Neutron is configured to use OVS hybrid plug, it will"},{"line_number":12,"context_line":"send the vif-plugged event as soon as the Nova compute manager updates"},{"line_number":13,"context_line":"the port binding to point to the source host. This causes the wait in"},{"line_number":14,"context_line":"the libvirt driver to time out because the event is received before"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"9fb8cfa7_d244c41d","line":11,"range":{"start_line":10,"start_character":0,"end_line":11,"end_character":9},"in_reply_to":"9fb8cfa7_870e398b","updated":"2019-06-07 15:55:52.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently, when reverting a resized instance back to the source host,"},{"line_number":10,"context_line":"the libvirt driver waits for vif-plugged events when spawning the"},{"line_number":11,"context_line":"instance. If Neutron is configured to use OVS hybrid plug, it will"},{"line_number":12,"context_line":"send the vif-plugged event as soon as the Nova compute manager updates"},{"line_number":13,"context_line":"the port binding to point to the source host. This causes the wait in"},{"line_number":14,"context_line":"the libvirt driver to time out because the event is received before"},{"line_number":15,"context_line":"Nova starts waiting for it. This patch checks the value of"},{"line_number":16,"context_line":"ovs_hybrid_plug in the port details to decide when to wait for the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"9fb8cfa7_878399cc","line":13,"range":{"start_line":12,"start_character":27,"end_line":13,"end_character":45},"updated":"2019-06-05 19:21:15.000000000","message":"And where does this happen? Oh I wish you\u0027d tell me so I don\u0027t have to grok all the code to figure it out first. That\u0027s the migrate_instance_finish call from finish_revert_resize, right? So we\u0027re triggering the event potentially before the driver\u0027s finish_revert_migration method is waiting for it. Good details to have in here.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1654a2c9b8a0d5621f134c0f48583cee495784c7","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Currently, when reverting a resized instance back to the source host,"},{"line_number":10,"context_line":"the libvirt driver waits for vif-plugged events when spawning the"},{"line_number":11,"context_line":"instance. If Neutron is configured to use OVS hybrid plug, it will"},{"line_number":12,"context_line":"send the vif-plugged event as soon as the Nova compute manager updates"},{"line_number":13,"context_line":"the port binding to point to the source host. This causes the wait in"},{"line_number":14,"context_line":"the libvirt driver to time out because the event is received before"},{"line_number":15,"context_line":"Nova starts waiting for it. This patch checks the value of"},{"line_number":16,"context_line":"ovs_hybrid_plug in the port details to decide when to wait for the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"9fb8cfa7_b241100e","line":13,"range":{"start_line":12,"start_character":27,"end_line":13,"end_character":45},"in_reply_to":"9fb8cfa7_878399cc","updated":"2019-06-07 15:55:52.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d796635236d8a24286f9ed3f37051051ec100f7","unresolved":false,"context_lines":[{"line_number":16,"context_line":"ovs_hybrid_plug in the port details to decide when to wait for the"},{"line_number":17,"context_line":"vif-plugged event. If hybrid plug is in use, wait in the compute"},{"line_number":18,"context_line":"manager. Otherwise, wait in the libvirt driver."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Partial-bug: 1813789"},{"line_number":21,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"9fb8cfa7_f1b8eac8","line":19,"updated":"2019-06-07 14:17:05.000000000","message":"Since we modify the virt driver API, I\u0027d love to see a release note mentioning the change (fortunately, that\u0027s a defaulted parameter) so that out-of-tree virt driver maintainers would know about this and adapt if they want.","commit_id":"05937e582783ef9cd727c8e4565203df52dd7cc4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1654a2c9b8a0d5621f134c0f48583cee495784c7","unresolved":false,"context_lines":[{"line_number":16,"context_line":"ovs_hybrid_plug in the port details to decide when to wait for the"},{"line_number":17,"context_line":"vif-plugged event. If hybrid plug is in use, wait in the compute"},{"line_number":18,"context_line":"manager. Otherwise, wait in the libvirt driver."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Partial-bug: 1813789"},{"line_number":21,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"9fb8cfa7_1766ce52","line":19,"in_reply_to":"9fb8cfa7_2c9f0960","updated":"2019-06-07 15:55:52.000000000","message":"Done","commit_id":"05937e582783ef9cd727c8e4565203df52dd7cc4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"53a02be091b215746922617ba2da232bf7781c71","unresolved":false,"context_lines":[{"line_number":16,"context_line":"ovs_hybrid_plug in the port details to decide when to wait for the"},{"line_number":17,"context_line":"vif-plugged event. If hybrid plug is in use, wait in the compute"},{"line_number":18,"context_line":"manager. Otherwise, wait in the libvirt driver."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Partial-bug: 1813789"},{"line_number":21,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"9fb8cfa7_2c9f0960","line":19,"in_reply_to":"9fb8cfa7_f1b8eac8","updated":"2019-06-07 14:40:04.000000000","message":"(9:37:33 AM) mriedem: oh right you want to backport that....\n(9:37:40 AM) mriedem: i was going to say we don\u0027t need a release note for a virt driver interface change,\n(9:37:44 AM) mriedem: but if we\u0027re backporting...\n(9:38:13 AM) ***artom shall buy a spot on CNN to make sure the message goes out ;)\n(9:38:20 AM) mriedem: we did this recently for an ironic driver related fix and handled a TypeError in the backportable version and removed it in a follow up on master\n(9:39:06 AM) mriedem: https://github.com/openstack/nova/blob/stable/rocky/nova/compute/manager.py#L7648\n(9:39:15 AM) mriedem: given that, we may want to follow the same pattern here\n(9:39:32 AM) mriedem: it is admittedly bending over backward for out of tree drivers which is being exceedingly nice","commit_id":"05937e582783ef9cd727c8e4565203df52dd7cc4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1654a2c9b8a0d5621f134c0f48583cee495784c7","unresolved":false,"context_lines":[{"line_number":16,"context_line":"ovs_hybrid_plug in the port details to decide when to wait for the"},{"line_number":17,"context_line":"vif-plugged event. If hybrid plug is in use, wait in the compute"},{"line_number":18,"context_line":"manager. Otherwise, wait in the libvirt driver."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Partial-bug: 1813789"},{"line_number":21,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"9fb8cfa7_f7621a3e","line":19,"in_reply_to":"9fb8cfa7_f1b8eac8","updated":"2019-06-07 15:55:52.000000000","message":"Done","commit_id":"05937e582783ef9cd727c8e4565203df52dd7cc4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10f7c1dd189cddb8506d246f3edbee909b8553a5","unresolved":false,"context_lines":[{"line_number":28,"context_line":"use, wait in the compute manager. Otherwise, wait in the libvirt"},{"line_number":29,"context_line":"driver."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Partial-bug: 1813789"},{"line_number":32,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"9fb8cfa7_865b7152","line":31,"updated":"2019-06-07 16:54:31.000000000","message":"This isn\u0027t really the correct bug. This bug is about evacuate:\n\nhttp://status.openstack.org/elastic-recheck/#1813789\n\nYour working on a revert resize issue with a specific type of vif, so you likely need a separate bug for that. At the very least it would be related to bug 1788403.","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"7fb2ed584df1e7a8402d9299e6ae2682a8451d31","unresolved":false,"context_lines":[{"line_number":28,"context_line":"use, wait in the compute manager. Otherwise, wait in the libvirt"},{"line_number":29,"context_line":"driver."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Partial-bug: 1813789"},{"line_number":32,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"9fb8cfa7_7c6fea91","line":31,"in_reply_to":"9fb8cfa7_865b7152","updated":"2019-06-07 18:01:51.000000000","message":"Done","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":18,"context_line":"network_api.migrate_instance_finish() in finish_revert_resize(), this"},{"line_number":19,"context_line":"updates the port binding back to the source host. If Neutron is"},{"line_number":20,"context_line":"configured to use OVS hybrid plug, it will send the vif-plugged event"},{"line_number":21,"context_line":"immediately after completing this request. This happens before the"},{"line_number":22,"context_line":"virt driver\u0027s finish_revert_migration() method is called. This causes"},{"line_number":23,"context_line":"the wait in the libvirt driver to time out because the event is"},{"line_number":24,"context_line":"received before Nova starts waiting for it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"This patch checks the value of ovs_hybrid_plug in the port details to"},{"line_number":27,"context_line":"decide when to wait for the vif-plugged event.  If hybrid plug is in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"9fb8cfa7_6ce2c5a2","line":24,"range":{"start_line":21,"start_character":43,"end_line":24,"end_character":43},"updated":"2019-06-10 16:00:29.000000000","message":"Well, it\u0027s a race, but sure - you\u0027re describing when we lose the race, right? You don\u0027t need to change this really, I\u0027m just thinking out loud.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":27,"context_line":"decide when to wait for the vif-plugged event.  If hybrid plug is in"},{"line_number":28,"context_line":"use, wait in the compute manager. Otherwise, wait in the libvirt"},{"line_number":29,"context_line":"driver."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Closes-bug: 1832028"},{"line_number":32,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":32,"id":"9fb8cfa7_6f3780e5","line":30,"updated":"2019-06-18 20:23:42.000000000","message":"Since I last looked at this, and brought up the issue of different types of ports being attached to the server potentially screwing up when/where we wait for the events, you\u0027ve added the bind/plug stuff but not mentioned any of that here - it would be good information to have in the commit message since that is slightly complicated.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":27,"context_line":"decide when to wait for the vif-plugged event.  If hybrid plug is in"},{"line_number":28,"context_line":"use, wait in the compute manager. Otherwise, wait in the libvirt"},{"line_number":29,"context_line":"driver."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Closes-bug: 1832028"},{"line_number":32,"context_line":"Change-Id: I51cdcae67be8c68a55bc939de4ea0aba2361dcc4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":32,"id":"9fb8cfa7_94c43794","line":30,"in_reply_to":"9fb8cfa7_6f3780e5","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":26,"context_line":"Events sent immediately after port binding update are hereafter known"},{"line_number":27,"context_line":"as \"bind-time\" events. For ports that do not use OVS hybrid plug,"},{"line_number":28,"context_line":"Neutron will continue to send vif-plugged events only when Nova"},{"line_number":29,"context_line":"actually plugs the VIF. This type of events is hereafter known as"},{"line_number":30,"context_line":"\"plug-time\" events. OVS hybrid plug is a Neutron-wide setting, so for"},{"line_number":31,"context_line":"a particular deployment, bind-time events are an all-or-nothing thing:"},{"line_number":32,"context_line":"either all ports have them, or no ports have them. The only exception"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_28640cb9","line":29,"range":{"start_line":29,"start_character":24,"end_line":29,"end_character":33},"updated":"2019-06-19 18:41:08.000000000","message":"These types","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"as \"bind-time\" events. For ports that do not use OVS hybrid plug,"},{"line_number":28,"context_line":"Neutron will continue to send vif-plugged events only when Nova"},{"line_number":29,"context_line":"actually plugs the VIF. This type of events is hereafter known as"},{"line_number":30,"context_line":"\"plug-time\" events. OVS hybrid plug is a Neutron-wide setting, so for"},{"line_number":31,"context_line":"a particular deployment, bind-time events are an all-or-nothing thing:"},{"line_number":32,"context_line":"either all ports have them, or no ports have them. The only exception"},{"line_number":33,"context_line":"is direct physical ports (ie, SRIOV). They will always have only"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_68fe84af","line":30,"range":{"start_line":30,"start_character":20,"end_line":30,"end_character":61},"updated":"2019-06-19 18:41:08.000000000","message":"It\u0027s not per neutron agent? For example I couldn\u0027t have some host aggregates with ovs hybrid plug agents and some host aggregates with linuxbridge?","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"714881726b29b8d923427ec28d8dbb78bde7f743","unresolved":false,"context_lines":[{"line_number":27,"context_line":"as \"bind-time\" events. For ports that do not use OVS hybrid plug,"},{"line_number":28,"context_line":"Neutron will continue to send vif-plugged events only when Nova"},{"line_number":29,"context_line":"actually plugs the VIF. This type of events is hereafter known as"},{"line_number":30,"context_line":"\"plug-time\" events. OVS hybrid plug is a Neutron-wide setting, so for"},{"line_number":31,"context_line":"a particular deployment, bind-time events are an all-or-nothing thing:"},{"line_number":32,"context_line":"either all ports have them, or no ports have them. The only exception"},{"line_number":33,"context_line":"is direct physical ports (ie, SRIOV). They will always have only"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_5545546f","line":30,"range":{"start_line":30,"start_character":20,"end_line":30,"end_character":61},"in_reply_to":"9fb8cfa7_68fe84af","updated":"2019-06-19 22:04:54.000000000","message":"yes it is per agent.\n\na very long time ago you could config only set this on the neutron server but as of pike \nhttps://github.com/openstack/neutron/commit/2f17a30ba04082889f3a703aca1884b031767942\n\nthis has been configuratle per agent.\n\non startup and periodically as it running the ovs neutron agent, send an agent report as a heartbeat which is stored\nin the neutron database.\n\nthe report contains an agent_sate dictionay that\npasses relevent information form the agents local config\n\nhttps://github.com/openstack/neutron/blob/a832a1d1b30c2eb4ab3d71fc793da766b83ec6de/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L290-L316\n\nso while the neutron ml2 driver still default initallised hybrid plug based on the neutron server config.\n\nhttps://github.com/openstack/neutron/blob/a832a1d1b30c2eb4ab3d71fc793da766b83ec6de/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#L59-L63\n\nif the firewall driver is defiend in the agents config it will be set hybrid plug in the agent configoutation dict and that value will be used to override the neutron servers default\n\nhttps://github.com/openstack/neutron/blob/a832a1d1b30c2eb4ab3d71fc793da766b83ec6de/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#L186-L189","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"a particular deployment, bind-time events are an all-or-nothing thing:"},{"line_number":32,"context_line":"either all ports have them, or no ports have them. The only exception"},{"line_number":33,"context_line":"is direct physical ports (ie, SRIOV). They will always have only"},{"line_number":34,"context_line":"bind-time events. If an instances mixes OVS hybrid-plugged ports with"},{"line_number":35,"context_line":"direct physical ports, it will have both kinds of events."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"This patch adds functions to the NetworkInfo model that return what"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_28096cdc","line":34,"range":{"start_line":34,"start_character":0,"end_line":34,"end_character":9},"updated":"2019-06-19 18:41:08.000000000","message":"I think you mean plug-time here...","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":31,"context_line":"a particular deployment, bind-time events are an all-or-nothing thing:"},{"line_number":32,"context_line":"either all ports have them, or no ports have them. The only exception"},{"line_number":33,"context_line":"is direct physical ports (ie, SRIOV). They will always have only"},{"line_number":34,"context_line":"bind-time events. If an instances mixes OVS hybrid-plugged ports with"},{"line_number":35,"context_line":"direct physical ports, it will have both kinds of events."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"This patch adds functions to the NetworkInfo model that return what"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_082d8845","line":34,"range":{"start_line":34,"start_character":24,"end_line":34,"end_character":33},"updated":"2019-06-19 18:41:08.000000000","message":"instance","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"727018b53f883729568ede4695be06dc47758ebb","unresolved":false,"context_lines":[{"line_number":31,"context_line":"a particular deployment, bind-time events are an all-or-nothing thing:"},{"line_number":32,"context_line":"either all ports have them, or no ports have them. The only exception"},{"line_number":33,"context_line":"is direct physical ports (ie, SRIOV). They will always have only"},{"line_number":34,"context_line":"bind-time events. If an instances mixes OVS hybrid-plugged ports with"},{"line_number":35,"context_line":"direct physical ports, it will have both kinds of events."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"This patch adds functions to the NetworkInfo model that return what"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_43391e3d","line":34,"range":{"start_line":34,"start_character":0,"end_line":34,"end_character":9},"in_reply_to":"9fb8cfa7_28096cdc","updated":"2019-06-20 00:21:11.000000000","message":"nope the neutron sriov nic agent does not suppor vnic_type\u003ddirect-physical.\nhttps://github.com/openstack/neutron/blob/a832a1d1b30c2eb4ab3d71fc793da766b83ec6de/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py#L131-L143\n\nso when we bind the port it pass None for the agent\nto try_to_bind_segment_for_agent\nwhich sets the port-status to active immediately.\n\nhttps://github.com/openstack/neutron/blob/a832a1d1b30c2eb4ab3d71fc793da766b83ec6de/neutron/plugins/ml2/drivers/mech_sriov/mech_driver/mech_driver.py#L163-L168\n\nsetting the port status to active is what actully trigger neutron to send the vif-plugged event\n\nhttps://github.com/openstack/neutron/blob/a832a1d1b30c2eb4ab3d71fc793da766b83ec6de/neutron/notifiers/nova.py#L182-L219\n\nthe mixed case is when you use port of vnic-type\u003ddirect-physical with ports of hybrid-plug\u003dfalse.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":38,"context_line":"kinds of events each VIF has. These are then used in the migration"},{"line_number":39,"context_line":"revert logic to decide when to wait for external events: in the"},{"line_number":40,"context_line":"compute manager, right after port binding update, for bind-time"},{"line_number":41,"context_line":"events, and/or in libvirt, right after plugging the VIFs, for"},{"line_number":42,"context_line":"plug-time events."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Closes-bug: 1832028"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"9fb8cfa7_2832cca8","line":41,"range":{"start_line":41,"start_character":18,"end_line":41,"end_character":25},"updated":"2019-06-19 18:41:08.000000000","message":"the libvirt driver","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"29fa8570a1fbb24bfdfcf99d8fbd8caecc69f154","unresolved":false,"context_lines":[{"line_number":55,"context_line":"kinds of events each VIF has. These are then used in the migration"},{"line_number":56,"context_line":"revert logic to decide when to wait for external events: in the"},{"line_number":57,"context_line":"compute manager, when binding the port, for bind-time events,"},{"line_number":58,"context_line":"and/or in libvirt, when plugging the VIFs, for plug-time events."},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"Closes-bug: 1832028"},{"line_number":61,"context_line":"Co-Authored-By: Sean Mooney work@seanmooney.info"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":37,"id":"9fb8cfa7_f51b1619","line":58,"range":{"start_line":58,"start_character":10,"end_line":58,"end_character":17},"updated":"2019-06-20 17:56:30.000000000","message":"nit: the libvirt driver","commit_id":"19f9b37721d9bc13bc1ed35a4f368b1d21b10a5b"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"321d1a9e66c8ce0caa7ba388e95f3d05ac8d30e5","unresolved":false,"context_lines":[{"line_number":4148,"context_line":"        :raises: eventlet.timeout.Timeout or"},{"line_number":4149,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4150,"context_line":"        \"\"\""},{"line_number":4151,"context_line":"        LOG.debug(\u0027artom: migrate_finish_and_wait start\u0027)"},{"line_number":4152,"context_line":"        events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4153,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4154,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"}],"source_content_type":"text/x-python","patch_set":15,"id":"bfb3d3c7_a1afcd92","line":4151,"range":{"start_line":4151,"start_character":8,"end_line":4151,"end_character":57},"updated":"2019-05-20 14:19:08.000000000","message":"So are you going to mark the change WIP in the commit title or what?","commit_id":"5f4f623f7b6de97bcbc62abefe3f1e6a426abf46"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f2e7d0eaab721a71a525dc51aa9c2e797ec6781d","unresolved":false,"context_lines":[{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"},{"line_number":4180,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4181,"context_line":"        If hybrid plug is not used, the event is expected further along in the"}],"source_content_type":"text/x-python","patch_set":18,"id":"bfb3d3c7_d4da2476","line":4178,"range":{"start_line":4178,"start_character":8,"end_line":4178,"end_character":30},"updated":"2019-05-29 14:16:27.000000000","message":"maybe you should add a \n\nTODO(sean-k-mooney): revisit this as part of fixing bug\nhttps://bugs.launchpad.net/neutron/+bug/1734320\nand\nhttps://bugs.launchpad.net/neutron/+bug/1815989\n\nhttps://review.opendev.org/#/c/602432/ will mean that\nlibvirt will no longer plug the vif so this might change the behavior. that said it wont change it for the hybrid_plug\u003dTrue case which is what starts the waiting here so perhaps that is not needed on second thought.\n\nlater\n-----\n\nya i would only be modifying the behavior of the false case so this so that change should have no effect on this logic since it only waits if if hybrid_plug\u003dtrue.","commit_id":"52386bfa324445d9aaeb114d02a89f8101c31e19"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"97b2d3b1bee8fd6078f20a17fc34118a4c0b2970","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        return any(["},{"line_number":4174,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_2925e9c2","line":4176,"updated":"2019-06-05 13:53:27.000000000","message":"Doesn\u0027t this also always return False for anything that is not OVS? That may be correct for the moment, I\u0027m not sure, but I think that needs to be confirmed, and a comment placed here for posterity in case some other backend wants to use eventing ever and we just skip it here on principle.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        return any(["},{"line_number":4174,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_d8097495","line":4176,"in_reply_to":"9fb8cfa7_2925e9c2","updated":"2019-06-05 23:26:34.000000000","message":"Yeah, this\u0027ll only return True if and only if OVS with hybrid plug is in use, which to my knowledge is currently the only case when we need to be ready to receive an event immediately after updating the port binding.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0bdba5e353045fec1b6f4c1a12535544ddd56251","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        return any(["},{"line_number":4174,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_6eea0276","line":4176,"in_reply_to":"9fb8cfa7_2925e9c2","updated":"2019-06-05 16:37:58.000000000","message":"hybrid plug is specific to ovs. ovs hooks into the kernel networking stack before the packet reach the tc/iptables filtering so hybrid plug was developed to enable iptables firewalling to work with ovs.  i dont think there is any other backend that sets this. there may be other cases where we want to wait or however. but the current code never waits here and always waits on the compute node so if there are other cases that currenly race this change does not fix that behavior but it also should not break previously working backends.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2ebeea8df0444b477b3a5de48c7cb6a9d705d591","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"        return any(["},{"line_number":4174,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_b12e1b41","line":4176,"in_reply_to":"9fb8cfa7_6eea0276","updated":"2019-06-05 17:00:30.000000000","message":"Okay, I think I was reversing the logic here. We\u0027ll return False for anything else, which will continue waiting for the events as in the past. So, yeah, nevermind.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"},{"line_number":4180,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4181,"context_line":"        If hybrid plug is not used, the event is expected further along in the"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_c7f0b16a","line":4178,"updated":"2019-06-05 19:21:15.000000000","message":"If you\u0027re going to leave this as-is, I\u0027d make it (1) private and (2) rename it to _finish_revert_resize_network_migrate_finish because as noted elsewhere, this same pattern shows up in other methods on the dest host and this method is extremely tightly coupled to the finish_revert_resize (on the source host) flow.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4175,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4176,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4177,"context_line":""},{"line_number":4178,"context_line":"    def network_migrate_finish(self, context, instance, migration):"},{"line_number":4179,"context_line":"        \"\"\"Causes port binding to be updated. If OVS hybrid plug is used, we"},{"line_number":4180,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4181,"context_line":"        If hybrid plug is not used, the event is expected further along in the"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_c2765fa5","line":4178,"in_reply_to":"9fb8cfa7_c7f0b16a","updated":"2019-06-05 23:26:34.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c41f7a29d14885807c2024763f0be19379f7e20e","unresolved":false,"context_lines":[{"line_number":4189,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_49793d2d","line":4192,"updated":"2019-06-05 13:49:21.000000000","message":"Is this something specific to migrate or is this a filter we should be applying to other cases, like spawn?","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4189,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_184b8cb0","line":4192,"in_reply_to":"9fb8cfa7_49793d2d","updated":"2019-06-05 23:26:34.000000000","message":"Specific to revert resize with hybrid plug in fact, because the instance remains destroyed (shut off) with the VIFs wired up on the source until we confirm the resize (or revert back to the source).","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0bdba5e353045fec1b6f4c1a12535544ddd56251","unresolved":false,"context_lines":[{"line_number":4189,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_ee2112fd","line":4192,"in_reply_to":"9fb8cfa7_49793d2d","updated":"2019-06-05 16:37:58.000000000","message":"for spawn this is not an issue so this is specific to cold migration and resize.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"01944e211f1ec062e06a9badf85d0c5e55a2ee3c","unresolved":false,"context_lines":[{"line_number":4189,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_a7733db1","line":4192,"in_reply_to":"9fb8cfa7_511d5f0d","updated":"2019-06-05 18:29:49.000000000","message":"soft reboot shoudl be fine.\n\nhard reboot we currently dont wait\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2945-L2952\nbeacues of linux bridge and the fact it does not relaible send events as it might miss the unplug and plug due to it polling interval.\n\nrebuild is a little more interesting. i think it would have the same issue as hard reboot. i dont know if we currently wait for events but i dont think we can due to linux bridge or other backend that only send event on port bind.\n\nunshelve from shelve offloaded effectivly works like spawn since there is no source node and therefore we cant race with a backend that is already set up.\n\nwe should teardown the host networking as part of shelve even before we offload so i think unshelve is fine even when  we have not offloaded yet. im at leat 90% sure on that.\n\nfinally for evacuate this would only be an issue for evacuate if we did a revert and i did not think that was something that could happen for evacuate?","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2ebeea8df0444b477b3a5de48c7cb6a9d705d591","unresolved":false,"context_lines":[{"line_number":4189,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_511d5f0d","line":4192,"in_reply_to":"9fb8cfa7_ee2112fd","updated":"2019-06-05 17:00:30.000000000","message":"But not reboot or rebuild? Unshelve and Evacuate are kinda like a migration, but without the temporal locality of a source and destination node working together (at the same time).","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c41f7a29d14885807c2024763f0be19379f7e20e","unresolved":false,"context_lines":[{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4196,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_09a7c551","line":4193,"updated":"2019-06-05 13:49:21.000000000","message":"Maybe I missed it, but I don\u0027t see anywhere that you actually check that this method calls the _uses_hybrid_plug() check, or that generating these events are conditional on its output. I see that you mock it out in a bunch of places (always false, BTW) and that you unit-test the actual helper, but not that you confirm that they work together and provide the behavior you\u0027re looking for.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4190,"context_line":"        \"\"\""},{"line_number":4191,"context_line":"        events \u003d []"},{"line_number":4192,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4193,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4194,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4195,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4196,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_98d45cbf","line":4193,"in_reply_to":"9fb8cfa7_09a7c551","updated":"2019-06-05 23:26:34.000000000","message":"Yeah, Matt also noted that I\u0027m missing a positive test. Will add.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5febd1b1a2df020c3bafcc9e832d199f353339bd","unresolved":false,"context_lines":[{"line_number":4197,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"},{"line_number":4198,"context_line":"                                                      deadline\u003ddeadline,"},{"line_number":4199,"context_line":"                                                      error_callback\u003derror_cb):"},{"line_number":4200,"context_line":"                self.network_api.setup_networks_on_host("},{"line_number":4201,"context_line":"                    context, instance, migration.source_compute)"},{"line_number":4202,"context_line":"                # NOTE(hanrong): we need to change migration_p[\u0027dest_compute\u0027]"},{"line_number":4203,"context_line":"                # to source host temporarily."},{"line_number":4204,"context_line":"                # \"network_api.migrate_instance_finish\" will setup the network"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_a331fa0d","line":4201,"range":{"start_line":4200,"start_character":0,"end_line":4201,"end_character":64},"updated":"2019-06-05 12:53:01.000000000","message":"This doesn\u0027t need to be here, as it doesn\u0027t generate the target event. It can be outside the wait scope.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4197,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"},{"line_number":4198,"context_line":"                                                      deadline\u003ddeadline,"},{"line_number":4199,"context_line":"                                                      error_callback\u003derror_cb):"},{"line_number":4200,"context_line":"                self.network_api.setup_networks_on_host("},{"line_number":4201,"context_line":"                    context, instance, migration.source_compute)"},{"line_number":4202,"context_line":"                # NOTE(hanrong): we need to change migration_p[\u0027dest_compute\u0027]"},{"line_number":4203,"context_line":"                # to source host temporarily."},{"line_number":4204,"context_line":"                # \"network_api.migrate_instance_finish\" will setup the network"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_0292379f","line":4201,"range":{"start_line":4200,"start_character":0,"end_line":4201,"end_character":64},"in_reply_to":"9fb8cfa7_274bcdb0","updated":"2019-06-05 23:26:34.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4197,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"},{"line_number":4198,"context_line":"                                                      deadline\u003ddeadline,"},{"line_number":4199,"context_line":"                                                      error_callback\u003derror_cb):"},{"line_number":4200,"context_line":"                self.network_api.setup_networks_on_host("},{"line_number":4201,"context_line":"                    context, instance, migration.source_compute)"},{"line_number":4202,"context_line":"                # NOTE(hanrong): we need to change migration_p[\u0027dest_compute\u0027]"},{"line_number":4203,"context_line":"                # to source host temporarily."},{"line_number":4204,"context_line":"                # \"network_api.migrate_instance_finish\" will setup the network"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_7b84020d","line":4201,"range":{"start_line":4200,"start_character":0,"end_line":4201,"end_character":64},"in_reply_to":"9fb8cfa7_a331fa0d","updated":"2019-06-05 23:26:34.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":4197,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"},{"line_number":4198,"context_line":"                                                      deadline\u003ddeadline,"},{"line_number":4199,"context_line":"                                                      error_callback\u003derror_cb):"},{"line_number":4200,"context_line":"                self.network_api.setup_networks_on_host("},{"line_number":4201,"context_line":"                    context, instance, migration.source_compute)"},{"line_number":4202,"context_line":"                # NOTE(hanrong): we need to change migration_p[\u0027dest_compute\u0027]"},{"line_number":4203,"context_line":"                # to source host temporarily."},{"line_number":4204,"context_line":"                # \"network_api.migrate_instance_finish\" will setup the network"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_274bcdb0","line":4201,"range":{"start_line":4200,"start_character":0,"end_line":4201,"end_character":64},"in_reply_to":"9fb8cfa7_a331fa0d","updated":"2019-06-05 19:21:15.000000000","message":"I wouldn\u0027t even have this in this method because this method is making an assumption about which compute to use (migration.source_compute) and it could be confused with other methods calling these same networking methods like _finish_resize. If you leave it in here, then you should generalize this for other methods like _finish_resize to use and pass in the compute you want to use, not hard-code this to use migration.source_compute.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6c907c2f909a4edbbaebf716cb5912c9fc3b6705","unresolved":false,"context_lines":[{"line_number":4208,"context_line":"                # migration_p[\u0027dest_compute\u0027] to source host at here."},{"line_number":4209,"context_line":"                with utils.temporary_mutation("},{"line_number":4210,"context_line":"                        migration, dest_compute\u003dmigration.source_compute):"},{"line_number":4211,"context_line":"                    self.network_api.migrate_instance_finish(context,"},{"line_number":4212,"context_line":"                                                             instance,"},{"line_number":4213,"context_line":"                                                             migration)"},{"line_number":4214,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4215,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4216,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events %(events)s \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_3e651de2","line":4213,"range":{"start_line":4211,"start_character":20,"end_line":4213,"end_character":71},"updated":"2019-06-05 12:45:12.000000000","message":"just because it was asked on irc this is what actully triggers the network-vif-plugged event to be sent.\ninterally this calls _update_port_binding_for_instance\n\nwhich updated the binding:host_id filed in the port binding details.\n\nhttps://github.com/openstack/nova/blob/f2b58bb20bf5a7b77023d2a2d689631991a8a5e8/nova/network/neutronv2/api.py#L3209\n\nwhen _update_port_binding_for_instance updates each of the ports https://github.com/openstack/nova/blob/f2b58bb20bf5a7b77023d2a2d689631991a8a5e8/nova/network/neutronv2/api.py#L3259\n\nwhen the hybrid plug is true since we have not called os-vif unplug yet via the host clean up on resize success the ovs port, veth pair and linux bridge used for hybrid plug still exist on teh source node. As a result of when the binding host_id is set back to the source host the when the neutron sees that the port is alreay wired up and sends the vif plugged event. when hybrid plug is false os-vif does not plug the vif libvirt does. When the domain was stopped on source node libvirt removed the port form ovs on the souce node. setting the binding:host_id back to the source node does not trigger an event in this case as the ovs port is not recrated until libvirt resumes the domain which happens later in the virt dirver. so this is why the behaivor is different and why we need to take acount of if hybrid plug is used or not.\n\ni have patch that will change this behavior so that os-vif is always responsible for pluggin the vif. if thoes merge we will alway wait here in the future as libvirt will never be plugging the vif and the port will always still be wired on the source host. that is tangential to this change however so i think this is the correct fix for now and is the correct backportable fix in general.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5febd1b1a2df020c3bafcc9e832d199f353339bd","unresolved":false,"context_lines":[{"line_number":4262,"context_line":"                           \u0027migration_uuid\u0027: migration.uuid})"},{"line_number":4263,"context_line":"                raise"},{"line_number":4264,"context_line":""},{"line_number":4265,"context_line":"            self.network_migrate_finish(context, instance, migration)"},{"line_number":4266,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":4267,"context_line":"                                                                 instance)"},{"line_number":4268,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_e32e12a4","line":4265,"range":{"start_line":4265,"start_character":0,"end_line":4265,"end_character":69},"updated":"2019-06-05 12:53:01.000000000","message":"Suggestion: this could return a boolean indicating whether or not vifs were plugged.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4262,"context_line":"                           \u0027migration_uuid\u0027: migration.uuid})"},{"line_number":4263,"context_line":"                raise"},{"line_number":4264,"context_line":""},{"line_number":4265,"context_line":"            self.network_migrate_finish(context, instance, migration)"},{"line_number":4266,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":4267,"context_line":"                                                                 instance)"},{"line_number":4268,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_82f3870a","line":4265,"range":{"start_line":4265,"start_character":0,"end_line":4265,"end_character":69},"in_reply_to":"9fb8cfa7_e32e12a4","updated":"2019-06-05 23:26:34.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5febd1b1a2df020c3bafcc9e832d199f353339bd","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"            self.driver.finish_revert_migration("},{"line_number":4283,"context_line":"                context, instance, network_info\u003dnetwork_info,"},{"line_number":4284,"context_line":"                block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4285,"context_line":"                vifs_already_plugged\u003dself._uses_hybrid_plug(instance))"},{"line_number":4286,"context_line":""},{"line_number":4287,"context_line":"            instance.drop_migration_context()"},{"line_number":4288,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_83171652","line":4285,"range":{"start_line":4285,"start_character":37,"end_line":4285,"end_character":69},"updated":"2019-06-05 12:53:01.000000000","message":"This is coupled to network_migrate_finish(), which is a bit far away and may change in the future. The above suggestion would eliminate this and improve maintainability.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"            self.driver.finish_revert_migration("},{"line_number":4283,"context_line":"                context, instance, network_info\u003dnetwork_info,"},{"line_number":4284,"context_line":"                block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4285,"context_line":"                vifs_already_plugged\u003dself._uses_hybrid_plug(instance))"},{"line_number":4286,"context_line":""},{"line_number":4287,"context_line":"            instance.drop_migration_context()"},{"line_number":4288,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_07fec936","line":4285,"range":{"start_line":4285,"start_character":37,"end_line":4285,"end_character":69},"in_reply_to":"9fb8cfa7_83171652","updated":"2019-06-05 19:21:15.000000000","message":"Agree, the tight coupling is nasty so let\u0027s minimize if possible.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4282,"context_line":"            self.driver.finish_revert_migration("},{"line_number":4283,"context_line":"                context, instance, network_info\u003dnetwork_info,"},{"line_number":4284,"context_line":"                block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4285,"context_line":"                vifs_already_plugged\u003dself._uses_hybrid_plug(instance))"},{"line_number":4286,"context_line":""},{"line_number":4287,"context_line":"            instance.drop_migration_context()"},{"line_number":4288,"context_line":"            instance.launched_at \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_5bcc9e60","line":4285,"range":{"start_line":4285,"start_character":37,"end_line":4285,"end_character":69},"in_reply_to":"9fb8cfa7_83171652","updated":"2019-06-05 23:26:34.000000000","message":"Smart. Done.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":4703,"context_line":"                    break"},{"line_number":4704,"context_line":"        instance.apply_migration_context()"},{"line_number":4705,"context_line":""},{"line_number":4706,"context_line":"        # NOTE(tr3buchet): setup networks on destination host"},{"line_number":4707,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":4708,"context_line":"                                                migration.dest_compute)"},{"line_number":4709,"context_line":"        # For neutron, migrate_instance_finish updates port bindings for this"},{"line_number":4710,"context_line":"        # host including any PCI devices claimed for SR-IOV ports."},{"line_number":4711,"context_line":"        self.network_api.migrate_instance_finish(context,"},{"line_number":4712,"context_line":"                                                 instance,"},{"line_number":4713,"context_line":"                                                 migration)"},{"line_number":4714,"context_line":""},{"line_number":4715,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":4716,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_87d159d0","line":4713,"range":{"start_line":4706,"start_character":8,"end_line":4713,"end_character":59},"updated":"2019-06-05 19:21:15.000000000","message":"Hi do I look familiar?!","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":4703,"context_line":"                    break"},{"line_number":4704,"context_line":"        instance.apply_migration_context()"},{"line_number":4705,"context_line":""},{"line_number":4706,"context_line":"        # NOTE(tr3buchet): setup networks on destination host"},{"line_number":4707,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":4708,"context_line":"                                                migration.dest_compute)"},{"line_number":4709,"context_line":"        # For neutron, migrate_instance_finish updates port bindings for this"},{"line_number":4710,"context_line":"        # host including any PCI devices claimed for SR-IOV ports."},{"line_number":4711,"context_line":"        self.network_api.migrate_instance_finish(context,"},{"line_number":4712,"context_line":"                                                 instance,"},{"line_number":4713,"context_line":"                                                 migration)"},{"line_number":4714,"context_line":""},{"line_number":4715,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":4716,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_a2f6cbf8","line":4713,"range":{"start_line":4706,"start_character":8,"end_line":4713,"end_character":59},"in_reply_to":"9fb8cfa7_87d159d0","updated":"2019-06-05 23:26:34.000000000","message":"hai2u2","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":7041,"context_line":"        LOG.info(\u0027Post operation of migration started\u0027,"},{"line_number":7042,"context_line":"                 instance\u003dinstance)"},{"line_number":7043,"context_line":""},{"line_number":7044,"context_line":"        # NOTE(tr3buchet): setup networks on destination host"},{"line_number":7045,"context_line":"        #                  this is called a second time because"},{"line_number":7046,"context_line":"        #                  multi_host does not create the bridge in"},{"line_number":7047,"context_line":"        #                  plug_vifs"},{"line_number":7048,"context_line":"        # NOTE(mriedem): This is a no-op for neutron."},{"line_number":7049,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":7050,"context_line":"                                                         self.host)"},{"line_number":7051,"context_line":"        migration \u003d {\u0027source_compute\u0027: instance.host,"},{"line_number":7052,"context_line":"                     \u0027dest_compute\u0027: self.host,"},{"line_number":7053,"context_line":"                     \u0027migration_type\u0027: \u0027live-migration\u0027}"},{"line_number":7054,"context_line":"        self.network_api.migrate_instance_finish(context,"},{"line_number":7055,"context_line":"                                                 instance,"},{"line_number":7056,"context_line":"                                                 migration)"},{"line_number":7057,"context_line":""},{"line_number":7058,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":7059,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_a7d49dbe","line":7056,"range":{"start_line":7044,"start_character":8,"end_line":7056,"end_character":59},"updated":"2019-06-05 19:21:15.000000000","message":"Oh hello again!","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":7041,"context_line":"        LOG.info(\u0027Post operation of migration started\u0027,"},{"line_number":7042,"context_line":"                 instance\u003dinstance)"},{"line_number":7043,"context_line":""},{"line_number":7044,"context_line":"        # NOTE(tr3buchet): setup networks on destination host"},{"line_number":7045,"context_line":"        #                  this is called a second time because"},{"line_number":7046,"context_line":"        #                  multi_host does not create the bridge in"},{"line_number":7047,"context_line":"        #                  plug_vifs"},{"line_number":7048,"context_line":"        # NOTE(mriedem): This is a no-op for neutron."},{"line_number":7049,"context_line":"        self.network_api.setup_networks_on_host(context, instance,"},{"line_number":7050,"context_line":"                                                         self.host)"},{"line_number":7051,"context_line":"        migration \u003d {\u0027source_compute\u0027: instance.host,"},{"line_number":7052,"context_line":"                     \u0027dest_compute\u0027: self.host,"},{"line_number":7053,"context_line":"                     \u0027migration_type\u0027: \u0027live-migration\u0027}"},{"line_number":7054,"context_line":"        self.network_api.migrate_instance_finish(context,"},{"line_number":7055,"context_line":"                                                 instance,"},{"line_number":7056,"context_line":"                                                 migration)"},{"line_number":7057,"context_line":""},{"line_number":7058,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":7059,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_02e79742","line":7056,"range":{"start_line":7044,"start_character":8,"end_line":7056,"end_character":59},"in_reply_to":"9fb8cfa7_a7d49dbe","updated":"2019-06-05 23:26:34.000000000","message":"hai3u3","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4acd154139530d5d6cef232f26d280095669e459","unresolved":false,"context_lines":[{"line_number":4228,"context_line":"                    self.network_api.migrate_instance_finish(context,"},{"line_number":4229,"context_line":"                                                             instance,"},{"line_number":4230,"context_line":"                                                             migration)"},{"line_number":4231,"context_line":"            return bool(events)"},{"line_number":4232,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4233,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4234,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events %(events)s \u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fb8cfa7_ad1792a3","line":4231,"range":{"start_line":4231,"start_character":19,"end_line":4231,"end_character":31},"updated":"2019-06-06 10:16:44.000000000","message":"Ewww, but ok.","commit_id":"95fc90852a22b666163aa4b443d61e5c37591bf0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8d796635236d8a24286f9ed3f37051051ec100f7","unresolved":false,"context_lines":[{"line_number":4187,"context_line":""},{"line_number":4188,"context_line":"    def _uses_hybrid_plug(self, instance):"},{"line_number":4189,"context_line":"        return any(["},{"line_number":4190,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4191,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4192,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4193,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"9fb8cfa7_91cc0e62","line":4190,"range":{"start_line":4190,"start_character":26,"end_line":4190,"end_character":45},"updated":"2019-06-07 14:17:05.000000000","message":"I was scared by this (thinking of a potential KeyError for all VIFs but OVS hybrid ones, but ooof, you make a conditional below)","commit_id":"05937e582783ef9cd727c8e4565203df52dd7cc4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d440e586b626e765b46e0f95acc53f38b7c1c85f","unresolved":false,"context_lines":[{"line_number":4306,"context_line":"                    context, instance, network_info\u003dnetwork_info,"},{"line_number":4307,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4308,"context_line":"                    vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":4309,"context_line":"            except TypeError:"},{"line_number":4310,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4311,"context_line":"                            \"\u0027vifs_already_plugged\u0027 parameter to the \""},{"line_number":4312,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""}],"source_content_type":"text/x-python","patch_set":22,"id":"9fb8cfa7_c65ac928","line":4309,"updated":"2019-06-07 16:23:24.000000000","message":"Follow this up with a change on master to remove this. This TypeError handling is only for out of tree drivers and backports.\n\nAlso, you should probably have a unit test wrinkle that this is handled and we fallback to the original version of the method without the new kwarg.","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"7fb2ed584df1e7a8402d9299e6ae2682a8451d31","unresolved":false,"context_lines":[{"line_number":4306,"context_line":"                    context, instance, network_info\u003dnetwork_info,"},{"line_number":4307,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4308,"context_line":"                    vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":4309,"context_line":"            except TypeError:"},{"line_number":4310,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4311,"context_line":"                            \"\u0027vifs_already_plugged\u0027 parameter to the \""},{"line_number":4312,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""}],"source_content_type":"text/x-python","patch_set":22,"id":"9fb8cfa7_fc63daaf","line":4309,"in_reply_to":"9fb8cfa7_c65ac928","updated":"2019-06-07 18:01:51.000000000","message":"Done","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4191,"context_line":""},{"line_number":4192,"context_line":"    def _uses_hybrid_plug(self, instance):"},{"line_number":4193,"context_line":"        return any(["},{"line_number":4194,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4195,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4196,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4197,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2c4e0d97","line":4194,"updated":"2019-06-10 16:00:29.000000000","message":"You can just use vif.is_hybrid_plug_enabled() here which will handle ovs_hybrid_plug not being in details and details is going to be in VIF as at least {} because of the construction of the object:\n\nhttps://github.com/openstack/nova/blob/1316c1c2850d2f966f335b628f7f5fe88cef611c/nova/network/model.py#L393\n\nwhich means...","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4193,"context_line":"        return any(["},{"line_number":4194,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4195,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4196,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4197,"context_line":""},{"line_number":4198,"context_line":"    def _finish_revert_resize_network_migrate_finish(self, context, instance,"},{"line_number":4199,"context_line":"                                                     migration):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ec43156b","line":4196,"updated":"2019-06-10 16:00:29.000000000","message":"...this can be removed if you\u0027re using is_hybrid_plug_enabled().","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":4193,"context_line":"        return any(["},{"line_number":4194,"context_line":"            vif[\u0027details\u0027][\u0027ovs_hybrid_plug\u0027]"},{"line_number":4195,"context_line":"            for vif in instance.info_cache.network_info"},{"line_number":4196,"context_line":"            if \u0027details\u0027 in vif and \u0027ovs_hybrid_plug\u0027 in vif[\u0027details\u0027]])"},{"line_number":4197,"context_line":""},{"line_number":4198,"context_line":"    def _finish_revert_resize_network_migrate_finish(self, context, instance,"},{"line_number":4199,"context_line":"                                                     migration):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_7d4fcd33","line":4196,"in_reply_to":"9fb8cfa7_ec43156b","updated":"2019-06-10 17:27:31.000000000","message":"0.)\n\nso if we remove _use_hybrid_plug and add a new method to the \n\nnova.network.model.vif class\n\ndef wait_in_manager(self):\n return self.is_hybrid_plug_enabled or self.get(\"vnic_type\") \u003d\u003d \"direct-physical\"\n\nhere https://github.com/openstack/nova/blob/master/nova/network/model.py#L460\n\nthen ...","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":4213,"context_line":"        \"\"\""},{"line_number":4214,"context_line":"        events \u003d []"},{"line_number":4215,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4216,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4217,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4218,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4219,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_3da8758b","line":4216,"range":{"start_line":4216,"start_character":26,"end_line":4216,"end_character":59},"updated":"2019-06-10 17:27:31.000000000","message":"1.)\n... we can modify _get_neutron_events_for_migration\n\nbelow to only return event if the vif.wait_in_manger() is true","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ad8a3b6d8814dc6f34df874d9dfc3923c52377a5","unresolved":false,"context_lines":[{"line_number":4213,"context_line":"        \"\"\""},{"line_number":4214,"context_line":"        events \u003d []"},{"line_number":4215,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4216,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4217,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4218,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4219,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_727db77d","line":4216,"range":{"start_line":4216,"start_character":26,"end_line":4216,"end_character":59},"in_reply_to":"9fb8cfa7_3da8758b","updated":"2019-06-10 20:22:19.000000000","message":"That will change the behavior for live migration as well. Is that desired?","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"062745fdce6ed3bb0313b1da126075f54a4e7299","unresolved":false,"context_lines":[{"line_number":4213,"context_line":"        \"\"\""},{"line_number":4214,"context_line":"        events \u003d []"},{"line_number":4215,"context_line":"        if self._uses_hybrid_plug(instance):"},{"line_number":4216,"context_line":"            events \u003d self._get_neutron_events_for_migration(instance)"},{"line_number":4217,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4218,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4219,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_156ea5c8","line":4216,"range":{"start_line":4216,"start_character":26,"end_line":4216,"end_character":59},"in_reply_to":"9fb8cfa7_727db77d","updated":"2019-06-12 11:25:32.000000000","message":"no its not artom introduced a new function just for revert resize/mirgate events to not have that issue.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4220,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"},{"line_number":4221,"context_line":"                                                      deadline\u003ddeadline,"},{"line_number":4222,"context_line":"                                                      error_callback\u003derror_cb):"},{"line_number":4223,"context_line":"                # NOTE(hanrong): we need to change migration_p[\u0027dest_compute\u0027]"},{"line_number":4224,"context_line":"                # to source host temporarily."},{"line_number":4225,"context_line":"                # \"network_api.migrate_instance_finish\" will setup the network"},{"line_number":4226,"context_line":"                # for the instance on the destination host. For revert resize,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2f783f8e","line":4223,"range":{"start_line":4223,"start_character":51,"end_line":4223,"end_character":78},"updated":"2019-06-10 16:00:29.000000000","message":"This must be old rebase damage because we\u0027re not using a migration primitive dict anymore. I know because I fixed that: I312d61383345ea0ac1ab0c277b4c468e6aa94656","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"01c6901acc24d72639eedf62d2040c0516054919","unresolved":false,"context_lines":[{"line_number":4220,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"},{"line_number":4221,"context_line":"                                                      deadline\u003ddeadline,"},{"line_number":4222,"context_line":"                                                      error_callback\u003derror_cb):"},{"line_number":4223,"context_line":"                # NOTE(hanrong): we need to change migration_p[\u0027dest_compute\u0027]"},{"line_number":4224,"context_line":"                # to source host temporarily."},{"line_number":4225,"context_line":"                # \"network_api.migrate_instance_finish\" will setup the network"},{"line_number":4226,"context_line":"                # for the instance on the destination host. For revert resize,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_1edfa70d","line":4223,"range":{"start_line":4223,"start_character":51,"end_line":4223,"end_character":78},"in_reply_to":"9fb8cfa7_2f783f8e","updated":"2019-06-10 19:57:22.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4226,"context_line":"                # for the instance on the destination host. For revert resize,"},{"line_number":4227,"context_line":"                # the instance will back to the source host, the setup of the"},{"line_number":4228,"context_line":"                # network for instance should be on the source host. So set the"},{"line_number":4229,"context_line":"                # migration_p[\u0027dest_compute\u0027] to source host at here."},{"line_number":4230,"context_line":"                with utils.temporary_mutation("},{"line_number":4231,"context_line":"                        migration, dest_compute\u003dmigration.source_compute):"},{"line_number":4232,"context_line":"                    self.network_api.migrate_instance_finish(context,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_4f7d737d","line":4229,"range":{"start_line":4229,"start_character":18,"end_line":4229,"end_character":45},"updated":"2019-06-10 16:00:29.000000000","message":"same","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"01c6901acc24d72639eedf62d2040c0516054919","unresolved":false,"context_lines":[{"line_number":4226,"context_line":"                # for the instance on the destination host. For revert resize,"},{"line_number":4227,"context_line":"                # the instance will back to the source host, the setup of the"},{"line_number":4228,"context_line":"                # network for instance should be on the source host. So set the"},{"line_number":4229,"context_line":"                # migration_p[\u0027dest_compute\u0027] to source host at here."},{"line_number":4230,"context_line":"                with utils.temporary_mutation("},{"line_number":4231,"context_line":"                        migration, dest_compute\u003dmigration.source_compute):"},{"line_number":4232,"context_line":"                    self.network_api.migrate_instance_finish(context,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_3ee4ebe0","line":4229,"range":{"start_line":4229,"start_character":18,"end_line":4229,"end_character":45},"in_reply_to":"9fb8cfa7_4f7d737d","updated":"2019-06-10 19:57:22.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":4232,"context_line":"                    self.network_api.migrate_instance_finish(context,"},{"line_number":4233,"context_line":"                                                             instance,"},{"line_number":4234,"context_line":"                                                             migration)"},{"line_number":4235,"context_line":"            return bool(events)"},{"line_number":4236,"context_line":"        except eventlet.timeout.Timeout:"},{"line_number":4237,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4238,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events %(events)s \u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_5df1a994","line":4235,"range":{"start_line":4235,"start_character":19,"end_line":4235,"end_character":31},"updated":"2019-06-10 17:27:31.000000000","message":"2.) \n\n... this would then become\n\nlen(events) \u003d\u003d len(instance.get_network_info())\n\ne.g. if all vif in the vm were handeled in the manger we retrun true to so that later we later can pass this value to vif_already_plugged. if all or some of the vif events should be handeled in the driver then we return false.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4307,"context_line":"            power_on \u003d old_vm_state !\u003d vm_states.STOPPED"},{"line_number":4308,"context_line":"            try:"},{"line_number":4309,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4310,"context_line":"                    context, instance, network_info\u003dnetwork_info,"},{"line_number":4311,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4312,"context_line":"                    vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":4313,"context_line":"            except TypeError:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_4f96d341","line":4310,"range":{"start_line":4310,"start_character":39,"end_line":4310,"end_character":51},"updated":"2019-06-10 16:00:29.000000000","message":"This isn\u0027t a kwarg so you don\u0027t need to treat it as such (it\u0027s unrelated damage).","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"01c6901acc24d72639eedf62d2040c0516054919","unresolved":false,"context_lines":[{"line_number":4307,"context_line":"            power_on \u003d old_vm_state !\u003d vm_states.STOPPED"},{"line_number":4308,"context_line":"            try:"},{"line_number":4309,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4310,"context_line":"                    context, instance, network_info\u003dnetwork_info,"},{"line_number":4311,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4312,"context_line":"                    vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":4313,"context_line":"            except TypeError:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_9efb9777","line":4310,"range":{"start_line":4310,"start_character":39,"end_line":4310,"end_character":51},"in_reply_to":"9fb8cfa7_4f96d341","updated":"2019-06-10 19:57:22.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4311,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4312,"context_line":"                    vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":4313,"context_line":"            except TypeError:"},{"line_number":4314,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4315,"context_line":"                            \"\u0027vifs_already_plugged\u0027 parameter to the \""},{"line_number":4316,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""},{"line_number":4317,"context_line":"                            \"please update your virt driver.\")"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_0fea1bab","line":4314,"updated":"2019-06-10 16:00:29.000000000","message":"nit: could leave a NOTE or TODO here that this is just for backports and will be removed on master. Or mention it in the commit message - I just don\u0027t want other reviewers to come along and trip up on it.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"01c6901acc24d72639eedf62d2040c0516054919","unresolved":false,"context_lines":[{"line_number":4311,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":4312,"context_line":"                    vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":4313,"context_line":"            except TypeError:"},{"line_number":4314,"context_line":"                LOG.warning(\"Your virt driver appears to not support the \""},{"line_number":4315,"context_line":"                            \"\u0027vifs_already_plugged\u0027 parameter to the \""},{"line_number":4316,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""},{"line_number":4317,"context_line":"                            \"please update your virt driver.\")"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_9e4077a7","line":4314,"in_reply_to":"9fb8cfa7_0fea1bab","updated":"2019-06-10 19:57:22.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":4316,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""},{"line_number":4317,"context_line":"                            \"please update your virt driver.\")"},{"line_number":4318,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4319,"context_line":"                    context, instance, network_info\u003dnetwork_info,"},{"line_number":4320,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on)"},{"line_number":4321,"context_line":""},{"line_number":4322,"context_line":"            instance.drop_migration_context()"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_6ff15795","line":4319,"range":{"start_line":4319,"start_character":39,"end_line":4319,"end_character":51},"updated":"2019-06-10 16:00:29.000000000","message":"same","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"01c6901acc24d72639eedf62d2040c0516054919","unresolved":false,"context_lines":[{"line_number":4316,"context_line":"                            \"\u0027finish_revert_migration\u0027 method; \""},{"line_number":4317,"context_line":"                            \"please update your virt driver.\")"},{"line_number":4318,"context_line":"                self.driver.finish_revert_migration("},{"line_number":4319,"context_line":"                    context, instance, network_info\u003dnetwork_info,"},{"line_number":4320,"context_line":"                    block_device_info\u003dblock_device_info, power_on\u003dpower_on)"},{"line_number":4321,"context_line":""},{"line_number":4322,"context_line":"            instance.drop_migration_context()"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_5ef19f94","line":4319,"range":{"start_line":4319,"start_character":39,"end_line":4319,"end_character":51},"in_reply_to":"9fb8cfa7_6ff15795","updated":"2019-06-10 19:57:22.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":6509,"context_line":"        # We don\u0027t generate events if CONF.vif_plugging_timeout\u003d0"},{"line_number":6510,"context_line":"        # meaning that the operator disabled using them."},{"line_number":6511,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":6512,"context_line":"            return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":6513,"context_line":"                    for vif in instance.get_network_info()]"},{"line_number":6514,"context_line":"        else:"},{"line_number":6515,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cf116354","line":6512,"updated":"2019-06-10 16:00:29.000000000","message":"Is it possible to have vifs attached where some use ovs_hybrid_plug and some don\u0027t? We\u0027re going to be waiting for an event if any use ovs_hybrid_plug but what if we have some ports attached that don\u0027t use that? Wouldn\u0027t we wait for an event from those that won\u0027t come as a result of the port binding change? Or would we expect to always get a network-vif-plugged event for a port binding change - and if so, why even filter on ovs_hybrid_plug in the first place?","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":6510,"context_line":"        # meaning that the operator disabled using them."},{"line_number":6511,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":6512,"context_line":"            return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":6513,"context_line":"                    for vif in instance.get_network_info()]"},{"line_number":6514,"context_line":"        else:"},{"line_number":6515,"context_line":"            return []"},{"line_number":6516,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_dd8119fd","line":6513,"range":{"start_line":6513,"start_character":20,"end_line":6513,"end_character":58},"updated":"2019-06-10 17:27:31.000000000","message":"for vif in instance.get_network_info() if vif.wait_in_manager()","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5234641cd0fe8ad103583cb8337b4bbc5053ab9","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                if decision is False:"},{"line_number":485,"context_line":"                    break"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _is_compute_manager_event(self, vif):"},{"line_number":488,"context_line":"        return vif.is_hybrid_plug_enabled()"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"    def get_manager_events_for_revert(self, instance):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fb8cfa7_3cf5c4ed","line":487,"updated":"2019-06-11 14:02:00.000000000","message":"This name seems completely unrelated to what it\u0027s doing...to the point of confusion.","commit_id":"1ce00985d34019c20f644fafffbb5f478b904175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5234641cd0fe8ad103583cb8337b4bbc5053ab9","unresolved":false,"context_lines":[{"line_number":499,"context_line":"            return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":500,"context_line":"                    for vif in instance.get_network_info()"},{"line_number":501,"context_line":"                    if not self._is_compute_manager_event(vif)]"},{"line_number":502,"context_line":"        return []"},{"line_number":503,"context_line":""},{"line_number":504,"context_line":""},{"line_number":505,"context_line":"class ComputeManager(manager.Manager):"}],"source_content_type":"text/x-python","patch_set":25,"id":"9fb8cfa7_7cefbc1d","line":502,"updated":"2019-06-11 14:02:00.000000000","message":"I think we need comments or docstrings for this. It feels like a really deep cut, and I suspect this is really libvirt leaking into the compute manager. I think what\u0027s going on here is:\n\nCompute manager should be waiting for hybrid-plugged vif, and the virt driver should be waiting for non-hybrid ones. The get_manager...() method returns those vifs and the get_virt...() one returns the others. I feel like the names could be a lot more descriptive. I also think that the above description means that the latter is really get_LIBvirt_events...(), unless you think it\u0027s likely that another driver would have hybrid vifs. If not, then I don\u0027t think putting this into virtapi is the right thing (even if it neatly arranges the two halves next to each other).\n\n(reading your comments on the previous patch about this new approach)\n\nIn summary, I think the split of who waits makes sense, but not putting them in virtapi, and definitely not the confusing naming above.","commit_id":"1ce00985d34019c20f644fafffbb5f478b904175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ce35e68de679e57e9a2c0641f299d7e6f5402643","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4174,"context_line":"        \"\"\""},{"line_number":4175,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":4176,"context_line":"            events \u003d instance.get_network_info().get_bind_time_events()"},{"line_number":4177,"context_line":"        else:"},{"line_number":4178,"context_line":"            events \u003d []"},{"line_number":4179,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"}],"source_content_type":"text/x-python","patch_set":30,"id":"9fb8cfa7_89230ca7","line":4176,"updated":"2019-06-14 14:55:08.000000000","message":"So, info_cache.network_info is nullable\u003dTrue, which means it can be set to None. I don\u0027t know when that would happen, but it means that we *could* be here with that set to None, and explode because we don\u0027t check it. It sucks to have to worry about that, but any time I see function_call().somethingelse, I fear the potential for a confusing explosion. We call network_api.setup_networks_on_host(...instance...) but it does not look to me that info_cache has to be set in order for that to succeed), which means that doesn\u0027t provide a gate for us.","commit_id":"3e1a30be6f83224f2ff23159dcecc507b6695eee"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"2680e9896734d1ebfe49cb32f324916bace62553","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4174,"context_line":"        \"\"\""},{"line_number":4175,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":4176,"context_line":"            events \u003d instance.get_network_info().get_bind_time_events()"},{"line_number":4177,"context_line":"        else:"},{"line_number":4178,"context_line":"            events \u003d []"},{"line_number":4179,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"}],"source_content_type":"text/x-python","patch_set":30,"id":"9fb8cfa7_e97e48ad","line":4176,"in_reply_to":"9fb8cfa7_89230ca7","updated":"2019-06-14 15:11:04.000000000","message":"I\u0027ve added a check. No harm in being extra careful - though we don\u0027t do any such check in libvirt\u0027s _get_neutron_events() or compute manager\u0027s _get_neutron_events_for_live_migration(). I guess if the instance has no networking info_cache.network_info will be the empty list and not None?","commit_id":"3e1a30be6f83224f2ff23159dcecc507b6695eee"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6c0cc0615dd0b39ab08329a80ef43eb22ae69a65","unresolved":false,"context_lines":[{"line_number":4173,"context_line":"                 exception.VirtualInterfacePlugException."},{"line_number":4174,"context_line":"        \"\"\""},{"line_number":4175,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":4176,"context_line":"            events \u003d instance.get_network_info().get_bind_time_events()"},{"line_number":4177,"context_line":"        else:"},{"line_number":4178,"context_line":"            events \u003d []"},{"line_number":4179,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"}],"source_content_type":"text/x-python","patch_set":30,"id":"9fb8cfa7_ad529b11","line":4176,"in_reply_to":"9fb8cfa7_e97e48ad","updated":"2019-06-14 15:14:30.000000000","message":"... and we\u0027re going to need unit tests for that new network_info check, aren\u0027t we?","commit_id":"3e1a30be6f83224f2ff23159dcecc507b6695eee"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"afe6eba18ae724a36f4eb17300cd45beb7bd59a1","unresolved":false,"context_lines":[{"line_number":4180,"context_line":"        \"\"\""},{"line_number":4181,"context_line":"        network_info \u003d instance.get_network_info()"},{"line_number":4182,"context_line":"        events \u003d []"},{"line_number":4183,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":4184,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4185,"context_line":"            LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"},{"line_number":4186,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"}],"source_content_type":"text/x-python","patch_set":31,"id":"9fb8cfa7_9c880723","line":4183,"updated":"2019-06-17 19:07:26.000000000","message":"\u003e ... and we\u0027re going to need unit tests for that new network_info\n \u003e check, aren\u0027t we?\n\nActually maybe not, given that the conditional is all \u0027and\u0027s - network_info is evaluated for truth in all cases, so we\u0027re definitely testing the positive case - and I\u0027m not sure there\u0027s value in testing the negative \"network_info is None\" case.","commit_id":"6942115c48e960cdedcbb61c3a4084d556b64edc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":4167,"context_line":"    def _finish_revert_resize_network_migrate_finish(self, context, instance,"},{"line_number":4168,"context_line":"                                                     migration):"},{"line_number":4169,"context_line":"        \"\"\"Causes port binding to be updated. In some Neutron or port"},{"line_number":4170,"context_line":"        configurations - see virtapi.get_manager_events_for_revert() - we"},{"line_number":4171,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4172,"context_line":"        ed. The rest of the time, the event is expected further along in the"},{"line_number":4173,"context_line":"        virt driver, so we don\u0027t wait here."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_2f3188ed","line":4170,"range":{"start_line":4170,"start_character":25,"end_line":4170,"end_character":68},"updated":"2019-06-18 20:23:42.000000000","message":"This doesn\u0027t exist. I\u0027m guessing you mean NetworkInfo.get_bind_time_events()?","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":4167,"context_line":"    def _finish_revert_resize_network_migrate_finish(self, context, instance,"},{"line_number":4168,"context_line":"                                                     migration):"},{"line_number":4169,"context_line":"        \"\"\"Causes port binding to be updated. In some Neutron or port"},{"line_number":4170,"context_line":"        configurations - see virtapi.get_manager_events_for_revert() - we"},{"line_number":4171,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4172,"context_line":"        ed. The rest of the time, the event is expected further along in the"},{"line_number":4173,"context_line":"        virt driver, so we don\u0027t wait here."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_620eb733","line":4170,"range":{"start_line":4170,"start_character":25,"end_line":4170,"end_character":68},"in_reply_to":"9fb8cfa7_2f3188ed","updated":"2019-06-19 16:32:12.000000000","message":"Bleah, didn\u0027t update the docstring. Done.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":4169,"context_line":"        \"\"\"Causes port binding to be updated. In some Neutron or port"},{"line_number":4170,"context_line":"        configurations - see virtapi.get_manager_events_for_revert() - we"},{"line_number":4171,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4172,"context_line":"        ed. The rest of the time, the event is expected further along in the"},{"line_number":4173,"context_line":"        virt driver, so we don\u0027t wait here."},{"line_number":4174,"context_line":""},{"line_number":4175,"context_line":"        :param context: The request context."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_4f3f1cb5","line":4172,"range":{"start_line":4172,"start_character":8,"end_line":4172,"end_character":12},"updated":"2019-06-18 20:23:42.000000000","message":"?","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":4169,"context_line":"        \"\"\"Causes port binding to be updated. In some Neutron or port"},{"line_number":4170,"context_line":"        configurations - see virtapi.get_manager_events_for_revert() - we"},{"line_number":4171,"context_line":"        expect the vif-plugged event from Neutron immediately and wait for it."},{"line_number":4172,"context_line":"        ed. The rest of the time, the event is expected further along in the"},{"line_number":4173,"context_line":"        virt driver, so we don\u0027t wait here."},{"line_number":4174,"context_line":""},{"line_number":4175,"context_line":"        :param context: The request context."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_021b3b73","line":4172,"range":{"start_line":4172,"start_character":8,"end_line":4172,"end_character":12},"in_reply_to":"9fb8cfa7_4f3f1cb5","updated":"2019-06-19 16:32:12.000000000","message":"Yeah man, you don\u0027t know Ed?\n\n(Done)","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":4183,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":4184,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4185,"context_line":"            if events:"},{"line_number":4186,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"},{"line_number":4187,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4188,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4189,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_2f466851","line":4186,"updated":"2019-06-18 20:23:42.000000000","message":"nit: throw the instance\u003dinstance kwarg in here for context","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":4183,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":4184,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4185,"context_line":"            if events:"},{"line_number":4186,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"},{"line_number":4187,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4188,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4189,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_c22683bb","line":4186,"in_reply_to":"9fb8cfa7_2f466851","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":4184,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4185,"context_line":"            if events:"},{"line_number":4186,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"},{"line_number":4187,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4188,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4189,"context_line":"        try:"},{"line_number":4190,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_2f69888a","line":4187,"updated":"2019-06-18 20:23:42.000000000","message":"nit: could define this above the conditional above and then you don\u0027t have to access the config option value twice","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":4184,"context_line":"            events \u003d network_info.get_bind_time_events()"},{"line_number":4185,"context_line":"            if events:"},{"line_number":4186,"context_line":"                LOG.debug(\u0027Will wait for bind-time events: %s\u0027, events)"},{"line_number":4187,"context_line":"        deadline \u003d CONF.vif_plugging_timeout"},{"line_number":4188,"context_line":"        error_cb \u003d self._neutron_failed_migration_callback"},{"line_number":4189,"context_line":"        try:"},{"line_number":4190,"context_line":"            with self.virtapi.wait_for_instance_event(instance, events,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_82208b9d","line":4187,"in_reply_to":"9fb8cfa7_2f69888a","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":4206,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4207,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events %(events)s \u0027"},{"line_number":4208,"context_line":"                          \u0027for instance %(instance)s\u0027,"},{"line_number":4209,"context_line":"                          {\u0027events\u0027: events, \u0027instance\u0027: instance})"},{"line_number":4210,"context_line":""},{"line_number":4211,"context_line":"    @wrap_exception()"},{"line_number":4212,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_0fbe04ff","line":4209,"range":{"start_line":4209,"start_character":57,"end_line":4209,"end_character":65},"updated":"2019-06-18 20:23:42.000000000","message":"Did you mean to log the full Instance object here? Or just the ID? If the latter, then do:\n\nLOG.error(\u0027Timeout waiting for Neutron events %s \u0027\n          \u0027for instance.\u0027, events, instance\u003dinstance)","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":4206,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4207,"context_line":"                LOG.error(\u0027Timeout waiting for Neutron events %(events)s \u0027"},{"line_number":4208,"context_line":"                          \u0027for instance %(instance)s\u0027,"},{"line_number":4209,"context_line":"                          {\u0027events\u0027: events, \u0027instance\u0027: instance})"},{"line_number":4210,"context_line":""},{"line_number":4211,"context_line":"    @wrap_exception()"},{"line_number":4212,"context_line":"    @reverts_task_state"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_8255ebfc","line":4209,"range":{"start_line":4209,"start_character":57,"end_line":4209,"end_character":65},"in_reply_to":"9fb8cfa7_0fbe04ff","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"}],"nova/network/model.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4aa7c31073cf95e0c2ca158f47a9ba10785103df","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                    \u0027ips\u0027: ips}"},{"line_number":462,"context_line":"        return []"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def has_bind_time_event(self):"},{"line_number":465,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":466,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":467,"context_line":"        reverting a resize or cold migration), the following Neutron/port"},{"line_number":468,"context_line":"        configurations cause network-vif-plugged events to be sent as soon as"},{"line_number":469,"context_line":"        the binding is updated:"},{"line_number":470,"context_line":"        - OVS with hybrid plug"},{"line_number":471,"context_line":"        \"\"\""},{"line_number":472,"context_line":"        return self.is_hybrid_plug_enabled()"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def is_hybrid_plug_enabled(self):"},{"line_number":475,"context_line":"        return self[\u0027details\u0027].get(VIF_DETAILS_OVS_HYBRID_PLUG, False)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9fb8cfa7_7db804e4","line":472,"range":{"start_line":464,"start_character":2,"end_line":472,"end_character":44},"updated":"2019-06-12 11:50:45.000000000","message":"port with hybrid_plug\u003dtrue only send events at binding time in the case of a cold migration/resize revert\n\nthe comment calls out that this is for reverts but\ni would prefer if this had revert in the function name.\n\ne.g. has_revirt_bind_time_events.\n\nml2/ovs is one of the few drivers that sends event correctly.\n\nin the hybrid plug case it is sending the event after the l2 agent has confirmed that the port is plugged on the source node. so its technically still sending the event at plug time.\n\nor rather the ovs l2 agent send vif_plugged events if and only if the port is plugged on the host and the port is bound to the host.\n\nso i want to avoid people getting confused and thinking that\nhybrid_plug\u003dtrue means it will aways send events at bind time. technically it never send them at bind time.\n\nthe the sriov pf case its actully sending the event as part of the port binding api call so that is really sending at bind time which is how ODL v1 ml2/driver used to work.\nhowever for sriov driect-phyical that is actully correct as there is no configuration for neutron to do. its all dont by the hypervior in the PF case.","commit_id":"87ca1948e862f13b06a1628b361726c7a64e7e36"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4aa7c31073cf95e0c2ca158f47a9ba10785103df","unresolved":false,"context_lines":[{"line_number":528,"context_line":"    def json(self):"},{"line_number":529,"context_line":"        return jsonutils.dumps(self)"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"    def get_bind_time_events(self):"},{"line_number":532,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":533,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":534,"context_line":"        reverting a resize or cold migration), return external events that are"}],"source_content_type":"text/x-python","patch_set":27,"id":"9fb8cfa7_ddc25034","line":531,"range":{"start_line":531,"start_character":7,"end_line":531,"end_character":28},"updated":"2019-06-12 11:50:45.000000000","message":"nit: get_revert_bind_time_events","commit_id":"87ca1948e862f13b06a1628b361726c7a64e7e36"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1ecd0caf74299793fd6d6c1efbb2353908214bae","unresolved":false,"context_lines":[{"line_number":528,"context_line":"    def json(self):"},{"line_number":529,"context_line":"        return jsonutils.dumps(self)"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"    def get_bind_time_events(self):"},{"line_number":532,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":533,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":534,"context_line":"        reverting a resize or cold migration), return external events that are"}],"source_content_type":"text/x-python","patch_set":27,"id":"9fb8cfa7_a58bc2c0","line":531,"range":{"start_line":531,"start_character":7,"end_line":531,"end_character":28},"in_reply_to":"9fb8cfa7_42d8bc79","updated":"2019-06-12 15:35:40.000000000","message":"Yeah, if we put the get_events() methods somewhere closer to the compute manager, I can see them containing \"revert\" in their name. But with them in the model, I agree with Dan that it should reflect the original cause, not the specific Nova operation.\n\nI\u0027m going to own my laziness and keep the method names as is unless someone comes in with an explicit -1 :)","commit_id":"87ca1948e862f13b06a1628b361726c7a64e7e36"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2cf47241f27ee7abb9abaa83485d18e8f87035d3","unresolved":false,"context_lines":[{"line_number":528,"context_line":"    def json(self):"},{"line_number":529,"context_line":"        return jsonutils.dumps(self)"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"    def get_bind_time_events(self):"},{"line_number":532,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":533,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":534,"context_line":"        reverting a resize or cold migration), return external events that are"}],"source_content_type":"text/x-python","patch_set":27,"id":"9fb8cfa7_42d8bc79","line":531,"range":{"start_line":531,"start_character":7,"end_line":531,"end_character":28},"in_reply_to":"9fb8cfa7_ddc25034","updated":"2019-06-12 14:29:52.000000000","message":"I don\u0027t think putting something into a generic place like this and using the term \"revert\" makes sense. We need to figure out what to name it that reflects the reason, not the symptom. Like \"bind_time_same_host_events\" or something like that.","commit_id":"87ca1948e862f13b06a1628b361726c7a64e7e36"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4aa7c31073cf95e0c2ca158f47a9ba10785103df","unresolved":false,"context_lines":[{"line_number":539,"context_line":"                    for vif in self if vif.has_bind_time_event()]"},{"line_number":540,"context_line":"        return []"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"    def get_plug_time_events(self):"},{"line_number":543,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":544,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":545,"context_line":"        reverting a resize or cold migration), return external events that are"}],"source_content_type":"text/x-python","patch_set":27,"id":"9fb8cfa7_1dd22801","line":542,"range":{"start_line":542,"start_character":8,"end_line":542,"end_character":28},"updated":"2019-06-12 11:50:45.000000000","message":"nit: get_revert_plug_time_events","commit_id":"87ca1948e862f13b06a1628b361726c7a64e7e36"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae40c4ead71a0c9d079af69233b768de85da479e","unresolved":false,"context_lines":[{"line_number":534,"context_line":"        reverting a resize or cold migration), return external events that are"},{"line_number":535,"context_line":"        sent as soon as the binding is updated."},{"line_number":536,"context_line":"        \"\"\""},{"line_number":537,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":538,"context_line":"            return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":539,"context_line":"                    for vif in self if vif.has_bind_time_event()]"},{"line_number":540,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":28,"id":"9fb8cfa7_139bf465","line":537,"range":{"start_line":537,"start_character":11,"end_line":537,"end_character":36},"updated":"2019-06-12 16:54:21.000000000","message":"I would prefer if this stays in the manager part. The behavior of the timeout, whether or not we wait, or treat it like an error, etc, is all behavior of the compute manager. This is \"the model\" not \"the controller\" and thus it should return steady-state information about the nature of the VIF. Let the manager decide what to do with that information.","commit_id":"505061ce94b97b18257f65880210047de06faac2"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4439739d8aa5ab19196e09dab46cfddbab1c6f2f","unresolved":false,"context_lines":[{"line_number":534,"context_line":"        reverting a resize or cold migration), return external events that are"},{"line_number":535,"context_line":"        sent as soon as the binding is updated."},{"line_number":536,"context_line":"        \"\"\""},{"line_number":537,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":538,"context_line":"            return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":539,"context_line":"                    for vif in self if vif.has_bind_time_event()]"},{"line_number":540,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":28,"id":"9fb8cfa7_6eab6774","line":537,"range":{"start_line":537,"start_character":11,"end_line":537,"end_character":36},"in_reply_to":"9fb8cfa7_139bf465","updated":"2019-06-12 17:39:37.000000000","message":"Done","commit_id":"505061ce94b97b18257f65880210047de06faac2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":535,"context_line":"                for vif in self if vif.has_bind_time_event()]"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"    def get_plug_time_events(self):"},{"line_number":538,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":539,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":540,"context_line":"        reverting a resize or cold migration), return external events that are"},{"line_number":541,"context_line":"        sent when the VIF is plugged."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_af2b382e","line":538,"range":{"start_line":538,"start_character":11,"end_line":538,"end_character":41},"updated":"2019-06-18 20:23:42.000000000","message":"This is a bit confusing since this isn\u0027t used when updating the port\u0027s host binding. Did you mean to leave this copied from above? I don\u0027t have a better alternative here though and it\u0027s probably OK given this description clarifies by saying \"sent when the VIF is plugged.\"","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":535,"context_line":"                for vif in self if vif.has_bind_time_event()]"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":"    def get_plug_time_events(self):"},{"line_number":538,"context_line":"        \"\"\"When updating the port binding to a host that already has the"},{"line_number":539,"context_line":"        instance in a shutoff state (in practice, this currently means"},{"line_number":540,"context_line":"        reverting a resize or cold migration), return external events that are"},{"line_number":541,"context_line":"        sent when the VIF is plugged."}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_02e57b38","line":538,"range":{"start_line":538,"start_character":11,"end_line":538,"end_character":41},"in_reply_to":"9fb8cfa7_af2b382e","updated":"2019-06-19 16:32:12.000000000","message":"It\u0027s meant to describe the context, and is indeed copy-pasted from above. I\u0027ll leave this for now since you seem to tolerate it, and we can think of a better docstring in a fup.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"}],"nova/tests/functional/notification_sample_tests/test_instance.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f2e7d0eaab721a71a525dc51aa9c2e797ec6781d","unresolved":false,"context_lines":[{"line_number":369,"context_line":"    def test_instance_action(self):"},{"line_number":370,"context_line":"        # NOTE(artom) We\u0027re not testing external events here, don\u0027t fail when"},{"line_number":371,"context_line":"        # we don\u0027t get them."},{"line_number":372,"context_line":"        self.flags(vif_plugging_timeout\u003d0)"},{"line_number":373,"context_line":"        self.flags(vif_plugging_is_fatal\u003dFalse)"},{"line_number":374,"context_line":"        # A single test case is used to test most of the instance action"},{"line_number":375,"context_line":"        # notifications to avoid booting up an instance for every action"}],"source_content_type":"text/x-python","patch_set":18,"id":"bfb3d3c7_54d95454","line":372,"range":{"start_line":372,"start_character":8,"end_line":372,"end_character":42},"updated":"2019-05-29 14:16:27.000000000","message":"this technically is not needed the\nvif_plugging_is_fatal shoudl be enough.\n\nare you setting it to 0 to just speed up the tests?","commit_id":"52386bfa324445d9aaeb114d02a89f8101c31e19"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c41f7a29d14885807c2024763f0be19379f7e20e","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        # NOTE(artom) We\u0027re not testing external events here, don\u0027t fail when"},{"line_number":371,"context_line":"        # we don\u0027t get them."},{"line_number":372,"context_line":"        self.flags(vif_plugging_timeout\u003d0)"},{"line_number":373,"context_line":"        self.flags(vif_plugging_is_fatal\u003dFalse)"},{"line_number":374,"context_line":"        # A single test case is used to test most of the instance action"},{"line_number":375,"context_line":"        # notifications to avoid booting up an instance for every action"},{"line_number":376,"context_line":"        # separately."}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_a91479a1","line":373,"updated":"2019-06-05 13:49:21.000000000","message":"What does this have to do with this change?","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        # NOTE(artom) We\u0027re not testing external events here, don\u0027t fail when"},{"line_number":371,"context_line":"        # we don\u0027t get them."},{"line_number":372,"context_line":"        self.flags(vif_plugging_timeout\u003d0)"},{"line_number":373,"context_line":"        self.flags(vif_plugging_is_fatal\u003dFalse)"},{"line_number":374,"context_line":"        # A single test case is used to test most of the instance action"},{"line_number":375,"context_line":"        # notifications to avoid booting up an instance for every action"},{"line_number":376,"context_line":"        # separately."}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_ac5d62ea","line":373,"in_reply_to":"9fb8cfa7_a91479a1","updated":"2019-06-05 23:26:34.000000000","message":"Left from before we discovered hybrid plug was the issue, you\u0027re right, don\u0027t think we need it anymore.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f2e7d0eaab721a71a525dc51aa9c2e797ec6781d","unresolved":false,"context_lines":[{"line_number":5999,"context_line":"        # migrate_instance_finish()."},{"line_number":6000,"context_line":"        with mock.patch.object("},{"line_number":6001,"context_line":"                self.compute, \u0027migrate_finish_and_wait\u0027,"},{"line_number":6002,"context_line":"                side_effect\u003dself.compute.network_api.migrate_instance_finish):"},{"line_number":6003,"context_line":"            self.compute.finish_revert_resize(self.context,"},{"line_number":6004,"context_line":"                                              migration\u003dmigration,"},{"line_number":6005,"context_line":"                                              instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bfb3d3c7_2fa48db9","line":6002,"range":{"start_line":6002,"start_character":28,"end_line":6002,"end_character":76},"updated":"2019-05-29 14:16:27.000000000","message":"so im assuming this is mocked out by the neturon fixture and not actually hitting neutron.  given that the unit test passed ill assume yes as it would have failed otherwise but just wanted to make sure this is valid.","commit_id":"52386bfa324445d9aaeb114d02a89f8101c31e19"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c41f7a29d14885807c2024763f0be19379f7e20e","unresolved":false,"context_lines":[{"line_number":5997,"context_line":"                               return_value\u003dFalse):"},{"line_number":5998,"context_line":"            self.compute.finish_revert_resize(self.context,"},{"line_number":5999,"context_line":"                                              migration\u003dmigration,"},{"line_number":6000,"context_line":"                                              instance\u003dinstance)"},{"line_number":6001,"context_line":"        mock_notify.assert_has_calls(["},{"line_number":6002,"context_line":"            mock.call(self.context, instance, \u0027fake-mini\u0027,"},{"line_number":6003,"context_line":"                      action\u003d\u0027resize_revert\u0027, phase\u003d\u0027start\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_89d415bf","line":6000,"updated":"2019-06-05 13:49:21.000000000","message":"This seems like an odd mock. Doesn\u0027t your new handler already return False because whatever vif info it gets doesn\u0027t have the pieces in place to be considered hybrid or whatever?","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":5997,"context_line":"                               return_value\u003dFalse):"},{"line_number":5998,"context_line":"            self.compute.finish_revert_resize(self.context,"},{"line_number":5999,"context_line":"                                              migration\u003dmigration,"},{"line_number":6000,"context_line":"                                              instance\u003dinstance)"},{"line_number":6001,"context_line":"        mock_notify.assert_has_calls(["},{"line_number":6002,"context_line":"            mock.call(self.context, instance, \u0027fake-mini\u0027,"},{"line_number":6003,"context_line":"                      action\u003d\u0027resize_revert\u0027, phase\u003d\u0027start\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_2c1732f0","line":6000,"in_reply_to":"9fb8cfa7_89d415bf","updated":"2019-06-05 23:26:34.000000000","message":"It access instance.info_cache.network_info, which isn\u0027t present in the tests, so I figured I\u0027d just mock the whole thing out.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afe39232fa501a658297b7e4ee213f55485bad3f","unresolved":false,"context_lines":[{"line_number":6110,"context_line":"        self.assertNotEqual(migration.dest_node, migration.source_node)"},{"line_number":6111,"context_line":"        self.assertEqual(NODENAME2, migration.dest_compute)"},{"line_number":6112,"context_line":""},{"line_number":6113,"context_line":"    def test_uses_hybrid_plug(self):"},{"line_number":6114,"context_line":"        instance \u003d self._create_fake_instance_obj("},{"line_number":6115,"context_line":"            {\u0027info_cache\u0027: objects.InstanceInfoCache("},{"line_number":6116,"context_line":"                 network_info\u003dnetwork_model.NetworkInfo(["}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_079329fc","line":6113,"updated":"2019-06-05 19:21:15.000000000","message":"nit: put this in test_compute_mgr because test_compute is a legacy mish-mash of compute API and manager tests and relies on a lot of db and mox setup stuff which is kind of gross to deal with in evolving tests - test_compute_mgr is much more straight-forward and mock driven and no db.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66642f2a25826f2a21734083f63d03b1f2800aef","unresolved":false,"context_lines":[{"line_number":6110,"context_line":"        self.assertNotEqual(migration.dest_node, migration.source_node)"},{"line_number":6111,"context_line":"        self.assertEqual(NODENAME2, migration.dest_compute)"},{"line_number":6112,"context_line":""},{"line_number":6113,"context_line":"    def test_uses_hybrid_plug(self):"},{"line_number":6114,"context_line":"        instance \u003d self._create_fake_instance_obj("},{"line_number":6115,"context_line":"            {\u0027info_cache\u0027: objects.InstanceInfoCache("},{"line_number":6116,"context_line":"                 network_info\u003dnetwork_model.NetworkInfo(["}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_e2d5a344","line":6113,"in_reply_to":"9fb8cfa7_079329fc","updated":"2019-06-05 23:26:34.000000000","message":"Done","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"4acd154139530d5d6cef232f26d280095669e459","unresolved":false,"context_lines":[{"line_number":5058,"context_line":"            source_compute\u003d\u0027fake-source\u0027,"},{"line_number":5059,"context_line":"            dest_compute\u003d\u0027fake-dest\u0027)"},{"line_number":5060,"context_line":"        with test.nested("},{"line_number":5061,"context_line":"            mock.patch.object(self.compute.instance_events,"},{"line_number":5062,"context_line":"                              \u0027prepare_for_instance_event\u0027),"},{"line_number":5063,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5064,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5065,"context_line":"        ) as (mock_prepare, mock_migrate_instance_finish):"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fb8cfa7_c0f08dcc","line":5062,"range":{"start_line":5061,"start_character":1,"end_line":5062,"end_character":60},"updated":"2019-06-06 10:16:44.000000000","message":"nit: It would more readable imho if these 2 unit tests were more similar. I\u0027d personally have them both mock prepare_for_instance_event and change the weird all([\u0027network-vif-plugged\u0027...) test above to assert a call to prepare_instance_event. And probably make much of the code common.\n\nNot worth a respin just for this, though. This is ok.","commit_id":"95fc90852a22b666163aa4b443d61e5c37591bf0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5021,"context_line":"                    self.context, fake_instance, fake_bdm)"},{"line_number":5022,"context_line":"        block_stats.assert_called_once_with(fake_instance, \u0027vda\u0027)"},{"line_number":5023,"context_line":""},{"line_number":5024,"context_line":"    def test_finish_revert_migration_signature(self):"},{"line_number":5025,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5026,"context_line":"        instance.system_metadata \u003d {}"},{"line_number":5027,"context_line":"        migration \u003d objects.Migration("}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cfb0838d","line":5024,"updated":"2019-06-10 16:00:29.000000000","message":"Could use a description of this test since it\u0027s non-trivial.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5030,"context_line":"            dest_compute\u003d\u0027fake-dest\u0027)"},{"line_number":5031,"context_line":"        with test.nested("},{"line_number":5032,"context_line":"            mock.patch.object(self.compute.driver, \u0027finish_revert_migration\u0027,"},{"line_number":5033,"context_line":"                              side_effect\u003d[TypeError, True]),"},{"line_number":5034,"context_line":"            mock.patch.object(self.compute,"},{"line_number":5035,"context_line":"                              \u0027_error_out_instance_on_exception\u0027),"},{"line_number":5036,"context_line":"            mock.patch.object(objects.BlockDeviceMappingList,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2f659f1e","line":5033,"range":{"start_line":5033,"start_character":54,"end_line":5033,"end_character":58},"updated":"2019-06-10 16:00:29.000000000","message":"None - the method doesn\u0027t return anything.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5065,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5066,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("},{"line_number":5067,"context_line":"             network_info\u003dnetwork_model.NetworkInfo(["},{"line_number":5068,"context_line":"                 {\u0027id\u0027: uuids.vif1,"},{"line_number":5069,"context_line":"                  \u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: True}},"},{"line_number":5070,"context_line":"                 {\u0027id\u0027: uuids.vif2,"},{"line_number":5071,"context_line":"                  \u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}}]))"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_0fed3b8a","line":5068,"updated":"2019-06-10 16:00:29.000000000","message":"Use real VIF model objects or just mock _uses_hybrid_plug to return True.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"730336480a41e1610baf0ded481c5d2e36865274","unresolved":false,"context_lines":[{"line_number":5065,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5066,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("},{"line_number":5067,"context_line":"             network_info\u003dnetwork_model.NetworkInfo(["},{"line_number":5068,"context_line":"                 {\u0027id\u0027: uuids.vif1,"},{"line_number":5069,"context_line":"                  \u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: True}},"},{"line_number":5070,"context_line":"                 {\u0027id\u0027: uuids.vif2,"},{"line_number":5071,"context_line":"                  \u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}}]))"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cba5bc48","line":5068,"in_reply_to":"9fb8cfa7_0fed3b8a","updated":"2019-06-11 18:43:15.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"        events \u003d self.compute._get_neutron_events_for_migration(instance)"},{"line_number":5076,"context_line":"        with test.nested("},{"line_number":5077,"context_line":"            mock.patch.object(self.compute.virtapi,"},{"line_number":5078,"context_line":"                              \u0027wait_for_instance_event\u0027),"},{"line_number":5079,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5080,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5081,"context_line":"        ) as (mock_wait, mock_migrate_instance_finish):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_0fbbfb71","line":5078,"range":{"start_line":5078,"start_character":31,"end_line":5078,"end_character":54},"updated":"2019-06-10 16:00:29.000000000","message":"Why not mock prepare_for_instance_event like below? We always call wait_for_instance_event but that doesn\u0027t mean we\u0027re passing events nor prepare_for_instance_event will be called.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"730336480a41e1610baf0ded481c5d2e36865274","unresolved":false,"context_lines":[{"line_number":5075,"context_line":"        events \u003d self.compute._get_neutron_events_for_migration(instance)"},{"line_number":5076,"context_line":"        with test.nested("},{"line_number":5077,"context_line":"            mock.patch.object(self.compute.virtapi,"},{"line_number":5078,"context_line":"                              \u0027wait_for_instance_event\u0027),"},{"line_number":5079,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5080,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5081,"context_line":"        ) as (mock_wait, mock_migrate_instance_finish):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2b59f88c","line":5078,"range":{"start_line":5078,"start_character":31,"end_line":5078,"end_character":54},"in_reply_to":"9fb8cfa7_0fbbfb71","updated":"2019-06-11 18:43:15.000000000","message":"Because if we mock further \"down\" than wait_for_instance_event, wait_for_instance_event() will actually try to wait, and then blow up. I think for consistency I\u0027ll change it in the test below as well - assert that wait_for_instance_event gets called with no events.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"670b8c74e6ebd4591bbfa9ce3322ca64dddd6138","unresolved":false,"context_lines":[{"line_number":5078,"context_line":"                              \u0027wait_for_instance_event\u0027),"},{"line_number":5079,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5080,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5081,"context_line":"        ) as (mock_wait, mock_migrate_instance_finish):"},{"line_number":5082,"context_line":"            self.assertTrue("},{"line_number":5083,"context_line":"                self.compute._finish_revert_resize_network_migrate_finish("},{"line_number":5084,"context_line":"                    self.context, instance, migration))"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_329d04bc","line":5081,"range":{"start_line":5081,"start_character":25,"end_line":5081,"end_character":53},"updated":"2019-06-10 16:05:19.000000000","message":"assert this is called","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"730336480a41e1610baf0ded481c5d2e36865274","unresolved":false,"context_lines":[{"line_number":5078,"context_line":"                              \u0027wait_for_instance_event\u0027),"},{"line_number":5079,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5080,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5081,"context_line":"        ) as (mock_wait, mock_migrate_instance_finish):"},{"line_number":5082,"context_line":"            self.assertTrue("},{"line_number":5083,"context_line":"                self.compute._finish_revert_resize_network_migrate_finish("},{"line_number":5084,"context_line":"                    self.context, instance, migration))"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ebb20094","line":5081,"range":{"start_line":5081,"start_character":25,"end_line":5081,"end_character":53},"in_reply_to":"9fb8cfa7_329d04bc","updated":"2019-06-11 18:43:15.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5092,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5093,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("},{"line_number":5094,"context_line":"             network_info\u003dnetwork_model.NetworkInfo(["},{"line_number":5095,"context_line":"                 {\u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}},"},{"line_number":5096,"context_line":"                 {\u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}}]))"},{"line_number":5097,"context_line":"        migration \u003d objects.Migration("},{"line_number":5098,"context_line":"            source_compute\u003d\u0027fake-source\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_efcea72b","line":5095,"updated":"2019-06-10 16:00:29.000000000","message":"Use real VIF model objects, or just mock _uses_hybrid_plug to return False.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"730336480a41e1610baf0ded481c5d2e36865274","unresolved":false,"context_lines":[{"line_number":5092,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5093,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("},{"line_number":5094,"context_line":"             network_info\u003dnetwork_model.NetworkInfo(["},{"line_number":5095,"context_line":"                 {\u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}},"},{"line_number":5096,"context_line":"                 {\u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}}]))"},{"line_number":5097,"context_line":"        migration \u003d objects.Migration("},{"line_number":5098,"context_line":"            source_compute\u003d\u0027fake-source\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ab2ba892","line":5095,"in_reply_to":"9fb8cfa7_efcea72b","updated":"2019-06-11 18:43:15.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"670b8c74e6ebd4591bbfa9ce3322ca64dddd6138","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                              \u0027prepare_for_instance_event\u0027),"},{"line_number":5103,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5104,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5105,"context_line":"        ) as (mock_prepare, mock_migrate_instance_finish):"},{"line_number":5106,"context_line":"            self.assertFalse("},{"line_number":5107,"context_line":"                self.compute._finish_revert_resize_network_migrate_finish("},{"line_number":5108,"context_line":"                    self.context, instance, migration))"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_f2a20cfc","line":5105,"range":{"start_line":5105,"start_character":28,"end_line":5105,"end_character":56},"updated":"2019-06-10 16:05:19.000000000","message":"assert this is called","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"730336480a41e1610baf0ded481c5d2e36865274","unresolved":false,"context_lines":[{"line_number":5102,"context_line":"                              \u0027prepare_for_instance_event\u0027),"},{"line_number":5103,"context_line":"            mock.patch.object(self.compute.network_api,"},{"line_number":5104,"context_line":"                              \u0027migrate_instance_finish\u0027)"},{"line_number":5105,"context_line":"        ) as (mock_prepare, mock_migrate_instance_finish):"},{"line_number":5106,"context_line":"            self.assertFalse("},{"line_number":5107,"context_line":"                self.compute._finish_revert_resize_network_migrate_finish("},{"line_number":5108,"context_line":"                    self.context, instance, migration))"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cb285c98","line":5105,"range":{"start_line":5105,"start_character":28,"end_line":5105,"end_character":56},"in_reply_to":"9fb8cfa7_f2a20cfc","updated":"2019-06-11 18:43:15.000000000","message":"Done","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5108,"context_line":"                    self.context, instance, migration))"},{"line_number":5109,"context_line":"            mock_prepare.assert_not_called()"},{"line_number":5110,"context_line":""},{"line_number":5111,"context_line":"    def test_uses_hybrid_plug(self):"},{"line_number":5112,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5113,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("},{"line_number":5114,"context_line":"             network_info\u003dnetwork_model.NetworkInfo(["}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2f64bf49","line":5111,"range":{"start_line":5111,"start_character":8,"end_line":5111,"end_character":29},"updated":"2019-06-10 16:00:29.000000000","message":"nit: a comment for each scenario would be nice","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":5112,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":5113,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("},{"line_number":5114,"context_line":"             network_info\u003dnetwork_model.NetworkInfo(["},{"line_number":5115,"context_line":"                 {\u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: True}},"},{"line_number":5116,"context_line":"                 {\u0027details\u0027: {\u0027ovs_hybrid_plug\u0027: False}}]))"},{"line_number":5117,"context_line":"        self.assertTrue(self.compute._uses_hybrid_plug(instance))"},{"line_number":5118,"context_line":"        instance.info_cache \u003d objects.InstanceInfoCache("}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_4fced32e","line":5115,"updated":"2019-06-10 16:00:29.000000000","message":"Use actual VIF model objects rather than dicts. is_hybrid_plug_enabled wouldn\u0027t work otherwise.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":7193,"context_line":""},{"line_number":7194,"context_line":"        reportclient \u003d self.compute.reportclient"},{"line_number":7195,"context_line":""},{"line_number":7196,"context_line":"        @mock.patch.object(self.compute, \u0027_uses_hybrid_plug\u0027,"},{"line_number":7197,"context_line":"                           return_value\u003dFalse)"},{"line_number":7198,"context_line":"        @mock.patch(\u0027nova.objects.ComputeNode.get_by_host_and_nodename\u0027)"},{"line_number":7199,"context_line":"        @mock.patch.object(self.compute.driver, \u0027finish_revert_migration\u0027)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_2f7b5ff5","line":7196,"updated":"2019-06-10 16:00:29.000000000","message":"As below, nix this since it\u0027s too low-level.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":7199,"context_line":"        @mock.patch.object(self.compute.driver, \u0027finish_revert_migration\u0027)"},{"line_number":7200,"context_line":"        @mock.patch.object(self.compute.network_api, \u0027get_instance_nw_info\u0027,"},{"line_number":7201,"context_line":"                           side_effect\u003d_get_instance_nw_info)"},{"line_number":7202,"context_line":"        @mock.patch.object(self.compute.network_api, \u0027migrate_instance_finish\u0027,"},{"line_number":7203,"context_line":"                           side_effect\u003d_migrate_instance_finish)"},{"line_number":7204,"context_line":"        @mock.patch.object(self.compute.network_api, \u0027setup_networks_on_host\u0027)"},{"line_number":7205,"context_line":"        @mock.patch.object(self.migration, \u0027save\u0027)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_ef80e7c1","line":7202,"range":{"start_line":7202,"start_character":54,"end_line":7202,"end_character":77},"updated":"2019-06-10 16:00:29.000000000","message":"And change this to _finish_revert_resize_network_migrate_finish.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":7302,"context_line":"            objects.BlockDeviceMapping(destination_type\u003d\u0027local\u0027)"},{"line_number":7303,"context_line":"        ])"},{"line_number":7304,"context_line":""},{"line_number":7305,"context_line":"        @mock.patch.object(self.compute, \u0027_uses_hybrid_plug\u0027,"},{"line_number":7306,"context_line":"                           return_value\u003dFalse)"},{"line_number":7307,"context_line":"        @mock.patch(\u0027nova.objects.Service.get_minimum_version\u0027,"},{"line_number":7308,"context_line":"                    return_value\u003d22)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_cf264316","line":7305,"updated":"2019-06-10 16:00:29.000000000","message":"This is a bit low-level of a mock for the method this is testing. Seems rather than mock this, you should change the mock on migrate_instance_finish to be _finish_revert_resize_network_migrate_finish.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":5072,"context_line":"            mock_wait.assert_called_once_with("},{"line_number":5073,"context_line":"                instance, events, deadline\u003dCONF.vif_plugging_timeout,"},{"line_number":5074,"context_line":"                error_callback\u003dself.compute._neutron_failed_migration_callback)"},{"line_number":5075,"context_line":"            # Fake the temporary_mutation() decorator"},{"line_number":5076,"context_line":"            migration.dest_compute \u003d migration.source_compute"},{"line_number":5077,"context_line":"            mock_migrate_instance_finish.assert_called_once_with("},{"line_number":5078,"context_line":"                self.context, instance, migration)"},{"line_number":5079,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_ef92904b","line":5076,"range":{"start_line":5075,"start_character":12,"end_line":5076,"end_character":61},"updated":"2019-06-18 20:23:42.000000000","message":"This makes no difference to the test, likely because Migration isn\u0027t a dict and mock is probably comparing the object id since Migration doesn\u0027t have an __eq__ override.\n\nTo really assert this mutation is happening, you need to add a side_effect method for migrate_instance_finish and do the assertion of the Migration.dest_compute value within that method, like is done in test_finish_revert_resize_validate_source_compute - interestingly that test is broken because it\u0027s stubbing the wrong thing nova.network.api.API.migrate_instance_finish rather than the neutronv2.api method.\n\nIf you want to fix that existing test\u0027s assertion and remove the thing that doesn\u0027t do anything here in your test, you can do this:\n\ndiff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py\nindex f3fbfba9ed..d117fbedeb 100644\n--- a/nova/tests/unit/compute/test_compute.py\n+++ b/nova/tests/unit/compute/test_compute.py\n@@ -6094,14 +6094,15 @@ class ComputeTestCase(BaseTestCase,\n \n         # NOTE(hanrong): Prove that we pass the right value to the\n         # \"self.network_api.migrate_instance_finish\".\n-        def fake_migrate_instance_finish(cls, context, instance, migration):\n+        def fake_migrate_instance_finish(context, instance, migration):\n             self.assertEqual(source_compute, migration[\u0027dest_compute\u0027])\n-        self.stub_out(\u0027nova.network.api.API.migrate_instance_finish\u0027,\n-                      fake_migrate_instance_finish)\n \n-        self.compute.finish_revert_resize(self.context,\n-                migration\u003dmigration,\n-                instance\u003dinstance)\n+        with mock.patch.object(self.compute.network_api,\n+                               \u0027migrate_instance_finish\u0027,\n+                               side_effect\u003dfake_migrate_instance_finish):\n+            self.compute.finish_revert_resize(self.context,\n+                    migration\u003dmigration,\n+                    instance\u003dinstance)\n \n         self.assertEqual(instance.host, migration.source_compute)\n         self.assertNotEqual(migration.dest_compute, migration.source_compute)","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":5072,"context_line":"            mock_wait.assert_called_once_with("},{"line_number":5073,"context_line":"                instance, events, deadline\u003dCONF.vif_plugging_timeout,"},{"line_number":5074,"context_line":"                error_callback\u003dself.compute._neutron_failed_migration_callback)"},{"line_number":5075,"context_line":"            # Fake the temporary_mutation() decorator"},{"line_number":5076,"context_line":"            migration.dest_compute \u003d migration.source_compute"},{"line_number":5077,"context_line":"            mock_migrate_instance_finish.assert_called_once_with("},{"line_number":5078,"context_line":"                self.context, instance, migration)"},{"line_number":5079,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_3451cb0f","line":5076,"range":{"start_line":5075,"start_character":12,"end_line":5076,"end_character":61},"in_reply_to":"9fb8cfa7_ef92904b","updated":"2019-06-19 16:32:12.000000000","message":"Fixed my own test, I can fix test_finish_revert_resize_validate_source_compute in a different patch.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":5103,"context_line":"            [])"},{"line_number":5104,"context_line":""},{"line_number":5105,"context_line":"    @mock.patch.object(utils, \u0027is_neutron\u0027, return_value\u003dFalse)"},{"line_number":5106,"context_line":"    def test_finish_revert_resize_network_migrate_finish_no_vif_neutron(self,"},{"line_number":5107,"context_line":"                                                                        _):"},{"line_number":5108,"context_line":"        self._test_finish_revert_resize_network_migrate_finish("},{"line_number":5109,"context_line":"            [network_model.VIF(id\u003duuids.hybrid_vif,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_efd61028","line":5106,"range":{"start_line":5106,"start_character":64,"end_line":5106,"end_character":71},"updated":"2019-06-18 20:23:42.000000000","message":"Wouldn\u0027t this make more sense to say \"not_neutron\" or \"nova_net\" or something, e.g. test_finish_revert_resize_network_migrate_finish_not_neutron. There are vifs, so \"no_vif\" isn\u0027t clear to me, the problem is we\u0027re not using neutron. A short description per test case would also be helpful.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":5103,"context_line":"            [])"},{"line_number":5104,"context_line":""},{"line_number":5105,"context_line":"    @mock.patch.object(utils, \u0027is_neutron\u0027, return_value\u003dFalse)"},{"line_number":5106,"context_line":"    def test_finish_revert_resize_network_migrate_finish_no_vif_neutron(self,"},{"line_number":5107,"context_line":"                                                                        _):"},{"line_number":5108,"context_line":"        self._test_finish_revert_resize_network_migrate_finish("},{"line_number":5109,"context_line":"            [network_model.VIF(id\u003duuids.hybrid_vif,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_c18433b4","line":5106,"range":{"start_line":5106,"start_character":64,"end_line":5106,"end_character":71},"in_reply_to":"9fb8cfa7_efd61028","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":5080,"context_line":"            mock_wait.assert_called_once_with("},{"line_number":5081,"context_line":"                instance, events, deadline\u003dCONF.vif_plugging_timeout,"},{"line_number":5082,"context_line":"                error_callback\u003dself.compute._neutron_failed_migration_callback)"},{"line_number":5083,"context_line":"            mock_migrate_instance_finish.assert_called()"},{"line_number":5084,"context_line":""},{"line_number":5085,"context_line":"    def test_finish_revert_resize_network_migrate_finish_wait(self):"},{"line_number":5086,"context_line":"        \"\"\"Test that we wait for bind-time events if we have a hybrid-plugged"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_c8dbf04c","line":5083,"range":{"start_line":5083,"start_character":41,"end_line":5083,"end_character":54},"updated":"2019-06-19 18:41:08.000000000","message":"This could still be:\n\nmock_migrate_instance_finish.assert_called_once_with(\n                self.context, instance, migration)","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae40c4ead71a0c9d079af69233b768de85da479e","unresolved":false,"context_lines":[{"line_number":19688,"context_line":"                mock.patch.object(drvr, \u0027_get_guest_xml\u0027,"},{"line_number":19689,"context_line":"                                  side_effect\u003dfake_get_guest_xml),"},{"line_number":19690,"context_line":"                mock.patch.object(network_model.NetworkInfo,"},{"line_number":19691,"context_line":"                                  \u0027get_bind_time_events\u0027, return_value\u003d[])):"},{"line_number":19692,"context_line":"            drvr.finish_revert_migration(\u0027\u0027, instance,"},{"line_number":19693,"context_line":"                                         network_model.NetworkInfo(),"},{"line_number":19694,"context_line":"                                         power_on\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9fb8cfa7_f3fe6069","line":19691,"range":{"start_line":19691,"start_character":35,"end_line":19691,"end_character":55},"updated":"2019-06-12 16:54:21.000000000","message":"This would never be called by the driver, right? Am I missing why this needs to be mocked out? Also, no assertion of whether or not it\u0027s called.","commit_id":"505061ce94b97b18257f65880210047de06faac2"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4439739d8aa5ab19196e09dab46cfddbab1c6f2f","unresolved":false,"context_lines":[{"line_number":19688,"context_line":"                mock.patch.object(drvr, \u0027_get_guest_xml\u0027,"},{"line_number":19689,"context_line":"                                  side_effect\u003dfake_get_guest_xml),"},{"line_number":19690,"context_line":"                mock.patch.object(network_model.NetworkInfo,"},{"line_number":19691,"context_line":"                                  \u0027get_bind_time_events\u0027, return_value\u003d[])):"},{"line_number":19692,"context_line":"            drvr.finish_revert_migration(\u0027\u0027, instance,"},{"line_number":19693,"context_line":"                                         network_model.NetworkInfo(),"},{"line_number":19694,"context_line":"                                         power_on\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9fb8cfa7_2e500f0b","line":19691,"range":{"start_line":19691,"start_character":35,"end_line":19691,"end_character":55},"in_reply_to":"9fb8cfa7_f3fe6069","updated":"2019-06-12 17:39:37.000000000","message":"Err, yeah, I messed that up. If we\u0027re passing a real NetworkInfo, we don\u0027t actually need to mock anything. Same for the other tests that I\u0027ve \"fixed\".","commit_id":"505061ce94b97b18257f65880210047de06faac2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"15674325439346fc46c7793a9c2aa0588b959f02","unresolved":false,"context_lines":[{"line_number":19607,"context_line":"            network_info \u003d network_model.NetworkInfo("},{"line_number":19608,"context_line":"                [network_model.VIF(id\u003duuids.vif)])"},{"line_number":19609,"context_line":"            self.events_passed_to_fake_create \u003d [(\u0027network-vif-plugged\u0027,"},{"line_number":19610,"context_line":"                                                  uuids.vif)]"},{"line_number":19611,"context_line":"            self.drvr.finish_revert_migration("},{"line_number":19612,"context_line":"                context.get_admin_context(), ins_ref,"},{"line_number":19613,"context_line":"                network_info, None, power_on)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9fb8cfa7_4e9b0317","line":19610,"updated":"2019-06-12 17:52:28.000000000","message":"This is used on L19562 to assert that we got events we expected, right? Don\u0027t we need another case here to make sure we\u0027re ignoring the  bind-time (or whatever) vifs?","commit_id":"3534a1785d8940422812ca976e80b13ee63b743b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcc8b53509c8849a37294f64fd894e9f26f2ebf9","unresolved":false,"context_lines":[{"line_number":19607,"context_line":"            network_info \u003d network_model.NetworkInfo("},{"line_number":19608,"context_line":"                [network_model.VIF(id\u003duuids.vif)])"},{"line_number":19609,"context_line":"            self.events_passed_to_fake_create \u003d [(\u0027network-vif-plugged\u0027,"},{"line_number":19610,"context_line":"                                                  uuids.vif)]"},{"line_number":19611,"context_line":"            self.drvr.finish_revert_migration("},{"line_number":19612,"context_line":"                context.get_admin_context(), ins_ref,"},{"line_number":19613,"context_line":"                network_info, None, power_on)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9fb8cfa7_ee6bd7ed","line":19610,"in_reply_to":"9fb8cfa7_4e9b0317","updated":"2019-06-12 19:16:32.000000000","message":"I\u0027ve added a VIF to the network_model that won\u0027t generate an event for the virt driver, yeah.","commit_id":"3534a1785d8940422812ca976e80b13ee63b743b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"088e1ab853e0066a84e67fc6bba27b95fdeaf899","unresolved":false,"context_lines":[{"line_number":17271,"context_line":"                elif (neutron_failure \u003d\u003d \u0027error\u0027 and"},{"line_number":17272,"context_line":"                          not CONF.vif_plugging_is_fatal):"},{"line_number":17273,"context_line":"                    event.wait.assert_called_once_with()"},{"line_number":17274,"context_line":"        else:"},{"line_number":17275,"context_line":"            self.assertEqual(0, prepare.call_count)"},{"line_number":17276,"context_line":""},{"line_number":17277,"context_line":"    def test_create_with_network_events_passed_in(self):"},{"line_number":17278,"context_line":"        self._test_create_with_network_events("}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_8324f6b0","line":17275,"range":{"start_line":17274,"start_character":7,"end_line":17275,"end_character":51},"updated":"2019-06-19 23:29:16.000000000","message":"this test the logic in \n_create_domain_and_network that takes the passed in events\nand set them to [] if is_neutron is false or timeout is 0\n\nhttps://review.opendev.org/#/c/644881/33/nova/virt/libvirt/driver.py@5734","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"088e1ab853e0066a84e67fc6bba27b95fdeaf899","unresolved":false,"context_lines":[{"line_number":19621,"context_line":"                [network_model.VIF(id\u003duuids.normal_plug_time_vif),"},{"line_number":19622,"context_line":"                 network_model.VIF(id\u003duuids.hybrid_bind_time_vif,"},{"line_number":19623,"context_line":"                                   details\u003d{\u0027ovs_hybrid_plug\u0027: True})])"},{"line_number":19624,"context_line":"            self.events_passed_to_fake_create \u003d None"},{"line_number":19625,"context_line":"            if wait_for_vifs:"},{"line_number":19626,"context_line":"                self.events_passed_to_fake_create \u003d ["},{"line_number":19627,"context_line":"                    (\u0027network-vif-plugged\u0027, uuids.normal_plug_time_vif)]"},{"line_number":19628,"context_line":"            self.drvr.finish_revert_migration("},{"line_number":19629,"context_line":"                context.get_admin_context(), ins_ref,"},{"line_number":19630,"context_line":"                network_info, None, power_on)"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_b865a51f","line":19627,"range":{"start_line":19624,"start_character":11,"end_line":19627,"end_character":72},"updated":"2019-06-19 23:29:16.000000000","message":"this shoudl always be\n\nself.events_passed_to_fake_create \u003d [\n                    (\u0027network-vif-plugged\u0027, uuids.normal_plug_time_vif)]\n\nas the filtering based on config is done in \n_create_domain_and_network\nnot in finish_revert_migration\n\nso fake_create_domain above which stubs out _create_domain_and_network\nwill always be called with the event regardless of the config value and they will be drop internally in _create_domain_and_network","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"088e1ab853e0066a84e67fc6bba27b95fdeaf899","unresolved":false,"context_lines":[{"line_number":19636,"context_line":"    def test_finish_revert_migration_power_off(self):"},{"line_number":19637,"context_line":"        self._test_finish_revert_migration(False)"},{"line_number":19638,"context_line":""},{"line_number":19639,"context_line":"    def test_finish_revert_migration_no_vif_timeout(self):"},{"line_number":19640,"context_line":"        self.flags(vif_plugging_timeout\u003d0)"},{"line_number":19641,"context_line":"        self._test_finish_revert_migration(False, wait_for_vifs\u003dFalse)"},{"line_number":19642,"context_line":""},{"line_number":19643,"context_line":"    @mock.patch.object(utils, \u0027is_neutron\u0027, return_value\u003dFalse)"},{"line_number":19644,"context_line":"    def test_finish_revert_migration_no_neutron(self, _):"},{"line_number":19645,"context_line":"        self._test_finish_revert_migration(False, wait_for_vifs\u003dFalse)"},{"line_number":19646,"context_line":""},{"line_number":19647,"context_line":"    def _test_finish_revert_migration_after_crash(self, backup_made\u003dTrue,"},{"line_number":19648,"context_line":"                                                  del_inst_failed\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_e3493260","line":19645,"range":{"start_line":19639,"start_character":4,"end_line":19645,"end_character":70},"updated":"2019-06-19 23:29:16.000000000","message":"these test dont actully test anything useful as we stub out the function that would be affect by those config.\nill remove them and add two to replace them that\ninvoke _test_create_with_network_events above like\n\nhttps://review.opendev.org/#/c/644881/33/nova/tests/unit/virt/libvirt/test_driver.py@17277\n\nwhich will actully assert the correct behavior.\n\ntest_create_with_network_events_non_neutron partly test this already but it does not acttuly pass network events so its not quite what we want but it i is the reason that\n_test_create_with_network_events already has the logic to assert the correct behavior.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"29fa8570a1fbb24bfdfcf99d8fbd8caecc69f154","unresolved":false,"context_lines":[{"line_number":19636,"context_line":"    def test_finish_revert_migration_power_off(self):"},{"line_number":19637,"context_line":"        self._test_finish_revert_migration(False)"},{"line_number":19638,"context_line":""},{"line_number":19639,"context_line":"    def test_finish_revert_migration_no_vif_timeout(self):"},{"line_number":19640,"context_line":"        self.flags(vif_plugging_timeout\u003d0)"},{"line_number":19641,"context_line":"        self._test_finish_revert_migration(False, wait_for_vifs\u003dFalse)"},{"line_number":19642,"context_line":""},{"line_number":19643,"context_line":"    @mock.patch.object(utils, \u0027is_neutron\u0027, return_value\u003dFalse)"},{"line_number":19644,"context_line":"    def test_finish_revert_migration_no_neutron(self, _):"},{"line_number":19645,"context_line":"        self._test_finish_revert_migration(False, wait_for_vifs\u003dFalse)"},{"line_number":19646,"context_line":""},{"line_number":19647,"context_line":"    def _test_finish_revert_migration_after_crash(self, backup_made\u003dTrue,"},{"line_number":19648,"context_line":"                                                  del_inst_failed\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_f5e0f6fa","line":19645,"range":{"start_line":19639,"start_character":4,"end_line":19645,"end_character":70},"in_reply_to":"9fb8cfa7_e3493260","updated":"2019-06-20 17:56:30.000000000","message":"Good catch.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"}],"nova/virt/driver.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"604eea07378dfff89e0c94680457941fcbd53b29","unresolved":false,"context_lines":[{"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":"                                vifs_already_plugged\u003dFalse):"},{"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":19,"id":"bfb3d3c7_a2107601","line":741,"range":{"start_line":741,"start_character":0,"end_line":741,"end_character":60},"updated":"2019-05-30 18:17:19.000000000","message":"Courtesy email to the ML for OOT drivers, if you please. \"We\u0027re changing the virt driver interface; you will have to update your overrides.\"","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b876f856404c76f3aa2e53e7ed867d89c4e5a1e1","unresolved":false,"context_lines":[{"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":"                                vifs_already_plugged\u003dFalse):"},{"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":19,"id":"bfb3d3c7_e2e2eed4","line":741,"range":{"start_line":741,"start_character":0,"end_line":741,"end_character":60},"in_reply_to":"bfb3d3c7_a2107601","updated":"2019-05-30 18:22:10.000000000","message":"Ack. Should I wait for at least 1 +2 to have some confidence that the genera direction proposed here is acceptable?","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3d7c75ff615870797b3f04be5cbbca8ab93bdf19","unresolved":false,"context_lines":[{"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":"                                vifs_already_plugged\u003dFalse):"},{"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":19,"id":"bfb3d3c7_76d8fced","line":741,"range":{"start_line":741,"start_character":0,"end_line":741,"end_character":60},"in_reply_to":"bfb3d3c7_e2e2eed4","updated":"2019-05-30 21:18:09.000000000","message":"Yup, that\u0027s a fine idea, just wanted to note it when I saw it :)","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"604eea07378dfff89e0c94680457941fcbd53b29","unresolved":false,"context_lines":[{"line_number":748,"context_line":"        :param power_on: True if the instance should be powered on, False"},{"line_number":749,"context_line":"                         otherwise"},{"line_number":750,"context_line":"        :param vifs_already_plugged: False if the virt driver should plug VIFs,"},{"line_number":751,"context_line":"                                     true otherwise."},{"line_number":752,"context_line":"        \"\"\""},{"line_number":753,"context_line":"        raise NotImplementedError()"},{"line_number":754,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"bfb3d3c7_2267a6b9","line":751,"range":{"start_line":751,"start_character":37,"end_line":751,"end_character":38},"updated":"2019-05-30 18:17:19.000000000","message":"T","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":748,"context_line":"        :param power_on: True if the instance should be powered on, False"},{"line_number":749,"context_line":"                         otherwise"},{"line_number":750,"context_line":"        :param vifs_already_plugged: False if the virt driver should plug VIFs,"},{"line_number":751,"context_line":"                                     true otherwise."},{"line_number":752,"context_line":"        \"\"\""},{"line_number":753,"context_line":"        raise NotImplementedError()"},{"line_number":754,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_4f6a53ee","line":751,"range":{"start_line":751,"start_character":37,"end_line":751,"end_character":41},"updated":"2019-06-10 16:00:29.000000000","message":"nit: True","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"}],"nova/virt/hyperv/driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"5febd1b1a2df020c3bafcc9e832d199f353339bd","unresolved":false,"context_lines":[{"line_number":329,"context_line":"                                vifs_already_plugged\u003dFalse):"},{"line_number":330,"context_line":"        self._migrationops.finish_revert_migration(context, instance,"},{"line_number":331,"context_line":"                                                   network_info,"},{"line_number":332,"context_line":"                                                   block_device_info, power_on)"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"},{"line_number":335,"context_line":"                         network_info, image_meta, resize_instance,"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_9ee9a91b","line":332,"updated":"2019-06-05 12:53:01.000000000","message":"It looks to me like we should pass vifs_already_plugged down here, which would then be passed to power_on. Otherwise we\u0027re going to plug them again. Guessing the same is true of other drivers where we\u0027ve only updated the call signature.\n\nNot sure if this case is even relevant to hyper-v, but I guess it was already broken and we\u0027re not breaking it here. I\u0027m ok with this.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0bdba5e353045fec1b6f4c1a12535544ddd56251","unresolved":false,"context_lines":[{"line_number":329,"context_line":"                                vifs_already_plugged\u003dFalse):"},{"line_number":330,"context_line":"        self._migrationops.finish_revert_migration(context, instance,"},{"line_number":331,"context_line":"                                                   network_info,"},{"line_number":332,"context_line":"                                                   block_device_info, power_on)"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    def finish_migration(self, context, migration, instance, disk_info,"},{"line_number":335,"context_line":"                         network_info, image_meta, resize_instance,"}],"source_content_type":"text/x-python","patch_set":19,"id":"9fb8cfa7_eeab521a","line":332,"in_reply_to":"9fb8cfa7_9ee9a91b","updated":"2019-06-05 16:37:58.000000000","message":"calling plug on an already plugged interfe in os-vif is safe. the plugings were written to handel that.\n\nhybrid_plug is specific to linux as iptables and linux bridge are not supported on windows.","commit_id":"58bf4be691274132c3a6e96d69f0c8eadded691b"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":5734,"context_line":"        if CONF.vif_plugging_is_fatal:"},{"line_number":5735,"context_line":"            raise exception.VirtualInterfaceCreateException()"},{"line_number":5736,"context_line":""},{"line_number":5737,"context_line":"    def _get_neutron_events(self, network_info):"},{"line_number":5738,"context_line":"        # NOTE(danms): We need to collect any VIFs that are currently"},{"line_number":5739,"context_line":"        # down that we expect a down-\u003eup event for. Anything that is"},{"line_number":5740,"context_line":"        # already up will not undergo that transition, and for"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_1d1c9176","line":5737,"range":{"start_line":5737,"start_character":7,"end_line":5737,"end_character":47},"updated":"2019-06-10 17:27:31.000000000","message":"3.) _get_neutron_events(self, network_info, revert\u003dFalse)","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":5741,"context_line":"        # anything that might be stale (cache-wise) assume it\u0027s"},{"line_number":5742,"context_line":"        # already up so we don\u0027t block on it."},{"line_number":5743,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":5744,"context_line":"                for vif in network_info if vif.get(\u0027active\u0027, True) is False]"},{"line_number":5745,"context_line":""},{"line_number":5746,"context_line":"    def _cleanup_failed_start(self, context, instance, network_info,"},{"line_number":5747,"context_line":"                              block_device_info, guest, destroy_disks):"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_bdb0e58b","line":5744,"range":{"start_line":5744,"start_character":16,"end_line":5744,"end_character":75},"updated":"2019-06-10 17:27:31.000000000","message":"4.) ... to be \nfor vif in network_info if vif.get(\u0027active\u0027, True) is False and (not vif.wait_in_manager() or revert \u003d\u003d False)\nthen back down in _create_domain_and_network ...","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":5753,"context_line":"                         block_device_info\u003dblock_device_info,"},{"line_number":5754,"context_line":"                         destroy_disks\u003ddestroy_disks)"},{"line_number":5755,"context_line":""},{"line_number":5756,"context_line":"    def _create_domain_and_network(self, context, xml, instance, network_info,"},{"line_number":5757,"context_line":"                                   block_device_info\u003dNone, power_on\u003dTrue,"},{"line_number":5758,"context_line":"                                   vifs_already_plugged\u003dFalse,"},{"line_number":5759,"context_line":"                                   post_xml_callback\u003dNone,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_1d97b1ee","line":5756,"range":{"start_line":5756,"start_character":8,"end_line":5756,"end_character":34},"updated":"2019-06-10 17:27:31.000000000","message":"1) as i say below we need to add a new kwarg\n\nrevert\u003dFalse which we will set to true in finish_revert_migration ...","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":5764,"context_line":"        if (self._conn_supports_start_paused and"},{"line_number":5765,"context_line":"            utils.is_neutron() and not"},{"line_number":5766,"context_line":"            vifs_already_plugged and power_on and timeout):"},{"line_number":5767,"context_line":"            events \u003d self._get_neutron_events(network_info)"},{"line_number":5768,"context_line":"        else:"},{"line_number":5769,"context_line":"            events \u003d []"},{"line_number":5770,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_9d8ac1bc","line":5767,"range":{"start_line":5767,"start_character":26,"end_line":5767,"end_character":45},"updated":"2019-06-10 17:27:31.000000000","message":"2.) ... we then modify this above passing in revert...","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":5771,"context_line":"        pause \u003d bool(events)"},{"line_number":5772,"context_line":"        guest \u003d None"},{"line_number":5773,"context_line":"        try:"},{"line_number":5774,"context_line":"            with self.virtapi.wait_for_instance_event("},{"line_number":5775,"context_line":"                    instance, events, deadline\u003dtimeout,"},{"line_number":5776,"context_line":"                    error_callback\u003dself._neutron_failed_callback):"},{"line_number":5777,"context_line":"                self.plug_vifs(instance, network_info)"},{"line_number":5778,"context_line":"                self.firewall_driver.setup_basic_filtering(instance,"},{"line_number":5779,"context_line":"                                                           network_info)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_f8242bb1","line":5776,"range":{"start_line":5774,"start_character":12,"end_line":5776,"end_character":66},"updated":"2019-06-10 17:27:31.000000000","message":"5.) ... we will now only wait for event that should be on the waited on in the driver.","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a19e1f9a263c37bdd9b5c59e20f387f28b2a14ff","unresolved":false,"context_lines":[{"line_number":9160,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info, disk_info,"},{"line_number":9161,"context_line":"                                  instance.image_meta,"},{"line_number":9162,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":9163,"context_line":"        self._create_domain_and_network("},{"line_number":9164,"context_line":"            context, xml, instance, network_info,"},{"line_number":9165,"context_line":"            block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":9166,"context_line":"            vifs_already_plugged\u003dvifs_already_plugged)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_fd5e5d74","line":9163,"range":{"start_line":9163,"start_character":13,"end_line":9163,"end_character":39},"updated":"2019-06-10 17:27:31.000000000","message":"0.) number are the order to read these connected comments in.\n\nin the driver (here) we would then want to extend this with a new revert flag which we set to true ...","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3b5a7964cec7a93718dfa99374488f3efba27704","unresolved":false,"context_lines":[{"line_number":9163,"context_line":"        self._create_domain_and_network("},{"line_number":9164,"context_line":"            context, xml, instance, network_info,"},{"line_number":9165,"context_line":"            block_device_info\u003dblock_device_info, power_on\u003dpower_on,"},{"line_number":9166,"context_line":"            vifs_already_plugged\u003dvifs_already_plugged)"},{"line_number":9167,"context_line":""},{"line_number":9168,"context_line":"        if power_on:"},{"line_number":9169,"context_line":"            timer \u003d loopingcall.FixedIntervalLoopingCall("}],"source_content_type":"text/x-python","patch_set":23,"id":"9fb8cfa7_8f7b6bbc","line":9166,"updated":"2019-06-10 16:00:29.000000000","message":"Technically we should probably have a libvirt driver unit test that asserts the kwarg from the finish_revert_migration signature gets passed through here, yeah?","commit_id":"1abb81a7b8646c4bf54e3ab0d11146e1c1d182a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"15674325439346fc46c7793a9c2aa0588b959f02","unresolved":false,"context_lines":[{"line_number":5764,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5765,"context_line":"        if external_events:"},{"line_number":5766,"context_line":"            events \u003d external_events"},{"line_number":5767,"context_line":"        elif (self._conn_supports_start_paused and"},{"line_number":5768,"context_line":"            utils.is_neutron() and not"},{"line_number":5769,"context_line":"            vifs_already_plugged and power_on and timeout):"},{"line_number":5770,"context_line":"            events \u003d self._get_neutron_events(network_info)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9fb8cfa7_eeb6778a","line":5767,"updated":"2019-06-12 17:52:28.000000000","message":"Maybe I\u0027m missing it in all the existing coverage, but.. are we sure that we\u0027re covering the case where events are provided? I think your tests might pass if we removed this bit because L5770 will yield the same thing, no?","commit_id":"3534a1785d8940422812ca976e80b13ee63b743b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcc8b53509c8849a37294f64fd894e9f26f2ebf9","unresolved":false,"context_lines":[{"line_number":5764,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5765,"context_line":"        if external_events:"},{"line_number":5766,"context_line":"            events \u003d external_events"},{"line_number":5767,"context_line":"        elif (self._conn_supports_start_paused and"},{"line_number":5768,"context_line":"            utils.is_neutron() and not"},{"line_number":5769,"context_line":"            vifs_already_plugged and power_on and timeout):"},{"line_number":5770,"context_line":"            events \u003d self._get_neutron_events(network_info)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9fb8cfa7_f9db9f4b","line":5767,"in_reply_to":"9fb8cfa7_eeb6778a","updated":"2019-06-12 19:16:32.000000000","message":"This bit of the code isn\u0027t actually tested, you\u0027re right. The stuff in fake_create_domain around test_driver.py@19558 tests lines 9165-9168 below, but not here. Especially with your comment at test_driver.py@19610 addressed, we\u0027ll be making sure that only the correct plug-time events are passed to _create_domain_and_network.\n\nI\u0027ll add a specific test to make sure that when _create_domain_and_network is passed events, we actually pass them on to wait_for_instance_event.","commit_id":"3534a1785d8940422812ca976e80b13ee63b743b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"15674325439346fc46c7793a9c2aa0588b959f02","unresolved":false,"context_lines":[{"line_number":9162,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info, disk_info,"},{"line_number":9163,"context_line":"                                  instance.image_meta,"},{"line_number":9164,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":9165,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":9166,"context_line":"            events \u003d network_info.get_plug_time_events()"},{"line_number":9167,"context_line":"        else:"},{"line_number":9168,"context_line":"            events \u003d []"}],"source_content_type":"text/x-python","patch_set":29,"id":"9fb8cfa7_ce8613ba","line":9165,"updated":"2019-06-12 17:52:28.000000000","message":"I don\u0027t think you\u0027re covering this conditional (the else case actually).","commit_id":"3534a1785d8940422812ca976e80b13ee63b743b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcc8b53509c8849a37294f64fd894e9f26f2ebf9","unresolved":false,"context_lines":[{"line_number":9162,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info, disk_info,"},{"line_number":9163,"context_line":"                                  instance.image_meta,"},{"line_number":9164,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":9165,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron():"},{"line_number":9166,"context_line":"            events \u003d network_info.get_plug_time_events()"},{"line_number":9167,"context_line":"        else:"},{"line_number":9168,"context_line":"            events \u003d []"}],"source_content_type":"text/x-python","patch_set":29,"id":"9fb8cfa7_349e1eb9","line":9165,"in_reply_to":"9fb8cfa7_ce8613ba","updated":"2019-06-12 19:16:32.000000000","message":"Done. And I need to do the same for the equivalent code in the compute manager, actually.","commit_id":"3534a1785d8940422812ca976e80b13ee63b743b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":5734,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5735,"context_line":"        if external_events:"},{"line_number":5736,"context_line":"            events \u003d external_events"},{"line_number":5737,"context_line":"        elif (self._conn_supports_start_paused and"},{"line_number":5738,"context_line":"            utils.is_neutron() and not"},{"line_number":5739,"context_line":"            vifs_already_plugged and power_on and timeout):"},{"line_number":5740,"context_line":"            events \u003d self._get_neutron_events(network_info)"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_4f02bc40","line":5737,"range":{"start_line":5737,"start_character":19,"end_line":5737,"end_character":46},"updated":"2019-06-18 20:23:42.000000000","message":"What about this flag in your external events calculation logic in finish_revert_migration? Kind of sucks that you\u0027ve missed 2 flags from this logic in that new duplicated logic (_conn_supports_start_paused and power_on). It would be nice to DRY that up somehow but I don\u0027t really have great suggestions.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":5734,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5735,"context_line":"        if external_events:"},{"line_number":5736,"context_line":"            events \u003d external_events"},{"line_number":5737,"context_line":"        elif (self._conn_supports_start_paused and"},{"line_number":5738,"context_line":"            utils.is_neutron() and not"},{"line_number":5739,"context_line":"            vifs_already_plugged and power_on and timeout):"},{"line_number":5740,"context_line":"            events \u003d self._get_neutron_events(network_info)"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_9b5552fd","line":5737,"range":{"start_line":5737,"start_character":19,"end_line":5737,"end_character":46},"in_reply_to":"9fb8cfa7_4f02bc40","updated":"2019-06-19 16:32:12.000000000","message":"Yeah, I\u0027ll consolidate the external events logic inside _create_domain_and_network. finish_revert_migration will always pass it external_events\u003dget_plug_time_events(), and _create_domain_and_network will use that additional data point when deciding what events to listen for.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":9104,"context_line":"                raise"},{"line_number":9105,"context_line":""},{"line_number":9106,"context_line":"    def finish_revert_migration(self, context, instance, network_info,"},{"line_number":9107,"context_line":"                                block_device_info\u003dNone, power_on\u003dTrue):"},{"line_number":9108,"context_line":"        LOG.debug(\"Starting finish_revert_migration\","},{"line_number":9109,"context_line":"                  instance\u003dinstance)"},{"line_number":9110,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_8fcf5428","line":9107,"range":{"start_line":9107,"start_character":56,"end_line":9107,"end_character":64},"updated":"2019-06-18 20:23:42.000000000","message":"What if power_on is False? Do we need to wait for events? _create_domain_and_network will not wait for events if power_on is False, so it seems you shouldn\u0027t either.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":9104,"context_line":"                raise"},{"line_number":9105,"context_line":""},{"line_number":9106,"context_line":"    def finish_revert_migration(self, context, instance, network_info,"},{"line_number":9107,"context_line":"                                block_device_info\u003dNone, power_on\u003dTrue):"},{"line_number":9108,"context_line":"        LOG.debug(\"Starting finish_revert_migration\","},{"line_number":9109,"context_line":"                  instance\u003dinstance)"},{"line_number":9110,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_bb5a9627","line":9107,"range":{"start_line":9107,"start_character":56,"end_line":9107,"end_character":64},"in_reply_to":"9fb8cfa7_8fcf5428","updated":"2019-06-19 16:32:12.000000000","message":"Done, see above.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":9132,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info, disk_info,"},{"line_number":9133,"context_line":"                                  instance.image_meta,"},{"line_number":9134,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":9135,"context_line":"        events \u003d None"},{"line_number":9136,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":9137,"context_line":"            events \u003d network_info.get_plug_time_events()"},{"line_number":9138,"context_line":"            if events:"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_8f5af480","line":9135,"updated":"2019-06-18 20:23:42.000000000","message":"It would be good to have a comment with this code.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":9132,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info, disk_info,"},{"line_number":9133,"context_line":"                                  instance.image_meta,"},{"line_number":9134,"context_line":"                                  block_device_info\u003dblock_device_info)"},{"line_number":9135,"context_line":"        events \u003d None"},{"line_number":9136,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":9137,"context_line":"            events \u003d network_info.get_plug_time_events()"},{"line_number":9138,"context_line":"            if events:"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_9b7c7273","line":9135,"in_reply_to":"9fb8cfa7_8f5af480","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe0ffbe51d5da5db37a1f20a02024cb0b852069c","unresolved":false,"context_lines":[{"line_number":9136,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":9137,"context_line":"            events \u003d network_info.get_plug_time_events()"},{"line_number":9138,"context_line":"            if events:"},{"line_number":9139,"context_line":"                LOG.debug(\u0027Will wait for plug-time events: %s\u0027, events)"},{"line_number":9140,"context_line":"        self._create_domain_and_network("},{"line_number":9141,"context_line":"            context, xml, instance, network_info,"},{"line_number":9142,"context_line":"            block_device_info\u003dblock_device_info, power_on\u003dpower_on,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_cf60ec38","line":9139,"updated":"2019-06-18 20:23:42.000000000","message":"Throw instance\u003dinstance in here for context.","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d4c6d9aff61a707ddf3801722f3e411121859c7","unresolved":false,"context_lines":[{"line_number":9136,"context_line":"        if CONF.vif_plugging_timeout and utils.is_neutron() and network_info:"},{"line_number":9137,"context_line":"            events \u003d network_info.get_plug_time_events()"},{"line_number":9138,"context_line":"            if events:"},{"line_number":9139,"context_line":"                LOG.debug(\u0027Will wait for plug-time events: %s\u0027, events)"},{"line_number":9140,"context_line":"        self._create_domain_and_network("},{"line_number":9141,"context_line":"            context, xml, instance, network_info,"},{"line_number":9142,"context_line":"            block_device_info\u003dblock_device_info, power_on\u003dpower_on,"}],"source_content_type":"text/x-python","patch_set":32,"id":"9fb8cfa7_5b3fdab4","line":9139,"in_reply_to":"9fb8cfa7_cf60ec38","updated":"2019-06-19 16:32:12.000000000","message":"Done","commit_id":"3ec1fc4e3d173aaae6b31fe011b22f40b5ef43f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"line_number":5734,"context_line":"        timeout \u003d CONF.vif_plugging_timeout"},{"line_number":5735,"context_line":"        if (self._conn_supports_start_paused and utils.is_neutron()"},{"line_number":5736,"context_line":"                and not vifs_already_plugged and power_on and timeout):"},{"line_number":5737,"context_line":"            events \u003d (external_events if external_events"},{"line_number":5738,"context_line":"                      else self._get_neutron_events(network_info))"},{"line_number":5739,"context_line":"        else:"},{"line_number":5740,"context_line":"            events \u003d []"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_68c5245b","line":5737,"updated":"2019-06-19 18:41:08.000000000","message":"Yeah this is a lot cleaner than PS32.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"088e1ab853e0066a84e67fc6bba27b95fdeaf899","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() if network_info else None"},{"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":33,"id":"9fb8cfa7_7802cd71","line":9138,"range":{"start_line":9138,"start_character":8,"end_line":9138,"end_character":78},"updated":"2019-06-19 23:29:16.000000000","message":"network_info should always be passed to this function and when get_plug_events is called on an empty network_info object it will retrun an empty list which will do the right thing in \"if events:\" below so this can be simplifed.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"29fa8570a1fbb24bfdfcf99d8fbd8caecc69f154","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() if network_info else None"},{"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":33,"id":"9fb8cfa7_95765a3e","line":9138,"range":{"start_line":9138,"start_character":8,"end_line":9138,"end_character":78},"in_reply_to":"9fb8cfa7_7802cd71","updated":"2019-06-20 17:56:30.000000000","message":"I want to say Dan asked for checking for None before as a precaution, but agree that network_info should always be passed in and should at least be an empty NetworkInfo object, i.e.:\n\nhttps://github.com/openstack/nova/blob/c18f7f47f628e266e5b69f4b9733a0f25ed4ffdd/nova/objects/instance.py#L1163\n\nhttps://github.com/openstack/nova/blob/c18f7f47f628e266e5b69f4b9733a0f25ed4ffdd/nova/network/neutronv2/api.py#L1763\n\nI\u0027m OK either way.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"47ea33c53cdcf0ac162a941fdd9e3acdc675eb7a","unresolved":false,"context_lines":[{"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() if network_info else None"},{"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)"},{"line_number":9142,"context_line":"        self._create_domain_and_network("},{"line_number":9143,"context_line":"            context, xml, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_c8b450f5","line":9140,"range":{"start_line":9140,"start_character":12,"end_line":9140,"end_character":58},"updated":"2019-06-19 18:41:08.000000000","message":"I think what you had makes more sense here:\n\nLOG.debug(\u0027Will wait for plug-time events:","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"56a55c48547a454db94987326b3f4356bcc58ef8","unresolved":false,"context_lines":[{"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() if network_info else None"},{"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)"},{"line_number":9142,"context_line":"        self._create_domain_and_network("},{"line_number":9143,"context_line":"            context, xml, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9fb8cfa7_a89ebc51","line":9140,"range":{"start_line":9140,"start_character":12,"end_line":9140,"end_character":58},"in_reply_to":"9fb8cfa7_c8b450f5","updated":"2019-06-19 18:47:46.000000000","message":"I changed it because at this point we don\u0027t know if we\u0027re actually going to wait (_create_domain_and_network makes that decision), so I tried to remove that \"promise\", while still indicating that plug-time events are expected.","commit_id":"de96e3c3b7a7e628ac1e7dcdb34d256b1b95e2f7"}],"releasenotes/notes/finish-revert-migration-method-signature-27bba16f9fdc0c4d.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d440e586b626e765b46e0f95acc53f38b7c1c85f","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"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, \u0027vifs_already_plugged\u0027. When set to \u0027True\u0027, this"},{"line_number":6,"context_line":"    indicates to the virt driver that VIFs have already been plugged in the"},{"line_number":7,"context_line":"    compute manager. If you have out of tree virt drivers, please update them."}],"source_content_type":"text/x-yaml","patch_set":22,"id":"9fb8cfa7_4646d951","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":18},"updated":"2019-06-07 16:23:24.000000000","message":"The ComputeDriver","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"7fb2ed584df1e7a8402d9299e6ae2682a8451d31","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"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, \u0027vifs_already_plugged\u0027. When set to \u0027True\u0027, this"},{"line_number":6,"context_line":"    indicates to the virt driver that VIFs have already been plugged in the"},{"line_number":7,"context_line":"    compute manager. If you have out of tree virt drivers, please update them."}],"source_content_type":"text/x-yaml","patch_set":22,"id":"9fb8cfa7_a60eb58a","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":18},"in_reply_to":"9fb8cfa7_4646d951","updated":"2019-06-07 18:01:51.000000000","message":"Had no idea that was the official class name. Done.","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d440e586b626e765b46e0f95acc53f38b7c1c85f","unresolved":false,"context_lines":[{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"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, \u0027vifs_already_plugged\u0027. When set to \u0027True\u0027, this"},{"line_number":6,"context_line":"    indicates to the virt driver that VIFs have already been plugged in the"},{"line_number":7,"context_line":"    compute manager. If you have out of tree virt drivers, please update them."}],"source_content_type":"text/x-yaml","patch_set":22,"id":"9fb8cfa7_067a2187","line":5,"range":{"start_line":5,"start_character":58,"end_line":5,"end_character":64},"updated":"2019-06-07 16:23:24.000000000","message":"True - it\u0027s not a string.","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"7fb2ed584df1e7a8402d9299e6ae2682a8451d31","unresolved":false,"context_lines":[{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"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, \u0027vifs_already_plugged\u0027. When set to \u0027True\u0027, this"},{"line_number":6,"context_line":"    indicates to the virt driver that VIFs have already been plugged in the"},{"line_number":7,"context_line":"    compute manager. If you have out of tree virt drivers, please update them."}],"source_content_type":"text/x-yaml","patch_set":22,"id":"9fb8cfa7_c61169ae","line":5,"range":{"start_line":5,"start_character":58,"end_line":5,"end_character":64},"in_reply_to":"9fb8cfa7_067a2187","updated":"2019-06-07 18:01:51.000000000","message":"Done","commit_id":"44515442f5fe1ae99e56884b8feb356060daa44b"}]}