)]}'
{"neutron/services/trunk/drivers/openvswitch/agent/driver.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"10d96cf13089707b1662a22a748526081705ad8c","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \"\"\""},{"line_number":38,"context_line":"        raise NotImplementedError()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":41,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":42,"context_line":"        if event_type \u003d\u003d events.CREATED:"},{"line_number":43,"context_line":"            for subport in subports:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_eb083c94","line":40,"updated":"2016-09-03 03:17:01.000000000","message":"bear in mind that these messages would be received by all agents, we\u0027d need to figure a clean way to discard those requests that don\u0027t pertain this agent, because as the code stands you add a subport to a trunk that might not belong to the host.","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"4d33ba6516cd32b4d8340566ccdd4ddd7efdda1d","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":41,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":42,"context_line":"        if event_type \u003d\u003d events.CREATED:"},{"line_number":43,"context_line":"            for subport in subports:"},{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_79b9672e","line":42,"updated":"2016-09-02 22:54:05.000000000","message":"If you look at [1], I could expect this to simply be:\n\nself.ovsdb_handler.wire_subports_for_trunk(...)\n\nor \n\nself.ovsdb_handler.unwire_subports_for_trunk(...)\n\nThere may be more bells and whistles attached, but at least we don\u0027t have to duplicate the entire binding/error handling logic.\n\n[1] https://review.openstack.org/#/c/365162/","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"45db6636d5807cbea06f8042574494bb1b81b69c","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"},{"line_number":46,"context_line":"                    port_id\u003dsubport.port_id,"},{"line_number":47,"context_line":"                    # TODO(jlibosva): Find out whether we need mac for subport"},{"line_number":48,"context_line":"                    port_mac\u003dNone,"},{"line_number":49,"context_line":"                    segmentation_id\u003dsubport.segmentation_id)"},{"line_number":50,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_4748075c","line":47,"updated":"2016-08-31 17:33:16.000000000","message":"I think we do need the MAC address. We need to be able to set \"attached-mac\" in the external-ids, so it needs to come from somewhere and preferably not from a RPC call :) It probably needs to be passed in, which makes me wonder if it\u0027s an attribute we could add the subport. If it\u0027s an attribute of the subport, then we could extract it with negligible overhead here.","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b1d67fdf030b88efc9d20511dc50570954437e1b","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"},{"line_number":46,"context_line":"                    port_id\u003dsubport.port_id,"},{"line_number":47,"context_line":"                    # TODO(jlibosva): Find out whether we need mac for subport"},{"line_number":48,"context_line":"                    port_mac\u003dNone,"},{"line_number":49,"context_line":"                    segmentation_id\u003dsubport.segmentation_id)"},{"line_number":50,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_b7f1f7cf","line":47,"in_reply_to":"fa7ab95a_173aa398","updated":"2016-08-31 22:37:05.000000000","message":"4 can obviously happen in \u0027bulk\u0027 as we have support for that.\n\nThe call back to the server may seem inefficient but in reality it allows us to make the subport_add server API call return without blocking (waiting for all bindings to complete) and have better load distribution across the server replicas (RPC workers)","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"2f5af51b5936942a0496134f7a2fdfe8ed4407c8","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"},{"line_number":46,"context_line":"                    port_id\u003dsubport.port_id,"},{"line_number":47,"context_line":"                    # TODO(jlibosva): Find out whether we need mac for subport"},{"line_number":48,"context_line":"                    port_mac\u003dNone,"},{"line_number":49,"context_line":"                    segmentation_id\u003dsubport.segmentation_id)"},{"line_number":50,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_dad6a2f0","line":47,"in_reply_to":"fa7ab95a_3ab4eef0","updated":"2016-08-31 23:00:27.000000000","message":"thing is: I see some shared logic between here and what the OVSDBHandler has to do. We can sort out how to dedup later, but let\u0027s not forget that.","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7921ce1c2dad322017b35adf231402a31f69ed68","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"},{"line_number":46,"context_line":"                    port_id\u003dsubport.port_id,"},{"line_number":47,"context_line":"                    # TODO(jlibosva): Find out whether we need mac for subport"},{"line_number":48,"context_line":"                    port_mac\u003dNone,"},{"line_number":49,"context_line":"                    segmentation_id\u003dsubport.segmentation_id)"},{"line_number":50,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_173aa398","line":47,"in_reply_to":"fa7ab95a_4748075c","updated":"2016-08-31 22:32:14.000000000","message":"The workflow on creation is as such:\n\n0) received the subports\n1) call server to update subports binding\n2) call 1 returns port details, MAC included\n3) trunk manager plug the subport, other OVS plumbing happens\n4) repeat\n5) once done, and you\u0027re happy, set the trunk to ACTIVE\n\nThis is somewhat similar to the workflow where we fetch all subports at once at the time the VM boot and the trunk is created for the first time.","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"c1c614b9ba4af816588dd78c7d51645b22e212a0","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"},{"line_number":46,"context_line":"                    port_id\u003dsubport.port_id,"},{"line_number":47,"context_line":"                    # TODO(jlibosva): Find out whether we need mac for subport"},{"line_number":48,"context_line":"                    port_mac\u003dNone,"},{"line_number":49,"context_line":"                    segmentation_id\u003dsubport.segmentation_id)"},{"line_number":50,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_3ab4eef0","line":47,"in_reply_to":"fa7ab95a_b7f1f7cf","updated":"2016-08-31 22:58:39.000000000","message":"OK, I\u0027m just totally scatter-brained today. If we perform the port binding first (like the design says), we\u0027ll get the port MAC address on the response. I totally missed that, should have been obvious.","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"330259addb0a51d762be28200999208f833541d2","unresolved":false,"context_lines":[{"line_number":44,"context_line":"                self.trunk_manager.add_sub_port("},{"line_number":45,"context_line":"                    trunk_id\u003dsubport.trunk_id,"},{"line_number":46,"context_line":"                    port_id\u003dsubport.port_id,"},{"line_number":47,"context_line":"                    # TODO(jlibosva): Find out whether we need mac for subport"},{"line_number":48,"context_line":"                    port_mac\u003dNone,"},{"line_number":49,"context_line":"                    segmentation_id\u003dsubport.segmentation_id)"},{"line_number":50,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_9aab3a1e","line":47,"in_reply_to":"fa7ab95a_dad6a2f0","updated":"2016-08-31 23:07:17.000000000","message":"Yes, maybe leave a note or a todo at a minimum.","commit_id":"513088c783d362927a9ffdf9b525cd1484bfe38f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d0185f5fe5b1ef177e45b1dc001486af53f7d3f","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_7c565ed3","line":50,"updated":"2016-09-06 11:13:17.000000000","message":"Why are we catching an exception here. Lets check the length of the subports","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"289a6ef7527be809247a2428fc0fd580cef4437b","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_82a9e1e1","line":50,"in_reply_to":"9a89bdaa_274e676f","updated":"2016-09-06 19:27:19.000000000","message":"When whoever the is notifier notifies with empty data for any reason. Do you think this case shouldn\u0027t be handled?","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"0f9e624dcd41a056b7328c45f52846ea82f05bc4","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_274e676f","line":50,"in_reply_to":"9a89bdaa_6e45ac6e","updated":"2016-09-06 19:04:38.000000000","message":"under what circumstances? When the RPC registry callback has a bug that rendered the whole system useless? What failure more are you trying to tackle?","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"10d750df9aaa51e4a0f4696a618ba14118f67dfa","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_6e45ac6e","line":50,"in_reply_to":"9a89bdaa_746f40c5","updated":"2016-09-06 18:09:28.000000000","message":"If we remove the check, we\u0027ll get the IndexError with the whole traceback","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"29a0d26f1df4b90d3287d8b6b0de08e84726052c","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_bc18e685","line":50,"in_reply_to":"9a89bdaa_7c565ed3","updated":"2016-09-06 11:24:29.000000000","message":"EAFP is preferred in Python: https://docs.python.org/2/glossary.html#term-eafp","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"02a5660074b9d39594bf2761373177d24d0b7229","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_bdfe0436","line":50,"in_reply_to":"9a89bdaa_82a9e1e1","updated":"2016-09-06 20:10:46.000000000","message":"If I look at [1], sending an empty list might be possible, but I wonder if this happens in practice or if we want to even plug the \u0027hole\u0027. Perhaps ajo can chime in. One thing is for certain: under the current server side circumstances, an empty payload is never sent.\n\n[1] https://github.com/openstack/neutron/blob/7f617e6a210243caba9b2637efac726a21a2d3a5/neutron/api/rpc/handlers/resources_rpc.py#L204","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ecccf20d3502832d5344740ad6f4014c37adfa0f","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_746f40c5","line":50,"in_reply_to":"9a89bdaa_8e6f55c4","updated":"2016-09-06 17:07:15.000000000","message":"I think we\u0027re missing the real point here: whether we expect subports to be part of the same trunk. There\u0027s nothing contractual about this though, and the notification is rather vague, anyhoo if we get this called, I think we can safely expect subports to be non-empty, otherwise there is a serious bug in the RPC callback registry, so I\u0027d personally vote to remove the check altogether.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3117fae347d9ef8dc21552d7749ef653c6ef5880","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_1900eda9","line":50,"in_reply_to":"9a89bdaa_8e6f55c4","updated":"2016-09-06 16:44:44.000000000","message":"It\u0027s expected to always get some subports to add. Sending rpc message to add no support doesn\u0027t make sense. So pre-checking if data are correct would be a C-style error handling.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"8357fb01d432287585fc92d3da1262f72206a721","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_8e6f55c4","line":50,"in_reply_to":"9a89bdaa_bc18e685","updated":"2016-09-06 16:25:51.000000000","message":"I\u0027m not sure I\u0027d apply EAFP here. While it has merit, I\u0027m not a fan of it for this type of check. To me this is a case where LBYL doesn\u0027t really present an issue and would read more clearly. I would personally just check the length of subports here.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"769bff5bf5f74e0be190db97a9652c4b5e2ba169","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":48,"context_line":"        try:"},{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_5fe1ccfd","line":50,"in_reply_to":"9a89bdaa_bdfe0436","updated":"2016-09-09 10:12:15.000000000","message":"Empty notification can\u0027t happen (at least in the current form of push), because resources_by_type dictionary would be empty after classification.\n\nIs there any place where trunk ports code can send empty list of subports?\n\nAnyway, having this check will hurt nobody ;) you\u0027re nitpicking (both sides) folks, focus! (partly joking)\n\nCheers,","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"8357fb01d432287585fc92d3da1262f72206a721","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"},{"line_number":54,"context_line":"            return"},{"line_number":55,"context_line":"        if event_type \u003d\u003d events.CREATED:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_ce215d4f","line":52,"updated":"2016-09-06 16:25:51.000000000","message":"Is this really an error case? Seems like something to log at DEBUG level if we log it at all.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3117fae347d9ef8dc21552d7749ef653c6ef5880","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            # Subports are always created with the same trunk_id"},{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"},{"line_number":54,"context_line":"            return"},{"line_number":55,"context_line":"        if event_type \u003d\u003d events.CREATED:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_7959f9fd","line":52,"in_reply_to":"9a89bdaa_ce215d4f","updated":"2016-09-06 16:44:44.000000000","message":"That means we got rpc message saying we should update no port. I don\u0027t think it should even happen.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"80c53f61ed16f11436eabf43b687070cf7ae7095","unresolved":false,"context_lines":[{"line_number":50,"context_line":"            trunk_id \u003d subports[0].trunk_id"},{"line_number":51,"context_line":"        except IndexError:"},{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"},{"line_number":54,"context_line":"            return"},{"line_number":55,"context_line":"        if event_type \u003d\u003d events.CREATED:"},{"line_number":56,"context_line":"            ctx \u003d self.ovsdb_handler.context"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_39a211af","line":53,"range":{"start_line":53,"start_character":22,"end_line":53,"end_character":30},"updated":"2016-09-06 16:46:17.000000000","message":"It should be event_type.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ecccf20d3502832d5344740ad6f4014c37adfa0f","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"},{"line_number":54,"context_line":"            return"},{"line_number":55,"context_line":"        if event_type \u003d\u003d events.CREATED:"},{"line_number":56,"context_line":"            ctx \u003d self.ovsdb_handler.context"},{"line_number":57,"context_line":"            self.ovsdb_handler.wire_subports_for_trunk(ctx, trunk_id, subports)"},{"line_number":58,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_74102002","line":55,"updated":"2016-09-06 17:07:15.000000000","message":"please see comment in base patch. I think it\u0027s makes more sense to check for the trunk being managed by this host here rather than in the methods being called.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"10d750df9aaa51e4a0f4696a618ba14118f67dfa","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            LOG.error(_LE(\"No subports received in notification: %s\"),"},{"line_number":53,"context_line":"                      subports)"},{"line_number":54,"context_line":"            return"},{"line_number":55,"context_line":"        if event_type \u003d\u003d events.CREATED:"},{"line_number":56,"context_line":"            ctx \u003d self.ovsdb_handler.context"},{"line_number":57,"context_line":"            self.ovsdb_handler.wire_subports_for_trunk(ctx, trunk_id, subports)"},{"line_number":58,"context_line":"        elif event_type \u003d\u003d events.DELETED:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_0e55001c","line":55,"in_reply_to":"9a89bdaa_74102002","updated":"2016-09-06 18:09:28.000000000","message":"I\u0027ll add it here once I\u0027m done with the base.","commit_id":"9240ee2440ad2ea30224f2aeb1b8f57c1830a351"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"912c95d3d9f9ece26e575a0e09d4f9a4eed19504","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        \"\"\""},{"line_number":44,"context_line":"        raise NotImplementedError()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":47,"context_line":"        # Subports are always created with the same trunk_id and there is"},{"line_number":48,"context_line":"        # always at least one item in subports list"},{"line_number":49,"context_line":"        trunk_id \u003d subports[0].trunk_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_d6d5afb9","line":46,"updated":"2016-09-09 16:52:56.000000000","message":"I tried this out and got the following stacktrace [1]. My steps to repro were:\n\n1) create trunk (no subports)\n2) boot a VM\n3) add a subport\n4) error below.\n\n[1] http://paste.openstack.org/show/570278/","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1529ba04dd1e79589390d966f197df725be21a77","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        \"\"\""},{"line_number":44,"context_line":"        raise NotImplementedError()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":47,"context_line":"        # Subports are always created with the same trunk_id and there is"},{"line_number":48,"context_line":"        # always at least one item in subports list"},{"line_number":49,"context_line":"        trunk_id \u003d subports[0].trunk_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_3fe11e9b","line":46,"in_reply_to":"9a89bdaa_592c70ab","updated":"2016-09-12 08:55:49.000000000","message":"/me digs his head in sand","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"eae796efe5ecd8e69cf252b0b603e97387ba63eb","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        \"\"\""},{"line_number":44,"context_line":"        raise NotImplementedError()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":47,"context_line":"        # Subports are always created with the same trunk_id and there is"},{"line_number":48,"context_line":"        # always at least one item in subports list"},{"line_number":49,"context_line":"        trunk_id \u003d subports[0].trunk_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_592c70ab","line":46,"in_reply_to":"9a89bdaa_79cf74b3","updated":"2016-09-09 17:39:39.000000000","message":"got it:\n\nhttps://review.openstack.org/#/c/368162/","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"0c95c32588e8e38f4f2a9f3a75c8e6e2dc41b725","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        \"\"\""},{"line_number":44,"context_line":"        raise NotImplementedError()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":47,"context_line":"        # Subports are always created with the same trunk_id and there is"},{"line_number":48,"context_line":"        # always at least one item in subports list"},{"line_number":49,"context_line":"        trunk_id \u003d subports[0].trunk_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_79cf74b3","line":46,"in_reply_to":"9a89bdaa_d6d5afb9","updated":"2016-09-09 16:57:47.000000000","message":"I suppose this means that the metadata were not set correctly. Not sure why yet.","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1529ba04dd1e79589390d966f197df725be21a77","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            LOG.debug(\"Event %s for subports: %s\", event_type, subports)"},{"line_number":53,"context_line":"            if event_type \u003d\u003d events.CREATED:"},{"line_number":54,"context_line":"                ctx \u003d self.ovsdb_handler.context"},{"line_number":55,"context_line":"                self.ovsdb_handler.wire_subports_for_trunk("},{"line_number":56,"context_line":"                    ctx, trunk_id, subports)"},{"line_number":57,"context_line":"            elif event_type \u003d\u003d events.DELETED:"},{"line_number":58,"context_line":"                subport_ids \u003d [subport.port_id for subport in subports]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_5f4a6284","line":55,"range":{"start_line":55,"start_character":35,"end_line":55,"end_character":58},"updated":"2016-09-12 08:55:49.000000000","message":"We should catch messaging errors here, I\u0027ll send a followup patch.","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"d6e55a6dbd35d1d0c3e5e4c85700f05f27862244","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                    ctx, trunk_id, subports)"},{"line_number":57,"context_line":"            elif event_type \u003d\u003d events.DELETED:"},{"line_number":58,"context_line":"                subport_ids \u003d [subport.port_id for subport in subports]"},{"line_number":59,"context_line":"                self.ovsdb_handler.unwire_subports_for_trunk("},{"line_number":60,"context_line":"                    trunk_id, subport_ids)"},{"line_number":61,"context_line":"            else:"},{"line_number":62,"context_line":"                LOG.error(_LE(\"Unknown event type %s\"), event_type)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_53061621","line":59,"updated":"2016-09-09 15:26:09.000000000","message":"I noticed that the subport parent port does not have its binding cleared when removed from the trunk. This blocks the port from being re-used as a subport after it is removed from the trunk. Is it possible to clear the subport binding inside of unwire_subports_for_trunk()?","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"92c0e8b4689187ffbfe27e6b4020ea49be371cc2","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                    ctx, trunk_id, subports)"},{"line_number":57,"context_line":"            elif event_type \u003d\u003d events.DELETED:"},{"line_number":58,"context_line":"                subport_ids \u003d [subport.port_id for subport in subports]"},{"line_number":59,"context_line":"                self.ovsdb_handler.unwire_subports_for_trunk("},{"line_number":60,"context_line":"                    trunk_id, subport_ids)"},{"line_number":61,"context_line":"            else:"},{"line_number":62,"context_line":"                LOG.error(_LE(\"Unknown event type %s\"), event_type)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_fcf8d7c6","line":59,"in_reply_to":"9a89bdaa_53061621","updated":"2016-09-09 16:01:38.000000000","message":"No, it does not (at least not in my experiments). Do you have other evidence?\n\nThe subport bindings are not cleared by the agent to avoid races between two agents in case the trunk is being moved. A subport can be removed/added to a different trunk and we don\u0027t look at its bindings. However, what we could add right now is that we clear the binding during a remove operation, but I don\u0027t believe it\u0027s necessary, because as soon as the subport gets added its bindings get updated by the new agent that takes over.","commit_id":"1087be35223d36ecda47162e85889ffe102666be"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"803bcd5c4419be524e7759a709f71d5bef155bd4","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                    ctx, trunk_id, subports)"},{"line_number":57,"context_line":"            elif event_type \u003d\u003d events.DELETED:"},{"line_number":58,"context_line":"                subport_ids \u003d [subport.port_id for subport in subports]"},{"line_number":59,"context_line":"                self.ovsdb_handler.unwire_subports_for_trunk("},{"line_number":60,"context_line":"                    trunk_id, subport_ids)"},{"line_number":61,"context_line":"            else:"},{"line_number":62,"context_line":"                LOG.error(_LE(\"Unknown event type %s\"), event_type)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a89bdaa_962a477d","line":59,"in_reply_to":"9a89bdaa_fcf8d7c6","updated":"2016-09-09 16:31:21.000000000","message":"I think the problem you\u0027re hinting is that on re-add, so, yeah clearing the subport bindings on API removal is needed.","commit_id":"1087be35223d36ecda47162e85889ffe102666be"}],"neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b634535aef12a03030bfed5b631009ce09147c94","unresolved":false,"context_lines":[{"line_number":227,"context_line":"                       \u0027error\u0027: str(e)})"},{"line_number":228,"context_line":"            return"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        trunk_bridge \u003d trunk_bridge or ovs_lib.OVSBridge("},{"line_number":231,"context_line":"            trunk_utils.gen_trunk_br_name(trunk_id))"},{"line_number":232,"context_line":"        parent_port \u003d parent_port or self._get_parent_port(trunk_bridge)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        self._set_trunk_metadata("},{"line_number":235,"context_line":"            trunk_bridge, parent_port, trunk_id, subport_ids)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a89bdaa_1103940e","line":232,"range":{"start_line":230,"start_character":0,"end_line":232,"end_character":72},"updated":"2016-09-05 21:14:07.000000000","message":"Whoops, that was supposed to be a fixup for previous patch.","commit_id":"dcb1ae52eef791c9152276cdeb65468a80e29108"}]}