)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"79d421d46602e1148f773f6ac59aa250501dbdad","unresolved":false,"context_lines":[{"line_number":51,"context_line":"- Subnet create changes (TBD soon)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Change-Id: I150da5938e79eeef0c947ddb1a4282e37d0515ee"},{"line_number":54,"context_line":"Partially-implements: blueprint multiple-ipv6-prefixes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da86d52c_49f02f91","line":54,"updated":"2015-02-04 22:46:49.000000000","message":"As a side-effect:\n\nPartial-Bug: 1357068","commit_id":"5a7f0f7c4498304a18c051f0fe40b2a73305bdb4"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"75f69aad3815682a447e03ea3d3469edbf4bb247","unresolved":false,"context_lines":[{"line_number":51,"context_line":"- Subnet create changes (TBD soon)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"Change-Id: I150da5938e79eeef0c947ddb1a4282e37d0515ee"},{"line_number":54,"context_line":"Partially-implements: blueprint multiple-ipv6-prefixes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da86d52c_75af337e","line":54,"in_reply_to":"da86d52c_49f02f91","updated":"2015-02-06 21:32:17.000000000","message":"Done","commit_id":"5a7f0f7c4498304a18c051f0fe40b2a73305bdb4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":12,"context_line":"routers. Some background on the changes included in this patchset:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"- The L3 driver\u0027s init_l3() method has been changed to accept a list"},{"line_number":15,"context_line":"  of gateway IPs, rather than a single gateway IP. This change"},{"line_number":16,"context_line":"  affects DVR, DHCP, and HA code since these features also call"},{"line_number":17,"context_line":"  the L3 driver\u0027s init_l3 method."},{"line_number":18,"context_line":"- The Neutron port dictionary\u0027s singular \u0027subnet\u0027 entry has been"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a80dd14_c8c89daa","line":15,"updated":"2015-03-08 02:56:06.000000000","message":"A list doesn\u0027t feel quite right.  Only one per address family is allowed, right?  So, wouldn\u0027t a dict indexed by family be more appropriate?","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":12,"context_line":"routers. Some background on the changes included in this patchset:"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"- The L3 driver\u0027s init_l3() method has been changed to accept a list"},{"line_number":15,"context_line":"  of gateway IPs, rather than a single gateway IP. This change"},{"line_number":16,"context_line":"  affects DVR, DHCP, and HA code since these features also call"},{"line_number":17,"context_line":"  the L3 driver\u0027s init_l3 method."},{"line_number":18,"context_line":"- The Neutron port dictionary\u0027s singular \u0027subnet\u0027 entry has been"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a80dd14_855ac3ef","line":15,"in_reply_to":"9a80dd14_c8c89daa","updated":"2015-03-10 18:28:08.000000000","message":"The blueprint design spec was written that way, saying that we would impose a restriction on gateway addresses of one address per family. However, in a review comment, baoli suggested that there\u0027s really no reason to impose the one address-per-family restriction for IPv6 addresses on gateway ports, and there may be a reason why an operator might want multiple IPv6 addresses. This would be simpler in that we wouldn\u0027t have to add any new checks on the number of addresses being used on a gateway port, beyond the (general) restrictions that are place when a Neutron port is created. If this is acceptable, then I can propose changing this in the blueprint design spec.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":16,"context_line":"  affects DVR, DHCP, and HA code since these features also call"},{"line_number":17,"context_line":"  the L3 driver\u0027s init_l3 method."},{"line_number":18,"context_line":"- The Neutron port dictionary\u0027s singular \u0027subnet\u0027 entry has been"},{"line_number":19,"context_line":"  replaced with a \u0027subnets\u0027 list, since ports can now be associated"},{"line_number":20,"context_line":"  with multiple subnets."},{"line_number":21,"context_line":"- The Neutron port dictionary no longer has a (singular) \u0027ip_cidr\u0027"},{"line_number":22,"context_line":"  entry, since a port can now be associated with multiple IP CIDRs"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a80dd14_e8cd61b8","line":19,"updated":"2015-03-08 02:56:06.000000000","message":"ditto.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":16,"context_line":"  affects DVR, DHCP, and HA code since these features also call"},{"line_number":17,"context_line":"  the L3 driver\u0027s init_l3 method."},{"line_number":18,"context_line":"- The Neutron port dictionary\u0027s singular \u0027subnet\u0027 entry has been"},{"line_number":19,"context_line":"  replaced with a \u0027subnets\u0027 list, since ports can now be associated"},{"line_number":20,"context_line":"  with multiple subnets."},{"line_number":21,"context_line":"- The Neutron port dictionary no longer has a (singular) \u0027ip_cidr\u0027"},{"line_number":22,"context_line":"  entry, since a port can now be associated with multiple IP CIDRs"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a80dd14_a5d34702","line":19,"in_reply_to":"9a80dd14_e8cd61b8","updated":"2015-03-10 18:28:08.000000000","message":"See response above. I don\u0027t know if you\u0027d want to combine the list of subnets and the list of gateway_ips into a combined list (of dictionaries), but this could be done as a followup change.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":23,"context_line":"  (e.g. up to one IP CIDR per IP family on gateway ports)."},{"line_number":24,"context_line":"  Instead, a \u0027prefixlen\u0027 entry has been added to the Neutron"},{"line_number":25,"context_line":"  fixed_ips dictionary, so that the port\u0027s (multiple) IP CIDRs can"},{"line_number":26,"context_line":"  be derived from the matching \u0027ip_addess\u0027 and \u0027prefixlen\u0027 pairs"},{"line_number":27,"context_line":"  in the port\u0027s fixed_ips."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"This patchset does not include the following work items for the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a80dd14_28ae99e6","line":26,"updated":"2015-03-08 02:56:06.000000000","message":"*ip_address","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":23,"context_line":"  (e.g. up to one IP CIDR per IP family on gateway ports)."},{"line_number":24,"context_line":"  Instead, a \u0027prefixlen\u0027 entry has been added to the Neutron"},{"line_number":25,"context_line":"  fixed_ips dictionary, so that the port\u0027s (multiple) IP CIDRs can"},{"line_number":26,"context_line":"  be derived from the matching \u0027ip_addess\u0027 and \u0027prefixlen\u0027 pairs"},{"line_number":27,"context_line":"  in the port\u0027s fixed_ips."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"This patchset does not include the following work items for the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9a80dd14_a556e759","line":26,"in_reply_to":"9a80dd14_28ae99e6","updated":"2015-03-10 18:28:08.000000000","message":"Done","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     41486020 (Merge \"Imported Translations from Transifex\")"},{"line_number":2,"context_line":"Author:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2015-03-18 02:17:17 -0400"},{"line_number":4,"context_line":"Commit:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-03-18 02:17:17 -0400"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"9a80dd14_fa59da35","line":2,"updated":"2015-03-18 19:48:31.000000000","message":"Why the sudden change in author?","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e251c5bcd099f4acc24d6b2e0f945375735fb7b0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     41486020 (Merge \"Imported Translations from Transifex\")"},{"line_number":2,"context_line":"Author:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2015-03-18 02:17:17 -0400"},{"line_number":4,"context_line":"Commit:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-03-18 02:17:17 -0400"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"9a80dd14_f5e9edb7","line":2,"in_reply_to":"9a80dd14_b5d5b51c","updated":"2015-03-18 20:23:36.000000000","message":"Your git client is mis-configured.  Normally, git will not reset the author without an explicit argument to do so.  It will reset the committer but not the author which is appropriate.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     41486020 (Merge \"Imported Translations from Transifex\")"},{"line_number":2,"context_line":"Author:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2015-03-18 02:17:17 -0400"},{"line_number":4,"context_line":"Commit:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-03-18 02:17:17 -0400"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"9a80dd14_7000ab02","line":2,"in_reply_to":"9a80dd14_fa59da35","updated":"2015-03-19 00:43:48.000000000","message":"Fixed. Will re-post.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6620,"name":"Abishek Subramanian","email":"absubram@cisco.com","username":"absubram"},"change_message_id":"e2a8c63e2c40f5aa32081748bd924226f9355b79","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     41486020 (Merge \"Imported Translations from Transifex\")"},{"line_number":2,"context_line":"Author:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2015-03-18 02:17:17 -0400"},{"line_number":4,"context_line":"Commit:     Abishek Subramanian \u003cabsubram@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-03-18 02:17:17 -0400"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"9a80dd14_b5d5b51c","line":2,"in_reply_to":"9a80dd14_fa59da35","updated":"2015-03-18 20:08:08.000000000","message":"that\u0027s my fault sorry :(\nI saw late last night that it needed a rebase so I did.\nMy bp is marked as dependent on this :p","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"}],"neutron/agent/l3/agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":648,"context_line":"                extra_subnets\u003dex_gw_port.get(\u0027extra_subnets\u0027, []),"},{"line_number":649,"context_line":"                preserve_ips\u003dpreserve_ips)"},{"line_number":650,"context_line":"            for fixed_ip in ex_gw_port[\u0027fixed_ips\u0027]:"},{"line_number":651,"context_line":"                ip_lib.send_gratuitous_arp(ns_name,"},{"line_number":652,"context_line":"                                           interface_name,"},{"line_number":653,"context_line":"                                           fixed_ip[\u0027ip_address\u0027],"},{"line_number":654,"context_line":"                                           self.conf.send_arp_for_ha)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_2895b986","line":651,"updated":"2015-03-08 02:56:06.000000000","message":"Thinking in terms of ipv4 vs ipv6, I don\u0027t think I like calling send_gratuitous_arp with ipv6 even though I know it checks the version internally.  I think this code should know better at this level.  What do you think?\n\nI tend to prefer that code not blindly ask for things that don\u0027t make sense even though the layer below will ignore it.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":648,"context_line":"                extra_subnets\u003dex_gw_port.get(\u0027extra_subnets\u0027, []),"},{"line_number":649,"context_line":"                preserve_ips\u003dpreserve_ips)"},{"line_number":650,"context_line":"            for fixed_ip in ex_gw_port[\u0027fixed_ips\u0027]:"},{"line_number":651,"context_line":"                ip_lib.send_gratuitous_arp(ns_name,"},{"line_number":652,"context_line":"                                           interface_name,"},{"line_number":653,"context_line":"                                           fixed_ip[\u0027ip_address\u0027],"},{"line_number":654,"context_line":"                                           self.conf.send_arp_for_ha)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_a0d33502","line":651,"in_reply_to":"9a80dd14_2895b986","updated":"2015-03-10 18:28:08.000000000","message":"I agree. I think it\u0027s less confusing and misleading to have the caller check in this case (there are 3 instances where checks are required). There was some back-and-forth discussion on this in this review: https://review.openstack.org/#/c/160799, maybe you can voice your opinion on that review?\nI should probably remove the check for IPv4 from this patch, and leave this issue to #160799.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":730,"context_line":"        if ri.is_ha:"},{"line_number":731,"context_line":"            ri._ha_disable_addressing_on_interface(interface_name)"},{"line_number":732,"context_line":"            for fixed_ip in fixed_ips:"},{"line_number":733,"context_line":"                ip_cidr \u003d \"%s/%s\" % (fixed_ip[\u0027ip_address\u0027],"},{"line_number":734,"context_line":"                                     fixed_ip[\u0027prefixlen\u0027])"},{"line_number":735,"context_line":"                ri._add_vip(ip_cidr, interface_name)"},{"line_number":736,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_c8f17d02","line":733,"updated":"2015-03-08 02:56:06.000000000","message":"I think this is the third time I\u0027ve seen this snippet of code in this file.  Do you think it is time to add a utility?","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":730,"context_line":"        if ri.is_ha:"},{"line_number":731,"context_line":"            ri._ha_disable_addressing_on_interface(interface_name)"},{"line_number":732,"context_line":"            for fixed_ip in fixed_ips:"},{"line_number":733,"context_line":"                ip_cidr \u003d \"%s/%s\" % (fixed_ip[\u0027ip_address\u0027],"},{"line_number":734,"context_line":"                                     fixed_ip[\u0027prefixlen\u0027])"},{"line_number":735,"context_line":"                ri._add_vip(ip_cidr, interface_name)"},{"line_number":736,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_c7ef5ee8","line":733,"in_reply_to":"9a80dd14_c8f17d02","updated":"2015-03-10 18:28:08.000000000","message":"Done","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                port2_filtered \u003d _get_filtered_dict(port2, keys_to_ignore)"},{"line_number":342,"context_line":"                return port1_filtered \u003d\u003d port2_filtered"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"            ri._set_subnet_info(ex_gw_port)"},{"line_number":345,"context_line":"            if not ri.ex_gw_port:"},{"line_number":346,"context_line":"                self.external_gateway_added(ri, ex_gw_port, interface_name)"},{"line_number":347,"context_line":"            elif not _gateway_ports_equal(ex_gw_port, ri.ex_gw_port):"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_4639393e","side":"PARENT","line":344,"updated":"2015-03-18 19:48:31.000000000","message":"Note:  I\u0027m actually pretty happy to see this method go.","commit_id":"41486020f6886b1a0f6601fd063b2e3a54b16c76"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                extra_subnets\u003dex_gw_port.get(\u0027extra_subnets\u0027, []),"},{"line_number":587,"context_line":"                preserve_ips\u003dpreserve_ips)"},{"line_number":588,"context_line":"            for fixed_ip in ex_gw_port[\u0027fixed_ips\u0027]:"},{"line_number":589,"context_line":"                ip_lib.send_gratuitous_arp(ns_name,"},{"line_number":590,"context_line":"                                           interface_name,"},{"line_number":591,"context_line":"                                           fixed_ip[\u0027ip_address\u0027],"},{"line_number":592,"context_line":"                                           self.conf.send_arp_for_ha)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_267605d9","line":589,"updated":"2015-03-18 19:48:31.000000000","message":"I don\u0027t think that this should be calling gratuitous arp for ipv6 addresses.  I know that the method skips ipv6 addresses internally but that is confusing to me.  When I see this, I think \"oh, we shouldn\u0027t be sending garp for ipv6\" and then I need to reassure myself that we aren\u0027t.  My $0.02.\n\nThis isn\u0027t a blocker and could be addressed in a different patch.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":586,"context_line":"                extra_subnets\u003dex_gw_port.get(\u0027extra_subnets\u0027, []),"},{"line_number":587,"context_line":"                preserve_ips\u003dpreserve_ips)"},{"line_number":588,"context_line":"            for fixed_ip in ex_gw_port[\u0027fixed_ips\u0027]:"},{"line_number":589,"context_line":"                ip_lib.send_gratuitous_arp(ns_name,"},{"line_number":590,"context_line":"                                           interface_name,"},{"line_number":591,"context_line":"                                           fixed_ip[\u0027ip_address\u0027],"},{"line_number":592,"context_line":"                                           self.conf.send_arp_for_ha)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_3ac1ad80","line":589,"in_reply_to":"9a80dd14_267605d9","updated":"2015-03-19 00:43:48.000000000","message":"I agree. I was expecting that we can revisit this in https://review.openstack.org/#/c/160799.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":603,"context_line":"            for p in ri.internal_ports:"},{"line_number":604,"context_line":"                gateway \u003d self._map_internal_interfaces(ri, p, snat_ports)"},{"line_number":605,"context_line":"                internal_interface \u003d ri.get_internal_device_name(p[\u0027id\u0027])"},{"line_number":606,"context_line":"                ri.snat_redirects_remove(gateway, p, internal_interface)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"            if (self.conf.agent_mode \u003d\u003d l3_constants.L3_AGENT_MODE_DVR_SNAT"},{"line_number":609,"context_line":"                and ri.get_gw_port_host() \u003d\u003d self.host):"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_c64c49a0","line":606,"updated":"2015-03-18 19:48:31.000000000","message":"nit:  Why change this method name?  Splitting hairs if you ask me.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":603,"context_line":"            for p in ri.internal_ports:"},{"line_number":604,"context_line":"                gateway \u003d self._map_internal_interfaces(ri, p, snat_ports)"},{"line_number":605,"context_line":"                internal_interface \u003d ri.get_internal_device_name(p[\u0027id\u0027])"},{"line_number":606,"context_line":"                ri.snat_redirects_remove(gateway, p, internal_interface)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"            if (self.conf.agent_mode \u003d\u003d l3_constants.L3_AGENT_MODE_DVR_SNAT"},{"line_number":609,"context_line":"                and ri.get_gw_port_host() \u003d\u003d self.host):"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_7aea45d9","line":606,"in_reply_to":"9a80dd14_c64c49a0","updated":"2015-03-19 00:43:48.000000000","message":"Reverted in followup patch (https://review.openstack.org/#/c/165664)","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"}],"neutron/agent/l3/dvr.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7e5658acc94bd21c1ab1060f6be6ed6fa8257ca9","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            subnet_cidr \u003d subnet[\u0027cidr\u0027]"},{"line_number":83,"context_line":"            ip_vers \u003d netaddr.IPNetwork(subnet_cidr).version"},{"line_number":84,"context_line":"            if ip_vers !\u003d 4:"},{"line_number":85,"context_line":"                continue"},{"line_number":86,"context_line":"            subnet_id \u003d subnet[\u0027id\u0027]"},{"line_number":87,"context_line":"            subnet_ports \u003d ("},{"line_number":88,"context_line":"                self.plugin_rpc.get_ports_by_subnet(self.context,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_9301886c","line":85,"updated":"2015-03-06 22:20:24.000000000","message":"ip_vers doesn\u0027t seem to be used after this, can simplify this to:\n\nif netaddr.IPNetwork(subnet_cidr).version !\u003d 4:","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            subnet_cidr \u003d subnet[\u0027cidr\u0027]"},{"line_number":83,"context_line":"            ip_vers \u003d netaddr.IPNetwork(subnet_cidr).version"},{"line_number":84,"context_line":"            if ip_vers !\u003d 4:"},{"line_number":85,"context_line":"                continue"},{"line_number":86,"context_line":"            subnet_id \u003d subnet[\u0027id\u0027]"},{"line_number":87,"context_line":"            subnet_ports \u003d ("},{"line_number":88,"context_line":"                self.plugin_rpc.get_ports_by_subnet(self.context,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_5ce47de0","line":85,"in_reply_to":"9a80dd14_9301886c","updated":"2015-03-06 23:30:31.000000000","message":"Done","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"b7a6696244848a2f5e956547fb0db0f4e0fc26bc","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _snat_redirect_add(self, ri, gateway, sn_port, sn_int):"},{"line_number":136,"context_line":"        \"\"\"Adds rules and routes for SNAT redirection.\"\"\""},{"line_number":137,"context_line":"        for ip_cidr in utils.fixed_ip_cidrs(sn_port[\u0027fixed_ips\u0027]):"},{"line_number":138,"context_line":"            try:"},{"line_number":139,"context_line":"                snat_idx \u003d self._get_snat_idx(ip_cidr)"},{"line_number":140,"context_line":"                ns_ipr \u003d ip_lib.IPRule(namespace\u003dri.ns_name)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_bf1d5964","line":137,"updated":"2015-03-13 18:28:53.000000000","message":"fixed_ips will include v6 addresses, I think, in a dual stack or v6 only setup. In addition the caller passes the gw port\u0027s fixedip[0], which is not appropriate in a dual stack case.\n\nshould find out if DVR is applicable to v4 only. If it is, then check can be made at the caller to make sure the call is made for v4 only","commit_id":"bc3d6014600b05aa6e8980e3a69530a93e2d65be"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"8d38db7ed24b028bd9ab5d1d8282dd8af2e2da36","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def _snat_redirect_add(self, ri, gateway, sn_port, sn_int):"},{"line_number":136,"context_line":"        \"\"\"Adds rules and routes for SNAT redirection.\"\"\""},{"line_number":137,"context_line":"        for ip_cidr in utils.fixed_ip_cidrs(sn_port[\u0027fixed_ips\u0027]):"},{"line_number":138,"context_line":"            try:"},{"line_number":139,"context_line":"                snat_idx \u003d self._get_snat_idx(ip_cidr)"},{"line_number":140,"context_line":"                ns_ipr \u003d ip_lib.IPRule(namespace\u003dri.ns_name)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_0850de09","line":137,"in_reply_to":"9a80dd14_bf1d5964","updated":"2015-03-14 00:22:14.000000000","message":"Based on our discussion and your recommendation, I\u0027ll need to add some logic to handle the dual stack case, e.g. redirect the v4 addresses to a v4 gateway, and v6 addresses to a v6 gateway. I\u0027ll do this in a followup commit.","commit_id":"bc3d6014600b05aa6e8980e3a69530a93e2d65be"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"b7a6696244848a2f5e956547fb0db0f4e0fc26bc","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                ns_ipd.route.add_gateway(gateway, table\u003dsnat_idx)"},{"line_number":144,"context_line":"                ns_ipr.rule.add(ip_cidr, snat_idx, snat_idx)"},{"line_number":145,"context_line":"                ns_ipwrapr.netns.execute([\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.\u0027"},{"line_number":146,"context_line":"                                          \u0027send_redirects\u003d0\u0027 % sn_int])"},{"line_number":147,"context_line":"            except Exception:"},{"line_number":148,"context_line":"                LOG.exception(_LE(\u0027DVR: error adding redirection logic\u0027))"},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_ff3a71cb","line":146,"updated":"2015-03-13 18:28:53.000000000","message":"this will be wrong with a v6 cidr.","commit_id":"bc3d6014600b05aa6e8980e3a69530a93e2d65be"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"b7a6696244848a2f5e956547fb0db0f4e0fc26bc","unresolved":false,"context_lines":[{"line_number":149,"context_line":""},{"line_number":150,"context_line":"    def _snat_redirect_remove(self, ri, gateway, sn_port, sn_int):"},{"line_number":151,"context_line":"        \"\"\"Removes rules and routes for SNAT redirection.\"\"\""},{"line_number":152,"context_line":"        for ip_cidr in utils.fixed_ip_cidrs(sn_port[\u0027fixed_ips\u0027]):"},{"line_number":153,"context_line":"            try:"},{"line_number":154,"context_line":"                snat_idx \u003d self._get_snat_idx(ip_cidr)"},{"line_number":155,"context_line":"                ns_ipr \u003d ip_lib.IPRule(namespace\u003dri.ns_name)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_5f2e1d8d","line":152,"updated":"2015-03-13 18:28:53.000000000","message":"ditto","commit_id":"bc3d6014600b05aa6e8980e3a69530a93e2d65be"}],"neutron/agent/l3/dvr_fip_ns.py":[{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"56d7736fec97bc2d9d38356370c6f63529d7fe6f","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                                 fixed_ip[\u0027prefixlen\u0027])"},{"line_number":100,"context_line":"            ip_addrs.append({\u0027cidr\u0027: ip_cidr})"},{"line_number":101,"context_line":"        self.driver.init_l3(interface_name, ip_addrs,"},{"line_number":102,"context_line":"                            namespace\u003dns_name)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        for ip_addr in ip_addrs:"},{"line_number":105,"context_line":"            ip_cidr \u003d ip_addr[\u0027cidr\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_373a04ce","line":102,"updated":"2015-03-04 20:24:11.000000000","message":"Shouldn\u0027t is_ext_gateway\u003dTrue be passed in here? I know it has no effect for IPv4, but would be nice to keep some consistency in the args.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                                 fixed_ip[\u0027prefixlen\u0027])"},{"line_number":100,"context_line":"            ip_addrs.append({\u0027cidr\u0027: ip_cidr})"},{"line_number":101,"context_line":"        self.driver.init_l3(interface_name, ip_addrs,"},{"line_number":102,"context_line":"                            namespace\u003dns_name)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        for ip_addr in ip_addrs:"},{"line_number":105,"context_line":"            ip_cidr \u003d ip_addr[\u0027cidr\u0027]"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_7656d92b","line":102,"in_reply_to":"9a80dd14_373a04ce","updated":"2015-03-06 23:30:31.000000000","message":"Yes, good catch. Though the is_ext_gateway will be getting removed in the next patch.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7e5658acc94bd21c1ab1060f6be6ed6fa8257ca9","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        for ip_addr in ip_addrs:"},{"line_number":105,"context_line":"            ip_cidr \u003d ip_addr[\u0027cidr\u0027]"},{"line_number":106,"context_line":"            ip_address \u003d ip_cidr.split(\u0027/\u0027)[0]"},{"line_number":107,"context_line":"            ip_lib.send_gratuitous_arp(ns_name,"},{"line_number":108,"context_line":"                                       interface_name,"},{"line_number":109,"context_line":"                                       ip_address,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_d6384ed5","line":106,"updated":"2015-03-06 22:20:24.000000000","message":"So we built a list of cidrs above to only strip it off here :(  You might as well just use \u0027for fixed_ip in ex_gw_port[\u0027fixed_ips\u0027]:\u0027 here too.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        for ip_addr in ip_addrs:"},{"line_number":105,"context_line":"            ip_cidr \u003d ip_addr[\u0027cidr\u0027]"},{"line_number":106,"context_line":"            ip_address \u003d ip_cidr.split(\u0027/\u0027)[0]"},{"line_number":107,"context_line":"            ip_lib.send_gratuitous_arp(ns_name,"},{"line_number":108,"context_line":"                                       interface_name,"},{"line_number":109,"context_line":"                                       ip_address,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_bc557951","line":106,"in_reply_to":"9a80dd14_d6384ed5","updated":"2015-03-06 23:30:31.000000000","message":"Done","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"}],"neutron/agent/l3/dvr_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":197,"context_line":"                LOG.exception(_LE(\"DVR: Failed updating arp entry\"))"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _set_subnet_arp_info(self, port):"},{"line_number":200,"context_line":"        \"\"\"Set ARP info retrieved from Plugin for existing ports.\"\"\""},{"line_number":201,"context_line":"        for subnet in port[\u0027subnets\u0027]:"},{"line_number":202,"context_line":"            if (\u0027id\u0027 not in subnet or"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_08c6359a","line":199,"updated":"2015-03-08 02:56:06.000000000","message":"No arp in ipv6.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":197,"context_line":"                LOG.exception(_LE(\"DVR: Failed updating arp entry\"))"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _set_subnet_arp_info(self, port):"},{"line_number":200,"context_line":"        \"\"\"Set ARP info retrieved from Plugin for existing ports.\"\"\""},{"line_number":201,"context_line":"        for subnet in port[\u0027subnets\u0027]:"},{"line_number":202,"context_line":"            if (\u0027id\u0027 not in subnet or"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_f01ed1ba","line":199,"in_reply_to":"9a80dd14_08c6359a","updated":"2015-03-10 18:28:08.000000000","message":"I\u0027ve already added a check for IPv4 (line 203). Maybe the best way to handle this would be to move the check for IPv4 to the calling method (which would require calling this method on a per-subnet basis). To minimize collateral changes, I can make the change in this patch, or add this change to the arping bug (https://review.openstack.org/#/c/160799), or add a new bug. Any preference?","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6620,"name":"Abishek Subramanian","email":"absubram@cisco.com","username":"absubram"},"change_message_id":"1752d1e7d941f33942e344a699657bc9ec87e83e","unresolved":false,"context_lines":[{"line_number":325,"context_line":"            interface_name,"},{"line_number":326,"context_line":"            dvr_snat_ns.SNAT_INT_DEV_PREFIX)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        self._set_subnet_arp_info(port)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def _dvr_internal_network_removed(self, port):"},{"line_number":331,"context_line":"        if not self.ex_gw_port:"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a80dd14_714bb15f","line":328,"updated":"2015-03-17 06:30:32.000000000","message":"should this not be subnet_id instead of port, based on the change above?","commit_id":"3a016e8cfde03282f3a0d7e8e089cc80df4fa9df"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a163db92080b8921c47101e8f56801db9c537897","unresolved":false,"context_lines":[{"line_number":325,"context_line":"            interface_name,"},{"line_number":326,"context_line":"            dvr_snat_ns.SNAT_INT_DEV_PREFIX)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        self._set_subnet_arp_info(port)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def _dvr_internal_network_removed(self, port):"},{"line_number":331,"context_line":"        if not self.ex_gw_port:"}],"source_content_type":"text/x-python","patch_set":25,"id":"9a80dd14_fd3f42b3","line":328,"in_reply_to":"9a80dd14_714bb15f","updated":"2015-03-18 00:24:42.000000000","message":"Done","commit_id":"3a016e8cfde03282f3a0d7e8e089cc80df4fa9df"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":252,"context_line":"            snat_idx \u003d net.value"},{"line_number":253,"context_line":"        return snat_idx"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def snat_redirects_add(self, gateway, sn_port, sn_int):"},{"line_number":256,"context_line":"        \"\"\"Adds rules and routes for SNAT redirections for a port.\"\"\""},{"line_number":257,"context_line":"        for port_fixed_ip in sn_port[\u0027fixed_ips\u0027]:"},{"line_number":258,"context_line":"            # Find the first gateway IP address matching this IP version"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_a92ac6e4","line":255,"updated":"2015-03-18 19:48:31.000000000","message":"nit:  I think the rename of this method to add a pretty subtle \u0027s\u0027 just decreases the SNR of the patch.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":252,"context_line":"            snat_idx \u003d net.value"},{"line_number":253,"context_line":"        return snat_idx"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def snat_redirects_add(self, gateway, sn_port, sn_int):"},{"line_number":256,"context_line":"        \"\"\"Adds rules and routes for SNAT redirections for a port.\"\"\""},{"line_number":257,"context_line":"        for port_fixed_ip in sn_port[\u0027fixed_ips\u0027]:"},{"line_number":258,"context_line":"            # Find the first gateway IP address matching this IP version"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_7ac1654d","line":255,"in_reply_to":"9a80dd14_a92ac6e4","updated":"2015-03-19 00:43:48.000000000","message":"Reverted in followup patch based on previous comment in agent.py.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":255,"context_line":"    def snat_redirects_add(self, gateway, sn_port, sn_int):"},{"line_number":256,"context_line":"        \"\"\"Adds rules and routes for SNAT redirections for a port.\"\"\""},{"line_number":257,"context_line":"        for port_fixed_ip in sn_port[\u0027fixed_ips\u0027]:"},{"line_number":258,"context_line":"            # Find the first gateway IP address matching this IP version"},{"line_number":259,"context_line":"            port_ip_addr \u003d port_fixed_ip[\u0027ip_address\u0027]"},{"line_number":260,"context_line":"            port_ip_vers \u003d netaddr.IPAddress(port_ip_addr).version"},{"line_number":261,"context_line":"            for gw_fixed_ip in gateway[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_4976aa40","line":258,"updated":"2015-03-18 19:48:31.000000000","message":"Does this imply that the loop at L261 should terminate after the body of the if statement on L263 is executed the first time?","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":255,"context_line":"    def snat_redirects_add(self, gateway, sn_port, sn_int):"},{"line_number":256,"context_line":"        \"\"\"Adds rules and routes for SNAT redirections for a port.\"\"\""},{"line_number":257,"context_line":"        for port_fixed_ip in sn_port[\u0027fixed_ips\u0027]:"},{"line_number":258,"context_line":"            # Find the first gateway IP address matching this IP version"},{"line_number":259,"context_line":"            port_ip_addr \u003d port_fixed_ip[\u0027ip_address\u0027]"},{"line_number":260,"context_line":"            port_ip_vers \u003d netaddr.IPAddress(port_ip_addr).version"},{"line_number":261,"context_line":"            for gw_fixed_ip in gateway[\u0027fixed_ips\u0027]:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_ccf37b71","line":258,"in_reply_to":"9a80dd14_4976aa40","updated":"2015-03-19 00:43:48.000000000","message":"Yes, this is missing a break. I\u0027ve added this in a followup commit: https://review.openstack.org/#/c/165664.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":261,"context_line":"            for gw_fixed_ip in gateway[\u0027fixed_ips\u0027]:"},{"line_number":262,"context_line":"                gw_ip_addr \u003d gw_fixed_ip[\u0027ip_address\u0027]"},{"line_number":263,"context_line":"                if netaddr.IPAddress(gw_ip_addr).version \u003d\u003d port_ip_vers:"},{"line_number":264,"context_line":"                    port_cidr \u003d \u0027%s/%s\u0027 % (port_ip_addr,"},{"line_number":265,"context_line":"                                           port_fixed_ip[\u0027prefixlen\u0027])"},{"line_number":266,"context_line":"                    self._snat_redirect_add(gw_ip_addr, port_cidr, sn_int)"},{"line_number":267,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_a99ac6c2","line":264,"updated":"2015-03-18 19:48:31.000000000","message":"There is common_utils.ip_to_cidr for this.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":261,"context_line":"            for gw_fixed_ip in gateway[\u0027fixed_ips\u0027]:"},{"line_number":262,"context_line":"                gw_ip_addr \u003d gw_fixed_ip[\u0027ip_address\u0027]"},{"line_number":263,"context_line":"                if netaddr.IPAddress(gw_ip_addr).version \u003d\u003d port_ip_vers:"},{"line_number":264,"context_line":"                    port_cidr \u003d \u0027%s/%s\u0027 % (port_ip_addr,"},{"line_number":265,"context_line":"                                           port_fixed_ip[\u0027prefixlen\u0027])"},{"line_number":266,"context_line":"                    self._snat_redirect_add(gw_ip_addr, port_cidr, sn_int)"},{"line_number":267,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_1d206348","line":264,"in_reply_to":"9a80dd14_a99ac6c2","updated":"2015-03-19 00:43:48.000000000","message":"Fixed in followup.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":275,"context_line":"            ns_ipd.route.add_gateway(gateway, table\u003dsnat_idx)"},{"line_number":276,"context_line":"            ns_ipr.rule.add(sn_port_cidr, snat_idx, snat_idx)"},{"line_number":277,"context_line":"            ns_ipwrapr.netns.execute([\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.\u0027"},{"line_number":278,"context_line":"                                      \u0027send_redirects\u003d0\u0027 % sn_int])"},{"line_number":279,"context_line":"        except Exception:"},{"line_number":280,"context_line":"            LOG.exception(_LE(\u0027DVR: error adding redirection logic\u0027))"},{"line_number":281,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_a95d66ee","line":278,"updated":"2015-03-18 19:48:31.000000000","message":"nit:  Try to avoid random whitespace changes.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"f1263ff8c517a459423d595f613636db74af2870","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                gw_ip_addr \u003d gw_fixed_ip[\u0027ip_address\u0027]"},{"line_number":290,"context_line":"                if netaddr.IPAddress(gw_ip_addr).version \u003d\u003d port_ip_vers:"},{"line_number":291,"context_line":"                    port_cidr \u003d \u0027%s/%s\u0027 % (port_fixed_ip[\u0027ip_address\u0027],"},{"line_number":292,"context_line":"                                           port_fixed_ip[\u0027prefixlen\u0027])"},{"line_number":293,"context_line":"                    self._snat_redirect_remove(gw_fixed_ip[\u0027ip_address\u0027],"},{"line_number":294,"context_line":"                                               port_cidr, sn_int)"},{"line_number":295,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_99f0faca","line":292,"updated":"2015-03-18 16:48:36.000000000","message":"This method up to here looks identical to snat_redirects_add(). A common method should be used to reduce code duplication.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                gw_ip_addr \u003d gw_fixed_ip[\u0027ip_address\u0027]"},{"line_number":290,"context_line":"                if netaddr.IPAddress(gw_ip_addr).version \u003d\u003d port_ip_vers:"},{"line_number":291,"context_line":"                    port_cidr \u003d \u0027%s/%s\u0027 % (port_fixed_ip[\u0027ip_address\u0027],"},{"line_number":292,"context_line":"                                           port_fixed_ip[\u0027prefixlen\u0027])"},{"line_number":293,"context_line":"                    self._snat_redirect_remove(gw_fixed_ip[\u0027ip_address\u0027],"},{"line_number":294,"context_line":"                                               port_cidr, sn_int)"},{"line_number":295,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_099cb25c","line":292,"in_reply_to":"9a80dd14_99f0faca","updated":"2015-03-18 19:48:31.000000000","message":"+1  While there are subtle differences, they do appear to be functionally the same.  Since there is quite a bit of complexity to the iteration, it would help to have a helper method to iterate them.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":289,"context_line":"                gw_ip_addr \u003d gw_fixed_ip[\u0027ip_address\u0027]"},{"line_number":290,"context_line":"                if netaddr.IPAddress(gw_ip_addr).version \u003d\u003d port_ip_vers:"},{"line_number":291,"context_line":"                    port_cidr \u003d \u0027%s/%s\u0027 % (port_fixed_ip[\u0027ip_address\u0027],"},{"line_number":292,"context_line":"                                           port_fixed_ip[\u0027prefixlen\u0027])"},{"line_number":293,"context_line":"                    self._snat_redirect_remove(gw_fixed_ip[\u0027ip_address\u0027],"},{"line_number":294,"context_line":"                                               port_cidr, sn_int)"},{"line_number":295,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_a063b018","line":292,"in_reply_to":"9a80dd14_99f0faca","updated":"2015-03-19 00:43:48.000000000","message":"Done in followup patch (https://review.openstack.org/#/c/165664).","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"f1263ff8c517a459423d595f613636db74af2870","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            snat_idx \u003d self._get_snat_idx(sn_port_cidr)"},{"line_number":300,"context_line":"            ns_ipr \u003d ip_lib.IPRule(namespace\u003dself.ns_name)"},{"line_number":301,"context_line":"            ns_ipd \u003d ip_lib.IPDevice(sn_int, namespace\u003dself.ns_name)"},{"line_number":302,"context_line":"            ns_ipd.route.delete_gateway(gateway, table\u003dsnat_idx)"},{"line_number":303,"context_line":"            ns_ipr.rule.delete(sn_port_cidr, snat_idx, snat_idx)"},{"line_number":304,"context_line":"        except Exception:"},{"line_number":305,"context_line":"            LOG.exception(_LE(\u0027DVR: removed snat failed\u0027))"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_bdd20932","line":302,"updated":"2015-03-18 16:48:36.000000000","message":"Even this first part is common with _add() and could be considered in the above code consolidation. But this could be left for a later refactor.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"9a2d0033497a1a533aaa35776b8096840d922807","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def snat_redirects_add(self, gateway, sn_port, sn_int):"},{"line_number":256,"context_line":"        \"\"\"Adds rules and routes for SNAT redirections for a port.\"\"\""},{"line_number":257,"context_line":"        for port_fixed_ip in sn_port[\u0027fixed_ips\u0027]:"},{"line_number":258,"context_line":"            # Find the first gateway IP address matching this IP version"},{"line_number":259,"context_line":"            port_ip_addr \u003d port_fixed_ip[\u0027ip_address\u0027]"},{"line_number":260,"context_line":"            port_ip_vers \u003d netaddr.IPAddress(port_ip_addr).version"}],"source_content_type":"text/x-python","patch_set":28,"id":"9a80dd14_96e97681","line":257,"updated":"2015-03-19 13:34:05.000000000","message":"There\u0027s a lot of duplication in this and snat_redirects_remove, and in L272-274, L299-301. You could use a conditional, like this (saves about 13 lines)\n\n    def _snat_redirect_modify(self, gateway, sn_port, sn_int, is_add):\n        for port_fixed_ip in sn_port[\u0027fixed_ips\u0027]:\n            # Find the first gateway IP address matching this IP version \n            port_ip_addr \u003d port_fixed_ip[\u0027ip_address\u0027]\n            port_ip_vers \u003d netaddr.IPAddress(port_ip_addr).version\n            for gw_fixed_ip in gateway[\u0027fixed_ips\u0027]:\n                gw_ip_addr \u003d gw_fixed_ip[\u0027ip_address\u0027]\n                if netaddr.IPAddress(gw_ip_addr).version \u003d\u003d port_ip_vers:\n                    port_cidr \u003d \u0027%s/%s\u0027 % (port_ip_addr,\n                                           port_fixed_ip[\u0027prefixlen\u0027])\n                # Assumes these three lines won\u0027t raise Exception\n                snat_idx \u003d self._get_snat_idx(port_cidr)\n                ns_ipr \u003d ip_lib.IPRule(namespace\u003dself.ns_name)\n                ns_ipd \u003d ip_lib.IPDevice(sn_int, namespace\u003dself.ns_name)\n                if is_add:\n                    # Could put these try blocks in named methods to reduce size\n                    try:\n                        ns_ipwrapr \u003d ip_lib.IPWrapper(namespace\u003dself.ns_name)\n                        ns_ipd.route.add_gateway(gw_ip_addr, table\u003dsnat_idx)\n                        ns_ipr.rule.add(port_cidr, snat_idx, snat_idx)\n                        ns_ipwrapr.netns.execute([\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.\u0027\n                                                  \u0027send_redirects\u003d0\u0027 % sn_int])\n                    except Exception:\n                        LOG.exception(_LE(\u0027DVR: error adding redirection logic\u0027))\n                else:\n                    try:\n                        ns_ipd.route.delete_gateway(gw_fixed_ip[\u0027ip_address\u0027],\n                                                    table\u003dsnat_idx)\n                        ns_ipr.rule.delete(port_cidr, snat_idx, snat_idx)\n                    except Exception:\n                        LOG.exception(_LE(\u0027DVR: removed snat failed\u0027))\n\n    def snat_redirects_add(self, gateway, sn_port, sn_int):\n        \"\"\"Adds rules and routes for SNAT redirections for a port.\"\"\"\n    \tself._snat_redirect_modify(gateway, sn_port, sn_int, True)\n\n    def snat_redirects_remove(self, gateway, sn_port, sn_int):\n        \"\"\"Removes rules and routes for SNAT redirections for a port.\"\"\"\n    \tself._snat_redirect_modify(gateway, sn_port, sn_int, False)","commit_id":"d1dee45689b277a1754877a74560bf37e5dd47ac"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"1570cc0c76029b59d02bfcd15338f6c428f6369d","unresolved":false,"context_lines":[{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def _snat_redirect_remove(self, gateway, sn_port, sn_int):"},{"line_number":297,"context_line":"        \"\"\"Removes rules and routes for SNAT redirection.\"\"\""},{"line_number":298,"context_line":"        self._snat_redirect_modify(gateway, sn_port, sn_int, is_add\u003dTrue)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def get_gw_port_host(self):"},{"line_number":301,"context_line":"        host \u003d self.router.get(\u0027gw_port_host\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_484604c1","line":298,"updated":"2015-03-23 13:18:28.000000000","message":"Whoops! is_add\u003dFalse!\n\nCHeck the unit tests too, as they should catch this.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    def _snat_redirect_remove(self, gateway, sn_port, sn_int):"},{"line_number":297,"context_line":"        \"\"\"Removes rules and routes for SNAT redirection.\"\"\""},{"line_number":298,"context_line":"        self._snat_redirect_modify(gateway, sn_port, sn_int, is_add\u003dTrue)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def get_gw_port_host(self):"},{"line_number":301,"context_line":"        host \u003d self.router.get(\u0027gw_port_host\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_b3651f47","line":298,"in_reply_to":"1a6ced46_484604c1","updated":"2015-03-23 20:37:40.000000000","message":"Oops. Good catch! I did  a quick change to the UT test case to catch this, but we should do a followup to harden the UT test cases so that we\u0027re checking calls deeper into the mocked methods.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"}],"neutron/agent/linux/interface.py":[{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"bf360d7b508cc4852df2d3633f33e3379aede095","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        self.conf \u003d conf"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def init_l3(self, device_name, ip_addrs, namespace\u003dNone,"},{"line_number":81,"context_line":"                preserve_ips\u003d[], gateway\u003dNone, extra_subnets\u003d[],"},{"line_number":82,"context_line":"                is_ext_gateway\u003dFalse):"},{"line_number":83,"context_line":"        \"\"\"Set the L3 settings for the interface using data from the port."},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_c06d2d34","line":81,"updated":"2015-03-05 18:39:23.000000000","message":"gateway doesn\u0027t have to be correlated with ip_addrs. The only addition is that we now may have multiple gateways, at least one from each of the address families. Therefore, the change that is needed is to change the \u0027gateway\u0027 argument to a list.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        self.conf \u003d conf"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def init_l3(self, device_name, ip_addrs, namespace\u003dNone,"},{"line_number":81,"context_line":"                preserve_ips\u003d[], gateway\u003dNone, extra_subnets\u003d[],"},{"line_number":82,"context_line":"                is_ext_gateway\u003dFalse):"},{"line_number":83,"context_line":"        \"\"\"Set the L3 settings for the interface using data from the port."},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_347299ad","line":81,"in_reply_to":"9a80dd14_c06d2d34","updated":"2015-03-06 23:30:31.000000000","message":"Absolutely right. I had introduced this correlation with the thought that I would need to be selective about setting accept_ra\u003d2 or not, based on whether any of the subnets involved had external gateway IPs set or not and whether the device address was set or not. But after our discussion, I agree that this selectivity is not needed. This will simplify things, thanks!","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"bf360d7b508cc4852df2d3633f33e3379aede095","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        return (self.DEV_NAME_PREFIX + port.id)[:self.DEV_NAME_LEN]"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    @staticmethod"},{"line_number":201,"context_line":"    def _enable_ipv6_forwarding(namespace, dev_name, gateway_ip):"},{"line_number":202,"context_line":"        \"\"\"Enable IPv6 forwarding on an interface.\"\"\""},{"line_number":203,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dnamespace)"},{"line_number":204,"context_line":"        ctl_str \u003d \u0027net.ipv6.conf.%s.forwarding\u003d1\u0027 % dev_name"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_6ab545ed","line":201,"updated":"2015-03-05 18:39:23.000000000","message":"gateway_ip is not used in this method\n\nall that is needed to be set is net.ipv6.conf.all.forwarding\u003d1\n\nwe can just move this back to l3 agent","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        return (self.DEV_NAME_PREFIX + port.id)[:self.DEV_NAME_LEN]"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    @staticmethod"},{"line_number":201,"context_line":"    def _enable_ipv6_forwarding(namespace, dev_name, gateway_ip):"},{"line_number":202,"context_line":"        \"\"\"Enable IPv6 forwarding on an interface.\"\"\""},{"line_number":203,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dnamespace)"},{"line_number":204,"context_line":"        ctl_str \u003d \u0027net.ipv6.conf.%s.forwarding\u003d1\u0027 % dev_name"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_ca3996a5","line":201,"in_reply_to":"9a80dd14_6ab545ed","updated":"2015-03-06 23:30:31.000000000","message":"Right. It looks like this was already added f or each namespace in namespaces.py and in dvr_fip_ns.py. I\u0027ll remove this.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"bf360d7b508cc4852df2d3633f33e3379aede095","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                          {\u0027ctl_str\u0027: ctl_str, \u0027ns\u0027: namespace})"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    @staticmethod"},{"line_number":214,"context_line":"    def _configure_ipv6_ra(namespace, dev_name, gateway_ip):"},{"line_number":215,"context_line":"        \"\"\"Configure acceptance of IPv6 route advertisements on an intf.\"\"\""},{"line_number":216,"context_line":"        if gateway_ip:"},{"line_number":217,"context_line":"            # Gateway IP is set, no need to learn it from RAs"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_83597f00","line":214,"updated":"2015-03-05 18:39:23.000000000","message":"l3 agent has a flag use_ipv6, and if it\u0027s set, the flags mentioned in this method can be enabled in the l3 agent module. \n\nall that\u0027s needed is to set accept_ra\u003d2.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                          {\u0027ctl_str\u0027: ctl_str, \u0027ns\u0027: namespace})"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    @staticmethod"},{"line_number":214,"context_line":"    def _configure_ipv6_ra(namespace, dev_name, gateway_ip):"},{"line_number":215,"context_line":"        \"\"\"Configure acceptance of IPv6 route advertisements on an intf.\"\"\""},{"line_number":216,"context_line":"        if gateway_ip:"},{"line_number":217,"context_line":"            # Gateway IP is set, no need to learn it from RAs"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_bd5025d5","line":214,"in_reply_to":"9a80dd14_83597f00","updated":"2015-03-06 23:30:31.000000000","message":"Right again. I\u0027ll leave this as default accept_ra\u003d2 for now, and maybe Abishek can add some logic in his patch to be more selective about setting accept_ra\u003d2.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        # add new addresses"},{"line_number":94,"context_line":"        for ip_cidr in ip_cidrs:"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            net \u003d netaddr.IPNetwork(ip_cidr)"},{"line_number":97,"context_line":"            # Convert to compact IPv6 address because the return values of"},{"line_number":98,"context_line":"            # \"ip addr list\" are compact."}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_68b97114","side":"PARENT","line":95,"updated":"2015-03-08 02:56:06.000000000","message":"Random whitespace change.  :(","commit_id":"b8ef0281824ab35c99d42304a989189de2fa29b5"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        # add new addresses"},{"line_number":94,"context_line":"        for ip_cidr in ip_cidrs:"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            net \u003d netaddr.IPNetwork(ip_cidr)"},{"line_number":97,"context_line":"            # Convert to compact IPv6 address because the return values of"},{"line_number":98,"context_line":"            # \"ip addr list\" are compact."}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_aaaa2fd0","side":"PARENT","line":95,"in_reply_to":"9a80dd14_68b97114","updated":"2015-03-10 18:28:08.000000000","message":"Done","commit_id":"b8ef0281824ab35c99d42304a989189de2fa29b5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d12e93105137263a2135b5aa1dc82e3dcb7806f4","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            device.addr.add(net.version, ip_cidr,"},{"line_number":112,"context_line":"                            str(net.broadcast.format(netaddr.ipv6_full)))"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        for gateway_ip in gateway_ips:"},{"line_number":115,"context_line":"            device.route.add_gateway(gateway_ip)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        # clean up any old addresses"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_883945a6","line":114,"updated":"2015-03-08 02:56:06.000000000","message":"Any particular reason this is moved ahead of cleaning up old addresses?  I\u0027m not sure it matters but if it doesn\u0027t matter, I\u0027d rather leave it below.  There could easily be something I missed.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3d8c4e11397f0b38103c8a9d8d59989f1540f42b","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            device.addr.add(net.version, ip_cidr,"},{"line_number":112,"context_line":"                            str(net.broadcast.format(netaddr.ipv6_full)))"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        for gateway_ip in gateway_ips:"},{"line_number":115,"context_line":"            device.route.add_gateway(gateway_ip)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        # clean up any old addresses"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a80dd14_6dce91b6","line":114,"in_reply_to":"9a80dd14_883945a6","updated":"2015-03-10 18:28:08.000000000","message":"No reason. Things got moved around in earlier iterations of the patch. I\u0027ll move it back.","commit_id":"6e5f161b86d5df68bf88df9505fade4627d0065e"},{"author":{"_account_id":6620,"name":"Abishek Subramanian","email":"absubram@cisco.com","username":"absubram"},"change_message_id":"62e236777ecb20d4bee839d3f1154af646702e5f","unresolved":false,"context_lines":[{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def init_l3(self, device_name, ip_cidrs, namespace\u003dNone,"},{"line_number":81,"context_line":"                preserve_ips\u003d[], gateway_ips\u003d[], extra_subnets\u003d[],"},{"line_number":82,"context_line":"                is_ext_gateway\u003dFalse):"},{"line_number":83,"context_line":"        \"\"\"Set the L3 settings for the interface using data from the port."},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        ip_cidrs: list of \u0027X.X.X.X/YY\u0027 strings"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_d3de533d","line":82,"updated":"2015-03-16 11:36:04.000000000","message":"I don\u0027t think is_ext_gateway is used anywhere?","commit_id":"9cead63d86facbe40008713db3ccdbd51e9611bd"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e124291513632ba1f59a287da714dd41f220db11","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        ip_cidrs: list of \u0027X.X.X.X/YY\u0027 strings"},{"line_number":85,"context_line":"        preserve_ips: list of ip cidrs that should not be removed from device"},{"line_number":86,"context_line":"        gateway_ips: For gateway ports, list of external gateway ip addresses"},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        device \u003d ip_lib.IPDevice(device_name, namespace\u003dnamespace)"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9a80dd14_1e034b0e","line":86,"updated":"2015-03-17 15:01:55.000000000","message":"Could add a note here, indicating that the gateway_ips is read only (not mutable), so the default is OK.\n\nCould do a cleanup commit later, to change all the methods to use the conventional None default and assign to [], if none. This would protect the methods, in case in the future changes are made to this list in one of the methods.","commit_id":"3a016e8cfde03282f3a0d7e8e089cc80df4fa9df"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a163db92080b8921c47101e8f56801db9c537897","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        ip_cidrs: list of \u0027X.X.X.X/YY\u0027 strings"},{"line_number":85,"context_line":"        preserve_ips: list of ip cidrs that should not be removed from device"},{"line_number":86,"context_line":"        gateway_ips: For gateway ports, list of external gateway ip addresses"},{"line_number":87,"context_line":"        \"\"\""},{"line_number":88,"context_line":"        device \u003d ip_lib.IPDevice(device_name, namespace\u003dnamespace)"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9a80dd14_9d4f266a","line":86,"in_reply_to":"9a80dd14_1e034b0e","updated":"2015-03-18 00:24:42.000000000","message":"Done","commit_id":"3a016e8cfde03282f3a0d7e8e089cc80df4fa9df"}],"neutron/agent/linux/ip_lib.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"f1263ff8c517a459423d595f613636db74af2870","unresolved":false,"context_lines":[{"line_number":596,"context_line":""},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"def device_exists_with_ips_and_mac(device_name, ip_cidrs, mac, namespace\u003dNone):"},{"line_number":599,"context_line":"    \"\"\"Return True if the device with the given IPs and MAC addresses"},{"line_number":600,"context_line":"    exists in the namespace."},{"line_number":601,"context_line":"    \"\"\""},{"line_number":602,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_e0013a05","line":599,"updated":"2015-03-18 16:48:36.000000000","message":"Nit: might want to reword so that it\u0027s clear only one MAC address.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":596,"context_line":""},{"line_number":597,"context_line":""},{"line_number":598,"context_line":"def device_exists_with_ips_and_mac(device_name, ip_cidrs, mac, namespace\u003dNone):"},{"line_number":599,"context_line":"    \"\"\"Return True if the device with the given IPs and MAC addresses"},{"line_number":600,"context_line":"    exists in the namespace."},{"line_number":601,"context_line":"    \"\"\""},{"line_number":602,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_eb7f49dd","line":599,"in_reply_to":"9a80dd14_e0013a05","updated":"2015-03-19 00:43:48.000000000","message":"Fixed in followup patch.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"30ae36eae0c2231d0ea81e2d679a160dc25d4de4","unresolved":false,"context_lines":[{"line_number":604,"context_line":"        if mac !\u003d device.link.address:"},{"line_number":605,"context_line":"            return False"},{"line_number":606,"context_line":"        for ip_cidr in ip_cidrs:"},{"line_number":607,"context_line":"            if ip_cidr not in [ip[\u0027cidr\u0027] for ip in device.addr.list()]:"},{"line_number":608,"context_line":"                return False"},{"line_number":609,"context_line":"    except RuntimeError:"},{"line_number":610,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_4e898b25","line":607,"updated":"2015-03-24 04:48:30.000000000","message":"This could be optimized a little by moving the list comprehension out of the for loop.","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"7d78365581f7eefc7e44ee7f3c1d53244ec329b0","unresolved":false,"context_lines":[{"line_number":604,"context_line":"        if mac !\u003d device.link.address:"},{"line_number":605,"context_line":"            return False"},{"line_number":606,"context_line":"        for ip_cidr in ip_cidrs:"},{"line_number":607,"context_line":"            if ip_cidr not in [ip[\u0027cidr\u0027] for ip in device.addr.list()]:"},{"line_number":608,"context_line":"                return False"},{"line_number":609,"context_line":"    except RuntimeError:"},{"line_number":610,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_d105f37a","line":607,"in_reply_to":"1a6ced46_4e898b25","updated":"2015-03-24 23:07:08.000000000","message":"Done","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7139f5db17f48d9d589c5c31e7a6013096beabe2","unresolved":false,"context_lines":[{"line_number":604,"context_line":"        if mac !\u003d device.link.address:"},{"line_number":605,"context_line":"            return False"},{"line_number":606,"context_line":"        for ip_cidr in ip_cidrs:"},{"line_number":607,"context_line":"            if ip_cidr not in [ip[\u0027cidr\u0027] for ip in device.addr.list()]:"},{"line_number":608,"context_line":"                return False"},{"line_number":609,"context_line":"    except RuntimeError:"},{"line_number":610,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_35eac10f","line":607,"in_reply_to":"1a6ced46_4e898b25","updated":"2015-03-24 16:35:18.000000000","message":"Right, is this going to make multiple calls to \u0027ip addr list\u0027 ?","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"}],"neutron/agent/linux/ra.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                    break"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                continue"},{"line_number":131,"context_line":"            break"},{"line_number":132,"context_line":"        else:"},{"line_number":133,"context_line":"            # Kill the daemon if it\u0027s running"},{"line_number":134,"context_line":"            self.disable()"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_5cc3c637","line":131,"updated":"2015-03-18 19:48:31.000000000","message":"This is out-of-hand crazy (ab)use of control flow logic.  I know that a loop’s else clause runs when no break occurs.  However, it isn\u0027t the most intuitive construct.  I can accept its use in a one-level loop.  But with this nested loop with multiple levels of break it just hurts my head.  I had to stop and read through this very carefully to be sure it does what it should.\n\nWhat this seems to be saying is \"disable and return if there are no ipv6 subnets on any router ports\".  The inverse of that is \"enable only if an ipv6 subnet is found.  Disable otherwise.\"  Isn\u0027t the following so much easier on the brain?\n\n  for p in router_ports:\n      for subnet in p[\u0027subnets\u0027]:\n          if netaddr.IPNetwork(subnet[\u0027cidr\u0027]).version \u003d\u003d 6:\n              (L137 - L139)\n              return\n\n  self.disable()","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                    break"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                continue"},{"line_number":131,"context_line":"            break"},{"line_number":132,"context_line":"        else:"},{"line_number":133,"context_line":"            # Kill the daemon if it\u0027s running"},{"line_number":134,"context_line":"            self.disable()"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_ebe4a9a9","line":131,"in_reply_to":"9a80dd14_10ae8fe7","updated":"2015-03-19 00:43:48.000000000","message":"Thanks, I\u0027ll leave this for your patch to clean up.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e251c5bcd099f4acc24d6b2e0f945375735fb7b0","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                    break"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                continue"},{"line_number":131,"context_line":"            break"},{"line_number":132,"context_line":"        else:"},{"line_number":133,"context_line":"            # Kill the daemon if it\u0027s running"},{"line_number":134,"context_line":"            self.disable()"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_10ae8fe7","line":131,"in_reply_to":"9a80dd14_353b85c9","updated":"2015-03-18 20:23:36.000000000","message":"If you want to leave this for now, I\u0027ve posted this [1].\n\n[1] https://review.openstack.org/165586","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5de56cb4c33714cc32ed0ae99adde43c13ecac01","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                    break"},{"line_number":129,"context_line":"            else:"},{"line_number":130,"context_line":"                continue"},{"line_number":131,"context_line":"            break"},{"line_number":132,"context_line":"        else:"},{"line_number":133,"context_line":"            # Kill the daemon if it\u0027s running"},{"line_number":134,"context_line":"            self.disable()"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_353b85c9","line":131,"in_reply_to":"9a80dd14_5cc3c637","updated":"2015-03-18 20:07:18.000000000","message":"+1 to that, I had to write a test to follow the flow.  When are goto\u0027s coming to python? :)","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":14844,"name":"John Joyce","email":"joycej@cisco.com","username":"john_a_joyce"},"change_message_id":"6d0ea13059d979b5960c45a048fc59ac55f19c99","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def enable(self, router_ports):"},{"line_number":125,"context_line":"        for p in router_ports:"},{"line_number":126,"context_line":"            for subnet in p[\u0027subnets\u0027]:"},{"line_number":127,"context_line":"                if netaddr.IPNetwork(subnet[\u0027cidr\u0027]).version \u003d\u003d 6:"},{"line_number":128,"context_line":"                    break"},{"line_number":129,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_752cb493","line":126,"updated":"2015-03-20 19:38:02.000000000","message":"I guess an update is pending that might clean this up,  but the code below is not intuitive that it is achieving what you want.","commit_id":"a695d50a12075d1f2d12eb5604d412104ace27d1"}],"neutron/common/utils.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"f1263ff8c517a459423d595f613636db74af2870","unresolved":false,"context_lines":[{"line_number":386,"context_line":"    return str(net)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"def fixed_ip_cidrs(fixed_ips):"},{"line_number":390,"context_line":"    \"\"\"Create a list of a port\u0027s fixed IPs in cidr notation."},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    :param fixed_ips: A neutron port\u0027s fixed_ips dictionary"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_39b38eea","line":389,"updated":"2015-03-18 16:48:36.000000000","message":"This common method is not specific to fixed IPs or ports. It\u0027s name should probably be ips_to_cidrs(ips) and just be a list version of the method above. Adjust the variable names and docstring accordingly.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"ceba6b845a2319f935c7df7c91585c99389175e8","unresolved":false,"context_lines":[{"line_number":386,"context_line":"    return str(net)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"def fixed_ip_cidrs(fixed_ips):"},{"line_number":390,"context_line":"    \"\"\"Create a list of a port\u0027s fixed IPs in cidr notation."},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    :param fixed_ips: A neutron port\u0027s fixed_ips dictionary"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_f2e748aa","line":389,"in_reply_to":"9a80dd14_39b38eea","updated":"2015-03-18 18:17:17.000000000","message":"Ooops, see below","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"ceba6b845a2319f935c7df7c91585c99389175e8","unresolved":false,"context_lines":[{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    :param fixed_ips: A neutron port\u0027s fixed_ips dictionary"},{"line_number":393,"context_line":"    \"\"\""},{"line_number":394,"context_line":"    return [ip_to_cidr(fixed_ip[\u0027ip_address\u0027], fixed_ip.get(\u0027prefixlen\u0027))"},{"line_number":395,"context_line":"            for fixed_ip in fixed_ips]"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_12b93c79","line":394,"updated":"2015-03-18 18:17:17.000000000","message":"Ah, I see you are actually using the fixed_ip dictionary here. It feels a little out of place in this module, but there isn\u0027t really a better place at the moment.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    :param fixed_ips: A neutron port\u0027s fixed_ips dictionary"},{"line_number":393,"context_line":"    \"\"\""},{"line_number":394,"context_line":"    return [ip_to_cidr(fixed_ip[\u0027ip_address\u0027], fixed_ip.get(\u0027prefixlen\u0027))"},{"line_number":395,"context_line":"            for fixed_ip in fixed_ips]"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_dcd096cb","line":394,"in_reply_to":"9a80dd14_12b93c79","updated":"2015-03-18 19:48:31.000000000","message":"+1 I thought it was out of place here too before reading Henry\u0027s comment.  This module doesn\u0027t really deal with ports or dictionaries from ports.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    :param fixed_ips: A neutron port\u0027s fixed_ips dictionary"},{"line_number":393,"context_line":"    \"\"\""},{"line_number":394,"context_line":"    return [ip_to_cidr(fixed_ip[\u0027ip_address\u0027], fixed_ip.get(\u0027prefixlen\u0027))"},{"line_number":395,"context_line":"            for fixed_ip in fixed_ips]"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_eb0e693c","line":394,"in_reply_to":"9a80dd14_dcd096cb","updated":"2015-03-19 00:43:48.000000000","message":"I\u0027m looking for suggestions. When I added this, I was thinking that this was not the right place for this, but there\u0027s no good place to put this. Seems we need a new utility module for things like this.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"dade667f3f086b07025453bb8eee06b165f35bac","unresolved":false,"context_lines":[{"line_number":905,"context_line":"                self._validate_shared_update(context, id, network, n)"},{"line_number":906,"context_line":"            # validate the subnets on the ext net"},{"line_number":907,"context_line":"            if network.external:"},{"line_number":908,"context_line":"                self._check_ext_network_subnets(context, network)"},{"line_number":909,"context_line":"            network.update(n)"},{"line_number":910,"context_line":"            # also update shared in all the subnets for this network"},{"line_number":911,"context_line":"            subnets \u003d self._get_subnets_by_network(context, id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_946d299b","line":908,"updated":"2015-03-05 21:59:02.000000000","message":"This check doesn\u0027t seem to be necessary if the number of subnets are not going to be changed as part of updating the network.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":905,"context_line":"                self._validate_shared_update(context, id, network, n)"},{"line_number":906,"context_line":"            # validate the subnets on the ext net"},{"line_number":907,"context_line":"            if network.external:"},{"line_number":908,"context_line":"                self._check_ext_network_subnets(context, network)"},{"line_number":909,"context_line":"            network.update(n)"},{"line_number":910,"context_line":"            # also update shared in all the subnets for this network"},{"line_number":911,"context_line":"            subnets \u003d self._get_subnets_by_network(context, id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_66ec5990","line":908,"in_reply_to":"9a80dd14_946d299b","updated":"2015-03-06 23:30:31.000000000","message":"This check matches what is written in the blueprint specification (up to one V4, up to one V6 external gateway addresses), but I agree that we shouldn\u0027t place an upper bound on the number of IPv6 addresses (some scenarios might call for additional gateway IP addresses e.g. as backup). I\u0027ll remove this check.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7e5658acc94bd21c1ab1060f6be6ed6fa8257ca9","unresolved":false,"context_lines":[{"line_number":992,"context_line":"                            \u0027netid\u0027: s[\u0027network_id\u0027]}"},{"line_number":993,"context_line":"                    msg \u003d _(\"Two IPv%(vers)s subnets disallowed \""},{"line_number":994,"context_line":"                            \"for external network: %(netid)s\") % data"},{"line_number":995,"context_line":"                    raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":996,"context_line":""},{"line_number":997,"context_line":"    def _validate_subnet(self, context, s, cur_subnet\u003dNone):"},{"line_number":998,"context_line":"        \"\"\"Validate a subnet spec.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_760de2e7","line":995,"updated":"2015-03-06 22:20:24.000000000","message":"From L981 down seems like a duplicate of _check_ext_network_subnets() above.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":992,"context_line":"                            \u0027netid\u0027: s[\u0027network_id\u0027]}"},{"line_number":993,"context_line":"                    msg \u003d _(\"Two IPv%(vers)s subnets disallowed \""},{"line_number":994,"context_line":"                            \"for external network: %(netid)s\") % data"},{"line_number":995,"context_line":"                    raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":996,"context_line":""},{"line_number":997,"context_line":"    def _validate_subnet(self, context, s, cur_subnet\u003dNone):"},{"line_number":998,"context_line":"        \"\"\"Validate a subnet spec.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_dcd94df4","line":995,"in_reply_to":"9a80dd14_760de2e7","updated":"2015-03-06 23:30:31.000000000","message":"This method will be removed per Robert\u0027s comments.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"dade667f3f086b07025453bb8eee06b165f35bac","unresolved":false,"context_lines":[{"line_number":1096,"context_line":"                                               s[\u0027allocation_pools\u0027])"},{"line_number":1097,"context_line":""},{"line_number":1098,"context_line":"        self._validate_subnet(context, s)"},{"line_number":1099,"context_line":"        self._validate_subnet_on_ext_network(context, s)"},{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"        tenant_id \u003d self._get_tenant_id_for_create(context, s)"},{"line_number":1102,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_d7dec310","line":1099,"updated":"2015-03-05 21:59:02.000000000","message":"regardless the number of subnets, the gateway port will be created the same way as other ports. Although it\u0027s not necessary to have more than one subnet from each family, I don\u0027t feel it is necessary to add the above check either.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":1096,"context_line":"                                               s[\u0027allocation_pools\u0027])"},{"line_number":1097,"context_line":""},{"line_number":1098,"context_line":"        self._validate_subnet(context, s)"},{"line_number":1099,"context_line":"        self._validate_subnet_on_ext_network(context, s)"},{"line_number":1100,"context_line":""},{"line_number":1101,"context_line":"        tenant_id \u003d self._get_tenant_id_for_create(context, s)"},{"line_number":1102,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_e6f76957","line":1099,"in_reply_to":"9a80dd14_d7dec310","updated":"2015-03-06 23:30:31.000000000","message":"Done","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"}],"neutron/db/l3_db.py":[{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"d5511e9df03d3af4318cbb883fa7f9920eb156dc","unresolved":false,"context_lines":[{"line_number":1071,"context_line":"            for port in ports:"},{"line_number":1072,"context_line":"                dev_owner \u003d port.get(\u0027device_owner\u0027)"},{"line_number":1073,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1074,"context_line":"                if len(fixed_ips) \u003e 1 and dev_owner !\u003d DEVICE_OWNER_ROUTER_GW:"},{"line_number":1075,"context_line":"                    LOG.info(_LI(\"Ignoring multiple IPs on \""},{"line_number":1076,"context_line":"                                 \"internal router port %s\"), port[\u0027id\u0027])"},{"line_number":1077,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_c2356347","line":1074,"updated":"2015-03-04 23:23:04.000000000","message":"Thought that this is applicable to router ports as well. Would patch #3 depend on this?","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":1071,"context_line":"            for port in ports:"},{"line_number":1072,"context_line":"                dev_owner \u003d port.get(\u0027device_owner\u0027)"},{"line_number":1073,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1074,"context_line":"                if len(fixed_ips) \u003e 1 and dev_owner !\u003d DEVICE_OWNER_ROUTER_GW:"},{"line_number":1075,"context_line":"                    LOG.info(_LI(\"Ignoring multiple IPs on \""},{"line_number":1076,"context_line":"                                 \"internal router port %s\"), port[\u0027id\u0027])"},{"line_number":1077,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_e6a5a947","line":1074,"in_reply_to":"9a80dd14_c2356347","updated":"2015-03-06 23:30:31.000000000","message":"Patch #3 will remove this check altogether. Might as well remove this now.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"d5511e9df03d3af4318cbb883fa7f9920eb156dc","unresolved":false,"context_lines":[{"line_number":1107,"context_line":"                        port[\u0027subnets\u0027].append(subnet_info)"},{"line_number":1108,"context_line":"                        prefixlen \u003d netaddr.IPNetwork("},{"line_number":1109,"context_line":"                            subnet[\u0027cidr\u0027]).prefixlen"},{"line_number":1110,"context_line":"                        fixed_ip[\u0027prefixlen\u0027] \u003d prefixlen"},{"line_number":1111,"context_line":"                        break"},{"line_number":1112,"context_line":"                else:"},{"line_number":1113,"context_line":"                    port[\u0027extra_subnets\u0027].append(subnet_info)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_8231ab0d","line":1110,"updated":"2015-03-04 23:23:04.000000000","message":"I was thinking about lines 1108 to 1110, and realized from line 1073 that fixed_ip is part of the port dictionary, and the above lines modify the content in the port dictionary. Some comments would help.","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a6b1063a301df0a003512501b1aec4d15e9d70df","unresolved":false,"context_lines":[{"line_number":1107,"context_line":"                        port[\u0027subnets\u0027].append(subnet_info)"},{"line_number":1108,"context_line":"                        prefixlen \u003d netaddr.IPNetwork("},{"line_number":1109,"context_line":"                            subnet[\u0027cidr\u0027]).prefixlen"},{"line_number":1110,"context_line":"                        fixed_ip[\u0027prefixlen\u0027] \u003d prefixlen"},{"line_number":1111,"context_line":"                        break"},{"line_number":1112,"context_line":"                else:"},{"line_number":1113,"context_line":"                    port[\u0027extra_subnets\u0027].append(subnet_info)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_7c8b06c3","line":1110,"in_reply_to":"9a80dd14_8231ab0d","updated":"2015-03-06 23:30:31.000000000","message":"Done","commit_id":"1643fbf49b194974a170d00e32dd89515190cc60"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8db908ad4c46c207b93c727e41f406b48599164b","unresolved":false,"context_lines":[{"line_number":1092,"context_line":"        if not ports:"},{"line_number":1093,"context_line":"            return"},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"        def each_port_with_ip():"},{"line_number":1096,"context_line":"            for port in ports:"},{"line_number":1097,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1098,"context_line":"                if not fixed_ips:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_9c945ee9","line":1095,"updated":"2015-03-18 19:48:31.000000000","message":"nit:  The semantics of this changed a bit.  It is now something like \"each port having fixed ips.\"  The each_port_with_ip seemed more appropriate when the ips where yielded on L1105.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"dd22d8de69323f372b4c9dd8dbc4ccc05491751d","unresolved":false,"context_lines":[{"line_number":1092,"context_line":"        if not ports:"},{"line_number":1093,"context_line":"            return"},{"line_number":1094,"context_line":""},{"line_number":1095,"context_line":"        def each_port_with_ip():"},{"line_number":1096,"context_line":"            for port in ports:"},{"line_number":1097,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1098,"context_line":"                if not fixed_ips:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_4c230b05","line":1095,"in_reply_to":"9a80dd14_9c945ee9","updated":"2015-03-19 00:43:48.000000000","message":"Is your preference to change the name of this method to something like \"each_port_having_fixed_ips\" to match the semantics? I\u0027ll do that in a followup patch, let me know if that\u0027s not what you intended.","commit_id":"65b94158b475924fc6e15105a49eb980ba5c8d6c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"ee1a8ce7846f6360fc86d407e967726d3f8cdfb5","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                                                          port_id, subnet_id,"},{"line_number":669,"context_line":"                                                          device_owner)"},{"line_number":670,"context_line":"        # remove_by_subnet is not used here, because the validation logic of"},{"line_number":671,"context_line":"        # _validate_interface_info ensures that at least one of remote_by_*"},{"line_number":672,"context_line":"        # is True."},{"line_number":673,"context_line":"        else:"},{"line_number":674,"context_line":"            port, subnet \u003d self._remove_interface_by_subnet("}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_9f02b46d","line":671,"updated":"2015-03-24 16:05:13.000000000","message":"remote -\u003e remove","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"7d78365581f7eefc7e44ee7f3c1d53244ec329b0","unresolved":false,"context_lines":[{"line_number":668,"context_line":"                                                          port_id, subnet_id,"},{"line_number":669,"context_line":"                                                          device_owner)"},{"line_number":670,"context_line":"        # remove_by_subnet is not used here, because the validation logic of"},{"line_number":671,"context_line":"        # _validate_interface_info ensures that at least one of remote_by_*"},{"line_number":672,"context_line":"        # is True."},{"line_number":673,"context_line":"        else:"},{"line_number":674,"context_line":"            port, subnet \u003d self._remove_interface_by_subnet("}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_31d647c6","line":671,"in_reply_to":"1a6ced46_9f02b46d","updated":"2015-03-24 23:07:08.000000000","message":"I\u0027d prefer to make this collateral change in a smaller (future) patch to keep the SNR as high as possible for this patch set.","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"30ae36eae0c2231d0ea81e2d679a160dc25d4de4","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"        if not ports:"},{"line_number":1100,"context_line":"            return"},{"line_number":1101,"context_line":""},{"line_number":1102,"context_line":"        def each_port_having_fixed_ips():"},{"line_number":1103,"context_line":"            for port in ports:"},{"line_number":1104,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1105,"context_line":"                if not fixed_ips:"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_aeebdfae","line":1102,"updated":"2015-03-24 04:48:30.000000000","message":"nit: each_port_with_fixed_ips()?","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":13409,"name":"Andrew Boik","email":"drew.boik@gmail.com","username":"dboik"},"change_message_id":"83aa6e3c723e259ff3af49f0ed1917684473e83d","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"        if not ports:"},{"line_number":1100,"context_line":"            return"},{"line_number":1101,"context_line":""},{"line_number":1102,"context_line":"        def each_port_having_fixed_ips():"},{"line_number":1103,"context_line":"            for port in ports:"},{"line_number":1104,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1105,"context_line":"                if not fixed_ips:"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_8ce36204","line":1102,"in_reply_to":"1a6ced46_31400756","updated":"2015-03-25 02:06:28.000000000","message":"Ok it just sounded strange to me when I read it. In that case I guess it\u0027s the next best thing.","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"7d78365581f7eefc7e44ee7f3c1d53244ec329b0","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"        if not ports:"},{"line_number":1100,"context_line":"            return"},{"line_number":1101,"context_line":""},{"line_number":1102,"context_line":"        def each_port_having_fixed_ips():"},{"line_number":1103,"context_line":"            for port in ports:"},{"line_number":1104,"context_line":"                fixed_ips \u003d port.get(\u0027fixed_ips\u0027, [])"},{"line_number":1105,"context_line":"                if not fixed_ips:"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_31400756","line":1102,"in_reply_to":"1a6ced46_aeebdfae","updated":"2015-03-24 23:07:08.000000000","message":"The change to \"...having_fixed_ips\" was requested in an earlier review comment, since the earlier meaning of \"port_with_fixed_ips\" was meant to signify that the helper method was returning a port with its fixed_ips, and now we\u0027re just returning the port.","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"}],"neutron/db/l3_dvr_db.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"ee1a8ce7846f6360fc86d407e967726d3f8cdfb5","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            port, subnet \u003d self._remove_interface_by_port("},{"line_number":330,"context_line":"                context, router_id, port_id, subnet_id, device_owner)"},{"line_number":331,"context_line":"        # remove_by_subnet is not used here, because the validation logic of"},{"line_number":332,"context_line":"        # _validate_interface_info ensures that at least one of remote_by_*"},{"line_number":333,"context_line":"        # is True."},{"line_number":334,"context_line":"        else:"},{"line_number":335,"context_line":"            port, subnet \u003d self._remove_interface_by_subnet("}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_3f1380b9","line":332,"updated":"2015-03-24 16:05:13.000000000","message":"remote -\u003e remove","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"7d78365581f7eefc7e44ee7f3c1d53244ec329b0","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            port, subnet \u003d self._remove_interface_by_port("},{"line_number":330,"context_line":"                context, router_id, port_id, subnet_id, device_owner)"},{"line_number":331,"context_line":"        # remove_by_subnet is not used here, because the validation logic of"},{"line_number":332,"context_line":"        # _validate_interface_info ensures that at least one of remote_by_*"},{"line_number":333,"context_line":"        # is True."},{"line_number":334,"context_line":"        else:"},{"line_number":335,"context_line":"            port, subnet \u003d self._remove_interface_by_subnet("}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_d1197389","line":332,"in_reply_to":"1a6ced46_3f1380b9","updated":"2015-03-24 23:07:08.000000000","message":"I\u0027d prefer to make this collateral change in a smaller (future) patch to keep the SNR as high as possible for this patch set.","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"}],"neutron/tests/functional/agent/test_l3_agent.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        return agent"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def generate_router_info(self, enable_ha, ip_version\u003d4, extra_routes\u003dTrue,"},{"line_number":99,"context_line":"                             enable_fip\u003dTrue, enable_snat\u003dTrue,"},{"line_number":100,"context_line":"                             dualstack\u003dFalse):"},{"line_number":101,"context_line":"        if ip_version \u003d\u003d 6 and not dualstack:"},{"line_number":102,"context_line":"            enable_snat \u003d False"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_ede89d7e","line":99,"updated":"2015-03-23 15:03:03.000000000","message":"dualstack should probably be named dual_stack but I\u0027m not going to seriously ask you that...","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        return agent"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def generate_router_info(self, enable_ha, ip_version\u003d4, extra_routes\u003dTrue,"},{"line_number":99,"context_line":"                             enable_fip\u003dTrue, enable_snat\u003dTrue,"},{"line_number":100,"context_line":"                             dualstack\u003dFalse):"},{"line_number":101,"context_line":"        if ip_version \u003d\u003d 6 and not dualstack:"},{"line_number":102,"context_line":"            enable_snat \u003d False"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_a9bbf0c2","line":99,"in_reply_to":"1a6ced46_ede89d7e","updated":"2015-03-23 20:37:40.000000000","message":"I\u0027ll go ahead and change this, I have to push up a new patch set anyway.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def test_legacy_router_lifecycle(self):"},{"line_number":331,"context_line":"        self._router_lifecycle(enable_ha\u003dFalse)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    def test_legacy_dualstack_router_lifecycle(self):"},{"line_number":334,"context_line":"        self._router_lifecycle(enable_ha\u003dFalse, dualstack\u003dTrue)"},{"line_number":335,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_6d4e6dc3","line":332,"updated":"2015-03-23 15:03:03.000000000","message":"Is there a reason to add another test and not modify test_legacy_router_lifecycle to pass dualstack\u003dTrue? These tests are pretty expensive.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def test_legacy_router_lifecycle(self):"},{"line_number":331,"context_line":"        self._router_lifecycle(enable_ha\u003dFalse)"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    def test_legacy_dualstack_router_lifecycle(self):"},{"line_number":334,"context_line":"        self._router_lifecycle(enable_ha\u003dFalse, dualstack\u003dTrue)"},{"line_number":335,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_7fb6063f","line":332,"in_reply_to":"1a6ced46_6d4e6dc3","updated":"2015-03-23 20:37:40.000000000","message":"No reason that I can think of; I think a combined test should catch everything. I\u0027ll combine these into one.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        utils.wait_until_true("},{"line_number":459,"context_line":"            lambda: self._metadata_proxy_exists(self.agent.conf, router))"},{"line_number":460,"context_line":"        self._assert_internal_devices(router)"},{"line_number":461,"context_line":"        self._assert_external_device(router)"},{"line_number":462,"context_line":"        if ip_version \u003d\u003d 4 or dualstack:"},{"line_number":463,"context_line":"            # Note(SridharG): enable the assert_gateway for IPv6 once"},{"line_number":464,"context_line":"            # keepalived on Ubuntu14.04 (i.e., check-neutron-dsvm-functional"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_bb317b0d","line":461,"updated":"2015-03-23 15:03:03.000000000","message":"This should be changed to:\n\n    if not (enable_ha and (ip_version \u003d\u003d 6 or dualstack))","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        utils.wait_until_true("},{"line_number":459,"context_line":"            lambda: self._metadata_proxy_exists(self.agent.conf, router))"},{"line_number":460,"context_line":"        self._assert_internal_devices(router)"},{"line_number":461,"context_line":"        self._assert_external_device(router)"},{"line_number":462,"context_line":"        if ip_version \u003d\u003d 4 or dualstack:"},{"line_number":463,"context_line":"            # Note(SridharG): enable the assert_gateway for IPv6 once"},{"line_number":464,"context_line":"            # keepalived on Ubuntu14.04 (i.e., check-neutron-dsvm-functional"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_44ad0331","line":461,"in_reply_to":"1a6ced46_bb317b0d","updated":"2015-03-23 20:37:40.000000000","message":"Done","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            self._assert_extra_routes(router)"},{"line_number":472,"context_line":"        self._assert_metadata_chains(router)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        if enable_ha:"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self._assert_ha_device(router)"},{"line_number":477,"context_line":"            self.assertTrue(router.keepalived_manager.get_process().active)"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_4d6a911b","line":474,"updated":"2015-03-23 15:03:03.000000000","message":"Beware of the newline of doom!","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            self._assert_extra_routes(router)"},{"line_number":472,"context_line":"        self._assert_metadata_chains(router)"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        if enable_ha:"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self._assert_ha_device(router)"},{"line_number":477,"context_line":"            self.assertTrue(router.keepalived_manager.get_process().active)"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_841a9b6c","line":474,"in_reply_to":"1a6ced46_4d6a911b","updated":"2015-03-23 20:37:40.000000000","message":"Argh! My \u003cEnter\u003e key finger has gotten too assertive, and has been put on notice. Fixed.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        self.assertTrue(self.device_exists_with_ips_and_mac("},{"line_number":489,"context_line":"            external_port, self.agent.get_external_device_name,"},{"line_number":490,"context_line":"            router.ns_name))"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    def _assert_gateway(self, router):"},{"line_number":493,"context_line":"        external_port \u003d router.get_ex_gw_port()"},{"line_number":494,"context_line":"        external_device_name \u003d self.agent.get_external_device_name("}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_764b9a59","line":491,"updated":"2015-03-23 15:03:03.000000000","message":"You should augment this method to check for both gateways if they exist. Right now you\u0027re still not actually asserting what this patch is supposed to be doing.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        self.assertTrue(self.device_exists_with_ips_and_mac("},{"line_number":489,"context_line":"            external_port, self.agent.get_external_device_name,"},{"line_number":490,"context_line":"            router.ns_name))"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    def _assert_gateway(self, router):"},{"line_number":493,"context_line":"        external_port \u003d router.get_ex_gw_port()"},{"line_number":494,"context_line":"        external_device_name \u003d self.agent.get_external_device_name("}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_9ce4ab0e","line":491,"in_reply_to":"1a6ced46_764b9a59","updated":"2015-03-23 20:37:40.000000000","message":"Ah, yes. Fixed.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"deadc80deb86ec677b7d0adf8acfcda80d58ee8e","unresolved":false,"context_lines":[{"line_number":522,"context_line":"        external_device_name \u003d self.agent.get_external_device_name("},{"line_number":523,"context_line":"            external_port[\u0027id\u0027])"},{"line_number":524,"context_line":"        external_device \u003d ip_lib.IPDevice(external_device_name,"},{"line_number":525,"context_line":"                                          namespace\u003drouter.ns_name)"},{"line_number":526,"context_line":"        for subnet in external_port[\u0027subnets\u0027]:"},{"line_number":527,"context_line":"            expected_gateway \u003d subnet[\u0027gateway_ip\u0027]"},{"line_number":528,"context_line":"            ip_vers \u003d netaddr.IPAddress(expected_gateway).version"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a6ced46_22de2d40","line":525,"updated":"2015-03-23 22:01:18.000000000","message":"These 5 lines are better than all of the unit tests for my money...","commit_id":"4c428fc4506adbf99685f4a7fe7b3052b16377ce"}],"neutron/tests/unit/test_l3_agent.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    mac_address.dialect \u003d netaddr.mac_unix"},{"line_number":80,"context_line":"    for i in range(current, current + count):"},{"line_number":81,"context_line":"        fixed_ips \u003d []"},{"line_number":82,"context_line":"        subnets \u003d []"},{"line_number":83,"context_line":"        if ip_version \u003d\u003d 4 or dualstack:"},{"line_number":84,"context_line":"            subnet_id \u003d _uuid()"},{"line_number":85,"context_line":"            fixed_ips.append({\u0027ip_address\u0027: ip_pool_4 % i,"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_0dedf9c3","line":82,"updated":"2015-03-23 15:03:03.000000000","message":"Please consider setting the values according to ip_version and dualstack and repeating yourself less.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    mac_address.dialect \u003d netaddr.mac_unix"},{"line_number":80,"context_line":"    for i in range(current, current + count):"},{"line_number":81,"context_line":"        fixed_ips \u003d []"},{"line_number":82,"context_line":"        subnets \u003d []"},{"line_number":83,"context_line":"        if ip_version \u003d\u003d 4 or dualstack:"},{"line_number":84,"context_line":"            subnet_id \u003d _uuid()"},{"line_number":85,"context_line":"            fixed_ips.append({\u0027ip_address\u0027: ip_pool_4 % i,"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_5f414559","line":82,"in_reply_to":"1a6ced46_0dedf9c3","updated":"2015-03-23 20:37:40.000000000","message":"Done","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c54f6e98bda5251a30ce3bc984eff07df4fe2a1c","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                        extra_routes\u003dFalse, dualstack\u003dFalse):"},{"line_number":119,"context_line":"    fixed_ips \u003d []"},{"line_number":120,"context_line":"    subnets \u003d []"},{"line_number":121,"context_line":"    if ip_version \u003d\u003d 4 or dualstack:"},{"line_number":122,"context_line":"        subnet_id \u003d _uuid()"},{"line_number":123,"context_line":"        fixed_ips.append({\u0027ip_address\u0027: \u002719.4.4.4\u0027,"},{"line_number":124,"context_line":"                          \u0027subnet_id\u0027: subnet_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_2da3959f","line":121,"updated":"2015-03-23 15:03:03.000000000","message":"The only difference here is the subnet_id, ip_address and prefixlen values, please refactor so that you don\u0027t repeat yourself like this.","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"3a07b720bbf329fa19865dd653254f9138f4a238","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                        extra_routes\u003dFalse, dualstack\u003dFalse):"},{"line_number":119,"context_line":"    fixed_ips \u003d []"},{"line_number":120,"context_line":"    subnets \u003d []"},{"line_number":121,"context_line":"    if ip_version \u003d\u003d 4 or dualstack:"},{"line_number":122,"context_line":"        subnet_id \u003d _uuid()"},{"line_number":123,"context_line":"        fixed_ips.append({\u0027ip_address\u0027: \u002719.4.4.4\u0027,"},{"line_number":124,"context_line":"                          \u0027subnet_id\u0027: subnet_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"1a6ced46_82960693","line":121,"in_reply_to":"1a6ced46_2da3959f","updated":"2015-03-23 20:37:40.000000000","message":"Done","commit_id":"9f746fad644e03d50752839dba95b8cfd93ec6bd"}]}