)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"22f0d5e63695bc8ed33e64c34b94cd3118953f16","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Hamdy Khader \u003chamdyk@mellanox.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-12-05 15:48:51 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove unbound SmartNIC port"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I94da5d6469111dab743cd3b748e1a2abd1770a60"},{"line_number":10,"context_line":"Closes-Bug: #1855260"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3fa7e38b_10a53e48","line":7,"updated":"2019-12-09 17:24:19.000000000","message":"1) Could you add some description here\n2) You say here \"remove unbound smartnic port\" but the code is adding it. What am I missing here?\n3) You should add UTs please.","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"}],"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py":[{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"70f234598cea0735ba5d05ef721bd4047f5eebc0","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            if port_binding[\u0027vnic_type\u0027] \u003d\u003d portbindings.VNIC_SMARTNIC:"},{"line_number":515,"context_line":"                if (port_binding[\u0027host\u0027] \u003d\u003d self.conf.host or"},{"line_number":516,"context_line":"                        self._should_remove_smartnic_port(port_binding)):"},{"line_number":517,"context_line":"                    self._add_port_to_updated_smartnic_ports(port_data,"},{"line_number":518,"context_line":"                                                             port_binding)"},{"line_number":519,"context_line":"                else:"},{"line_number":520,"context_line":"                    # The port doesn\u0027t belong to this Smart NIC,"},{"line_number":521,"context_line":"                    # the reason for this could be multi Smart NIC"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_a7a3e0b0","line":518,"range":{"start_line":517,"start_character":0,"end_line":518,"end_character":74},"updated":"2019-12-08 13:22:32.000000000","message":"you rely here on ironic behaviour that doesnt remove the binding profile when it unbinds the port.[1]\n\nyou will get a key error in _add_port_to_updated_smartnic_ports() in case ironic does not contain the change. (this change is not backward compatible with old ironic)\n\nneed to have some logic to deal with this case or alternatively not rely on ironic to keep the binding profile, but this will take the two commits to another direction.\n\n[1] https://review.opendev.org/#/c/697471/","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"45c58c1ed3f0aee41a1c6c076932b450bfd10dac","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            if port_binding[\u0027vnic_type\u0027] \u003d\u003d portbindings.VNIC_SMARTNIC:"},{"line_number":515,"context_line":"                if (port_binding[\u0027host\u0027] \u003d\u003d self.conf.host or"},{"line_number":516,"context_line":"                        self._should_remove_smartnic_port(port_binding)):"},{"line_number":517,"context_line":"                    self._add_port_to_updated_smartnic_ports(port_data,"},{"line_number":518,"context_line":"                                                             port_binding)"},{"line_number":519,"context_line":"                else:"},{"line_number":520,"context_line":"                    # The port doesn\u0027t belong to this Smart NIC,"},{"line_number":521,"context_line":"                    # the reason for this could be multi Smart NIC"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d5479f1d","line":518,"range":{"start_line":517,"start_character":0,"end_line":518,"end_character":74},"in_reply_to":"3fa7e38b_27229002","updated":"2019-12-09 09:37:59.000000000","message":"I see, thanks.\n\nAlso what would happen in case you get a port update of an unbinded port that belongs to another Smart-NIC ?","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"},{"author":{"_account_id":22948,"name":"Hamdy Khader","email":"hamdyk@mellanox.com","username":"hamdyk"},"change_message_id":"c945d6f49ca1172026a93afb73faf6ae0c270eb3","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            if port_binding[\u0027vnic_type\u0027] \u003d\u003d portbindings.VNIC_SMARTNIC:"},{"line_number":515,"context_line":"                if (port_binding[\u0027host\u0027] \u003d\u003d self.conf.host or"},{"line_number":516,"context_line":"                        self._should_remove_smartnic_port(port_binding)):"},{"line_number":517,"context_line":"                    self._add_port_to_updated_smartnic_ports(port_data,"},{"line_number":518,"context_line":"                                                             port_binding)"},{"line_number":519,"context_line":"                else:"},{"line_number":520,"context_line":"                    # The port doesn\u0027t belong to this Smart NIC,"},{"line_number":521,"context_line":"                    # the reason for this could be multi Smart NIC"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_27229002","line":518,"range":{"start_line":517,"start_character":0,"end_line":518,"end_character":74},"in_reply_to":"3fa7e38b_a7a3e0b0","updated":"2019-12-08 14:29:19.000000000","message":"if Ironic did clear the binding profile, then the for at line 513 will not be executed (no key error will happen) and the port will not be removed until the next agent sync.","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"},{"author":{"_account_id":22948,"name":"Hamdy Khader","email":"hamdyk@mellanox.com","username":"hamdyk"},"change_message_id":"5f72766edf942a919a011405b8d9168a3a7951be","unresolved":false,"context_lines":[{"line_number":514,"context_line":"            if port_binding[\u0027vnic_type\u0027] \u003d\u003d portbindings.VNIC_SMARTNIC:"},{"line_number":515,"context_line":"                if (port_binding[\u0027host\u0027] \u003d\u003d self.conf.host or"},{"line_number":516,"context_line":"                        self._should_remove_smartnic_port(port_binding)):"},{"line_number":517,"context_line":"                    self._add_port_to_updated_smartnic_ports(port_data,"},{"line_number":518,"context_line":"                                                             port_binding)"},{"line_number":519,"context_line":"                else:"},{"line_number":520,"context_line":"                    # The port doesn\u0027t belong to this Smart NIC,"},{"line_number":521,"context_line":"                    # the reason for this could be multi Smart NIC"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_f835f2db","line":518,"range":{"start_line":517,"start_character":0,"end_line":518,"end_character":74},"in_reply_to":"3fa7e38b_d5479f1d","updated":"2019-12-09 10:29:45.000000000","message":"In that case, we need to make sure that the port is added to OVS db before removing it, will update the code accordingly.","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"22f0d5e63695bc8ed33e64c34b94cd3118953f16","unresolved":false,"context_lines":[{"line_number":525,"context_line":"                             {\u0027port_id\u0027: port[\u0027id\u0027],"},{"line_number":526,"context_line":"                              \u0027host\u0027: self.conf.host})"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"    def _should_remove_smartnic_port(self, port_binding):"},{"line_number":529,"context_line":"        return (not port_binding[\u0027host\u0027] and"},{"line_number":530,"context_line":"                port_binding[\u0027vif_type\u0027] \u003d\u003d portbindings.VIF_TYPE_UNBOUND)"},{"line_number":531,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_508656bc","line":528,"updated":"2019-12-09 17:24:19.000000000","message":"I don\u0027t think the function name is the most adequate one","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"22f0d5e63695bc8ed33e64c34b94cd3118953f16","unresolved":false,"context_lines":[{"line_number":526,"context_line":"                              \u0027host\u0027: self.conf.host})"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"    def _should_remove_smartnic_port(self, port_binding):"},{"line_number":529,"context_line":"        return (not port_binding[\u0027host\u0027] and"},{"line_number":530,"context_line":"                port_binding[\u0027vif_type\u0027] \u003d\u003d portbindings.VIF_TYPE_UNBOUND)"},{"line_number":531,"context_line":""},{"line_number":532,"context_line":"    def treat_smartnic_port(self, smartnic_port_data):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_d09246fc","line":529,"updated":"2019-12-09 17:24:19.000000000","message":"Sorry, I don\u0027t understand this logic. Why a port without host description and unbound should be added to this host smartnic?","commit_id":"5c6cee7686fec9d4102c9a7b82f5a274e05682d1"}]}