)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b6f2df02742e61f0b3d175d61e8e187a510ecae3","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     liuchengqian90 \u003cxieling1990.happy@163.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-12-31 15:16:58 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"When there is no rule in the policy received after the ovs-agent restarts,"},{"line_number":8,"context_line":"all rules of the inventory should be deleted."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: Iaaff10dfa8ac6ab8c9dead3124e2bb3caa03a665"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffd0ebdf_8c9f7bb3","line":7,"updated":"2019-01-03 09:30:52.000000000","message":"Please write some short summary of change in first line and then more detailed description in next lines of commit message.","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"}],"neutron/agent/l2/extensions/qos.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"956d878f4d0160b5171038901f6693a666c57afa","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            if old_qos_policy:"},{"line_number":272,"context_line":"                self.qos_driver.delete(port, old_qos_policy)"},{"line_number":273,"context_line":"                self.qos_driver.update(port, qos_policy)"},{"line_number":274,"context_line":"            elif not getattr(qos_policy, \u0027rules\u0027, None):"},{"line_number":275,"context_line":"                # There are no rules in new qos_policy,"},{"line_number":276,"context_line":"                # so it should be deleted."},{"line_number":277,"context_line":"                # But new policy has a relationship with the port,"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfd5e7cf_c1b1aee3","line":274,"updated":"2019-01-08 08:42:22.000000000","message":"I don\u0027t understand why are you using getattr. qos_policy object always have \"rules\" attribute.","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"00c371fc8cd52ec62ec819022bdca2d744a4a211","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            if old_qos_policy:"},{"line_number":272,"context_line":"                self.qos_driver.delete(port, old_qos_policy)"},{"line_number":273,"context_line":"                self.qos_driver.update(port, qos_policy)"},{"line_number":274,"context_line":"            elif not getattr(qos_policy, \u0027rules\u0027, None):"},{"line_number":275,"context_line":"                # There are no rules in new qos_policy,"},{"line_number":276,"context_line":"                # so it should be deleted."},{"line_number":277,"context_line":"                # But new policy has a relationship with the port,"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfd5e7cf_af2eb5f7","line":274,"in_reply_to":"dfd5e7cf_8ea2d899","updated":"2019-01-09 11:10:33.000000000","message":"+1, lets just use qos_policy.rules here","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"},{"author":{"_account_id":15309,"name":"zhaobo","email":"bzhaojyathousandy@gmail.com","username":"ZhaoBo"},"change_message_id":"e655145faa8c107931e6f703c6637d2050fe82db","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            if old_qos_policy:"},{"line_number":272,"context_line":"                self.qos_driver.delete(port, old_qos_policy)"},{"line_number":273,"context_line":"                self.qos_driver.update(port, qos_policy)"},{"line_number":274,"context_line":"            elif not getattr(qos_policy, \u0027rules\u0027, None):"},{"line_number":275,"context_line":"                # There are no rules in new qos_policy,"},{"line_number":276,"context_line":"                # so it should be deleted."},{"line_number":277,"context_line":"                # But new policy has a relationship with the port,"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfd5e7cf_8ea2d899","line":274,"in_reply_to":"dfd5e7cf_a77a7a21","updated":"2019-01-09 09:28:57.000000000","message":"why not just qos_policy.rules ?","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"},{"author":{"_account_id":25643,"name":"Chengqian Liu","email":"xieling1990.happy@163.com","username":"25643"},"change_message_id":"82124466c5a82483d59a2197fddcc34c43cb8c68","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            if old_qos_policy:"},{"line_number":272,"context_line":"                self.qos_driver.delete(port, old_qos_policy)"},{"line_number":273,"context_line":"                self.qos_driver.update(port, qos_policy)"},{"line_number":274,"context_line":"            elif not getattr(qos_policy, \u0027rules\u0027, None):"},{"line_number":275,"context_line":"                # There are no rules in new qos_policy,"},{"line_number":276,"context_line":"                # so it should be deleted."},{"line_number":277,"context_line":"                # But new policy has a relationship with the port,"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfd5e7cf_fed54de6","line":274,"in_reply_to":"dfd5e7cf_af2eb5f7","updated":"2019-01-09 16:40:43.000000000","message":"Two reasons：\n1. We know that qos must have a \u0027rules\u0027 attribute because setattr is called on the server side to set this attribute.  So \u0027getattr\u0027 and \u0027policy.rules\u0027 are the same effect.\n2.  policy.QosPolicy is used in https://github.com/openstack/neutron/blob/master/neutron/tests/unit/agent/l2/extensions/test_qos.py but does not have the \u0027rules\u0027 attribute. If \u0027policy.rules\u0027 is used here , we need to change a lot of test cases , for example \u0027test_handle_known_port\u0027 and \u0027test_delete_known_port\u0027.","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"},{"author":{"_account_id":25643,"name":"Chengqian Liu","email":"xieling1990.happy@163.com","username":"25643"},"change_message_id":"a6ea44190fc639311fcb8982915410135b2f8761","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            if old_qos_policy:"},{"line_number":272,"context_line":"                self.qos_driver.delete(port, old_qos_policy)"},{"line_number":273,"context_line":"                self.qos_driver.update(port, qos_policy)"},{"line_number":274,"context_line":"            elif not getattr(qos_policy, \u0027rules\u0027, None):"},{"line_number":275,"context_line":"                # There are no rules in new qos_policy,"},{"line_number":276,"context_line":"                # so it should be deleted."},{"line_number":277,"context_line":"                # But new policy has a relationship with the port,"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfd5e7cf_a77a7a21","line":274,"in_reply_to":"dfd5e7cf_c1b1aee3","updated":"2019-01-08 10:30:36.000000000","message":"thank you for your review.\nAs you said, qos_policy object always have \"rules\" attribute.\nSo, I use it to get the value of \u0027rules\u0027 to check.","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"7e2d10056f9a6dc1e306df4a11b0b26326a18e52","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            if old_qos_policy:"},{"line_number":272,"context_line":"                self.qos_driver.delete(port, old_qos_policy)"},{"line_number":273,"context_line":"                self.qos_driver.update(port, qos_policy)"},{"line_number":274,"context_line":"            elif not getattr(qos_policy, \u0027rules\u0027, None):"},{"line_number":275,"context_line":"                # There are no rules in new qos_policy,"},{"line_number":276,"context_line":"                # so it should be deleted."},{"line_number":277,"context_line":"                # But new policy has a relationship with the port,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfdaf3ff_5309d2da","line":274,"in_reply_to":"dfd5e7cf_fed54de6","updated":"2019-01-16 11:37:55.000000000","message":"I tried with qos_policy.rules and no unit test failed for me (I run for neutron.tests.unit.agent.l2.extensions.test_qos).\n\nOtherwise: if in that case if I have to choose between changing unit tests and keep the \"production code\" cleaner I would choose to change unit tests, and keep the \"production code\" cleaner.","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"}],"neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b6f2df02742e61f0b3d175d61e8e187a510ecae3","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                      \"It seems that bandwidth_limit is already deleted\","},{"line_number":76,"context_line":"                      port_id)"},{"line_number":77,"context_line":"            return"},{"line_number":78,"context_line":"        vif_port \u003d vif_port or port.get(\u0027vif_port\u0027)"},{"line_number":79,"context_line":"        self.br_int.delete_egress_bw_limit_for_port(vif_port.port_name)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def delete_bandwidth_limit_ingress(self, port):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_ccbb430c","line":78,"updated":"2019-01-03 09:30:52.000000000","message":"You don\u0027t need this. If via_port is None then it means that it was None in L67 and nothing would change from L67 to this one :)","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"},{"author":{"_account_id":25643,"name":"Chengqian Liu","email":"xieling1990.happy@163.com","username":"25643"},"change_message_id":"87bc181de7afdaf6c4e75b26d7697ae6345831a9","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                      \"It seems that bandwidth_limit is already deleted\","},{"line_number":76,"context_line":"                      port_id)"},{"line_number":77,"context_line":"            return"},{"line_number":78,"context_line":"        vif_port \u003d vif_port or port.get(\u0027vif_port\u0027)"},{"line_number":79,"context_line":"        self.br_int.delete_egress_bw_limit_for_port(vif_port.port_name)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def delete_bandwidth_limit_ingress(self, port):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_c7031c0f","line":78,"in_reply_to":"ffd0ebdf_ccbb430c","updated":"2019-01-03 11:01:06.000000000","message":"Thank you for your review.\n\n1. It should get information from the cache(self.ports) when deleting a vm because vif_port in L67  is None.\n\n2. It should get information from vif_port in L67 when restarting neutron-openvswitch-agent because there is no cache for now.\n\nSo I think L78 is necessary.","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b6f2df02742e61f0b3d175d61e8e187a510ecae3","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                      \"It seems that bandwidth_limit is already deleted\","},{"line_number":91,"context_line":"                      port_id)"},{"line_number":92,"context_line":"            return"},{"line_number":93,"context_line":"        vif_port \u003d vif_port or port.get(\u0027vif_port\u0027)"},{"line_number":94,"context_line":"        self.br_int.delete_ingress_bw_limit_for_port(vif_port.port_name)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def create_dscp_marking(self, port, rule):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_2c40cf07","line":93,"updated":"2019-01-03 09:30:52.000000000","message":"ditto","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"},{"author":{"_account_id":25643,"name":"Chengqian Liu","email":"xieling1990.happy@163.com","username":"25643"},"change_message_id":"87bc181de7afdaf6c4e75b26d7697ae6345831a9","unresolved":false,"context_lines":[{"line_number":90,"context_line":"                      \"It seems that bandwidth_limit is already deleted\","},{"line_number":91,"context_line":"                      port_id)"},{"line_number":92,"context_line":"            return"},{"line_number":93,"context_line":"        vif_port \u003d vif_port or port.get(\u0027vif_port\u0027)"},{"line_number":94,"context_line":"        self.br_int.delete_ingress_bw_limit_for_port(vif_port.port_name)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def create_dscp_marking(self, port, rule):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_992c9c9a","line":93,"in_reply_to":"ffd0ebdf_2c40cf07","updated":"2019-01-03 11:01:06.000000000","message":"Same as I said above.","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b6f2df02742e61f0b3d175d61e8e187a510ecae3","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                      port[\u0027port_id\u0027])"},{"line_number":149,"context_line":"            return"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        vif_port \u003d vif_port or dscp_port.get(\u0027vif_port\u0027)"},{"line_number":152,"context_line":"        port_num \u003d vif_port.ofport"},{"line_number":153,"context_line":"        self.br_int.uninstall_flows(in_port\u003dport_num, table_id\u003d0, reg2\u003d0)"},{"line_number":154,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_ac535fe2","line":151,"updated":"2019-01-03 09:30:52.000000000","message":"You should move this line above L145 as now You will never try to get vif_port from dscp_port if \"vif_port\" is None","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"},{"author":{"_account_id":25643,"name":"Chengqian Liu","email":"xieling1990.happy@163.com","username":"25643"},"change_message_id":"87bc181de7afdaf6c4e75b26d7697ae6345831a9","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                      port[\u0027port_id\u0027])"},{"line_number":149,"context_line":"            return"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        vif_port \u003d vif_port or dscp_port.get(\u0027vif_port\u0027)"},{"line_number":152,"context_line":"        port_num \u003d vif_port.ofport"},{"line_number":153,"context_line":"        self.br_int.uninstall_flows(in_port\u003dport_num, table_id\u003d0, reg2\u003d0)"},{"line_number":154,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_595b942d","line":151,"in_reply_to":"ffd0ebdf_ac535fe2","updated":"2019-01-03 11:01:06.000000000","message":"Same as I said above.","commit_id":"4e88e06a2631c75be4d140b75f70634d69a20027"},{"author":{"_account_id":15309,"name":"zhaobo","email":"bzhaojyathousandy@gmail.com","username":"ZhaoBo"},"change_message_id":"e655145faa8c107931e6f703c6637d2050fe82db","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                      \"It seems that bandwidth_limit is already deleted\","},{"line_number":76,"context_line":"                      port_id)"},{"line_number":77,"context_line":"            return"},{"line_number":78,"context_line":"        vif_port \u003d vif_port or port.get(\u0027vif_port\u0027)"},{"line_number":79,"context_line":"        self.br_int.delete_egress_bw_limit_for_port(vif_port.port_name)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def delete_bandwidth_limit_ingress(self, port):"}],"source_content_type":"text/x-python","patch_set":9,"id":"dfd5e7cf_293725e6","line":78,"range":{"start_line":78,"start_character":19,"end_line":78,"end_character":51},"updated":"2019-01-09 09:28:57.000000000","message":"I know \"port\" has been overrided by L68 finally, so it\u0027s good for change it from \"port\" to \"cached_port\".\nSo here,\n1. When agent restart, the cached_port must be None in the first loop. So we need to check the port\u0027s vif_port. That\u0027s correct.\n2. Agent runs a period, the cached_port stores the original port info, eventhough we \"can\" update the vif_port, we have to process with the new one if the vif_port get from the new requested port.If we failed, we can still use the cached info to reset the rules.\nI agree this process. This seems cover the all cases.","commit_id":"a74bbb5e0e85adb2462acaad070130cb2add9275"}]}