)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11682,"name":"Ryan Moats","email":"rmoats@us.ibm.com","username":"regXboi"},"change_message_id":"22a6069da28713e9b707bfe5b4defd4622d107ed","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Allow _get_sync_floating_ips to filter host"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Allow _get_sync_floating_ips to filter floatingips based on"},{"line_number":10,"context_line":"given host."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ifec8c58436228ba54635025b59a49b5916bdf444"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da6ed579_6ef38274","line":10,"updated":"2016-01-12 14:38:01.000000000","message":"Is there a bug reference explaining why this is necessary?","commit_id":"5267323b641e43a6ea88a5f4a28f29b045c87557"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"2534743b96c3353b2309a179949ddb491cc936b4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Previous processing in _get_dvr_sync_data will do host filter"},{"line_number":10,"context_line":"for floatingip(in python logic, not by DB query), and for that"},{"line_number":11,"context_line":"its processing seems unnecessary, for:"},{"line_number":12,"context_line":"* when agent is in \u0027dvr\u0027 mode, it should onyl return floatingips"},{"line_number":13,"context_line":"  hosted by given agent/host."},{"line_number":14,"context_line":"* when agent is not in \u0027dvr\u0027 mode, it should return:"},{"line_number":15,"context_line":"  * all floatingips when router is not DVR router;"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da6ed579_ced713ec","line":12,"range":{"start_line":12,"start_character":41,"end_line":12,"end_character":45},"updated":"2016-01-17 10:43:28.000000000","message":"nit: only","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8dd9d3bc9bdc70da3ee5bec32f1968180e901e80","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"When above logic can be done in DB query, we can save those"},{"line_number":20,"context_line":"unnecessary processing for floatingip. And this is the point"},{"line_number":21,"context_line":"this patch try to optimize _get_dvr_sync_data."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Closes-Bug: #1533313"},{"line_number":24,"context_line":"Change-Id: Ifec8c58436228ba54635025b59a49b5916bdf444"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"9aed3d3a_f1ccf4bc","line":21,"updated":"2016-02-29 15:23:18.000000000","message":"out of interest do you have numbers to justify the optimization?","commit_id":"7adb1031a002918f33dd519363b1956d70c1d264"}],"neutron/db/l3_db.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"7670c0b0b6fc925e2369c081fa052105873a3493","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"        d[\u0027fixed_ip_address_scope\u0027] \u003d scope_id"},{"line_number":1198,"context_line":"        return d"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _get_sync_floating_ips(self, context, router_ids, host\u003dNone,"},{"line_number":1201,"context_line":"                               dvr_router_ids\u003dNone):"},{"line_number":1202,"context_line":"        \"\"\"Query floating_ips that relate to list of router_ids with scope."},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        This is different than the regular get_floatingips in that it finds the"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_3fa112aa","line":1201,"range":{"start_line":1200,"start_character":46,"end_line":1201,"end_character":50},"updated":"2016-01-18 15:54:55.000000000","message":"might be good to add description of parameters to the docstring","commit_id":"ba775461de037b6b40ed967f4e455b4328775f35"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"2517403f13601cef1ec3edd1518be35f1042092a","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"        d[\u0027fixed_ip_address_scope\u0027] \u003d scope_id"},{"line_number":1198,"context_line":"        return d"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _get_sync_floating_ips(self, context, router_ids, host\u003dNone,"},{"line_number":1201,"context_line":"                               dvr_router_ids\u003dNone):"},{"line_number":1202,"context_line":"        \"\"\"Query floating_ips that relate to list of router_ids with scope."},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        This is different than the regular get_floatingips in that it finds the"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_c5282436","line":1201,"range":{"start_line":1200,"start_character":46,"end_line":1201,"end_character":50},"in_reply_to":"da6ed579_3fa112aa","updated":"2016-01-18 16:29:27.000000000","message":"Done","commit_id":"ba775461de037b6b40ed967f4e455b4328775f35"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"7670c0b0b6fc925e2369c081fa052105873a3493","unresolved":false,"context_lines":[{"line_number":1211,"context_line":"        if not router_ids:"},{"line_number":1212,"context_line":"            return []"},{"line_number":1213,"context_line":"        if dvr_router_ids:"},{"line_number":1214,"context_line":"            router_ids \u003d set(router_ids) - set(dvr_router_ids)"},{"line_number":1215,"context_line":""},{"line_number":1216,"context_line":"        query \u003d context.session.query(FloatingIP,"},{"line_number":1217,"context_line":"                                      models_v2.SubnetPool.address_scope_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_ffe7faa9","line":1214,"range":{"start_line":1214,"start_character":12,"end_line":1214,"end_character":62},"updated":"2016-01-18 15:54:55.000000000","message":"this shouldn\u0027t be done if host is None right? Please move this after 1231","commit_id":"ba775461de037b6b40ed967f4e455b4328775f35"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"2517403f13601cef1ec3edd1518be35f1042092a","unresolved":false,"context_lines":[{"line_number":1211,"context_line":"        if not router_ids:"},{"line_number":1212,"context_line":"            return []"},{"line_number":1213,"context_line":"        if dvr_router_ids:"},{"line_number":1214,"context_line":"            router_ids \u003d set(router_ids) - set(dvr_router_ids)"},{"line_number":1215,"context_line":""},{"line_number":1216,"context_line":"        query \u003d context.session.query(FloatingIP,"},{"line_number":1217,"context_line":"                                      models_v2.SubnetPool.address_scope_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da6ed579_851b0c1c","line":1214,"range":{"start_line":1214,"start_character":12,"end_line":1214,"end_character":62},"in_reply_to":"da6ed579_ffe7faa9","updated":"2016-01-18 16:29:27.000000000","message":"Done","commit_id":"ba775461de037b6b40ed967f4e455b4328775f35"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"d680ffd6ad422640dd34a094a64f258ce168c861","unresolved":false,"context_lines":[{"line_number":1240,"context_line":"                router_ids \u003d set(router_ids) - set(dvr_router_ids)"},{"line_number":1241,"context_line":"                query \u003d query.filter(sa.or_("},{"line_number":1242,"context_line":"                    FloatingIP.router_id.in_(router_ids),"},{"line_number":1243,"context_line":"                    ((FloatingIP.router_id.in_(dvr_router_ids)) \u0026"},{"line_number":1244,"context_line":"                     (ml2_models.PortBinding.host \u003d\u003d host))))"},{"line_number":1245,"context_line":"            else:"},{"line_number":1246,"context_line":"                query \u003d query.filter(FloatingIP.router_id.in_(router_ids))"},{"line_number":1247,"context_line":"                query \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d host)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba63e191_23793afd","line":1244,"range":{"start_line":1243,"start_character":20,"end_line":1244,"end_character":61},"updated":"2016-01-19 02:56:38.000000000","message":"Extra parenthesis here.","commit_id":"505574f4beebbc93bb64b3dd650fd44f2495d32a"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"ccd18f5b439f91dd9d097867bd0d71bfb5e95d4a","unresolved":false,"context_lines":[{"line_number":1240,"context_line":"                router_ids \u003d set(router_ids) - set(dvr_router_ids)"},{"line_number":1241,"context_line":"                query \u003d query.filter(sa.or_("},{"line_number":1242,"context_line":"                    FloatingIP.router_id.in_(router_ids),"},{"line_number":1243,"context_line":"                    ((FloatingIP.router_id.in_(dvr_router_ids)) \u0026"},{"line_number":1244,"context_line":"                     (ml2_models.PortBinding.host \u003d\u003d host))))"},{"line_number":1245,"context_line":"            else:"},{"line_number":1246,"context_line":"                query \u003d query.filter(FloatingIP.router_id.in_(router_ids))"},{"line_number":1247,"context_line":"                query \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d host)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba63e191_065c3924","line":1244,"range":{"start_line":1243,"start_character":20,"end_line":1244,"end_character":61},"in_reply_to":"ba63e191_23793afd","updated":"2016-01-19 06:45:42.000000000","message":"They are necessary indeed, not for common python statement, but for sqlalchemy.","commit_id":"505574f4beebbc93bb64b3dd650fd44f2495d32a"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"d680ffd6ad422640dd34a094a64f258ce168c861","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"            else:"},{"line_number":1246,"context_line":"                query \u003d query.filter(FloatingIP.router_id.in_(router_ids))"},{"line_number":1247,"context_line":"                query \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d host)"},{"line_number":1248,"context_line":"        else:"},{"line_number":1249,"context_line":"            query \u003d query.filter(FloatingIP.router_id.in_(router_ids))"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"        return [self._make_floatingip_dict_with_scope(*row)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba63e191_01b42bf0","line":1248,"updated":"2016-01-19 02:56:38.000000000","message":"I think this block is a bit complicating. And the result  highly depends on the input parameters.\n\n1) \u0027host\u0027+\u0027dvr_router_ids\u0027 means dvr_snat agent\n\n2) only \u0027host\u0027 means dvr agent.\n\n3) no \u0027host\u0027 and no \u0027dvr_router_ids\u0027 means other agent.\n\nAs a base class, (I think)L3_NAT_dbonly_mixin should be unaware of router type. Maybe you can override this method in l3_dvr_db.py to better fit the requirement of DVR.","commit_id":"505574f4beebbc93bb64b3dd650fd44f2495d32a"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"ccd18f5b439f91dd9d097867bd0d71bfb5e95d4a","unresolved":false,"context_lines":[{"line_number":1245,"context_line":"            else:"},{"line_number":1246,"context_line":"                query \u003d query.filter(FloatingIP.router_id.in_(router_ids))"},{"line_number":1247,"context_line":"                query \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d host)"},{"line_number":1248,"context_line":"        else:"},{"line_number":1249,"context_line":"            query \u003d query.filter(FloatingIP.router_id.in_(router_ids))"},{"line_number":1250,"context_line":""},{"line_number":1251,"context_line":"        return [self._make_floatingip_dict_with_scope(*row)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba63e191_26d05d53","line":1248,"in_reply_to":"ba63e191_01b42bf0","updated":"2016-01-19 06:45:42.000000000","message":"Yes, I updated comments to explain what parameters \u0027host\u0027 and \u0027dvr_router_ids\u0027 work for, but that doesn\u0027t mean L3_NAT_dbonly_mixin should be aware of router type here. Indeed, no agent object is using in _get_sync_floating_ips or in _get_router_info_list. \n\nIf override _get_sync_floating_ips in l3_dvr_db.py, _get_router_info_list seems need override too. That way comes more expensive.","commit_id":"505574f4beebbc93bb64b3dd650fd44f2495d32a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"eee1bcf37c3de44d4a44ecaa05ab717de8ba232e","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"        d[\u0027fixed_ip_address_scope\u0027] \u003d scope_id"},{"line_number":1198,"context_line":"        return d"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _get_sync_floating_ips(self, context, router_ids, host\u003dNone,"},{"line_number":1201,"context_line":"                               dvr_router_ids\u003dNone):"},{"line_number":1202,"context_line":"        \"\"\"Query floating_ips that relate to list of router_ids with scope."},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        This is different than the regular get_floatingips in that it finds the"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a68dd71_64415a0b","line":1201,"range":{"start_line":1200,"start_character":8,"end_line":1201,"end_character":51},"updated":"2016-01-21 07:14:01.000000000","message":"This method indeed became too complicated, I thought parameters description will make things better but on a fresh look I see it\u0027s not, sorry for a mess. I\u0027d suggest to not pass dvr_router_ids here, instead call it 2 times from _get_dvr_sync_data() for dvr_snat agent: first time passing host and dvr router_ids, second time just non-dvr router ids and no host. This will keep dvr logic in l3_dvr_db","commit_id":"b6246a5c013821773fcac12394a5c4a8ceb62111"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"c434c890c3796537230bd6a4fb6734869f642b37","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"        d[\u0027fixed_ip_address_scope\u0027] \u003d scope_id"},{"line_number":1198,"context_line":"        return d"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"    def _get_sync_floating_ips(self, context, router_ids, host\u003dNone,"},{"line_number":1201,"context_line":"                               dvr_router_ids\u003dNone):"},{"line_number":1202,"context_line":"        \"\"\"Query floating_ips that relate to list of router_ids with scope."},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        This is different than the regular get_floatingips in that it finds the"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a68dd71_81731bb1","line":1201,"range":{"start_line":1200,"start_character":8,"end_line":1201,"end_character":51},"in_reply_to":"9a68dd71_64415a0b","updated":"2016-01-21 09:06:53.000000000","message":"Well, previously, I modified L1236-L1249 is try to decrease unnecessary DB queries, seems those code become disaster now.\nwhat a pity.\n\nOK, split DVR and non-DVR router ids to two queries is ok to me.","commit_id":"b6246a5c013821773fcac12394a5c4a8ceb62111"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8dd9d3bc9bdc70da3ee5bec32f1968180e901e80","unresolved":false,"context_lines":[{"line_number":43,"context_line":"from neutron import manager"},{"line_number":44,"context_line":"from neutron.plugins.common import constants"},{"line_number":45,"context_line":"from neutron.plugins.common import utils as p_utils"},{"line_number":46,"context_line":"from neutron.plugins.ml2 import models as ml2_models"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9aed3d3a_314adc26","line":46,"updated":"2016-02-29 15:23:18.000000000","message":"i do not think that generic code should be importing specific plugin models.","commit_id":"7adb1031a002918f33dd519363b1956d70c1d264"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8dd9d3bc9bdc70da3ee5bec32f1968180e901e80","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"        # Filter out on fixed_ip binding host if host is given"},{"line_number":1365,"context_line":"        if host:"},{"line_number":1366,"context_line":"            query \u003d query.join(ml2_models.PortBinding,"},{"line_number":1367,"context_line":"                ml2_models.PortBinding.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":1368,"context_line":"            query \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d host)"},{"line_number":1369,"context_line":"        # Outer join of Subnet can cause each ip to have more than one row."},{"line_number":1370,"context_line":"        query \u003d query.outerjoin(models_v2.Subnet,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9aed3d3a_d10610cd","line":1367,"updated":"2016-02-29 15:23:18.000000000","message":"what if i am using plugin foo that is not a ml2 plugin?","commit_id":"7adb1031a002918f33dd519363b1956d70c1d264"}],"neutron/db/l3_dvr_db.py":[{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"3de59c399436f6e31b147ceaabcf7e8d81288060","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        if self._get_agent_mode(agent) \u003d\u003d l3_const.L3_AGENT_MODE_DVR:"},{"line_number":496,"context_line":"            dvr_host \u003d host"},{"line_number":497,"context_line":"        else:"},{"line_number":498,"context_line":"            dvr_host \u003d None"},{"line_number":499,"context_line":"        routers, interfaces, floating_ips \u003d self._get_router_info_list("},{"line_number":500,"context_line":"            context, router_ids\u003drouter_ids, active\u003dactive,"},{"line_number":501,"context_line":"            device_owners\u003dl3_const.ROUTER_INTERFACE_OWNERS, host\u003ddvr_host)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da6ed579_311f996d","line":498,"range":{"start_line":498,"start_character":23,"end_line":498,"end_character":27},"updated":"2016-01-14 19:27:05.000000000","message":"I don\u0027t understand why we don\u0027t populate the host if it is not a \u0027dvr\u0027 agent.\nwhat is the state when we have a single node scenario?","commit_id":"5778abdb437ef1209bfa74671fbb4b303eff746b"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"605c7a63427efb390108b8107d455a998543a9dd","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        if self._get_agent_mode(agent) \u003d\u003d l3_const.L3_AGENT_MODE_DVR:"},{"line_number":496,"context_line":"            dvr_host \u003d host"},{"line_number":497,"context_line":"        else:"},{"line_number":498,"context_line":"            dvr_host \u003d None"},{"line_number":499,"context_line":"        routers, interfaces, floating_ips \u003d self._get_router_info_list("},{"line_number":500,"context_line":"            context, router_ids\u003drouter_ids, active\u003dactive,"},{"line_number":501,"context_line":"            device_owners\u003dl3_const.ROUTER_INTERFACE_OWNERS, host\u003ddvr_host)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da6ed579_8f888b72","line":498,"range":{"start_line":498,"start_character":23,"end_line":498,"end_character":27},"in_reply_to":"da6ed579_311f996d","updated":"2016-01-15 02:50:59.000000000","message":"Oops, that\u0027s a stupid mistake I made. It should be following in L508:\n    fip[\u0027host\u0027] \u003d host\n\nThanks for your review :)","commit_id":"5778abdb437ef1209bfa74671fbb4b303eff746b"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"4c50e12275165edb60cb4d23d259b6ceccc4eca5","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            router \u003d routers_dict.get(floating_ip[\u0027router_id\u0027])"},{"line_number":472,"context_line":"            # NOTE(lizk): For DVR router on \u0027dvr_snat\u0027 mode agent, only"},{"line_number":473,"context_line":"            # return floatingips bound the host."},{"line_number":474,"context_line":"            if (fips_on_host and"},{"line_number":475,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":476,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":477,"context_line":"                continue"},{"line_number":478,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":479,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da6ed579_ec8a6863","line":476,"range":{"start_line":474,"start_character":12,"end_line":476,"end_character":54},"updated":"2016-01-15 09:21:48.000000000","message":"I don\u0027t see a check for \u0027dvr_snat\u0027 agent mode here. Am I missing something?","commit_id":"2f63c6f1fd5ef961f616f48e102c44a7ec20b09a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"e450dade7b74a987443486efa37e4a953543c85e","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            router \u003d routers_dict.get(floating_ip[\u0027router_id\u0027])"},{"line_number":472,"context_line":"            # NOTE(lizk): For DVR router on \u0027dvr_snat\u0027 mode agent, only"},{"line_number":473,"context_line":"            # return floatingips bound the host."},{"line_number":474,"context_line":"            if (fips_on_host and"},{"line_number":475,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":476,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":477,"context_line":"                continue"},{"line_number":478,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":479,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da6ed579_cc98bb7a","line":476,"range":{"start_line":474,"start_character":12,"end_line":476,"end_character":54},"in_reply_to":"da6ed579_4029f488","updated":"2016-01-15 11:51:42.000000000","message":"While I agree with your points, I think it is quite hard to keep in mind all of that, so adding check here for agent mode seems not a big deal, wouldn\u0027t you agree?","commit_id":"2f63c6f1fd5ef961f616f48e102c44a7ec20b09a"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"cda19589ede354c840c48a55938dbeb01475fc83","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            router \u003d routers_dict.get(floating_ip[\u0027router_id\u0027])"},{"line_number":472,"context_line":"            # NOTE(lizk): For DVR router on \u0027dvr_snat\u0027 mode agent, only"},{"line_number":473,"context_line":"            # return floatingips bound the host."},{"line_number":474,"context_line":"            if (fips_on_host and"},{"line_number":475,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":476,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":477,"context_line":"                continue"},{"line_number":478,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":479,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da6ed579_4029f488","line":476,"range":{"start_line":474,"start_character":12,"end_line":476,"end_character":54},"in_reply_to":"da6ed579_a2531916","updated":"2016-01-15 10:58:00.000000000","message":"Previously, I think https://review.openstack.org/#/c/239908 (parent patch of this one) can make only \u0027dvr_snat\u0027 or \u0027dvr\u0027 mode agent call _get_dvr_sync_data, and by that, routers on \u0027legacy\u0027 mode agent will be processed by get_sync_data.\n\nBut now, only one thing I can\u0027t confirm, if _get_dvr_sync_data can be called for agent in \u0027legacy\u0027 mode.\n\nPer [2] and [1], it seems in current implement, _get_active_l3_agent_routers_sync_data will be called, not _get_dvr_sync_data. And per [3], only agent mode belongs to DVR will be processed by _get_dvr_sync_data. If all these things work as I considered, we can ignore \u0027legacy\u0027 mode agent here.\n\nBut I\u0027m not sure in what kind of scenario will  supported_extension_aliases not contain \"l3-ha\"/L3_HA_MODE_EXT_ALIAS.\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/services/l3_router/l3_router_plugin.py#L37-L56\n[2] https://github.com/openstack/neutron/blob/master/neutron/db/l3_dvrscheduler_db.py#L385\n[3] https://review.openstack.org/#/c/239908/12/neutron/db/l3_hamode_db.py","commit_id":"2f63c6f1fd5ef961f616f48e102c44a7ec20b09a"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"1c16969a63d68b10386a6f6028ff6a9f129bc380","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            router \u003d routers_dict.get(floating_ip[\u0027router_id\u0027])"},{"line_number":472,"context_line":"            # NOTE(lizk): For DVR router on \u0027dvr_snat\u0027 mode agent, only"},{"line_number":473,"context_line":"            # return floatingips bound the host."},{"line_number":474,"context_line":"            if (fips_on_host and"},{"line_number":475,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":476,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":477,"context_line":"                continue"},{"line_number":478,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":479,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da6ed579_7a54dcc8","line":476,"range":{"start_line":474,"start_character":12,"end_line":476,"end_character":54},"in_reply_to":"da6ed579_cc98bb7a","updated":"2016-01-15 13:59:26.000000000","message":"Sure, that\u0027s OK to me.","commit_id":"2f63c6f1fd5ef961f616f48e102c44a7ec20b09a"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a639b41c8494c21a4e1e2dec342fcf1562c42887","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            router \u003d routers_dict.get(floating_ip[\u0027router_id\u0027])"},{"line_number":472,"context_line":"            # NOTE(lizk): For DVR router on \u0027dvr_snat\u0027 mode agent, only"},{"line_number":473,"context_line":"            # return floatingips bound the host."},{"line_number":474,"context_line":"            if (fips_on_host and"},{"line_number":475,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":476,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":477,"context_line":"                continue"},{"line_number":478,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":479,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da6ed579_a2531916","line":476,"range":{"start_line":474,"start_character":12,"end_line":476,"end_character":54},"in_reply_to":"da6ed579_e7b3634b","updated":"2016-01-15 09:53:50.000000000","message":"Seems you\u0027re missing agents in legacy mode","commit_id":"2f63c6f1fd5ef961f616f48e102c44a7ec20b09a"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"62abe7da8aa5286c6e4abb1d76164dfee70a97aa","unresolved":false,"context_lines":[{"line_number":471,"context_line":"            router \u003d routers_dict.get(floating_ip[\u0027router_id\u0027])"},{"line_number":472,"context_line":"            # NOTE(lizk): For DVR router on \u0027dvr_snat\u0027 mode agent, only"},{"line_number":473,"context_line":"            # return floatingips bound the host."},{"line_number":474,"context_line":"            if (fips_on_host and"},{"line_number":475,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":476,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":477,"context_line":"                continue"},{"line_number":478,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":479,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da6ed579_e7b3634b","line":476,"range":{"start_line":474,"start_character":12,"end_line":476,"end_character":54},"in_reply_to":"da6ed579_ec8a6863","updated":"2016-01-15 09:38:10.000000000","message":"First, dvr_host will only be None in L499-L502 when it\u0027s a \u0027dvr_snat\u0027 mode agent.\nThen in L508-L513, fips_on_host will be filled when dvr_host is None, which means it\u0027s a \u0027dvr_snat\u0027 mode agent. And in there, as you can see, only floatingips on DVR router and bound to \u0027dvr_snat\u0027 mode agent will be selected.\nFinally, in L474, if fips_on_host is not empty, we do processing like comments in L472-473 described. If fips_on_host is empty, all floatingips we have in @param floating_ips should be configured by agent.\n\nHope that can help you to understand, or maybe I need update comments in L472-L473 ?","commit_id":"2f63c6f1fd5ef961f616f48e102c44a7ec20b09a"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"2534743b96c3353b2309a179949ddb491cc936b4","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        return routers_dict"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def _process_floating_ips_dvr(self, context, routers_dict,"},{"line_number":467,"context_line":"                                  floating_ips, host, agent, fips_on_host):"},{"line_number":468,"context_line":"        fip_sync_interfaces \u003d None"},{"line_number":469,"context_line":"        LOG.debug(\"FIP Agent : %s \", agent.id)"},{"line_number":470,"context_line":"        for floating_ip in floating_ips:"}],"source_content_type":"text/x-python","patch_set":8,"id":"da6ed579_aec3bf37","line":467,"range":{"start_line":467,"start_character":48,"end_line":467,"end_character":52},"updated":"2016-01-17 10:43:28.000000000","message":"According to your intention, the host in param list is no longer needed.","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e6c669b0f4daafa92e25e0f7a47b505b62511fb1","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        return routers_dict"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def _process_floating_ips_dvr(self, context, routers_dict,"},{"line_number":467,"context_line":"                                  floating_ips, host, agent, fips_on_host):"},{"line_number":468,"context_line":"        fip_sync_interfaces \u003d None"},{"line_number":469,"context_line":"        LOG.debug(\"FIP Agent : %s \", agent.id)"},{"line_number":470,"context_line":"        for floating_ip in floating_ips:"}],"source_content_type":"text/x-python","patch_set":8,"id":"da6ed579_3fe61be9","line":467,"range":{"start_line":467,"start_character":48,"end_line":467,"end_character":52},"in_reply_to":"da6ed579_aec3bf37","updated":"2016-01-17 17:43:42.000000000","message":"Yes, it should removed.","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"2534743b96c3353b2309a179949ddb491cc936b4","unresolved":false,"context_lines":[{"line_number":474,"context_line":"            if (self._get_agent_mode(agent) \u003d\u003d l3_const.L3_AGENT_MODE_DVR_SNAT"},{"line_number":475,"context_line":"                and fips_on_host and"},{"line_number":476,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":477,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":478,"context_line":"                continue"},{"line_number":479,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":480,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da6ed579_2eafcf7e","line":477,"updated":"2016-01-17 10:43:28.000000000","message":"I think the logic here is a bit complicating. And comparing to the original code, it seems like you only consider dvr_snat agent here. I don\u0027t know why that agent mode is special in this method. \n\nI think you can filter floatingips that are not in the host outside this method(just like what you have done to the dvr agent mode). That will make things simpler.","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e6c669b0f4daafa92e25e0f7a47b505b62511fb1","unresolved":false,"context_lines":[{"line_number":474,"context_line":"            if (self._get_agent_mode(agent) \u003d\u003d l3_const.L3_AGENT_MODE_DVR_SNAT"},{"line_number":475,"context_line":"                and fips_on_host and"},{"line_number":476,"context_line":"                router[\u0027distributed\u0027] and"},{"line_number":477,"context_line":"                not floating_ip[\u0027id\u0027] in fips_on_host):"},{"line_number":478,"context_line":"                continue"},{"line_number":479,"context_line":"            router.setdefault(l3_const.FLOATINGIP_KEY, [])"},{"line_number":480,"context_line":"            router[l3_const.FLOATINGIP_KEY].append(floating_ip)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da6ed579_3ffdbb6e","line":477,"in_reply_to":"da6ed579_2eafcf7e","updated":"2016-01-17 17:43:42.000000000","message":"L474 is result of discussion in PS6. Since https://review.openstack.org/#/c/239908/ is merged, I think we can remove L474 now, and comments should be updated to tell that, only agent in \u0027dvr\u0027 or \u0027dvr_snat\u0027 mode will enter this method for processing now.","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"2534743b96c3353b2309a179949ddb491cc936b4","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                context, router_ids\u003drouter_ids, host\u003dhost)"},{"line_number":512,"context_line":"            for fip in agent_hosted_floating_ips:"},{"line_number":513,"context_line":"                if routers_dict[fip[\u0027router_id\u0027]][\u0027distributed\u0027]:"},{"line_number":514,"context_line":"                    fips_on_host.add(fip[\u0027id\u0027])"},{"line_number":515,"context_line":"        self._process_floating_ips_dvr(context, routers_dict,"},{"line_number":516,"context_line":"                                       floating_ips, host, agent, fips_on_host)"},{"line_number":517,"context_line":"        ports_to_populate \u003d []"}],"source_content_type":"text/x-python","patch_set":8,"id":"da6ed579_2ee6efe8","line":514,"updated":"2016-01-17 10:43:28.000000000","message":"I think you can eliminate line 507 to line 517, by adding DVR_SNAT at line 500.","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e6c669b0f4daafa92e25e0f7a47b505b62511fb1","unresolved":false,"context_lines":[{"line_number":511,"context_line":"                context, router_ids\u003drouter_ids, host\u003dhost)"},{"line_number":512,"context_line":"            for fip in agent_hosted_floating_ips:"},{"line_number":513,"context_line":"                if routers_dict[fip[\u0027router_id\u0027]][\u0027distributed\u0027]:"},{"line_number":514,"context_line":"                    fips_on_host.add(fip[\u0027id\u0027])"},{"line_number":515,"context_line":"        self._process_floating_ips_dvr(context, routers_dict,"},{"line_number":516,"context_line":"                                       floating_ips, host, agent, fips_on_host)"},{"line_number":517,"context_line":"        ports_to_populate \u003d []"}],"source_content_type":"text/x-python","patch_set":8,"id":"da6ed579_5f5e6f77","line":514,"in_reply_to":"da6ed579_2ee6efe8","updated":"2016-01-17 17:43:42.000000000","message":"I guess \"fips_on_host\" is confusing you, but I\u0027m not sure what kind of name should be better.\n\nWhile agent in \u0027dvr\u0027 mode can only be scheduled DVR router, agent in \u0027dvr_snat\u0027 mode can host all kind of routers. That will make it complex to judge whether floatingips should return.\n\n[1]For agent in \u0027dvr_snat\u0027 mode, when a router is hosted on it:\n  * for DVR router, we should only return floatingip bound on it;\n  * for non-DVR router, we should return all floatingips belong to this router.\n\nSo we are going to filter floatingips bound on agent in \u0027dvr_snat\u0027 mode here, then let L474-478 do processing in [1].","commit_id":"ef8af926a95dc2e5ba744db1405aa57becb7e0a5"}]}