)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e27688a581b8b543f766948643f68eb25003996","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t wait for vif plug events during _hard_reboot"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Because we unplug vifs locally, the change may go undetected by the"},{"line_number":10,"context_line":"neutron agent or server, so neutron may not know the vif was unplugged"},{"line_number":11,"context_line":"in the first place and may never send a plug event."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f93b717_b7e42e3d","line":9,"range":{"start_line":9,"start_character":23,"end_line":9,"end_character":30},"updated":"2018-02-02 14:21:28.000000000","message":"I think this came from me in one of the comments in the libvirt driver code, but I\u0027m not really sure what this means anymore.","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2f8ba75c2160a2d016c29ca57b452b73ca61afd1","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Don\u0027t wait for vif plug events during _hard_reboot"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Because we unplug vifs locally, the change may go undetected by the"},{"line_number":10,"context_line":"neutron agent or server, so neutron may not know the vif was unplugged"},{"line_number":11,"context_line":"in the first place and may never send a plug event."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f93b717_177ba227","line":9,"range":{"start_line":9,"start_character":23,"end_line":9,"end_character":30},"in_reply_to":"5f93b717_b7e42e3d","updated":"2018-02-02 14:32:05.000000000","message":"It comes from the fact we unplug the VIFs when rebooting here since we destroy the instance by passing the network info https://github.com/openstack/nova/blob/59e3ce09c60b0f6191f9a7207f92ac01e1e8b404/nova/virt/libvirt/driver.py#L2674\nwhich itselfs later calls cleanup() which unplugs the VIFs :\nhttps://github.com/openstack/nova/blob/59e3ce09c60b0f6191f9a7207f92ac01e1e8b404/nova/virt/libvirt/driver.py#L1004-L1005\n\nSo, since the problem is about Neutron not notified that we unplugged the VIFs, we can safely assume that if we go to https://github.com/openstack/nova/blob/59e3ce09c60b0f6191f9a7207f92ac01e1e8b404/nova/virt/libvirt/driver.py#L2711 we haven\u0027t got an exception when unplugging, hence this is successful and we don\u0027t need to wait for Neutron.","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50b7bd090a4c57501c4ab1db40c493729fa9721f","unresolved":false,"context_lines":[{"line_number":2709,"context_line":"        # Initialize all the necessary networking, block devices and"},{"line_number":2710,"context_line":"        # start the instance."},{"line_number":2711,"context_line":"        # NOTE(melwitt): Pass vifs_already_plugged\u003dTrue here even though we\u0027ve"},{"line_number":2712,"context_line":"        # unplugged vifs earlier. Because we unplug VIFs locally, the status"},{"line_number":2713,"context_line":"        # change in the port might go undetected by the neutron L2 agent (or"},{"line_number":2714,"context_line":"        # neutron server) so neutron may not know that the VIF was unplugged in"},{"line_number":2715,"context_line":"        # the first place and never send an event."}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_573ccadf","line":2712,"range":{"start_line":2712,"start_character":57,"end_line":2712,"end_character":64},"updated":"2018-02-02 14:48:21.000000000","message":"Not sure what this means, and how it\u0027s different from a nova plug on server create and unplug on server delete.","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"743615d3ac0d115468773da5792c9a92b5143fca","unresolved":false,"context_lines":[{"line_number":2709,"context_line":"        # Initialize all the necessary networking, block devices and"},{"line_number":2710,"context_line":"        # start the instance."},{"line_number":2711,"context_line":"        # NOTE(melwitt): Pass vifs_already_plugged\u003dTrue here even though we\u0027ve"},{"line_number":2712,"context_line":"        # unplugged vifs earlier. Because we unplug VIFs locally, the status"},{"line_number":2713,"context_line":"        # change in the port might go undetected by the neutron L2 agent (or"},{"line_number":2714,"context_line":"        # neutron server) so neutron may not know that the VIF was unplugged in"},{"line_number":2715,"context_line":"        # the first place and never send an event."}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_9aa037f6","line":2712,"range":{"start_line":2712,"start_character":57,"end_line":2712,"end_character":64},"in_reply_to":"5f93b717_573ccadf","updated":"2018-02-02 14:56:32.000000000","message":"If your comment is about s/locally//, I\u0027m not opposed to that. I\u0027m just saying that I see the VIF unplug call before we enter that section of code because when hard rebooting, we first destroy and cleanup the instance, which has some nice effect to unplug the attached VIFs.","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7db2b38be7ccc6b0eb33d42d1c022ebd605e4610","unresolved":false,"context_lines":[{"line_number":2709,"context_line":"        # Initialize all the necessary networking, block devices and"},{"line_number":2710,"context_line":"        # start the instance."},{"line_number":2711,"context_line":"        # NOTE(melwitt): Pass vifs_already_plugged\u003dTrue here even though we\u0027ve"},{"line_number":2712,"context_line":"        # unplugged vifs earlier. Because we unplug VIFs locally, the status"},{"line_number":2713,"context_line":"        # change in the port might go undetected by the neutron L2 agent (or"},{"line_number":2714,"context_line":"        # neutron server) so neutron may not know that the VIF was unplugged in"},{"line_number":2715,"context_line":"        # the first place and never send an event."}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_bd2c0dac","line":2712,"range":{"start_line":2712,"start_character":57,"end_line":2712,"end_character":64},"in_reply_to":"5f93b717_9aa037f6","updated":"2018-02-02 15:59:13.000000000","message":"I don\u0027t see how saying that we\u0027ve already unplugged the VIFs means that we don\u0027t need to wait for the VIFs to be plugged, which happens in _create_domain_and_network.\n\nThe bug was reported against the linuxbridge job and according to Sean Mooney, the LB agent is different from the OVS agent in how it detects changes, and there is a window where it could miss the changes in the LB agent:\n\n(9:47:21 AM) sean-k-mooney: mriedem: for ovs neuton can subscrible to notification form the ovsdb for port add/remove events\n(9:47:45 AM) sean-k-mooney: melwitt: for linuxbridge i belive they have to poll\n(9:50:13 AM) sean-k-mooney: mriedem: if you do a hard reboot we will actully terminate the qemu process and remove the tap and re add it\n(9:50:28 AM) sean-k-mooney: if you are polling you may miss that\n(9:50:55 AM) sean-k-mooney: with ovs you will get the notifcation from the ovsdb for the add and remove","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e5836a5a5fef10e9243ec068c61131c4cafcfa68","unresolved":false,"context_lines":[{"line_number":2715,"context_line":"        # the first place and never send an event."},{"line_number":2716,"context_line":"        self._create_domain_and_network(context, xml, instance, network_info,"},{"line_number":2717,"context_line":"                                        block_device_info\u003dblock_device_info,"},{"line_number":2718,"context_line":"                                        vifs_already_plugged\u003dTrue)"},{"line_number":2719,"context_line":"        self._prepare_pci_devices_for_use("},{"line_number":2720,"context_line":"            pci_manager.get_instance_pci_devs(instance, \u0027all\u0027))"},{"line_number":2721,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_eb1414eb","line":2718,"range":{"start_line":2718,"start_character":40,"end_line":2718,"end_character":60},"updated":"2018-02-02 10:16:10.000000000","message":"the naming of that parameter is super confusing, because even if the VIFs are already plugged, we still plug them again within _create_domain_and_network().\n\nBy default, passing that argument to True just makes Nova not waiting for Neutron events, that\u0027s it...\n\nAnyway, just food for thoughts, nothing related to that change which now that I looked at the libvirt code makes understandable to pass vifs_already_plugged\u003dTrue (ie. \"please Nova, don\u0027t wait for Neutron\") given we did the unplug locally.\n\nActually, with looking at the code, the only case where we explicitly wait for Neutron is during a spawn(). All the other server actions *WON\u0027T* wait for it once we merge that change. \nWorth reconsidering the interest of that parameter, nope?","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"c8a09b1f770a824dd712725087ab7a37edbab98e","unresolved":false,"context_lines":[{"line_number":2715,"context_line":"        # the first place and never send an event."},{"line_number":2716,"context_line":"        self._create_domain_and_network(context, xml, instance, network_info,"},{"line_number":2717,"context_line":"                                        block_device_info\u003dblock_device_info,"},{"line_number":2718,"context_line":"                                        vifs_already_plugged\u003dTrue)"},{"line_number":2719,"context_line":"        self._prepare_pci_devices_for_use("},{"line_number":2720,"context_line":"            pci_manager.get_instance_pci_devs(instance, \u0027all\u0027))"},{"line_number":2721,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_a6d40349","line":2718,"range":{"start_line":2718,"start_character":40,"end_line":2718,"end_character":60},"in_reply_to":"5f93b717_eb1414eb","updated":"2018-02-02 11:04:49.000000000","message":"Agreed, something like wait_for_vif_plugged_event and flip the conditional in _create_domain_and_network?","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7db2b38be7ccc6b0eb33d42d1c022ebd605e4610","unresolved":false,"context_lines":[{"line_number":5369,"context_line":"        # anything that might be stale (cache-wise) assume it\u0027s"},{"line_number":5370,"context_line":"        # already up so we don\u0027t block on it."},{"line_number":5371,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":5372,"context_line":"                for vif in network_info if vif.get(\u0027active\u0027, True) is False]"},{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"    def _cleanup_failed_start(self, context, instance, network_info,"},{"line_number":5375,"context_line":"                              block_device_info, guest, destroy_disks):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_802a067c","line":5372,"range":{"start_line":5372,"start_character":43,"end_line":5372,"end_character":66},"updated":"2018-02-02 15:59:13.000000000","message":"At this point we likely have a stale cache, because we haven\u0027t refreshed it after we unplugged the VIFs (and we didn\u0027t wait for the vif-unplugged event). I wonder if this has any impact on what we\u0027re waiting for, i.e. are we waiting for a vif-plugged on VIFs actually active VIFs and the notification has already happened? Maybe we need to be waiting for the vif-unplugged event before we go further with plugging VIFs to know they are actually inactive? Else, we just assume they will be plugged and we just don\u0027t wait.","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3d0e72e4a3d4b7911ce272bbeb42ebac5f849264","unresolved":false,"context_lines":[{"line_number":5369,"context_line":"        # anything that might be stale (cache-wise) assume it\u0027s"},{"line_number":5370,"context_line":"        # already up so we don\u0027t block on it."},{"line_number":5371,"context_line":"        return [(\u0027network-vif-plugged\u0027, vif[\u0027id\u0027])"},{"line_number":5372,"context_line":"                for vif in network_info if vif.get(\u0027active\u0027, True) is False]"},{"line_number":5373,"context_line":""},{"line_number":5374,"context_line":"    def _cleanup_failed_start(self, context, instance, network_info,"},{"line_number":5375,"context_line":"                              block_device_info, guest, destroy_disks):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f93b717_a0ee221f","line":5372,"range":{"start_line":5372,"start_character":43,"end_line":5372,"end_character":66},"in_reply_to":"5f93b717_802a067c","updated":"2018-02-02 16:06:05.000000000","message":"We will only wait for vifs that have active\u003dFalse and in the logs we can see that the vif goes from active\u003dTrue to active\u003dFalse during the rebuild. Weirdly, we get a plug event during the libvirt domain teardown *before* we even unplug the vif. But then we never receive another plug event.","commit_id":"0e2fad4a198a31e3740ca6215e609bcf3b86d348"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79a7fd67cdeb586475203b249c9cee5023c4d691","unresolved":false,"context_lines":[{"line_number":2708,"context_line":""},{"line_number":2709,"context_line":"        # Initialize all the necessary networking, block devices and"},{"line_number":2710,"context_line":"        # start the instance."},{"line_number":2711,"context_line":"        # NOTE(melwitt): Pass vifs_already_plugged\u003dTrue here even though we\u0027ve"},{"line_number":2712,"context_line":"        # unplugged vifs earlier. The behavior of neutron plug events depends"},{"line_number":2713,"context_line":"        # on which vif type we\u0027re using and we are working with a stale network"},{"line_number":2714,"context_line":"        # info cache here, so won\u0027t rely on waiting for neutron plug events."},{"line_number":2715,"context_line":"        # vifs_already_plugged\u003dTrue means \"do not wait for neutron plug events\""},{"line_number":2716,"context_line":"        self._create_domain_and_network(context, xml, instance, network_info,"},{"line_number":2717,"context_line":"                                        block_device_info\u003dblock_device_info,"},{"line_number":2718,"context_line":"                                        vifs_already_plugged\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_711c6e77","line":2715,"range":{"start_line":2711,"start_character":8,"end_line":2715,"end_character":79},"updated":"2018-02-02 18:41:52.000000000","message":"well I don\u0027t really have a better alternative so I guess this makes sense for now at least.\n\nI still think it might be work looking into can we notify neutron explicitly somehow that we just rebooted or are about to reboot the vm. that way it know it will have work to do then we can re enable waiting for the plug events but that is not easily back portable so this is probably the best we can do for queens.","commit_id":"236bb54493385bcfe2034e8b0fd7a387fa66635a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"78ec438bdc29ad1a94f9b70a8dd215eebe75292a","unresolved":false,"context_lines":[{"line_number":2715,"context_line":"        # vifs_already_plugged\u003dTrue means \"do not wait for neutron plug events\""},{"line_number":2716,"context_line":"        self._create_domain_and_network(context, xml, instance, network_info,"},{"line_number":2717,"context_line":"                                        block_device_info\u003dblock_device_info,"},{"line_number":2718,"context_line":"                                        vifs_already_plugged\u003dTrue)"},{"line_number":2719,"context_line":"        self._prepare_pci_devices_for_use("},{"line_number":2720,"context_line":"            pci_manager.get_instance_pci_devs(instance, \u0027all\u0027))"},{"line_number":2721,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa0c359_9b80f98d","line":2718,"updated":"2018-02-06 14:49:57.000000000","message":"This disables it for all VIF types. I think it would be a lot better if we just did it for linuxbridge VIFs, otherwise we\u0027re opening up the original race (from like 2012) to all people here.","commit_id":"236bb54493385bcfe2034e8b0fd7a387fa66635a"}]}