)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"089ebc3488781881e98fdd35e18b3fe9a4837d96","unresolved":false,"context_lines":[{"line_number":7,"context_line":"ovsdb monitor: handle modified ports"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch ensures that the ovsdb monitor propagates the events"},{"line_number":10,"context_line":"recevived for modified ports."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"We\u0027ll use a new list for the modified ports, which the neutron ovs"},{"line_number":13,"context_line":"agent can handle using the same code path as the \"added\" ones."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5faad753_337f2116","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":9},"updated":"2019-09-15 18:00:41.000000000","message":"received","commit_id":"2a22b0103f2f02bc4934bc7f7e0aebc99d60d727"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"2ea81746698f58fe5e8332ce0d9437580a59a1cc","unresolved":false,"context_lines":[{"line_number":7,"context_line":"ovsdb monitor: handle modified ports"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch ensures that the ovsdb monitor propagates the events"},{"line_number":10,"context_line":"recevived for modified ports."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"We\u0027ll use a new list for the modified ports, which the neutron ovs"},{"line_number":13,"context_line":"agent can handle using the same code path as the \"added\" ones."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5faad753_bad8be7a","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":9},"in_reply_to":"5faad753_337f2116","updated":"2019-09-16 08:11:00.000000000","message":"Whops, thanks.","commit_id":"2a22b0103f2f02bc4934bc7f7e0aebc99d60d727"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c504eb4ca80fa3e5414072f8a5ad96e4823b28a3","unresolved":false,"context_lines":[{"line_number":18,"context_line":"ofport when the VMs are powered back on (different than the initial"},{"line_number":19,"context_line":"one). With this patch applied, \"modify\" events will be propagated"},{"line_number":20,"context_line":"to the ovs agent, which will then update the OpenFlow rules."},{"line_number":21,"context_line":"The old rules are cleaned up by \"update_stale_ofport_rules\"."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Closes-Bug: #1843870"},{"line_number":24,"context_line":"Co-authored-by: Alin Serdean \u003caserdean@cloudbasesolutions.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5faad753_15dc45e2","line":21,"updated":"2019-09-16 09:33:36.000000000","message":"nit: \n\"The old rules are cleaned up by \"update_stale_ofport_rules\" in the first event, when the ofport becomes -1 (invalid)\"","commit_id":"327b3c60eedf40a9ca85c734048ee51d5c3e9781"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"dcd888af7b869c9c935d739a23a8b0816886d52f","unresolved":false,"context_lines":[{"line_number":18,"context_line":"ofport when the VMs are powered back on (different than the initial"},{"line_number":19,"context_line":"one). With this patch applied, \"modify\" events will be propagated"},{"line_number":20,"context_line":"to the ovs agent, which will then update the OpenFlow rules."},{"line_number":21,"context_line":"The old rules are cleaned up by \"update_stale_ofport_rules\"."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Closes-Bug: #1843870"},{"line_number":24,"context_line":"Co-authored-by: Alin Serdean \u003caserdean@cloudbasesolutions.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5faad753_9d9a9bb6","line":21,"in_reply_to":"5faad753_15dc45e2","updated":"2019-09-16 09:54:30.000000000","message":"Done","commit_id":"327b3c60eedf40a9ca85c734048ee51d5c3e9781"}],"neutron/agent/common/ovsdb_monitor.py":[{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"7d6702f9077ad47e267d54bf2d696d5a8fe88876","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                elif action \u003d\u003d OVSDB_ACTION_DELETE:"},{"line_number":117,"context_line":"                    devices_removed.append(device)"},{"line_number":118,"context_line":"                elif action \u003d\u003d OVSDB_ACTION_NEW:"},{"line_number":119,"context_line":"                    devices_added.append(device)"},{"line_number":120,"context_line":"                    dev_to_ofport[name] \u003d ofport"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.new_events[\u0027added\u0027].extend(devices_added)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_257373b3","line":119,"range":{"start_line":119,"start_character":20,"end_line":119,"end_character":48},"updated":"2019-09-13 09:00:07.000000000","message":"By the way, I\u0027m also thinking about having a separate list for modified ports.","commit_id":"3612167406a4b4471851a0ca9f6cf5a7718e33e6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0e590be600bfd4dd75c503c4a68e00436e730fb0","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                elif action \u003d\u003d OVSDB_ACTION_DELETE:"},{"line_number":117,"context_line":"                    devices_removed.append(device)"},{"line_number":118,"context_line":"                elif action \u003d\u003d OVSDB_ACTION_NEW:"},{"line_number":119,"context_line":"                    devices_added.append(device)"},{"line_number":120,"context_line":"                    dev_to_ofport[name] \u003d ofport"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.new_events[\u0027added\u0027].extend(devices_added)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_428c6535","line":119,"updated":"2019-09-13 08:35:21.000000000","message":"There is one (big) inconvenience if the ofport changes: the OVS firewall. Being considered as a new (\"added\") port will trigger the \"self.sg_agent.setup_port_filters\" method, adding this new port to the firewall.\n\nBut the \"old\" one (actually the same but with an nonexistent ofport) is still present in the firewall.\n\nConsidering this new port as \"updated\" won\u0027t update the firewall rules, so you need to find a way to:\n- Inform the OVS agent about this change.\n- Change correctly this information in the firewall.","commit_id":"3612167406a4b4471851a0ca9f6cf5a7718e33e6"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"cc031471001bb87dc396f00b91cd62a10c7129ff","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                elif action \u003d\u003d OVSDB_ACTION_DELETE:"},{"line_number":117,"context_line":"                    devices_removed.append(device)"},{"line_number":118,"context_line":"                elif action \u003d\u003d OVSDB_ACTION_NEW:"},{"line_number":119,"context_line":"                    devices_added.append(device)"},{"line_number":120,"context_line":"                    dev_to_ofport[name] \u003d ofport"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        self.new_events[\u0027added\u0027].extend(devices_added)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_454f8f6a","line":119,"in_reply_to":"5faad753_428c6535","updated":"2019-09-13 08:58:26.000000000","message":"Indeed, that\u0027s the main issue that we were trying to fix. After applying this patch, the old ofport rules were cleaned up by this: https://github.com/openstack/neutron/blob/8e73de8bc42067c0a6796df3cca9938d25ae754e/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1486-L1504","commit_id":"3612167406a4b4471851a0ca9f6cf5a7718e33e6"}],"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c504eb4ca80fa3e5414072f8a5ad96e4823b28a3","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1443,"context_line":"        # since the processing workflow is the same."},{"line_number":1444,"context_line":"        added_ports \u003d {p[\u0027name\u0027]"},{"line_number":1445,"context_line":"                       for p in events[\u0027added\u0027] + events.get(\u0027modified\u0027, [])}"},{"line_number":1446,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1447,"context_line":"        ports_removed_and_added \u003d added_ports \u0026 removed_ports"},{"line_number":1448,"context_line":"        for p in ports_removed_and_added:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_d54c6dc9","line":1445,"range":{"start_line":1445,"start_character":71,"end_line":1445,"end_character":75},"updated":"2019-09-16 09:33:36.000000000","message":"small nit: because of [1], this is not needed\n\n[1] https://review.opendev.org/#/c/681984/3/neutron/agent/common/ovsdb_monitor.py@131","commit_id":"327b3c60eedf40a9ca85c734048ee51d5c3e9781"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"dcd888af7b869c9c935d739a23a8b0816886d52f","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1443,"context_line":"        # since the processing workflow is the same."},{"line_number":1444,"context_line":"        added_ports \u003d {p[\u0027name\u0027]"},{"line_number":1445,"context_line":"                       for p in events[\u0027added\u0027] + events.get(\u0027modified\u0027, [])}"},{"line_number":1446,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1447,"context_line":"        ports_removed_and_added \u003d added_ports \u0026 removed_ports"},{"line_number":1448,"context_line":"        for p in ports_removed_and_added:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5faad753_7de0ff26","line":1445,"range":{"start_line":1445,"start_character":71,"end_line":1445,"end_character":75},"in_reply_to":"5faad753_d54c6dc9","updated":"2019-09-16 09:54:30.000000000","message":"Indeed. I\u0027ve just updated the unit tests to expect this new list.","commit_id":"327b3c60eedf40a9ca85c734048ee51d5c3e9781"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cf716e48397ee7cc7b62cdedbc23be3aaa788f65","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1443,"context_line":"        # since the processing workflow is the same."},{"line_number":1444,"context_line":"        added_ports \u003d {p[\u0027name\u0027]"},{"line_number":1445,"context_line":"                       for p in events[\u0027added\u0027] + events[\u0027modified\u0027]}"},{"line_number":1446,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1447,"context_line":"        ports_removed_and_added \u003d added_ports \u0026 removed_ports"},{"line_number":1448,"context_line":"        for p in ports_removed_and_added:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_02645821","line":1445,"updated":"2019-09-23 07:04:58.000000000","message":"why don\u0027t You add those ports to \"updated_ports\"? IMO it seems more logical choice rather than \"added\" but maybe there is some other reason, like updated_ports are treated in slightly different way?","commit_id":"45fe3e60e7b87d92b778d59339547ff944a948f5"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"a15490a81a7f799534a92b18efc24ac6a3ff1274","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1443,"context_line":"        # since the processing workflow is the same."},{"line_number":1444,"context_line":"        added_ports \u003d {p[\u0027name\u0027]"},{"line_number":1445,"context_line":"                       for p in events[\u0027added\u0027] + events[\u0027modified\u0027]}"},{"line_number":1446,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1447,"context_line":"        ports_removed_and_added \u003d added_ports \u0026 removed_ports"},{"line_number":1448,"context_line":"        for p in ports_removed_and_added:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_85aff2e1","line":1445,"in_reply_to":"3fa7e38b_02645821","updated":"2019-09-23 07:35:17.000000000","message":"The security groups seem to be applied only for the \"added\" ports: https://github.com/openstack/neutron/blob/0550c0e1f66b26e7e947958fdddb60d8f309e76f/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1994-L1995\n\nThis optimization was introduced by this patch: https://github.com/openstack/neutron/commit/75edc1ff28a460342a9b5e5b7d63c6f4fb59862d.\n\nWe can either revert the above or just keep it like this and make the inline comment a bit more descriptive.","commit_id":"45fe3e60e7b87d92b778d59339547ff944a948f5"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"f32481c02732bf6eb1aa635f308372d62f0405ea","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1443,"context_line":"        # since the processing workflow is the same."},{"line_number":1444,"context_line":"        added_ports \u003d {p[\u0027name\u0027]"},{"line_number":1445,"context_line":"                       for p in events[\u0027added\u0027] + events[\u0027modified\u0027]}"},{"line_number":1446,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1447,"context_line":"        ports_removed_and_added \u003d added_ports \u0026 removed_ports"},{"line_number":1448,"context_line":"        for p in ports_removed_and_added:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_fc356da9","line":1445,"in_reply_to":"3fa7e38b_85aff2e1","updated":"2019-10-03 06:10:19.000000000","message":"Never mind, \"updated\" ports do get handled fine so the following patch set uses that list instead. I finally got the time to test that.","commit_id":"45fe3e60e7b87d92b778d59339547ff944a948f5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"66d179f82ddb777c32ef973f14b337ee19ff2539","unresolved":false,"context_lines":[{"line_number":1443,"context_line":"        # can\u0027t know the order of the operations, check the status of the port"},{"line_number":1444,"context_line":"        # to determine if the port was added or deleted."},{"line_number":1445,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1446,"context_line":"        # since the processing workflow is the same."},{"line_number":1447,"context_line":"        added_ports \u003d {p[\u0027name\u0027] for p in events[\u0027added\u0027]}"},{"line_number":1448,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1449,"context_line":"        updated_ports.update({p[\u0027name\u0027] for p in events[\u0027modified\u0027]})"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_c8f2fa9a","line":1446,"updated":"2019-10-07 15:26:36.000000000","message":"This comment seems wrong, modified ports are not being added to the list of added ports below","commit_id":"17900640d2956e269bc30430fa4c11cad2929584"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"881d80dc688fca354468d3ac472f4e59e2070921","unresolved":false,"context_lines":[{"line_number":1443,"context_line":"        # can\u0027t know the order of the operations, check the status of the port"},{"line_number":1444,"context_line":"        # to determine if the port was added or deleted."},{"line_number":1445,"context_line":"        # For convenience, we\u0027ll add modified ports to the list of added ports"},{"line_number":1446,"context_line":"        # since the processing workflow is the same."},{"line_number":1447,"context_line":"        added_ports \u003d {p[\u0027name\u0027] for p in events[\u0027added\u0027]}"},{"line_number":1448,"context_line":"        removed_ports \u003d {p[\u0027name\u0027] for p in events[\u0027removed\u0027]}"},{"line_number":1449,"context_line":"        updated_ports.update({p[\u0027name\u0027] for p in events[\u0027modified\u0027]})"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fa7e38b_8a2e2ba3","line":1446,"in_reply_to":"3fa7e38b_c8f2fa9a","updated":"2019-10-08 07:57:12.000000000","message":"Right, this is a left over. Thanks for bringing it up!a","commit_id":"17900640d2956e269bc30430fa4c11cad2929584"}]}
