)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"efb6aceb8ababb9778ef5319cb0f4bad07270c2b","unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Fixed metering for DVR routers"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Fixed race condition when adding router before its namespaces are created"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: If7f78b72fd1e1be05636c164bd559862cc3498d0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_b974e539","line":13,"range":{"start_line":13,"start_character":6,"end_line":13,"end_character":20},"updated":"2018-12-01 14:38:41.000000000","message":"Please add a LP bug with some details:\n(1) which version of neutron you find this issue? stable? master?\n(2) how to reproduce the issue?\n(3) what will happened after the issue?\n\nNeutron LP bug link:\nhttps://bugs.launchpad.net/neutron","commit_id":"b65b8e557b896ce66976333be540eca427c966bc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"efb6aceb8ababb9778ef5319cb0f4bad07270c2b","unresolved":false,"context_lines":[{"line_number":11,"context_line":"Fixed metering for DVR routers"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Fixed race condition when adding router before its namespaces are created"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: If7f78b72fd1e1be05636c164bd559862cc3498d0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_198079f6","line":14,"updated":"2018-12-01 14:38:41.000000000","message":"Thanks for submit the fix, it\u0027s better to add a bug here to trace the change.","commit_id":"b65b8e557b896ce66976333be540eca427c966bc"}],"neutron/services/metering/drivers/iptables/iptables_driver.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fe392f7b5df342f920de25a0f70c144f4ab996ea","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        created \u003d False"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        if self.snat_iptables_manager is None and self.router[\u0027distributed\u0027]:"},{"line_number":90,"context_line":"            # If distributed routers then we need to apply the"},{"line_number":91,"context_line":"            # metering agent label rules in the snat namespace as well."},{"line_number":92,"context_line":"            snat_ns_name \u003d dvr_snat_ns.SnatNamespace.get_snat_ns_name("}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_dd0fdc34","line":89,"updated":"2019-01-28 21:16:30.000000000","message":"This should probably be reversed - distributed check first.  That way with non-DVR routers we only check one thing not two.","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":28655,"name":"Alexandru Sorodoc","email":"alex@privacysystems.eu","username":"bno1"},"change_message_id":"3ba5e4ec00b59ccd48fda60d780addc7ed64b8c5","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        created \u003d False"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        if self.snat_iptables_manager is None and self.router[\u0027distributed\u0027]:"},{"line_number":90,"context_line":"            # If distributed routers then we need to apply the"},{"line_number":91,"context_line":"            # metering agent label rules in the snat namespace as well."},{"line_number":92,"context_line":"            snat_ns_name \u003d dvr_snat_ns.SnatNamespace.get_snat_ns_name("}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_368aabbe","line":89,"in_reply_to":"9fdfeff1_dd0fdc34","updated":"2019-01-30 11:21:28.000000000","message":"Done","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fe392f7b5df342f920de25a0f70c144f4ab996ea","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                    # in case the selected manager is None pick another one"},{"line_number":163,"context_line":"                    old_rm_im \u003d (old_rm_im"},{"line_number":164,"context_line":"                                 or old_rm.snat_iptables_manager"},{"line_number":165,"context_line":"                                 or old_rm.iptables_manager)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"                    if old_rm_im:"},{"line_number":168,"context_line":"                        with IptablesManagerTransaction(old_rm_im):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_5dc0cce1","line":165,"updated":"2019-01-28 21:16:30.000000000","message":"But this could choose the wrong router namespace, it doesn\u0027t look correct.","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":28655,"name":"Alexandru Sorodoc","email":"alex@privacysystems.eu","username":"bno1"},"change_message_id":"3ba5e4ec00b59ccd48fda60d780addc7ed64b8c5","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                    # in case the selected manager is None pick another one"},{"line_number":163,"context_line":"                    old_rm_im \u003d (old_rm_im"},{"line_number":164,"context_line":"                                 or old_rm.snat_iptables_manager"},{"line_number":165,"context_line":"                                 or old_rm.iptables_manager)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"                    if old_rm_im:"},{"line_number":168,"context_line":"                        with IptablesManagerTransaction(old_rm_im):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_d9a110fa","line":165,"in_reply_to":"9fdfeff1_5dc0cce1","updated":"2019-01-30 11:21:28.000000000","message":"_process_disassociate_metering_label disassociates the metering label for each namespace. This worked fine when only one namespace was used, but when the snat namespace was added those transactions on the namespace weren\u0027t updated too. Here I tried to lock any namespace if the first assignment of old_rm_im is None. Otherwise the agent would fail to disassociate all the iptables rules in certain conditions (eg router is distributed and snat_iptables_manager is None but rules were added to iptables_manager). It\u0027s not perfect but I believe it\u0027s the least broken way of doing this without major modifications.\nIdeally, functions like _process_disassociate_metering_label should be rewritten to take a namespace argument and do the iteration over namespaces and transaction in high-level functions like this one.","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fe392f7b5df342f920de25a0f70c144f4ab996ea","unresolved":false,"context_lines":[{"line_number":443,"context_line":"            # create the managers for them. when a new manager is created, the"},{"line_number":444,"context_line":"            # metering rules have to be added to it."},{"line_number":445,"context_line":"            if rm.create_iptables_managers():"},{"line_number":446,"context_line":"                self._process_associate_metering_label(router)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"            for label_id in rm.metering_labels:"},{"line_number":449,"context_line":"                chain_acc \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_300c0f3a","line":446,"updated":"2019-01-28 21:16:30.000000000","message":"Constantly calling create_iptables_managers() seems wasteful, we can probably cache some status in the object to this effect given the type of router we have.","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":28655,"name":"Alexandru Sorodoc","email":"alex@privacysystems.eu","username":"bno1"},"change_message_id":"3ba5e4ec00b59ccd48fda60d780addc7ed64b8c5","unresolved":false,"context_lines":[{"line_number":443,"context_line":"            # create the managers for them. when a new manager is created, the"},{"line_number":444,"context_line":"            # metering rules have to be added to it."},{"line_number":445,"context_line":"            if rm.create_iptables_managers():"},{"line_number":446,"context_line":"                self._process_associate_metering_label(router)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"            for label_id in rm.metering_labels:"},{"line_number":449,"context_line":"                chain_acc \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_b929ac8a","line":446,"in_reply_to":"9fdfeff1_300c0f3a","updated":"2019-01-30 11:21:28.000000000","message":"If the namespaces needed by the router are found then the call will end up running two `if False` statements. I could add a status cache, something like `if not rm.has_all_namespaces and rm.create_iptables_managers(): ...`. Do you think it is worth implementing this to save a function call and an `if False` check?","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fe392f7b5df342f920de25a0f70c144f4ab996ea","unresolved":false,"context_lines":[{"line_number":447,"context_line":""},{"line_number":448,"context_line":"            for label_id in rm.metering_labels:"},{"line_number":449,"context_line":"                chain_acc \u003d None"},{"line_number":450,"context_line":"                snat_chain_acc \u003d None"},{"line_number":451,"context_line":""},{"line_number":452,"context_line":"            for label_id in rm.metering_labels:"},{"line_number":453,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_bd545822","line":450,"updated":"2019-01-28 21:16:30.000000000","message":"These look like they are supposed to be below.","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"},{"author":{"_account_id":28655,"name":"Alexandru Sorodoc","email":"alex@privacysystems.eu","username":"bno1"},"change_message_id":"3ba5e4ec00b59ccd48fda60d780addc7ed64b8c5","unresolved":false,"context_lines":[{"line_number":447,"context_line":""},{"line_number":448,"context_line":"            for label_id in rm.metering_labels:"},{"line_number":449,"context_line":"                chain_acc \u003d None"},{"line_number":450,"context_line":"                snat_chain_acc \u003d None"},{"line_number":451,"context_line":""},{"line_number":452,"context_line":"            for label_id in rm.metering_labels:"},{"line_number":453,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fdfeff1_d669dff8","line":450,"in_reply_to":"9fdfeff1_bd545822","updated":"2019-01-30 11:21:28.000000000","message":"Done","commit_id":"072795e36058c944a9dc3c2554aac96ad6408091"}]}