)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"827fa8f8b5a05c4ea0954ccf6eb0609eda30d59c","unresolved":false,"context_lines":[{"line_number":12,"context_line":"because the server declared it as dead."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I164153c79313f2ae7a1fca0414736d5987656185"},{"line_number":15,"context_line":"Partially-implements: blueprint vlan-aware-vms"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a8ec9b2_19cbb154","line":15,"updated":"2016-09-14 23:32:55.000000000","message":"Please target [1] too, for RC purposes.\n\n[1] https://bugs.launchpad.net/neutron/+bug/1623708","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I164153c79313f2ae7a1fca0414736d5987656185"},{"line_number":18,"context_line":"Partially-implements: blueprint vlan-aware-vms"},{"line_number":19,"context_line":"Closes-bug: #1623708"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7a8ec9b2_745e6c13","line":19,"updated":"2016-09-21 00:27:11.000000000","message":"So far this patch takes care of the failure case where the agent dies, subports are added/removed (but the VM is kept in place) and once the agent is restarted, the trunk must be brought in sync.\n\nThere\u0027s another case where the VM is removed while the agent is inactive, and the trunk should be cleaned up as consequence. This is currently missing from this patch. Do you want to address this in the context of this patch or do you want to keep this a partial and have a different patch tackle this?","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I164153c79313f2ae7a1fca0414736d5987656185"},{"line_number":18,"context_line":"Partially-implements: blueprint vlan-aware-vms"},{"line_number":19,"context_line":"Closes-bug: #1623708"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"7a8ec9b2_3cbf43ab","line":19,"in_reply_to":"7a8ec9b2_745e6c13","updated":"2016-09-21 15:07:21.000000000","message":"in the sake of merging this quickly I will keep it partial and tackle the cleanup after VM deletion in another patch","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     rossella \u003crsblendido@suse.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-09-21 15:06:16 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle agent restart for trunks"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the ovs agent is restarted the trunk driver should make sure"},{"line_number":10,"context_line":"that the configuration of the trunk present on the machine"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7a8ec9b2_66003467","line":7,"updated":"2016-09-22 00:16:31.000000000","message":"This patch only handles add/remove subports events loss due to agent failures\n\nWhen the agent restarts, the state is reconciled.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     rossella \u003crsblendido@suse.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-09-21 15:06:16 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle agent restart for trunks"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the ovs agent is restarted the trunk driver should make sure"},{"line_number":10,"context_line":"that the configuration of the trunk present on the machine"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5a8bc5a2_2474f833","line":7,"in_reply_to":"7a8ec9b2_66003467","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the ovs agent is restarted the trunk driver should make sure"},{"line_number":10,"context_line":"that the configuration of the trunk present on the machine"},{"line_number":11,"context_line":"matches the desidered one. If a trunk parent port already exists,"},{"line_number":12,"context_line":"then the subports are retrieved and the stale ones are deleted."},{"line_number":13,"context_line":"This patch adds a functional test to demonstrate that there\u0027s"},{"line_number":14,"context_line":"no need to handle the resync in the trunk driver, since the ovs"},{"line_number":15,"context_line":"agent already takes care of it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7a8ec9b2_a6cf9c54","line":12,"range":{"start_line":11,"start_character":27,"end_line":12,"end_character":63},"updated":"2016-09-22 00:16:31.000000000","message":"This sentence is a bit cryptic, though I understand what you mean. Basically here you mean that upon restart the agent reconciles the logical with the physical state by removing/adding physical subports that are used to be/are current present in the logical view.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the ovs agent is restarted the trunk driver should make sure"},{"line_number":10,"context_line":"that the configuration of the trunk present on the machine"},{"line_number":11,"context_line":"matches the desidered one. If a trunk parent port already exists,"},{"line_number":12,"context_line":"then the subports are retrieved and the stale ones are deleted."},{"line_number":13,"context_line":"This patch adds a functional test to demonstrate that there\u0027s"},{"line_number":14,"context_line":"no need to handle the resync in the trunk driver, since the ovs"},{"line_number":15,"context_line":"agent already takes care of it."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5a8bc5a2_448f4423","line":12,"range":{"start_line":11,"start_character":27,"end_line":12,"end_character":63},"in_reply_to":"7a8ec9b2_a6cf9c54","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"}],"neutron/callbacks/events.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9f87e867bf0a38bcd6968259201ba1c17044ccc","unresolved":false,"context_lines":[{"line_number":44,"context_line":"BEFORE \u003d \u0027before_\u0027"},{"line_number":45,"context_line":"PRECOMMIT \u003d \u0027precommit_\u0027"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"RESYNC \u003d \u0027resync\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_5d7fa6df","line":47,"updated":"2016-09-12 18:41:54.000000000","message":"my suggestion is to localize this closer to where it is used.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"abc012d65420e0d29eff935bf096537ddd06eaf5","unresolved":false,"context_lines":[{"line_number":44,"context_line":"BEFORE \u003d \u0027before_\u0027"},{"line_number":45,"context_line":"PRECOMMIT \u003d \u0027precommit_\u0027"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"RESYNC \u003d \u0027resync\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_de85fa59","line":47,"in_reply_to":"9a89bdaa_5d7fa6df","updated":"2016-09-19 15:19:45.000000000","message":"Done","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"}],"neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9f87e867bf0a38bcd6968259201ba1c17044ccc","unresolved":false,"context_lines":[{"line_number":149,"context_line":"# callback resource for notifying to ovsdb handler"},{"line_number":150,"context_line":"OVSDB_RESOURCE \u003d \u0027ovsdb\u0027"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"AGENT \u003d \u0027agent\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_9d85aed0","line":152,"updated":"2016-09-12 18:41:54.000000000","message":"maybe here.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"}],"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8af19f0ad93ae3d0f3ade5a76ca894be084d71c3","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                    ports.clear()"},{"line_number":1834,"context_line":"                    ancillary_ports.clear()"},{"line_number":1835,"context_line":"                    consecutive_resyncs \u003d 0"},{"line_number":1836,"context_line":"                registry.notify(constants.AGENT, callback_events.RESYNC,"},{"line_number":1837,"context_line":"                                self)"},{"line_number":1838,"context_line":"            else:"},{"line_number":1839,"context_line":"                consecutive_resyncs \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_a43666b2","line":1836,"updated":"2016-09-13 18:16:13.000000000","message":"I am starting to wonder if this is the right place where to trigger a resync event that is meant to be consumed by things like the trunk component. \n\nThis block seems to be executed by either a fullsync or a an exception on line 2082. I am tempted to say that to start with, we should only worry about a resync as triggered by line 324 (_report_state), and the event payload should carry out this information too in case in the future we want to expand the resync capabilities.\n\nUltimately what failures modes do we want to worry about? I can think of primarily this one:\n\nThe trunk is up and running (i.e. in use by a VM), but the agent is temporarily down. This means that the logical state and the physical state may be out of sync, i.e. users may have added/removed subports, or deleted the VM, while the agent couldn\u0027t handle the events.\n\nDo you see any other? To reconcile the logical and the physical view we\u0027d have to inspect the local trunk bridges and assess what needs to be done:\n\na) for trunk bridges that have a parent port connected (i.e. VM alive and kicking), we\u0027d need to get the latest logical view and realize it on the data plane. This means removing OVS subports that do not map the logical ones, or add OVS subports that are present amongst the logical ones.\n\nb) for trunk bridges that have no parent port connected (i.e. VM is gone), we\u0027d need to wipe out the local OVS resources.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"685422e771b0ca1f04bf468302f94f6e1efc2ce4","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                    ports.clear()"},{"line_number":1834,"context_line":"                    ancillary_ports.clear()"},{"line_number":1835,"context_line":"                    consecutive_resyncs \u003d 0"},{"line_number":1836,"context_line":"                registry.notify(constants.AGENT, callback_events.RESYNC,"},{"line_number":1837,"context_line":"                                self)"},{"line_number":1838,"context_line":"            else:"},{"line_number":1839,"context_line":"                consecutive_resyncs \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_cb9b2cc8","line":1836,"in_reply_to":"7a8ec9b2_a43666b2","updated":"2016-09-14 23:58:07.000000000","message":"if agent temporarily down is the result of an agent crash, let\u0027s remember that an agent restart will trigger notification @1873, hence we want to be careful at avoiding having two threads step on each other\u0027s toes.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"abc012d65420e0d29eff935bf096537ddd06eaf5","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                    ports.clear()"},{"line_number":1834,"context_line":"                    ancillary_ports.clear()"},{"line_number":1835,"context_line":"                    consecutive_resyncs \u003d 0"},{"line_number":1836,"context_line":"                registry.notify(constants.AGENT, callback_events.RESYNC,"},{"line_number":1837,"context_line":"                                self)"},{"line_number":1838,"context_line":"            else:"},{"line_number":1839,"context_line":"                consecutive_resyncs \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_fb8008fa","line":1836,"in_reply_to":"7a8ec9b2_cb9b2cc8","updated":"2016-09-19 15:19:45.000000000","message":"I agree let\u0027s reduce the scope only to full resync (triggered by line 324). Even if the agent gets an exception when processing the patch port on br-int, there\u0027s nothing that should be done by the trunk driver.\nIf the agent crashes, the restart will trigger notification line @1873 but sync will be false. What do you mean exactly by \"avoiding having two threads step on each other\u0027s toes\"?","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"7ba2dab2da6b7f4c921d1a222491bf819651696a","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                    ports.clear()"},{"line_number":1834,"context_line":"                    ancillary_ports.clear()"},{"line_number":1835,"context_line":"                    consecutive_resyncs \u003d 0"},{"line_number":1836,"context_line":"                registry.notify(constants.AGENT, callback_events.RESYNC,"},{"line_number":1837,"context_line":"                                self)"},{"line_number":1838,"context_line":"            else:"},{"line_number":1839,"context_line":"                consecutive_resyncs \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_c7ac3b8f","line":1836,"in_reply_to":"7a8ec9b2_fb8008fa","updated":"2016-09-19 21:40:57.000000000","message":"If the agent is down for longer than cfg.CONF.agent_down_time (default \u003d 75), sync will be True.\n\nWe can avoid having two threads step on each other\u0027s toes if we just avoid doing double work on a cold agent restart (i.e. agent was down for longer than cfg.CONF.agent_down_time).\n\nAlthough, the case I\u0027m worried about is when the agent has lost connectivity for *less* than cfg.CONF.agent_down_time and subport CRUD has occurred during that window. In that case, neither the ovsdb READ or RESYNC notifications are triggered, and ultimately those notifications are lost.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"685422e771b0ca1f04bf468302f94f6e1efc2ce4","unresolved":false,"context_lines":[{"line_number":1870,"context_line":"                                          failed_devices,"},{"line_number":1871,"context_line":"                                          failed_ancillary_devices,"},{"line_number":1872,"context_line":"                                          updated_ports_copy))"},{"line_number":1873,"context_line":"            registry.notify("},{"line_number":1874,"context_line":"                constants.OVSDB_RESOURCE,"},{"line_number":1875,"context_line":"                callback_events.AFTER_READ,"},{"line_number":1876,"context_line":"                self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_8b63e4db","line":1873,"updated":"2016-09-14 23:58:07.000000000","message":"here","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"7ba2dab2da6b7f4c921d1a222491bf819651696a","unresolved":false,"context_lines":[{"line_number":1958,"context_line":"            if self.fullsync:"},{"line_number":1959,"context_line":"                LOG.info(_LI(\"rpc_loop doing a full sync.\"))"},{"line_number":1960,"context_line":"                sync \u003d True"},{"line_number":1961,"context_line":"                self.fullsync \u003d False"},{"line_number":1962,"context_line":"            port_info \u003d {}"},{"line_number":1963,"context_line":"            ancillary_port_info \u003d {}"},{"line_number":1964,"context_line":"            start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_58d56cb7","line":1961,"updated":"2016-09-19 21:40:57.000000000","message":"Because we ideally don\u0027t want to do double work on a cold agent restart, we should set self.trunk_sync \u003d True here (or L324), then set self.trunk_sync \u003d False when we start to process the trunk resync.\n\nThen in the ovsdb_handler resync logic, we can check if trunk_sync is True before doing the operation. We could check this directly from the agent that\u0027s passed in the notification payload or pass it as a separate item in the payload.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"}],"neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py":[{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"b45773f85fa11aa6819033962bbd393c4b7d552e","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                    for trunk_br_port in trunk_br_ports:"},{"line_number":108,"context_line":"                        if not is_trunk_service_port(trunk_br_port[\u0027name\u0027]):"},{"line_number":109,"context_line":"                            self.create_trunk(trunk_br, trunk_br_port)"},{"line_number":110,"context_line":"                            break"},{"line_number":111,"context_line":"                else:"},{"line_number":112,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":113,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a89bdaa_34d4d81e","line":110,"updated":"2016-09-06 17:16:31.000000000","message":"I\u0027m not seeing where we handle sync of the subports. This handles basic wiring of the trunk in response to a VM being booted up while the agent was offline. However, there could very well be cases where subports were created at the same time the trunk was, and cases where subports were added/removed while the agent was offline. In these cases, we can\u0027t avoid an RPC call back to the server to fetch the subports. Subports aren\u0027t going to leave any bread crumbs on the node, we have to go back to the server every time we re-sync to get each trunk (ideally with a bulk call).","commit_id":"1154247756916254ea3c8d40d3d2a48fdee7feb6"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"0f19e498a15cb9d99ed2694d9f200f6a49005dcf","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                    for trunk_br_port in trunk_br_ports:"},{"line_number":108,"context_line":"                        if not is_trunk_service_port(trunk_br_port[\u0027name\u0027]):"},{"line_number":109,"context_line":"                            self.create_trunk(trunk_br, trunk_br_port)"},{"line_number":110,"context_line":"                            break"},{"line_number":111,"context_line":"                else:"},{"line_number":112,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":113,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a89bdaa_dd72b510","line":110,"in_reply_to":"9a89bdaa_34d4d81e","updated":"2016-09-07 15:22:12.000000000","message":"Line 195, that\u0027s where subports are processed","commit_id":"1154247756916254ea3c8d40d3d2a48fdee7feb6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"9ed3c534a92b9446475d62f7f9b72e9de336e708","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                    for trunk_br_port in trunk_br_ports:"},{"line_number":108,"context_line":"                        if not is_trunk_service_port(trunk_br_port[\u0027name\u0027]):"},{"line_number":109,"context_line":"                            self.create_trunk(trunk_br, trunk_br_port)"},{"line_number":110,"context_line":"                            break"},{"line_number":111,"context_line":"                else:"},{"line_number":112,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":113,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a89bdaa_7cf4355a","line":110,"in_reply_to":"9a89bdaa_dd72b510","updated":"2016-09-08 00:15:58.000000000","message":"I see it now. Maybe we should leave a note or todo to revisit, it looks like during a re-sync this would make an RPC call per trunk. Maybe we can skate by here because there may not be a large number of trunks on any given node, but the more they stack up the more RPC round-trips we will be making.","commit_id":"1154247756916254ea3c8d40d3d2a48fdee7feb6"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"7ba2dab2da6b7f4c921d1a222491bf819651696a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                    self.handle_trunk_remove, port_event)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def process_resync(self, resource, event, trigger):"},{"line_number":117,"context_line":"        bridges \u003d ovs_lib.get_bridges()"},{"line_number":118,"context_line":"        for bridge in bridges:"},{"line_number":119,"context_line":"            if is_trunk_bridge(bridge):"},{"line_number":120,"context_line":"                trunk_br \u003d ovs_lib.OVSBridge(bridge)"},{"line_number":121,"context_line":"                if bridge_has_instance_port(trunk_br):"},{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_820b012b","line":121,"range":{"start_line":117,"start_character":8,"end_line":121,"end_character":54},"updated":"2016-09-19 21:40:57.000000000","message":"Should this be simplified?\n\ntrunk_bridges \u003d {b for b in ovs_lib.get_bridges() if is_trunk_bridge(b)}\ntrunks_to_sync \u003d {b for b in trunk_bridges if has_instance_port(b)}\ntrunks_to_remove \u003d trunk_bridges - trunks_to_add\nfor bridge in trunks_to_add:\n    ...\nfor bridge in trunks_to_remove:\n    ...","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"277db475d97708f6e257699c30703c758d15d817","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                    self.handle_trunk_remove, port_event)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    def process_resync(self, resource, event, trigger):"},{"line_number":117,"context_line":"        bridges \u003d ovs_lib.get_bridges()"},{"line_number":118,"context_line":"        for bridge in bridges:"},{"line_number":119,"context_line":"            if is_trunk_bridge(bridge):"},{"line_number":120,"context_line":"                trunk_br \u003d ovs_lib.OVSBridge(bridge)"},{"line_number":121,"context_line":"                if bridge_has_instance_port(trunk_br):"},{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_62a512b4","line":121,"range":{"start_line":117,"start_character":8,"end_line":121,"end_character":54},"in_reply_to":"7a8ec9b2_820b012b","updated":"2016-09-19 21:42:27.000000000","message":"s/trunks_to_add/trunks_to_sync/","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9f87e867bf0a38bcd6968259201ba1c17044ccc","unresolved":false,"context_lines":[{"line_number":119,"context_line":"            if is_trunk_bridge(bridge):"},{"line_number":120,"context_line":"                trunk_br \u003d ovs_lib.OVSBridge(bridge)"},{"line_number":121,"context_line":"                if bridge_has_instance_port(trunk_br):"},{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_fde23a9f","line":122,"updated":"2016-09-12 18:41:54.000000000","message":"this should be spawned too, no?","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"abc012d65420e0d29eff935bf096537ddd06eaf5","unresolved":false,"context_lines":[{"line_number":119,"context_line":"            if is_trunk_bridge(bridge):"},{"line_number":120,"context_line":"                trunk_br \u003d ovs_lib.OVSBridge(bridge)"},{"line_number":121,"context_line":"                if bridge_has_instance_port(trunk_br):"},{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_9e737215","line":122,"in_reply_to":"7a8ec9b2_04d63ac5","updated":"2016-09-19 15:19:45.000000000","message":"yeah, we need to modify this. Actually we need to modify handle_trunk_add. Imagine the agent is restarted and misses a subport delete notification. It will come up, reprocess the trunk but it will never delete the sub port","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"7ba2dab2da6b7f4c921d1a222491bf819651696a","unresolved":false,"context_lines":[{"line_number":119,"context_line":"            if is_trunk_bridge(bridge):"},{"line_number":120,"context_line":"                trunk_br \u003d ovs_lib.OVSBridge(bridge)"},{"line_number":121,"context_line":"                if bridge_has_instance_port(trunk_br):"},{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_9d228600","line":122,"in_reply_to":"7a8ec9b2_9e737215","updated":"2016-09-19 21:40:57.000000000","message":"\u003e we\u0027d have to perform a reconciliation rather than blindly add the trunk, which might have been already provisioned successfully.\n\nThe \"add\" operation should be idempotent, but maybe we still want to avoid doing the whole operation twice when it\u0027s unneeded?\n\nSomehow we need to make a distinction between two RESYNC cases: A) \"agent restarted\" and B) \"agent not restarted but regained lost connectivity\".\n\nWe could simply trigger this RESYNC logic only when we see case B (i.e., when self.fullsync gets set to True) and avoid doing double work for case A. See my comment in ovs_neutron_agent.py.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8af19f0ad93ae3d0f3ade5a76ca894be084d71c3","unresolved":false,"context_lines":[{"line_number":119,"context_line":"            if is_trunk_bridge(bridge):"},{"line_number":120,"context_line":"                trunk_br \u003d ovs_lib.OVSBridge(bridge)"},{"line_number":121,"context_line":"                if bridge_has_instance_port(trunk_br):"},{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_04d63ac5","line":122,"in_reply_to":"9a89bdaa_fde23a9f","updated":"2016-09-13 18:16:13.000000000","message":"This is not quite right, actually. Thinking about the failure mode, we\u0027d have to perform a reconciliation rather than blindly add the trunk, which might have been already provisioned successfully.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8af19f0ad93ae3d0f3ade5a76ca894be084d71c3","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"},{"line_number":126,"context_line":"                        trunk_br_port \u003d trunk_br.get_ports_attributes("},{"line_number":127,"context_line":"                            \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027],"},{"line_number":128,"context_line":"                            ports\u003d[bridge], if_exists\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_44ff6234","line":125,"updated":"2016-09-13 18:16:13.000000000","message":"I am not sure we need to hold this registered_trunk_brs in memory. I think it suffices if we simply list the bridges that are meant to be associated to trunks as you did on line 117. For bridges that have no parent port we\u0027d simply have to wipe out all the resources associated with it, for that we can simply leverage ovs_lib, to fetch ports and peers associated with the bridge in question and perform the cleanup. If you think that getting the external IDs from the bridge it allows you to reuse the trunk manager building blocks, I\u0027d be fine with that, though it\u0027s not strictly necessary.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"abc012d65420e0d29eff935bf096537ddd06eaf5","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                    self.handle_trunk_add(bridge)"},{"line_number":123,"context_line":"                else:"},{"line_number":124,"context_line":"                    # the trunk needs to be cleaned up"},{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"},{"line_number":126,"context_line":"                        trunk_br_port \u003d trunk_br.get_ports_attributes("},{"line_number":127,"context_line":"                            \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027],"},{"line_number":128,"context_line":"                            ports\u003d[bridge], if_exists\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_9ee1b29d","line":125,"in_reply_to":"7a8ec9b2_44ff6234","updated":"2016-09-19 15:19:45.000000000","message":"OK, let\u0027s keep it simple.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9f87e867bf0a38bcd6968259201ba1c17044ccc","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"},{"line_number":126,"context_line":"                        trunk_br_port \u003d trunk_br.get_ports_attributes("},{"line_number":127,"context_line":"                            \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027],"},{"line_number":128,"context_line":"                            ports\u003d[bridge], if_exists\u003dTrue)"},{"line_number":129,"context_line":"                        eventlet.spawn_n(self.handle_trunk_remove,"},{"line_number":130,"context_line":"                                         trunk_br_port[0])"},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_1d8efecf","line":128,"updated":"2016-09-12 18:41:54.000000000","message":"wouldn\u0027t we fail here if for whatever reason the port does not exist? Error handling?","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"abc012d65420e0d29eff935bf096537ddd06eaf5","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                    if bridge in self.registered_trunk_brs:"},{"line_number":126,"context_line":"                        trunk_br_port \u003d trunk_br.get_ports_attributes("},{"line_number":127,"context_line":"                            \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027],"},{"line_number":128,"context_line":"                            ports\u003d[bridge], if_exists\u003dTrue)"},{"line_number":129,"context_line":"                        eventlet.spawn_n(self.handle_trunk_remove,"},{"line_number":130,"context_line":"                                         trunk_br_port[0])"},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_34b43dd0","line":128,"in_reply_to":"9a89bdaa_1d8efecf","updated":"2016-09-19 15:19:45.000000000","message":"it\u0027s the local port of the bridge. If this port doesn\u0027t exist then the bridge doesn\u0027t exist","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"7ba2dab2da6b7f4c921d1a222491bf819651696a","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                    \u0027in %(timeout)d seconds. Cleaning up the bridge\u0027),"},{"line_number":156,"context_line":"                {\u0027br_name\u0027: bridge.br_name,"},{"line_number":157,"context_line":"                 \u0027timeout\u0027: WAIT_FOR_PORT_TIMEOUT})"},{"line_number":158,"context_line":"            bridge.destroy()"},{"line_number":159,"context_line":"            return"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # Once we get hold of the trunk parent port, we can provision"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_228dca7b","line":158,"updated":"2016-09-19 21:40:57.000000000","message":"This can also occur if the VM was deleted while the agent was down. We should also check for and remove any stale patch ports on br-int.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8af19f0ad93ae3d0f3ade5a76ca894be084d71c3","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                      {\u0027bridge_name\u0027: bridge.br_name,"},{"line_number":174,"context_line":"                       \u0027err\u0027: e})"},{"line_number":175,"context_line":"        else:"},{"line_number":176,"context_line":"            self.registered_trunk_brs.add(bridge_name)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    @lock_on_bridge_name"},{"line_number":179,"context_line":"    def handle_trunk_remove(self, port):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_240c1642","line":176,"updated":"2016-09-13 18:16:13.000000000","message":"see comment above.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9f87e867bf0a38bcd6968259201ba1c17044ccc","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                       \u0027err\u0027: te})"},{"line_number":198,"context_line":"        else:"},{"line_number":199,"context_line":"            LOG.debug(\"Deleted resources associated to trunk: %s\", trunk_id)"},{"line_number":200,"context_line":"            bridge_name \u003d trunk_utils.gen_trunk_br_name(trunk_id)"},{"line_number":201,"context_line":"            self.registered_trunk_brs.remove(bridge_name)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def manages_this_trunk(self, trunk_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_fdf95a4f","line":200,"updated":"2016-09-12 18:41:54.000000000","message":"this is being added in [1]\n\n[1] https://review.openstack.org/#/c/368691/","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9f87e867bf0a38bcd6968259201ba1c17044ccc","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        trunk_br.set_db_attribute("},{"line_number":342,"context_line":"            \u0027Interface\u0027, port[\u0027name\u0027], \u0027external_ids\u0027, port[\u0027external_ids\u0027])"},{"line_number":343,"context_line":"        # store the metadata in the bridge too so that can be used when"},{"line_number":344,"context_line":"        # performing a resync"},{"line_number":345,"context_line":"        trunk_br.set_db_attribute("},{"line_number":346,"context_line":"            \u0027Interface\u0027, trunk_br.br_name, \u0027external_ids\u0027,"},{"line_number":347,"context_line":"            port[\u0027external_ids\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_9db84e77","line":344,"updated":"2016-09-12 18:41:54.000000000","message":"you sure this is strictly needed?","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8af19f0ad93ae3d0f3ade5a76ca894be084d71c3","unresolved":false,"context_lines":[{"line_number":342,"context_line":"            \u0027Interface\u0027, port[\u0027name\u0027], \u0027external_ids\u0027, port[\u0027external_ids\u0027])"},{"line_number":343,"context_line":"        # store the metadata in the bridge too so that can be used when"},{"line_number":344,"context_line":"        # performing a resync"},{"line_number":345,"context_line":"        trunk_br.set_db_attribute("},{"line_number":346,"context_line":"            \u0027Interface\u0027, trunk_br.br_name, \u0027external_ids\u0027,"},{"line_number":347,"context_line":"            port[\u0027external_ids\u0027])"},{"line_number":348,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_a7fe6803","line":345,"updated":"2016-09-13 18:16:13.000000000","message":"a) this is trunk_bridge, and b) this should be done in a transaction context.","commit_id":"47325566e73ec47ab24dc5c32157543bd12e8c6e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        try:"},{"line_number":162,"context_line":"            common_utils.wait_until_true("},{"line_number":163,"context_line":"                bridge_has_port_predicate,"},{"line_number":164,"context_line":"                timeout\u003dWAIT_FOR_PORT_TIMEOUT)"},{"line_number":165,"context_line":"        except eventlet.TimeoutError:"},{"line_number":166,"context_line":"            LOG.error("},{"line_number":167,"context_line":"                _LE(\u0027No port appeared on trunk bridge %(br_name)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_9405b8c3","line":164,"updated":"2016-09-21 00:27:11.000000000","message":"My suggestion would be that after this has completed, check if the trunk is active (i.e. it has been processed already, it has subports, metadata and all), if so, rather than calling into _wire_trunk call into a new method _rewire_trunk whose objective is to fetch the latest server\u0027s subports, get the local subports, compute the diff and call wire_subports_for_trunk/unwire_subports_for_trunk. It\u0027s slightly more work of what you did, but it keeps things more decoupled, IMO.\n\nThat leads me to think about an issue with the metadata we save, because right now we overwrite them on wire and do not refresh them on unwire.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        try:"},{"line_number":162,"context_line":"            common_utils.wait_until_true("},{"line_number":163,"context_line":"                bridge_has_port_predicate,"},{"line_number":164,"context_line":"                timeout\u003dWAIT_FOR_PORT_TIMEOUT)"},{"line_number":165,"context_line":"        except eventlet.TimeoutError:"},{"line_number":166,"context_line":"            LOG.error("},{"line_number":167,"context_line":"                _LE(\u0027No port appeared on trunk bridge %(br_name)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_c293e259","line":164,"in_reply_to":"7a8ec9b2_9405b8c3","updated":"2016-09-21 15:07:21.000000000","message":"I\u0027ve refactored it, tbh I think having 2 methods _wire_trunk and _rewire_trunk to be a bit redundant, see if you like the next PS otherwise I\u0027ll create two methods.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        bridge_name \u003d utils.gen_trunk_br_name(trunk_id)"},{"line_number":216,"context_line":"        return ovs_lib.BaseOVS().bridge_exists(bridge_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_subports_ids(self, bridge):"},{"line_number":219,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":220,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":221,"context_line":"        subports_ids \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_5435f0d7","line":218,"updated":"2016-09-21 00:27:11.000000000","message":"I\u0027d personally make this a public method that takes the trunk ID, as it may turn out to be useful in other contexts. So for instance do:\n\ndef get_connected_subports_for_trunk(self, trunk_id):\n    \"\"\"Return the list of subports actively participating in trunk ID.\"\"\"","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        bridge_name \u003d utils.gen_trunk_br_name(trunk_id)"},{"line_number":216,"context_line":"        return ovs_lib.BaseOVS().bridge_exists(bridge_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_subports_ids(self, bridge):"},{"line_number":219,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":220,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":221,"context_line":"        subports_ids \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_77ead275","line":218,"in_reply_to":"7a8ec9b2_5435f0d7","updated":"2016-09-21 15:07:21.000000000","message":"Done","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return ovs_lib.BaseOVS().bridge_exists(bridge_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_subports_ids(self, bridge):"},{"line_number":219,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":220,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":221,"context_line":"        subports_ids \u003d []"},{"line_number":222,"context_line":"        for port in ports:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_f4a8bcdd","line":219,"updated":"2016-09-21 00:27:11.000000000","message":"so you should add (if you agree with my suggestion above)\n\nbridge \u003d ovs_lib.OVSBridge(utils.gen_trunk_br_name(trunk_id)).\n\nIf the bridge does not exist, we should probably still return [] rather than fail.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return ovs_lib.BaseOVS().bridge_exists(bridge_name)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def _get_subports_ids(self, bridge):"},{"line_number":219,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":220,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":221,"context_line":"        subports_ids \u003d []"},{"line_number":222,"context_line":"        for port in ports:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_9703de41","line":219,"in_reply_to":"7a8ec9b2_f4a8bcdd","updated":"2016-09-21 15:07:21.000000000","message":"Done","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            trunk_br \u003d ovs_lib.OVSBridge(utils.gen_trunk_br_name(trunk_id))"},{"line_number":257,"context_line":"            old_subport_ids \u003d self._get_subports_ids(trunk_br)"},{"line_number":258,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - set(subport_ids)"},{"line_number":259,"context_line":"            self.unwire_subports_for_trunk(trunk_id, subports_to_delete)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        try:"},{"line_number":262,"context_line":"            self._set_trunk_metadata("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_d4cae0a0","line":259,"updated":"2016-09-21 00:27:11.000000000","message":"A method called wire_subports_for_trunk that does unwire_subports_for_trunk smells for me and it\u0027s telling me that this could be more modular. I think you\u0027re on the right track but I feel that this is reconciliation should be done elsewhere.\n\nPersonally what I had envisioned doing this type of work in hande_trunk_add. See comment above.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":256,"context_line":"            trunk_br \u003d ovs_lib.OVSBridge(utils.gen_trunk_br_name(trunk_id))"},{"line_number":257,"context_line":"            old_subport_ids \u003d self._get_subports_ids(trunk_br)"},{"line_number":258,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - set(subport_ids)"},{"line_number":259,"context_line":"            self.unwire_subports_for_trunk(trunk_id, subports_to_delete)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        try:"},{"line_number":262,"context_line":"            self._set_trunk_metadata("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_029a2a6e","line":259,"in_reply_to":"7a8ec9b2_d4cae0a0","updated":"2016-09-21 15:07:21.000000000","message":"Done","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            self.unwire_subports_for_trunk(trunk_id, subports_to_delete)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        try:"},{"line_number":262,"context_line":"            self._set_trunk_metadata("},{"line_number":263,"context_line":"                trunk_bridge, parent_port, trunk_id, subport_ids)"},{"line_number":264,"context_line":"        except RuntimeError:"},{"line_number":265,"context_line":"            LOG.error(_LE(\"Failed to set metadata for trunk %s\"), trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_b408f4da","line":262,"updated":"2016-09-21 00:27:11.000000000","message":"I just realized that we don\u0027t reset metadata during unwire_subports_for_trunk, it looks like our management of metadata needs a little work. I\u0027ll file a bug and take care of it.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            self.unwire_subports_for_trunk(trunk_id, subports_to_delete)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        try:"},{"line_number":262,"context_line":"            self._set_trunk_metadata("},{"line_number":263,"context_line":"                trunk_bridge, parent_port, trunk_id, subport_ids)"},{"line_number":264,"context_line":"        except RuntimeError:"},{"line_number":265,"context_line":"            LOG.error(_LE(\"Failed to set metadata for trunk %s\"), trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_dd2ca7ae","line":262,"in_reply_to":"7a8ec9b2_b408f4da","updated":"2016-09-21 15:07:21.000000000","message":"I am leaning toward not using metadata at all. We spare one query but we add more complexity in terms of info that we need to keep updated and that might get stale","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":332,"context_line":"            LOG.error(_LE(\"Port %s has no trunk associated.\"), parent_port_id)"},{"line_number":333,"context_line":"            return"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        if self.trunk_manager.trunk_exists(trunk.id, trunk.port_id,"},{"line_number":336,"context_line":"                port[\u0027external_ids\u0027].get(\u0027attached-mac\u0027)):"},{"line_number":337,"context_line":"            reconcile \u003d True"},{"line_number":338,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_74ed8cee","line":335,"updated":"2016-09-21 00:27:11.000000000","message":"I think this should be one level up (done by the caller).","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":332,"context_line":"            LOG.error(_LE(\"Port %s has no trunk associated.\"), parent_port_id)"},{"line_number":333,"context_line":"            return"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        if self.trunk_manager.trunk_exists(trunk.id, trunk.port_id,"},{"line_number":336,"context_line":"                port[\u0027external_ids\u0027].get(\u0027attached-mac\u0027)):"},{"line_number":337,"context_line":"            reconcile \u003d True"},{"line_number":338,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_627f6e17","line":335,"in_reply_to":"7a8ec9b2_74ed8cee","updated":"2016-09-21 15:07:21.000000000","message":"Done","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            return"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        if self.trunk_manager.trunk_exists(trunk.id, trunk.port_id,"},{"line_number":336,"context_line":"                port[\u0027external_ids\u0027].get(\u0027attached-mac\u0027)):"},{"line_number":337,"context_line":"            reconcile \u003d True"},{"line_number":338,"context_line":"        try:"},{"line_number":339,"context_line":"            self.trunk_manager.create_trunk("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_74458cb1","line":336,"updated":"2016-09-21 00:27:11.000000000","message":"that said if you need to check that the trunk is indeed active (i.e. it has a VM connected to it) shouldn\u0027t using method bridge_has_instance_port() suffice?","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            return"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        if self.trunk_manager.trunk_exists(trunk.id, trunk.port_id,"},{"line_number":336,"context_line":"                port[\u0027external_ids\u0027].get(\u0027attached-mac\u0027)):"},{"line_number":337,"context_line":"            reconcile \u003d True"},{"line_number":338,"context_line":"        try:"},{"line_number":339,"context_line":"            self.trunk_manager.create_trunk("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_91420e3d","line":336,"in_reply_to":"7a8ec9b2_74458cb1","updated":"2016-09-21 15:07:21.000000000","message":"what I am checking here is that it\u0027s not the first time that we process this trunk, so the existence of patch ports corresponding to a trunk. bridge_has_instance_port() does the opposite but I created a new method","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"80f483e055627458bd8be8f13aba87b68e98391a","unresolved":false,"context_lines":[{"line_number":95,"context_line":"               if not is_trunk_service_port(iface))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"def bridge_has_service_port(bridge):"},{"line_number":99,"context_line":"    \"\"\"True if there is an OVS port that used to implement a trunk."},{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_7cbb80e5","line":98,"range":{"start_line":98,"start_character":4,"end_line":98,"end_character":27},"updated":"2016-09-21 15:46:16.000000000","message":"This looks very similar like L81-L95. How about:\n\n def bridge_has_port(bridge, is_port_predicate):\n      L85-L93\n      return any(iface for iface in ifaces if is_port_predicate(port)\n\n def bridge_has_instance_port(bridge):\n      is_instance_port \u003d lambda p: not is_trunk_service_port(p)\n      return bridge_has_port(bridge, is_instance_port)\n\n def bridge_has_service_port(bridge):\n      return bridge_has_port(bridge, is_trunk_service_port)","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":95,"context_line":"               if not is_trunk_service_port(iface))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"def bridge_has_service_port(bridge):"},{"line_number":99,"context_line":"    \"\"\"True if there is an OVS port that used to implement a trunk."},{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_19092896","line":98,"range":{"start_line":98,"start_character":4,"end_line":98,"end_character":27},"in_reply_to":"7a8ec9b2_062cd000","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":95,"context_line":"               if not is_trunk_service_port(iface))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"def bridge_has_service_port(bridge):"},{"line_number":99,"context_line":"    \"\"\"True if there is an OVS port that used to implement a trunk."},{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_062cd000","line":98,"range":{"start_line":98,"start_character":4,"end_line":98,"end_character":27},"in_reply_to":"7a8ec9b2_7cbb80e5","updated":"2016-09-22 00:16:31.000000000","message":"++, but I would not freak out if you didn\u0027t.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"80f483e055627458bd8be8f13aba87b68e98391a","unresolved":false,"context_lines":[{"line_number":187,"context_line":"            bridge.destroy()"},{"line_number":188,"context_line":"            return"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        rewire \u003d False"},{"line_number":191,"context_line":"        if bridge_has_service_port(bridge):"},{"line_number":192,"context_line":"            rewire \u003d True"},{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_39f02a91","line":192,"range":{"start_line":190,"start_character":0,"end_line":192,"end_character":25},"updated":"2016-09-21 15:46:16.000000000","message":"How about:\n\n rewire \u003d bridge_has_service_port(bridge)","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":187,"context_line":"            bridge.destroy()"},{"line_number":188,"context_line":"            return"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        rewire \u003d False"},{"line_number":191,"context_line":"        if bridge_has_service_port(bridge):"},{"line_number":192,"context_line":"            rewire \u003d True"},{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_4632585f","line":192,"range":{"start_line":190,"start_character":0,"end_line":192,"end_character":25},"in_reply_to":"7a8ec9b2_39f02a91","updated":"2016-09-22 00:16:31.000000000","message":"++ to Kuba\u0027s suggestion. As for the other case, let\u0027s crawl :)\n\nThat said, I think this whole block 170-192 could nicely be extracted in its own method, which returns a bool (rewire needed) but that\u0027s perhaps for another day. If you do, you get extra brownie points.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"0b77d7cf37928a5e9ca24ee689d018842d6f0c87","unresolved":false,"context_lines":[{"line_number":187,"context_line":"            bridge.destroy()"},{"line_number":188,"context_line":"            return"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        rewire \u003d False"},{"line_number":191,"context_line":"        if bridge_has_service_port(bridge):"},{"line_number":192,"context_line":"            rewire \u003d True"},{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_fbe4efa8","line":192,"range":{"start_line":190,"start_character":0,"end_line":192,"end_character":25},"in_reply_to":"7a8ec9b2_39f02a91","updated":"2016-09-21 17:56:34.000000000","message":"This could be moved to L174, and after destroying the bridge on L187 we check \"if rewire\" then make sure to remove any stale patch port peers on br-int.\n\nI\u0027m not sure if you want to handle that in this patch, but this would handle the case where a VM was deleted while the agent was offline.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":187,"context_line":"            bridge.destroy()"},{"line_number":188,"context_line":"            return"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        rewire \u003d False"},{"line_number":191,"context_line":"        if bridge_has_service_port(bridge):"},{"line_number":192,"context_line":"            rewire \u003d True"},{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_191768a4","line":192,"range":{"start_line":190,"start_character":0,"end_line":192,"end_character":25},"in_reply_to":"7a8ec9b2_4632585f","updated":"2016-09-22 14:59:23.000000000","message":"doh, thanks for the suggestion Kuba!","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"0d6b0ab4e63e052c7ed703195145f3e337035e15","unresolved":false,"context_lines":[{"line_number":187,"context_line":"            bridge.destroy()"},{"line_number":188,"context_line":"            return"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        rewire \u003d False"},{"line_number":191,"context_line":"        if bridge_has_service_port(bridge):"},{"line_number":192,"context_line":"            rewire \u003d True"},{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_f20a491f","line":192,"range":{"start_line":190,"start_character":0,"end_line":192,"end_character":25},"in_reply_to":"7a8ec9b2_fbe4efa8","updated":"2016-09-21 20:39:04.000000000","message":"I\u0027m not sure that helps us with VM\u0027s getting deleted while the agent was offline. On a cold start, the polling_manager wioAt L178 we wait for a VIF that will never appear, then destroy the bridge after the timeout. It seems to me that to handle cleanup of the stale spi ports requires us to clean them up before we return on L188. It seems like it would require special handling of stale spi ports as well. I don\u0027t think _wire_trunk() can be leveraged for that cleanup.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"},{"line_number":196,"context_line":"            self._wire_trunk(bridge, self._get_parent_port(bridge), rewire)"},{"line_number":197,"context_line":"        except oslo_messaging.MessagingException as e:"},{"line_number":198,"context_line":"            LOG.error(_LE(\"Got messaging error while processing trunk bridge \""},{"line_number":199,"context_line":"                          \"%(bridge_name)s: %(err)s\"),"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_668594cd","line":196,"updated":"2016-09-22 00:16:31.000000000","message":"nice","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":194,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":195,"context_line":"        try:"},{"line_number":196,"context_line":"            self._wire_trunk(bridge, self._get_parent_port(bridge), rewire)"},{"line_number":197,"context_line":"        except oslo_messaging.MessagingException as e:"},{"line_number":198,"context_line":"            LOG.error(_LE(\"Got messaging error while processing trunk bridge \""},{"line_number":199,"context_line":"                          \"%(bridge_name)s: %(err)s\"),"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_b92034f9","line":196,"in_reply_to":"7a8ec9b2_668594cd","updated":"2016-09-22 14:59:23.000000000","message":":)","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"80f483e055627458bd8be8f13aba87b68e98391a","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            return []"},{"line_number":242,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":243,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":244,"context_line":"        subports_ids \u003d []"},{"line_number":245,"context_line":"        for port in ports:"},{"line_number":246,"context_line":"            if is_subport(port[\u0027name\u0027]):"},{"line_number":247,"context_line":"                port_id \u003d self.trunk_manager.get_port_uuid_from_external_ids("},{"line_number":248,"context_line":"                    port)"},{"line_number":249,"context_line":"                subports_ids.append(port_id)"},{"line_number":250,"context_line":"        return subports_ids"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def wire_subports_for_trunk(self, context, trunk_id, subports,"},{"line_number":253,"context_line":"                                trunk_bridge\u003dNone, parent_port\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_597c16b5","line":250,"range":{"start_line":244,"start_character":0,"end_line":250,"end_character":27},"updated":"2016-09-21 15:46:16.000000000","message":"How would following look like?\n\n return [\n     self.trunk_manager.get_port_uuid_from_external_ids(port)\n     for port in ports if is_subport(port[\u0027name\u0027])\n ]","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            return []"},{"line_number":242,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":243,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":244,"context_line":"        subports_ids \u003d []"},{"line_number":245,"context_line":"        for port in ports:"},{"line_number":246,"context_line":"            if is_subport(port[\u0027name\u0027]):"},{"line_number":247,"context_line":"                port_id \u003d self.trunk_manager.get_port_uuid_from_external_ids("},{"line_number":248,"context_line":"                    port)"},{"line_number":249,"context_line":"                subports_ids.append(port_id)"},{"line_number":250,"context_line":"        return subports_ids"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    def wire_subports_for_trunk(self, context, trunk_id, subports,"},{"line_number":253,"context_line":"                                trunk_bridge\u003dNone, parent_port\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_398344ce","line":250,"range":{"start_line":244,"start_character":0,"end_line":250,"end_character":27},"in_reply_to":"7a8ec9b2_597c16b5","updated":"2016-09-22 14:59:23.000000000","message":"doh again :)","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    def _wire_trunk(self, trunk_br, port, rewire\u003dFalse):"},{"line_number":330,"context_line":"        \"\"\"Wire trunk bridge with integration bridge."},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        The method calls into trunk manager to create patch ports for trunk and"},{"line_number":333,"context_line":"        patch ports for all subports associated with this trunk."},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        :param trunk_br: OVSBridge object representing the trunk bridge."}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_866e202d","line":332,"updated":"2016-09-22 00:16:31.000000000","message":"this needs to be tweaked a bit too.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    def _wire_trunk(self, trunk_br, port, rewire\u003dFalse):"},{"line_number":330,"context_line":"        \"\"\"Wire trunk bridge with integration bridge."},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        The method calls into trunk manager to create patch ports for trunk and"},{"line_number":333,"context_line":"        patch ports for all subports associated with this trunk."},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        :param trunk_br: OVSBridge object representing the trunk bridge."}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_e6dfe4d3","line":332,"updated":"2016-09-22 00:16:31.000000000","message":"this needs tweaking a bit","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    def _wire_trunk(self, trunk_br, port, rewire\u003dFalse):"},{"line_number":330,"context_line":"        \"\"\"Wire trunk bridge with integration bridge."},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        The method calls into trunk manager to create patch ports for trunk and"},{"line_number":333,"context_line":"        patch ports for all subports associated with this trunk."},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        :param trunk_br: OVSBridge object representing the trunk bridge."}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_c13a66cd","line":332,"in_reply_to":"7a8ec9b2_e6dfe4d3","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        :param trunk_br: OVSBridge object representing the trunk bridge."},{"line_number":336,"context_line":"        :param port: Parent port dict."},{"line_number":337,"context_line":"        :param rewire: True if it\u0027s not a brand new trunk and we need to rewire"},{"line_number":338,"context_line":"            it."},{"line_number":339,"context_line":"        \"\"\""},{"line_number":340,"context_line":"        ctx \u003d self.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_a66b1c1d","line":337,"updated":"2016-09-22 00:16:31.000000000","message":"how about: True if local trunk state must be reconciled with server\u0027s","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        :param trunk_br: OVSBridge object representing the trunk bridge."},{"line_number":336,"context_line":"        :param port: Parent port dict."},{"line_number":337,"context_line":"        :param rewire: True if it\u0027s not a brand new trunk and we need to rewire"},{"line_number":338,"context_line":"            it."},{"line_number":339,"context_line":"        \"\"\""},{"line_number":340,"context_line":"        ctx \u003d self.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_26f46c59","line":337,"updated":"2016-09-22 00:16:31.000000000","message":"what about: True if local trunk state must be reconciled with server\u0027s state.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        :param trunk_br: OVSBridge object representing the trunk bridge."},{"line_number":336,"context_line":"        :param port: Parent port dict."},{"line_number":337,"context_line":"        :param rewire: True if it\u0027s not a brand new trunk and we need to rewire"},{"line_number":338,"context_line":"            it."},{"line_number":339,"context_line":"        \"\"\""},{"line_number":340,"context_line":"        ctx \u003d self.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_d990a0ef","line":337,"in_reply_to":"7a8ec9b2_a66b1c1d","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":338,"context_line":"            it."},{"line_number":339,"context_line":"        \"\"\""},{"line_number":340,"context_line":"        ctx \u003d self.context"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        try:"},{"line_number":343,"context_line":"            parent_port_id \u003d ("},{"line_number":344,"context_line":"                self.trunk_manager.get_port_uuid_from_external_ids(port))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_46e5f8a1","line":341,"updated":"2016-09-22 00:16:31.000000000","message":"extraneous line!!!","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":338,"context_line":"            it."},{"line_number":339,"context_line":"        \"\"\""},{"line_number":340,"context_line":"        ctx \u003d self.context"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        try:"},{"line_number":343,"context_line":"            parent_port_id \u003d ("},{"line_number":344,"context_line":"                self.trunk_manager.get_port_uuid_from_external_ids(port))"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_21322af2","line":341,"in_reply_to":"7a8ec9b2_46e5f8a1","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"976a099788025e70d3448a28feef65cdf88b62b6","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_d8a019f7","line":370,"range":{"start_line":370,"start_character":12,"end_line":370,"end_character":27},"updated":"2016-09-21 17:25:11.000000000","message":"Would this be better termed as \"local_subport_ids\", and rename \"subports\" to \"model_subport_ids\" on the line below?","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_218eaa78","line":370,"range":{"start_line":370,"start_character":12,"end_line":370,"end_character":27},"in_reply_to":"7a8ec9b2_d8a019f7","updated":"2016-09-22 14:59:23.000000000","message":"not sure your suggestion makes it more clear...","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"80f483e055627458bd8be8f13aba87b68e98391a","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_9c05ac12","line":371,"range":{"start_line":371,"start_character":27,"end_line":371,"end_character":28},"updated":"2016-09-21 15:46:16.000000000","message":"Nit: Brackets are not needed","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"976a099788025e70d3448a28feef65cdf88b62b6","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_d568e2f7","line":371,"range":{"start_line":371,"start_character":27,"end_line":371,"end_character":28},"in_reply_to":"7a8ec9b2_9c05ac12","updated":"2016-09-21 17:25:11.000000000","message":"further nit: set notation?\n\n \n{p[\u0027port_id\u0027] for p in trunk.sub_ports}","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_e165a2e1","line":371,"range":{"start_line":371,"start_character":27,"end_line":371,"end_character":28},"in_reply_to":"7a8ec9b2_d568e2f7","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"976a099788025e70d3448a28feef65cdf88b62b6","unresolved":false,"context_lines":[{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_781aa570","line":372,"range":{"start_line":372,"start_character":33,"end_line":372,"end_character":36},"updated":"2016-09-21 17:25:11.000000000","message":"Can this conversion be done on L370? unwire_subports_for_trunk would accept a set just fine","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_01bfeefb","line":372,"range":{"start_line":372,"start_character":33,"end_line":372,"end_character":36},"in_reply_to":"7a8ec9b2_781aa570","updated":"2016-09-22 14:59:23.000000000","message":"it could but then the line would be too long and I will have to split it...","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":365,"context_line":"                                               constants.ERROR_STATUS)"},{"line_number":366,"context_line":"            return"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_2669ccf9","line":374,"range":{"start_line":368,"start_character":0,"end_line":374,"end_character":76},"updated":"2016-09-22 00:16:31.000000000","message":"this would status/metadata handling, my suggestion is to rebase this change on top of [1] to make sure we don\u0027t clash and everything is handled nicely together.\n\n[1] https://review.openstack.org/#/c/373600/","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":365,"context_line":"                                               constants.ERROR_STATUS)"},{"line_number":366,"context_line":"            return"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"        # We need to remove stale subports"},{"line_number":369,"context_line":"        if rewire:"},{"line_number":370,"context_line":"            old_subport_ids \u003d self.get_connected_subports_for_trunk(trunk.id)"},{"line_number":371,"context_line":"            subports \u003d set([p[\u0027port_id\u0027] for p in trunk.sub_ports])"},{"line_number":372,"context_line":"            subports_to_delete \u003d set(old_subport_ids) - subports"},{"line_number":373,"context_line":"            if subports_to_delete:"},{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_c1afc6e1","line":374,"range":{"start_line":368,"start_character":0,"end_line":374,"end_character":76},"in_reply_to":"7a8ec9b2_2669ccf9","updated":"2016-09-22 14:59:23.000000000","message":"Done","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"},{"line_number":378,"context_line":"            parent_port\u003dport)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def _set_trunk_metadata(self, trunk_bridge, port, trunk_id, subport_ids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_663d94dc","line":377,"updated":"2016-09-22 00:16:31.000000000","message":"Here we are \u0027re-adding\u0027 subports that may already be present. This is fine, though one might wonder that these should be skipped but I am thinking about the case where the port ID is the same but the segmentation ID has changed (e.g. agent crashed, subport removed and readded with different seg_id). This should be refreshed seamlessly but can you confirm? In any case a note would be nice to help us not forget ;)","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"afccc8268b42cc9442f2710f17d69075f3138372","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"},{"line_number":378,"context_line":"            parent_port\u003dport)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def _set_trunk_metadata(self, trunk_bridge, port, trunk_id, subport_ids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_f6ff2232","line":377,"in_reply_to":"5a8bc5a2_10bf36fc","updated":"2016-09-22 16:32:15.000000000","message":"got your point now, I tested it manually, it works. It\u0027s worth adding a func test but I\u0027d skip it for this patch.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"2cf3cbea248a214ed2ed0f0895ddf10918fa49e7","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"},{"line_number":378,"context_line":"            parent_port\u003dport)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def _set_trunk_metadata(self, trunk_bridge, port, trunk_id, subport_ids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_10bf36fc","line":377,"in_reply_to":"5a8bc5a2_21530ae3","updated":"2016-09-22 15:42:32.000000000","message":"when I said refreshed seamlessly I mean without traffic loss.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"005565b02558cb6298a82c06708abfb97b62864e","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"},{"line_number":378,"context_line":"            parent_port\u003dport)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def _set_trunk_metadata(self, trunk_bridge, port, trunk_id, subport_ids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_b90ca643","line":377,"in_reply_to":"5a8bc5a2_f6ff2232","updated":"2016-09-22 18:15:43.000000000","message":"ok, ok","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"9c104f22979a66d77cfbff04bf9c24b047fb206e","unresolved":false,"context_lines":[{"line_number":374,"context_line":"                self.unwire_subports_for_trunk(trunk.id, subports_to_delete)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        self.wire_subports_for_trunk("},{"line_number":377,"context_line":"            ctx, trunk.id, trunk.sub_ports, trunk_bridge\u003dtrunk_br,"},{"line_number":378,"context_line":"            parent_port\u003dport)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def _set_trunk_metadata(self, trunk_bridge, port, trunk_id, subport_ids):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a8bc5a2_21530ae3","line":377,"in_reply_to":"7a8ec9b2_663d94dc","updated":"2016-09-22 14:59:23.000000000","message":"it should work, let me add a test for it","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"def bridge_has_service_port(bridge):"},{"line_number":105,"context_line":"    \"\"\"True if there is an OVS port that used to implement a trunk."},{"line_number":106,"context_line":"    \"\"\""},{"line_number":107,"context_line":"    return bridge_has_port(bridge, is_trunk_service_port)"},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_748305cd","line":105,"updated":"2016-09-22 18:51:25.000000000","message":"that is* used","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"def bridge_has_service_port(bridge):"},{"line_number":105,"context_line":"    \"\"\"True if there is an OVS port that used to implement a trunk."},{"line_number":106,"context_line":"    \"\"\""},{"line_number":107,"context_line":"    return bridge_has_port(bridge, is_trunk_service_port)"},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_cd07cc1a","line":105,"in_reply_to":"5a8bc5a2_748305cd","updated":"2016-09-23 10:31:38.000000000","message":"Done","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":183,"context_line":"            bridge.destroy()"},{"line_number":184,"context_line":"            return"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        rewire \u003d bridge_has_service_port(bridge)"},{"line_number":187,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":188,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":189,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_9417b150","line":186,"updated":"2016-09-22 18:51:25.000000000","message":"I\u0027d add a note here that says:\n\n# Check if the trunk was provisioned in a previous run. This can happen\n# at agent startup when existing trunks are notified as added events.","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":183,"context_line":"            bridge.destroy()"},{"line_number":184,"context_line":"            return"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        rewire \u003d bridge_has_service_port(bridge)"},{"line_number":187,"context_line":"        # Once we get hold of the trunk parent port, we can provision"},{"line_number":188,"context_line":"        # the OVS dataplane for the trunk."},{"line_number":189,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_4821ea9e","line":186,"in_reply_to":"5a8bc5a2_9417b150","updated":"2016-09-23 10:31:38.000000000","message":"Done","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        return ovs_lib.BaseOVS().bridge_exists(bridge_name)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def get_connected_subports_for_trunk(self, trunk_id):"},{"line_number":238,"context_line":"        \"\"\"Return the list of subports present on the trunk bridge.\"\"\""},{"line_number":239,"context_line":"        bridge \u003d ovs_lib.OVSBridge(utils.gen_trunk_br_name(trunk_id))"},{"line_number":240,"context_line":"        if not bridge.bridge_exists(bridge.br_name):"},{"line_number":241,"context_line":"            return []"},{"line_number":242,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":243,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":244,"context_line":"        return ["},{"line_number":245,"context_line":"            self.trunk_manager.get_port_uuid_from_external_ids(port)"},{"line_number":246,"context_line":"            for port in ports if is_subport(port[\u0027name\u0027])"},{"line_number":247,"context_line":"        ]"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def wire_subports_for_trunk(self, context, trunk_id, subports,"},{"line_number":250,"context_line":"                                trunk_bridge\u003dNone, parent_port\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_4fee7023","line":247,"range":{"start_line":238,"start_character":0,"end_line":247,"end_character":9},"updated":"2016-09-22 18:51:25.000000000","message":"this could fail with a runtime error, do we want to protect ourselves in some way or do we let bubble up?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        return ovs_lib.BaseOVS().bridge_exists(bridge_name)"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def get_connected_subports_for_trunk(self, trunk_id):"},{"line_number":238,"context_line":"        \"\"\"Return the list of subports present on the trunk bridge.\"\"\""},{"line_number":239,"context_line":"        bridge \u003d ovs_lib.OVSBridge(utils.gen_trunk_br_name(trunk_id))"},{"line_number":240,"context_line":"        if not bridge.bridge_exists(bridge.br_name):"},{"line_number":241,"context_line":"            return []"},{"line_number":242,"context_line":"        ports \u003d bridge.get_ports_attributes("},{"line_number":243,"context_line":"                        \u0027Interface\u0027, columns\u003d[\u0027name\u0027, \u0027external_ids\u0027])"},{"line_number":244,"context_line":"        return ["},{"line_number":245,"context_line":"            self.trunk_manager.get_port_uuid_from_external_ids(port)"},{"line_number":246,"context_line":"            for port in ports if is_subport(port[\u0027name\u0027])"},{"line_number":247,"context_line":"        ]"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def wire_subports_for_trunk(self, context, trunk_id, subports,"},{"line_number":250,"context_line":"                                trunk_bridge\u003dNone, parent_port\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_a36451e5","line":247,"range":{"start_line":238,"start_character":0,"end_line":247,"end_character":9},"in_reply_to":"5a8bc5a2_4fee7023","updated":"2016-09-23 10:31:38.000000000","message":"ok let\u0027s be protective","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # NOTE(status_police): inform the server whether the operation"},{"line_number":383,"context_line":"        # was a partial or complete success. Do not inline status."},{"line_number":384,"context_line":"        status \u003d self.wire_subports_for_trunk("},{"line_number":385,"context_line":"            ctx, trunk.id, trunk.sub_ports,"},{"line_number":386,"context_line":"            trunk_bridge\u003dtrunk_br, parent_port\u003dport)"},{"line_number":387,"context_line":"        self.report_trunk_status(ctx, trunk.id, status)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_cfc1a082","line":384,"updated":"2016-09-22 18:51:25.000000000","message":"forgot to add a note about why we\u0027re rewiring existing ports.","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # NOTE(status_police): inform the server whether the operation"},{"line_number":383,"context_line":"        # was a partial or complete success. Do not inline status."},{"line_number":384,"context_line":"        status \u003d self.wire_subports_for_trunk("},{"line_number":385,"context_line":"            ctx, trunk.id, trunk.sub_ports,"},{"line_number":386,"context_line":"            trunk_bridge\u003dtrunk_br, parent_port\u003dport)"},{"line_number":387,"context_line":"        self.report_trunk_status(ctx, trunk.id, status)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_a3ab71d5","line":384,"in_reply_to":"5a8bc5a2_cfc1a082","updated":"2016-09-23 10:31:38.000000000","message":"ooops sorry","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"2f08789de26910223bd0abcb9a4c66021040817e","unresolved":false,"context_lines":[{"line_number":249,"context_line":"                for port in ports if is_subport(port[\u0027name\u0027])"},{"line_number":250,"context_line":"            ]"},{"line_number":251,"context_line":"        except (RuntimeError, tman.TrunkManagerError) as e:"},{"line_number":252,"context_line":"            LOG.error(_LE(\"Failed to get subports for bridge %(bridge)s: \""},{"line_number":253,"context_line":"                          \"%(err)s\"), {\u0027bridge\u0027: bridge.br_name, \u0027err\u0027: e})"},{"line_number":254,"context_line":"            return []"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a8bc5a2_f6427d3d","line":252,"updated":"2016-09-23 17:58:45.000000000","message":"would have been nice to have tested this error path","commit_id":"08f2af18f996c9ec89ddb6554a1132aa9605b5af"}],"neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        patch_int_attrs \u003d get_patch_peer_attrs("},{"line_number":135,"context_line":"            self.patch_port_trunk_name, self.port_mac, self.port_id)"},{"line_number":136,"context_line":"        patch_trunk_attrs \u003d get_patch_peer_attrs(self.patch_port_int_name,"},{"line_number":137,"context_line":"                                                 self.port_mac, self.port_id)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        with self.ovsdb_transaction() as txn:"},{"line_number":140,"context_line":"            txn.add(ovsdb.add_port(br_int.br_name,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_f4aefc96","line":137,"updated":"2016-09-21 00:27:11.000000000","message":"is this needed because of exists()?","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        patch_int_attrs \u003d get_patch_peer_attrs("},{"line_number":135,"context_line":"            self.patch_port_trunk_name, self.port_mac, self.port_id)"},{"line_number":136,"context_line":"        patch_trunk_attrs \u003d get_patch_peer_attrs(self.patch_port_int_name,"},{"line_number":137,"context_line":"                                                 self.port_mac, self.port_id)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        with self.ovsdb_transaction() as txn:"},{"line_number":140,"context_line":"            txn.add(ovsdb.add_port(br_int.br_name,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_683e6f8c","line":137,"in_reply_to":"7a8ec9b2_f4aefc96","updated":"2016-09-21 15:07:21.000000000","message":"nope this is needed to get directly from the trunk bridge the ids of the subports. Otherwise I need to:\n\n1) get the ports attributes from the trunk bridge\n2) identify the subports\n3) get the pair port on br-int\n4) get the attributes of the port on br-int to get the port id\n\nI think a shortcut is worth it, otherwise we need one query more for each pair to get the port attributes from br-int","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            txn.add(ovsdb.del_port(self.patch_port_int_name,"},{"line_number":162,"context_line":"                                   bridge.br_name))"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def exists(self):"},{"line_number":165,"context_line":"        \"\"\"Check if the patch port on br-int exists\"\"\""},{"line_number":166,"context_line":"        return self.bridge.port_exists(self.patch_port_int_name)"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_146e082f","line":164,"updated":"2016-09-21 00:27:11.000000000","message":"not sure if these changes are actually needed.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            txn.add(ovsdb.del_port(self.patch_port_int_name,"},{"line_number":162,"context_line":"                                   bridge.br_name))"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def exists(self):"},{"line_number":165,"context_line":"        \"\"\"Check if the patch port on br-int exists\"\"\""},{"line_number":166,"context_line":"        return self.bridge.port_exists(self.patch_port_int_name)"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_8843bb12","line":164,"in_reply_to":"7a8ec9b2_146e082f","updated":"2016-09-21 15:07:21.000000000","message":"Done","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110f2d288bfe724189bf8da140d85e5c75ff0247","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        except RuntimeError as e:"},{"line_number":247,"context_line":"            raise TrunkManagerError(error\u003de)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def trunk_exists(self, trunk_id, port_id, port_mac):"},{"line_number":250,"context_line":"        \"\"\"Check if it\u0027s not the first time we process this trunk\"\"\""},{"line_number":251,"context_line":"        trunk \u003d TrunkParentPort(trunk_id, port_id, port_mac)"},{"line_number":252,"context_line":"        return trunk.exists()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_b44d549b","line":249,"updated":"2016-09-21 00:27:11.000000000","message":"neither is this one.","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914ddf26e89e2cf9d4330beae7aa87d1aaa02dab","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        except RuntimeError as e:"},{"line_number":247,"context_line":"            raise TrunkManagerError(error\u003de)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def trunk_exists(self, trunk_id, port_id, port_mac):"},{"line_number":250,"context_line":"        \"\"\"Check if it\u0027s not the first time we process this trunk\"\"\""},{"line_number":251,"context_line":"        trunk \u003d TrunkParentPort(trunk_id, port_id, port_mac)"},{"line_number":252,"context_line":"        return trunk.exists()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_c8494333","line":249,"in_reply_to":"7a8ec9b2_b44d549b","updated":"2016-09-21 15:07:21.000000000","message":"Done","commit_id":"4a299e934c1d3741769270c37053927439716d3b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"89a0c291d32e9c217b14b067d0551b3830e1ce57","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        patch_int_attrs \u003d get_patch_peer_attrs("},{"line_number":135,"context_line":"            self.patch_port_trunk_name, self.port_mac, self.port_id)"},{"line_number":136,"context_line":"        patch_trunk_attrs \u003d get_patch_peer_attrs(self.patch_port_int_name,"},{"line_number":137,"context_line":"                                                 self.port_mac, self.port_id)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        with self.ovsdb_transaction() as txn:"},{"line_number":140,"context_line":"            txn.add(ovsdb.add_port(br_int.br_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_460191ae","line":137,"updated":"2016-09-21 15:32:10.000000000","message":"Even though adding these params when they are None is equivalent to omitting them, I am still confused by your response on PS3. Where exactly you see this used? Let me see if I can make sense of this change, I am not quite sure I understand its effect.","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"976a099788025e70d3448a28feef65cdf88b62b6","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        patch_int_attrs \u003d get_patch_peer_attrs("},{"line_number":135,"context_line":"            self.patch_port_trunk_name, self.port_mac, self.port_id)"},{"line_number":136,"context_line":"        patch_trunk_attrs \u003d get_patch_peer_attrs(self.patch_port_int_name,"},{"line_number":137,"context_line":"                                                 self.port_mac, self.port_id)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        with self.ovsdb_transaction() as txn:"},{"line_number":140,"context_line":"            txn.add(ovsdb.add_port(br_int.br_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_58f7a9d2","line":137,"in_reply_to":"7a8ec9b2_460191ae","updated":"2016-09-21 17:25:11.000000000","message":"They are used to get the child port IDs off of the \u0027spt\u0027 patch ports [1].\n\n[1] https://review.openstack.org/#/c/365176/4/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py@247","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"57c7fab834be9745b2e7335370a67ddc37998889","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        patch_int_attrs \u003d get_patch_peer_attrs("},{"line_number":135,"context_line":"            self.patch_port_trunk_name, self.port_mac, self.port_id)"},{"line_number":136,"context_line":"        patch_trunk_attrs \u003d get_patch_peer_attrs(self.patch_port_int_name,"},{"line_number":137,"context_line":"                                                 self.port_mac, self.port_id)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        with self.ovsdb_transaction() as txn:"},{"line_number":140,"context_line":"            txn.add(ovsdb.add_port(br_int.br_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8ec9b2_261e2c70","line":137,"in_reply_to":"7a8ec9b2_58f7a9d2","updated":"2016-09-22 00:16:31.000000000","message":"yeah, thanks Rossella helped me see the light :)","commit_id":"226a23eac73efc59afba6e8408cdd373c7dec3f7"}],"neutron/tests/functional/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports)"},{"line_number":161,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":162,"context_line":"        self.agent.fullsync \u003d True"},{"line_number":163,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def test_restart(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_ef9f3c5d","line":162,"updated":"2016-09-22 18:51:25.000000000","message":"wouldn\u0027t this test be somewhat a no-op in that a fullsync does not generate a notification with the list of OVSDB events?\n\nCan you elaborate a bit more on the nature of this test?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports)"},{"line_number":161,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":162,"context_line":"        self.agent.fullsync \u003d True"},{"line_number":163,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def test_restart(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_03afc494","line":162,"in_reply_to":"5a8bc5a2_ef9f3c5d","updated":"2016-09-23 10:31:38.000000000","message":"it\u0027s a no op for the ovsdb handler right. The resync is managed entirely by the ovs agent even for trunk ports. This test is here to make sure that the resync mechanism of the agent doesn\u0027t change in a way that might affect the trunks. Does it make sense?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"4eb9d30f14ddaa167d6fd431760007e6ba1faed5","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        self.agent.fullsync \u003d True"},{"line_number":163,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def test_restart(self):"},{"line_number":166,"context_line":"        ports \u003d self.create_test_ports(amount\u003d3)"},{"line_number":167,"context_line":"        self.trunk_dict[\u0027port_id\u0027] \u003d ports[0][\u0027id\u0027]"},{"line_number":168,"context_line":"        self.trunk_dict[\u0027sub_ports\u0027] \u003d [trunk_obj.SubPort("}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_6fb7ab39","line":165,"updated":"2016-09-22 18:54:04.000000000","message":"I gotta ask to scare gate demons away: is this race-y?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        self.agent.fullsync \u003d True"},{"line_number":163,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def test_restart(self):"},{"line_number":166,"context_line":"        ports \u003d self.create_test_ports(amount\u003d3)"},{"line_number":167,"context_line":"        self.trunk_dict[\u0027port_id\u0027] \u003d ports[0][\u0027id\u0027]"},{"line_number":168,"context_line":"        self.trunk_dict[\u0027sub_ports\u0027] \u003d [trunk_obj.SubPort("}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_63a088a0","line":165,"in_reply_to":"5a8bc5a2_6fb7ab39","updated":"2016-09-23 10:31:38.000000000","message":"it shouldn\u0027t :)","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":178,"context_line":"        self.stop_agent(self.agent, self.agent_thread)"},{"line_number":179,"context_line":"        self.polling_manager.stop()"},{"line_number":180,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports)"},{"line_number":181,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        # restart and simulate a subport delete"},{"line_number":184,"context_line":"        deleted_port \u003d self.ports[2]"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_ef765c3d","line":181,"range":{"start_line":180,"start_character":0,"end_line":181,"end_character":55},"updated":"2016-09-22 18:51:25.000000000","message":"why are we doing this?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":178,"context_line":"        self.stop_agent(self.agent, self.agent_thread)"},{"line_number":179,"context_line":"        self.polling_manager.stop()"},{"line_number":180,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports)"},{"line_number":181,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        # restart and simulate a subport delete"},{"line_number":184,"context_line":"        deleted_port \u003d self.ports[2]"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_83a9147e","line":181,"range":{"start_line":180,"start_character":0,"end_line":181,"end_character":55},"in_reply_to":"5a8bc5a2_ef765c3d","updated":"2016-09-23 10:31:38.000000000","message":"it\u0027s a simple restart without any port deletion in the middle, I think I can skip it since I added the port deletion in the following lines.","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        deleted_port \u003d self.ports[2]"},{"line_number":185,"context_line":"        deleted_sp \u003d trunk_manager.SubPort("},{"line_number":186,"context_line":"            self.trunk_dict[\u0027id\u0027], deleted_port[\u0027id\u0027])"},{"line_number":187,"context_line":"        self.stop_agent(self.agent, self.agent_thread)"},{"line_number":188,"context_line":"        self.polling_manager.stop()"},{"line_number":189,"context_line":"        self.trunk_dict[\u0027sub_ports\u0027] \u003d self.trunk_dict[\u0027sub_ports\u0027][:1]"},{"line_number":190,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports[:2])"},{"line_number":191,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_efc49c41","line":188,"range":{"start_line":187,"start_character":0,"end_line":188,"end_character":35},"updated":"2016-09-22 18:51:25.000000000","message":"didn\u0027t we do this already? Is that because the process monitor restarts the agent?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        deleted_port \u003d self.ports[2]"},{"line_number":185,"context_line":"        deleted_sp \u003d trunk_manager.SubPort("},{"line_number":186,"context_line":"            self.trunk_dict[\u0027id\u0027], deleted_port[\u0027id\u0027])"},{"line_number":187,"context_line":"        self.stop_agent(self.agent, self.agent_thread)"},{"line_number":188,"context_line":"        self.polling_manager.stop()"},{"line_number":189,"context_line":"        self.trunk_dict[\u0027sub_ports\u0027] \u003d self.trunk_dict[\u0027sub_ports\u0027][:1]"},{"line_number":190,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports[:2])"},{"line_number":191,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_a304507e","line":188,"range":{"start_line":187,"start_character":0,"end_line":188,"end_character":35},"in_reply_to":"5a8bc5a2_efc49c41","updated":"2016-09-23 10:31:38.000000000","message":"that\u0027s the second restart, as said in the previous comment I will simply the test and do only one restart with port deletion, instead of doing (1) restart (2) restart with port deletion","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a833f39895a049c22a3a9c3e437a117797606200","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        self.polling_manager.stop()"},{"line_number":189,"context_line":"        self.trunk_dict[\u0027sub_ports\u0027] \u003d self.trunk_dict[\u0027sub_ports\u0027][:1]"},{"line_number":190,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports[:2])"},{"line_number":191,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":192,"context_line":"        common_utils.wait_until_true("},{"line_number":193,"context_line":"            lambda: (deleted_sp.patch_port_trunk_name not in"},{"line_number":194,"context_line":"                self.trunk_br.get_port_name_list()))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_2f184484","line":191,"updated":"2016-09-22 18:51:25.000000000","message":"shouldn\u0027t we wait only for a subset of ports?","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"b7d090c3f873c739743683fcc3d08dafaf9a3b29","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        self.polling_manager.stop()"},{"line_number":189,"context_line":"        self.trunk_dict[\u0027sub_ports\u0027] \u003d self.trunk_dict[\u0027sub_ports\u0027][:1]"},{"line_number":190,"context_line":"        self.setup_agent_and_ports(port_dicts\u003dports[:2])"},{"line_number":191,"context_line":"        self.wait_until_ports_state(self.ports, up\u003dTrue)"},{"line_number":192,"context_line":"        common_utils.wait_until_true("},{"line_number":193,"context_line":"            lambda: (deleted_sp.patch_port_trunk_name not in"},{"line_number":194,"context_line":"                self.trunk_br.get_port_name_list()))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a8bc5a2_0310a4a7","line":191,"in_reply_to":"5a8bc5a2_2f184484","updated":"2016-09-23 10:31:38.000000000","message":"it\u0027s tricky, when you do setup_agent_and_ports the port_dicts passed it\u0027s saved in self.ports so we wait for a subset here","commit_id":"b3cf561728065de73b3ce2a3d5025ef082b7a509"}]}