)]}'
{"whitebox_tempest_plugin/api/compute/test_sriov.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":62,"context_line":"            return \u0027hostdev\u0027"},{"line_number":63,"context_line":"        elif vnic_type \u003d\u003d \u0027macvtap\u0027:"},{"line_number":64,"context_line":"            return \u0027direct\u0027"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _create_sriov_net(self):"},{"line_number":67,"context_line":"        \"\"\"Create an IPv4 L2 vlan network and subnet.  Physical network"},{"line_number":68,"context_line":"        provider comes from sriov_physnet provided in tempest config"}],"source_content_type":"text/x-python","patch_set":2,"id":"722daffa_b8553cb2","line":65,"updated":"2021-02-01 20:16:25.000000000","message":"+1","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":73,"context_line":"            CONF.network_feature_enabled.provider_net_base_segmentation_id"},{"line_number":74,"context_line":"        physical_net \u003d CONF.whitebox_hardware.sriov_physnet"},{"line_number":75,"context_line":"        net_dict \u003d {"},{"line_number":76,"context_line":"            \u0027shared\u0027: True,"},{"line_number":77,"context_line":"            \u0027provider:network_type\u0027: \u0027vlan\u0027,"},{"line_number":78,"context_line":"            \u0027provider:physical_network\u0027: physical_net,"},{"line_number":79,"context_line":"            \u0027provider:segmentation_id\u0027: vlan_id"}],"source_content_type":"text/x-python","patch_set":2,"id":"427a83b0_f9784d05","line":76,"range":{"start_line":76,"start_character":4,"end_line":76,"end_character":27},"updated":"2021-02-01 20:16:25.000000000","message":"it should not need to be shared and in fact probably should not be as that can break test that expect only 1 network.\n\nwe currently run test tests serially but it would still be better to set this to False or just not set it as that is the default.\n\nshared networks are used wehn you want to share it between different tenants which is not what we want to do.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            CONF.network_feature_enabled.provider_net_base_segmentation_id"},{"line_number":74,"context_line":"        physical_net \u003d CONF.whitebox_hardware.sriov_physnet"},{"line_number":75,"context_line":"        net_dict \u003d {"},{"line_number":76,"context_line":"            \u0027shared\u0027: True,"},{"line_number":77,"context_line":"            \u0027provider:network_type\u0027: \u0027vlan\u0027,"},{"line_number":78,"context_line":"            \u0027provider:physical_network\u0027: physical_net,"},{"line_number":79,"context_line":"            \u0027provider:segmentation_id\u0027: vlan_id"}],"source_content_type":"text/x-python","patch_set":2,"id":"110224cd_87e31239","line":76,"range":{"start_line":76,"start_character":4,"end_line":76,"end_character":27},"in_reply_to":"427a83b0_f9784d05","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":80,"context_line":"        }"},{"line_number":81,"context_line":"        net \u003d self.networks_client.create_network(name\u003dname_net,"},{"line_number":82,"context_line":"                                                  **net_dict)"},{"line_number":83,"context_line":"        self.addCleanup(self.networks_client.delete_network,"},{"line_number":84,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        subnet \u003d self.subnets_client.create_subnet("}],"source_content_type":"text/x-python","patch_set":2,"id":"e9ad4fa1_abd5d50f","line":83,"range":{"start_line":83,"start_character":13,"end_line":83,"end_character":23},"updated":"2021-02-01 20:16:25.000000000","message":"when does this run? i though it was at the end of the function scope but based on its use\nthis look like its at teh end of the test case execution correct. if so then this look ok but i would almost prefer this to be done outside of this funciton","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":true,"context_lines":[{"line_number":80,"context_line":"        }"},{"line_number":81,"context_line":"        net \u003d self.networks_client.create_network(name\u003dname_net,"},{"line_number":82,"context_line":"                                                  **net_dict)"},{"line_number":83,"context_line":"        self.addCleanup(self.networks_client.delete_network,"},{"line_number":84,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        subnet \u003d self.subnets_client.create_subnet("}],"source_content_type":"text/x-python","patch_set":2,"id":"da6c7cea_df25843f","line":83,"range":{"start_line":83,"start_character":13,"end_line":83,"end_character":23},"in_reply_to":"e9ad4fa1_abd5d50f","updated":"2021-02-01 20:43:05.000000000","message":"correct its done outside the scope of the function, I\u0027m not aware of any parent methods that allow for sriov port creation in tempest that also address cleanup but I can update/refactor if I can find a cleaner solution.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":83,"context_line":"        self.addCleanup(self.networks_client.delete_network,"},{"line_number":84,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        subnet \u003d self.subnets_client.create_subnet("},{"line_number":87,"context_line":"            network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":88,"context_line":"            cidr\u003dCONF.network.project_network_cidr,"},{"line_number":89,"context_line":"            ip_version\u003d4"}],"source_content_type":"text/x-python","patch_set":2,"id":"df3b6ae7_affec6cb","line":86,"range":{"start_line":86,"start_character":3,"end_line":86,"end_character":51},"updated":"2021-02-01 20:16:25.000000000","message":"similarly for the subnet creation its nice to have but might be nice if it was in a different function but im ok with it for now.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":true,"context_lines":[{"line_number":83,"context_line":"        self.addCleanup(self.networks_client.delete_network,"},{"line_number":84,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        subnet \u003d self.subnets_client.create_subnet("},{"line_number":87,"context_line":"            network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":88,"context_line":"            cidr\u003dCONF.network.project_network_cidr,"},{"line_number":89,"context_line":"            ip_version\u003d4"}],"source_content_type":"text/x-python","patch_set":2,"id":"b27f7ac3_a858053b","line":86,"range":{"start_line":86,"start_character":3,"end_line":86,"end_character":51},"in_reply_to":"df3b6ae7_affec6cb","updated":"2021-02-01 20:43:05.000000000","message":"Same as comment 83.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":105,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":106,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":107,"context_line":"                                             **vnic_type)"},{"line_number":108,"context_line":"        self.addCleanup(self.ports_client.delete_port,"},{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"11682f88_8c20698d","line":108,"range":{"start_line":108,"start_character":7,"end_line":108,"end_character":23},"updated":"2021-02-01 20:16:25.000000000","message":"again im not sure im a fan of auto registring the resouce to be cleaned up here but i guess its ok.\n\nside effect like that make test hard to read but eiser to write after you know the pattern.\n\nif we do this i would prefer the fucntion to be _create_and_cleanup_sriov_port or similar","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":true,"context_lines":[{"line_number":105,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":106,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":107,"context_line":"                                             **vnic_type)"},{"line_number":108,"context_line":"        self.addCleanup(self.ports_client.delete_port,"},{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fd4c9d7d_b5546d47","line":108,"range":{"start_line":108,"start_character":7,"end_line":108,"end_character":23},"in_reply_to":"11682f88_8c20698d","updated":"2021-02-01 20:43:05.000000000","message":"Same as comment 83, if a proper alternative is not found, I will update the method name to be more reflective of the method actions.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"edf23b6854806c48022f8763a2cebf5f0468c435","unresolved":true,"context_lines":[{"line_number":105,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":106,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":107,"context_line":"                                             **vnic_type)"},{"line_number":108,"context_line":"        self.addCleanup(self.ports_client.delete_port,"},{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"4868edc9_41b00763","line":108,"range":{"start_line":108,"start_character":7,"end_line":108,"end_character":23},"in_reply_to":"fd4c9d7d_b5546d47","updated":"2021-02-02 14:52:37.000000000","message":"It\u0027s a tempest pattern - you a create a thing, then register a cleanup function that gets called during tearDown(). The order they\u0027re called is the reverse of the order they were registered - which makes sense. The thing that was created last gets cleaned up first.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _get_xml_interface_devices(self, server_id, port, interface_type):"},{"line_number":113,"context_line":"        \"\"\"Returns xml interface element that matches provided port mac"},{"line_number":114,"context_line":"        and interface type. It is technically possible to have multiple ports"},{"line_number":115,"context_line":"        with the same MAC address in an instance, so method functionality may"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa3675e7_0966c166","line":112,"range":{"start_line":112,"start_character":8,"end_line":112,"end_character":34},"updated":"2021-02-01 20:16:25.000000000","message":"nit:this should proably be in a common utils module.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"4839b127bef9746ad878447eef459a8adfb556d6","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _get_xml_interface_devices(self, server_id, port, interface_type):"},{"line_number":113,"context_line":"        \"\"\"Returns xml interface element that matches provided port mac"},{"line_number":114,"context_line":"        and interface type. It is technically possible to have multiple ports"},{"line_number":115,"context_line":"        with the same MAC address in an instance, so method functionality may"}],"source_content_type":"text/x-python","patch_set":2,"id":"3b5b7ccf_8173f48d","line":112,"range":{"start_line":112,"start_character":8,"end_line":112,"end_character":34},"in_reply_to":"0f2d9c05_43fed012","updated":"2021-02-02 15:42:45.000000000","message":"Yes that does make sense a refactor around XML helpers would be beneficial.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"edf23b6854806c48022f8763a2cebf5f0468c435","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _get_xml_interface_devices(self, server_id, port, interface_type):"},{"line_number":113,"context_line":"        \"\"\"Returns xml interface element that matches provided port mac"},{"line_number":114,"context_line":"        and interface type. It is technically possible to have multiple ports"},{"line_number":115,"context_line":"        with the same MAC address in an instance, so method functionality may"}],"source_content_type":"text/x-python","patch_set":2,"id":"0f2d9c05_43fed012","line":112,"range":{"start_line":112,"start_character":8,"end_line":112,"end_character":34},"in_reply_to":"9d8c57f7_51e8a7ed","updated":"2021-02-02 14:52:37.000000000","message":"I\u0027d leave it as is for now - I don\u0027t disagree, but we have a whole bunch of these \"get something out of the XML\" helpers scattered all over the place, so we could use with a refactoring patch that puts them all in something like a XMLHelperMixin class.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":110,"context_line":"        return port"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _get_xml_interface_devices(self, server_id, port, interface_type):"},{"line_number":113,"context_line":"        \"\"\"Returns xml interface element that matches provided port mac"},{"line_number":114,"context_line":"        and interface type. It is technically possible to have multiple ports"},{"line_number":115,"context_line":"        with the same MAC address in an instance, so method functionality may"}],"source_content_type":"text/x-python","patch_set":2,"id":"9d8c57f7_51e8a7ed","line":112,"range":{"start_line":112,"start_character":8,"end_line":112,"end_character":34},"in_reply_to":"fa3675e7_0966c166","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"edf23b6854806c48022f8763a2cebf5f0468c435","unresolved":true,"context_lines":[{"line_number":118,"context_line":"        :param server_id: str, id of the instance to analyze"},{"line_number":119,"context_line":"        :param port: dictionary describing port to find"},{"line_number":120,"context_line":"        :param interface_type: str, interface type to look for in the xml"},{"line_number":121,"context_line":"        return intf: A list of xml elements that match the port"},{"line_number":122,"context_line":"        search criteria"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        root \u003d self.get_server_xml(server_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"850a3169_f773e41a","line":121,"updated":"2021-02-02 14:52:37.000000000","message":"nit: I know this is copied from below, but it\u0027s missing a colon","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"4839b127bef9746ad878447eef459a8adfb556d6","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        :param server_id: str, id of the instance to analyze"},{"line_number":119,"context_line":"        :param port: dictionary describing port to find"},{"line_number":120,"context_line":"        :param interface_type: str, interface type to look for in the xml"},{"line_number":121,"context_line":"        return intf: A list of xml elements that match the port"},{"line_number":122,"context_line":"        search criteria"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        root \u003d self.get_server_xml(server_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1e23c111_04031d7a","line":121,"in_reply_to":"850a3169_f773e41a","updated":"2021-02-02 15:42:45.000000000","message":"Ack","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        )"},{"line_number":130,"context_line":"        return interface_list"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def _validate_vf_port_xml_attributes(self, server_id, port):"},{"line_number":133,"context_line":"        \"\"\"Validates port count and vlan are accurate in server\u0027s XML"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param server_id: str, id of the instance to analyze"}],"source_content_type":"text/x-python","patch_set":2,"id":"978dc742_9e969db4","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":40},"updated":"2021-02-01 20:16:25.000000000","message":"also thsi feels like it would be useful elsewhere but i guess its ok for now","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"edf23b6854806c48022f8763a2cebf5f0468c435","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        )"},{"line_number":130,"context_line":"        return interface_list"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def _validate_vf_port_xml_attributes(self, server_id, port):"},{"line_number":133,"context_line":"        \"\"\"Validates port count and vlan are accurate in server\u0027s XML"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param server_id: str, id of the instance to analyze"}],"source_content_type":"text/x-python","patch_set":2,"id":"13da6339_c3aee17c","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":40},"in_reply_to":"85bcb09f_85499d4e","updated":"2021-02-02 14:52:37.000000000","message":"It\u0027s specific enough to this test that I\u0027m not sure it should move. See my comment on L133.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        )"},{"line_number":130,"context_line":"        return interface_list"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def _validate_vf_port_xml_attributes(self, server_id, port):"},{"line_number":133,"context_line":"        \"\"\"Validates port count and vlan are accurate in server\u0027s XML"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param server_id: str, id of the instance to analyze"}],"source_content_type":"text/x-python","patch_set":2,"id":"85bcb09f_85499d4e","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":40},"in_reply_to":"978dc742_9e969db4","updated":"2021-02-01 20:43:05.000000000","message":"I could maybe move it the whitebox utils potentially.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"edf23b6854806c48022f8763a2cebf5f0468c435","unresolved":true,"context_lines":[{"line_number":130,"context_line":"        return interface_list"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def _validate_vf_port_xml_attributes(self, server_id, port):"},{"line_number":133,"context_line":"        \"\"\"Validates port count and vlan are accurate in server\u0027s XML"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param server_id: str, id of the instance to analyze"},{"line_number":136,"context_line":"        :param port: dictionary describing port to find"}],"source_content_type":"text/x-python","patch_set":2,"id":"30b3af31_e0d36cc2","line":133,"range":{"start_line":133,"start_character":45,"end_line":133,"end_character":53},"updated":"2021-02-02 14:52:37.000000000","message":"nit: \"accurate\" is... vague? What it\u0027s asserting is that there\u0027s a single XML element for the port, and that the element\u0027s VLAN is the correct one. I\u0027d rename the method and/or improve the docstring.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"4839b127bef9746ad878447eef459a8adfb556d6","unresolved":true,"context_lines":[{"line_number":130,"context_line":"        return interface_list"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def _validate_vf_port_xml_attributes(self, server_id, port):"},{"line_number":133,"context_line":"        \"\"\"Validates port count and vlan are accurate in server\u0027s XML"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        :param server_id: str, id of the instance to analyze"},{"line_number":136,"context_line":"        :param port: dictionary describing port to find"}],"source_content_type":"text/x-python","patch_set":2,"id":"df275378_354b2682","line":133,"range":{"start_line":133,"start_character":45,"end_line":133,"end_character":53},"in_reply_to":"30b3af31_e0d36cc2","updated":"2021-02-02 15:42:45.000000000","message":"ACK","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":156,"context_line":"                         \u0027have vlan tag %s but instead it is tagged \u0027"},{"line_number":157,"context_line":"                         \u0027with %s\u0027 % (net_vlan, interface_vlan))"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def _get_port_attribute(self, port_id, attribute):"},{"line_number":160,"context_line":"        \"\"\"Get a specific attribute for provided port id"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        :param port_id, str the port id to search for"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f91e627_1067dbfb","line":159,"range":{"start_line":159,"start_character":8,"end_line":159,"end_character":27},"updated":"2021-02-01 20:16:25.000000000","message":"personally i would have mad ethis not raise an excption by taking a default value.\n_get_port_attribute(self, port_id, attribute, default\u003dNone):\n   ...\n return port.get(attribute,default) but i guess this works","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":156,"context_line":"                         \u0027have vlan tag %s but instead it is tagged \u0027"},{"line_number":157,"context_line":"                         \u0027with %s\u0027 % (net_vlan, interface_vlan))"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def _get_port_attribute(self, port_id, attribute):"},{"line_number":160,"context_line":"        \"\"\"Get a specific attribute for provided port id"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        :param port_id, str the port id to search for"}],"source_content_type":"text/x-python","patch_set":2,"id":"cd6de5e4_62040271","line":159,"range":{"start_line":159,"start_character":8,"end_line":159,"end_character":27},"in_reply_to":"7f91e627_1067dbfb","updated":"2021-02-01 20:43:05.000000000","message":"I can update this.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":181,"context_line":"        with db_client.cursor(db) as cursor:"},{"line_number":182,"context_line":"            result \u003d cursor.execute(\u0027SELECT address,status,dev_type FROM \u0027"},{"line_number":183,"context_line":"                                    \u0027pci_devices WHERE %s \u003d \"%s\"\u0027"},{"line_number":184,"context_line":"                                    % (column, value))"},{"line_number":185,"context_line":"            self.assertGreater(result, 0)"},{"line_number":186,"context_line":"            data \u003d cursor.fetchall()"},{"line_number":187,"context_line":"        return data"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ebbc32b_4e514c6d","line":184,"updated":"2021-02-01 20:16:25.000000000","message":"nit: you shoudl really use hanging indents here and anywere else you need to put\narguments across multiple lines.\n\nresult \u003d cursor.execute(\n  \u0027SELECT address,status,dev_type FROM \u0027\n  \u0027pci_devices WHERE %s \u003d \"%s\"\u0027 % (column, value))","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        with db_client.cursor(db) as cursor:"},{"line_number":182,"context_line":"            result \u003d cursor.execute(\u0027SELECT address,status,dev_type FROM \u0027"},{"line_number":183,"context_line":"                                    \u0027pci_devices WHERE %s \u003d \"%s\"\u0027"},{"line_number":184,"context_line":"                                    % (column, value))"},{"line_number":185,"context_line":"            self.assertGreater(result, 0)"},{"line_number":186,"context_line":"            data \u003d cursor.fetchall()"},{"line_number":187,"context_line":"        return data"}],"source_content_type":"text/x-python","patch_set":2,"id":"d60d17b1_b2fb1312","line":184,"in_reply_to":"1ebbc32b_4e514c6d","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":197,"context_line":"        \"\"\""},{"line_number":198,"context_line":"        binding_profile \u003d self._get_port_attribute(port_id, \u0027binding:profile\u0027)"},{"line_number":199,"context_line":"        pci_info \u003d self._get_pci_device_info(\u0027instance_uuid\u0027, server_id)"},{"line_number":200,"context_line":"        for pf in pci_info:"},{"line_number":201,"context_line":"            self.assertEqual(\"allocated\", pf[\u0027status\u0027], \u0027Physical function %s \u0027"},{"line_number":202,"context_line":"                             \u0027is in status %s and not in status allocated\u0027 %"},{"line_number":203,"context_line":"                             (pf[\u0027address\u0027], pf[\u0027status\u0027]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"44ed660d_bbefbd6b","line":200,"range":{"start_line":200,"start_character":12,"end_line":200,"end_character":14},"updated":"2021-02-01 20:16:25.000000000","message":"nit: this shoudl be device or similar.\nwe are more likely to be testing with a VF then a PF anyway but we should be generic.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        \"\"\""},{"line_number":198,"context_line":"        binding_profile \u003d self._get_port_attribute(port_id, \u0027binding:profile\u0027)"},{"line_number":199,"context_line":"        pci_info \u003d self._get_pci_device_info(\u0027instance_uuid\u0027, server_id)"},{"line_number":200,"context_line":"        for pf in pci_info:"},{"line_number":201,"context_line":"            self.assertEqual(\"allocated\", pf[\u0027status\u0027], \u0027Physical function %s \u0027"},{"line_number":202,"context_line":"                             \u0027is in status %s and not in status allocated\u0027 %"},{"line_number":203,"context_line":"                             (pf[\u0027address\u0027], pf[\u0027status\u0027]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"a91d4436_9ae4550c","line":200,"range":{"start_line":200,"start_character":12,"end_line":200,"end_character":14},"in_reply_to":"44ed660d_bbefbd6b","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        binding_profile \u003d self._get_port_attribute(port_id, \u0027binding:profile\u0027)"},{"line_number":199,"context_line":"        pci_info \u003d self._get_pci_device_info(\u0027instance_uuid\u0027, server_id)"},{"line_number":200,"context_line":"        for pf in pci_info:"},{"line_number":201,"context_line":"            self.assertEqual(\"allocated\", pf[\u0027status\u0027], \u0027Physical function %s \u0027"},{"line_number":202,"context_line":"                             \u0027is in status %s and not in status allocated\u0027 %"},{"line_number":203,"context_line":"                             (pf[\u0027address\u0027], pf[\u0027status\u0027]))"},{"line_number":204,"context_line":"            self.assertIn(str(pf[\u0027address\u0027]), str(binding_profile), \u0027PCI \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3d331b5a_d20358b7","line":201,"range":{"start_line":201,"start_character":0,"end_line":201,"end_character":12},"updated":"2021-02-01 20:16:25.000000000","message":"+1","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            self.assertEqual(\"allocated\", pf[\u0027status\u0027], \u0027Physical function %s \u0027"},{"line_number":202,"context_line":"                             \u0027is in status %s and not in status allocated\u0027 %"},{"line_number":203,"context_line":"                             (pf[\u0027address\u0027], pf[\u0027status\u0027]))"},{"line_number":204,"context_line":"            self.assertIn(str(pf[\u0027address\u0027]), str(binding_profile), \u0027PCI \u0027"},{"line_number":205,"context_line":"                          \u0027device information in Nova and and Binding profile \u0027"},{"line_number":206,"context_line":"                          \u0027information in Neutron mismatch\u0027)"},{"line_number":207,"context_line":"            self.assertEqual(dev_type, pf[\u0027dev_type\u0027], \u0027Expected PCI device \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"0311f184_d891d122","line":204,"range":{"start_line":204,"start_character":11,"end_line":204,"end_character":25},"updated":"2021-02-01 20:16:25.000000000","message":"you should load the port binding as a json blob and assert that the device addres is in the pci_slot key.\n\ne.g. self.assertEqual(device[\u0027address\u0027], json.loads(binding_profile)[\u0027pci_slot\u0027], ...)","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            self.assertEqual(\"allocated\", pf[\u0027status\u0027], \u0027Physical function %s \u0027"},{"line_number":202,"context_line":"                             \u0027is in status %s and not in status allocated\u0027 %"},{"line_number":203,"context_line":"                             (pf[\u0027address\u0027], pf[\u0027status\u0027]))"},{"line_number":204,"context_line":"            self.assertIn(str(pf[\u0027address\u0027]), str(binding_profile), \u0027PCI \u0027"},{"line_number":205,"context_line":"                          \u0027device information in Nova and and Binding profile \u0027"},{"line_number":206,"context_line":"                          \u0027information in Neutron mismatch\u0027)"},{"line_number":207,"context_line":"            self.assertEqual(dev_type, pf[\u0027dev_type\u0027], \u0027Expected PCI device \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"41a5b8dc_48ea434a","line":204,"range":{"start_line":204,"start_character":11,"end_line":204,"end_character":25},"in_reply_to":"0311f184_d891d122","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            self.assertIn(str(pf[\u0027address\u0027]), str(binding_profile), \u0027PCI \u0027"},{"line_number":205,"context_line":"                          \u0027device information in Nova and and Binding profile \u0027"},{"line_number":206,"context_line":"                          \u0027information in Neutron mismatch\u0027)"},{"line_number":207,"context_line":"            self.assertEqual(dev_type, pf[\u0027dev_type\u0027], \u0027Expected PCI device \u0027"},{"line_number":208,"context_line":"                             \u0027type to be %s but instead it is %s\u0027 %"},{"line_number":209,"context_line":"                             (pf[\u0027dev_type\u0027], dev_type))"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e1531b34_793e727b","line":207,"range":{"start_line":207,"start_character":17,"end_line":207,"end_character":28},"updated":"2021-02-01 20:16:25.000000000","message":"im not sure this add value \nit woudl be better to do something like this\n\nif port[\u0027vnic_type\u0027] \u003d\u003d \u0027direct-physical\u0027:\n self.assertEqual(dev_type, \u0027type-PF\u0027)\nelse:\n # vnic_type direct, macvtap or virtio-forwarder can use VF or type pci devices.\n self.assertIn(dev_type, [\u0027type-VF\u0027,\u0027type-PCI\u0027])\n\n\nthe current assertion basically reads the value for the db two different ways and assert they are the same.\none passed in as dev type and the second via a direct select","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":204,"context_line":"            self.assertIn(str(pf[\u0027address\u0027]), str(binding_profile), \u0027PCI \u0027"},{"line_number":205,"context_line":"                          \u0027device information in Nova and and Binding profile \u0027"},{"line_number":206,"context_line":"                          \u0027information in Neutron mismatch\u0027)"},{"line_number":207,"context_line":"            self.assertEqual(dev_type, pf[\u0027dev_type\u0027], \u0027Expected PCI device \u0027"},{"line_number":208,"context_line":"                             \u0027type to be %s but instead it is %s\u0027 %"},{"line_number":209,"context_line":"                             (pf[\u0027dev_type\u0027], dev_type))"},{"line_number":210,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1407d63e_fa56a50b","line":207,"range":{"start_line":207,"start_character":17,"end_line":207,"end_character":28},"in_reply_to":"e1531b34_793e727b","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":505,"context_line":"            vnic_type\u003d{\u0027binding:vnic_type\u0027: vnic_type}"},{"line_number":506,"context_line":"        )"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        server_b \u003d self.create_test_server("},{"line_number":509,"context_line":"            clients\u003dself.os_admin,"},{"line_number":510,"context_line":"            flavor\u003dflavor[\u0027id\u0027],"},{"line_number":511,"context_line":"            networks\u003d[{\u0027port\u0027: port_b[\u0027port\u0027][\u0027id\u0027]}],"}],"source_content_type":"text/x-python","patch_set":2,"id":"bff00ac9_b8d96b3b","line":508,"range":{"start_line":508,"start_character":7,"end_line":508,"end_character":43},"updated":"2021-02-01 20:16:25.000000000","message":"why are you creating a second server.\n\nyou are not using it to asser that the pci addresses chagne as part of a migration\nor otherwise makeing any assertion between both servers so this does not seam to add value to the test.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":true,"context_lines":[{"line_number":505,"context_line":"            vnic_type\u003d{\u0027binding:vnic_type\u0027: vnic_type}"},{"line_number":506,"context_line":"        )"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        server_b \u003d self.create_test_server("},{"line_number":509,"context_line":"            clients\u003dself.os_admin,"},{"line_number":510,"context_line":"            flavor\u003dflavor[\u0027id\u0027],"},{"line_number":511,"context_line":"            networks\u003d[{\u0027port\u0027: port_b[\u0027port\u0027][\u0027id\u0027]}],"}],"source_content_type":"text/x-python","patch_set":2,"id":"e882e5c2_b8ac4bbf","line":508,"range":{"start_line":508,"start_character":7,"end_line":508,"end_character":43},"in_reply_to":"bff00ac9_b8d96b3b","updated":"2021-02-01 20:43:05.000000000","message":"This could be reduced to a single server, the thought process was just validating that having multiple servers/ports live migrating do not impact each other in a negative way.  I.e. server_a\u0027s port somehow become associated incorrectly with server_b.  Also confirming that when VF\u0027s are available that server_b should still be able to migrate even though server_a is on the host.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":565,"context_line":"    def test_sriov_direct_live_migration(self):"},{"line_number":566,"context_line":"        \"\"\"Verify sriov live migration using direct type ports"},{"line_number":567,"context_line":"        \"\"\""},{"line_number":568,"context_line":"        self._base_test_live_migration(vnic_type\u003d\u0027direct\u0027, dev_type\u003d\u0027type-VF\u0027)"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"    def test_sriov_macvtap_live_migration(self):"},{"line_number":571,"context_line":"        \"\"\"Verify sriov live migration using macvtap type ports"}],"source_content_type":"text/x-python","patch_set":2,"id":"2fc4bd2d_8273c5d3","line":568,"range":{"start_line":568,"start_character":59,"end_line":568,"end_character":77},"updated":"2021-02-01 20:16:25.000000000","message":"nit:this can select type-pci also.","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":565,"context_line":"    def test_sriov_direct_live_migration(self):"},{"line_number":566,"context_line":"        \"\"\"Verify sriov live migration using direct type ports"},{"line_number":567,"context_line":"        \"\"\""},{"line_number":568,"context_line":"        self._base_test_live_migration(vnic_type\u003d\u0027direct\u0027, dev_type\u003d\u0027type-VF\u0027)"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"    def test_sriov_macvtap_live_migration(self):"},{"line_number":571,"context_line":"        \"\"\"Verify sriov live migration using macvtap type ports"}],"source_content_type":"text/x-python","patch_set":2,"id":"433e5563_31428c9d","line":568,"range":{"start_line":568,"start_character":59,"end_line":568,"end_character":77},"in_reply_to":"2fc4bd2d_8273c5d3","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7739d85a3e04a8858c03d6c18a220309c52c9d26","unresolved":true,"context_lines":[{"line_number":570,"context_line":"    def test_sriov_macvtap_live_migration(self):"},{"line_number":571,"context_line":"        \"\"\"Verify sriov live migration using macvtap type ports"},{"line_number":572,"context_line":"        \"\"\""},{"line_number":573,"context_line":"        self._base_test_live_migration(vnic_type\u003d\u0027macvtap\u0027, dev_type\u003d\u0027type-VF\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"08594aa0_3b9832f1","line":573,"range":{"start_line":573,"start_character":60,"end_line":573,"end_character":79},"updated":"2021-02-01 20:16:25.000000000","message":"as can this.\n\nit will only select type-PCI if the nic does not support sriov but form a crorrectnes point of view this is not valid to hard code.\n\nalternitively you can just add a comment stating that test test require nics that support sriov VFs","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"edf23b6854806c48022f8763a2cebf5f0468c435","unresolved":false,"context_lines":[{"line_number":570,"context_line":"    def test_sriov_macvtap_live_migration(self):"},{"line_number":571,"context_line":"        \"\"\"Verify sriov live migration using macvtap type ports"},{"line_number":572,"context_line":"        \"\"\""},{"line_number":573,"context_line":"        self._base_test_live_migration(vnic_type\u003d\u0027macvtap\u0027, dev_type\u003d\u0027type-VF\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"22f44af4_e47c19d0","line":573,"range":{"start_line":573,"start_character":60,"end_line":573,"end_character":79},"in_reply_to":"029fbe54_173b2ddd","updated":"2021-02-02 14:52:37.000000000","message":"I guess the sriov_physnet config option that we skip on implies a VF-capable SRIOV nic? At least the way this test is written, it assumes we can get at least 2 VFs out of it. So should probably update the description of the parameter?","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c679bab5d5ef520b2a341334b6699a7d4dfb133d","unresolved":false,"context_lines":[{"line_number":570,"context_line":"    def test_sriov_macvtap_live_migration(self):"},{"line_number":571,"context_line":"        \"\"\"Verify sriov live migration using macvtap type ports"},{"line_number":572,"context_line":"        \"\"\""},{"line_number":573,"context_line":"        self._base_test_live_migration(vnic_type\u003d\u0027macvtap\u0027, dev_type\u003d\u0027type-VF\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"029fbe54_173b2ddd","line":573,"range":{"start_line":573,"start_character":60,"end_line":573,"end_character":79},"in_reply_to":"08594aa0_3b9832f1","updated":"2021-02-01 20:43:05.000000000","message":"Done","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"4839b127bef9746ad878447eef459a8adfb556d6","unresolved":false,"context_lines":[{"line_number":570,"context_line":"    def test_sriov_macvtap_live_migration(self):"},{"line_number":571,"context_line":"        \"\"\"Verify sriov live migration using macvtap type ports"},{"line_number":572,"context_line":"        \"\"\""},{"line_number":573,"context_line":"        self._base_test_live_migration(vnic_type\u003d\u0027macvtap\u0027, dev_type\u003d\u0027type-VF\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"064e8b82_cc638898","line":573,"range":{"start_line":573,"start_character":60,"end_line":573,"end_character":79},"in_reply_to":"22f44af4_e47c19d0","updated":"2021-02-02 15:42:45.000000000","message":"Yes either update the test parameter description to highlight this or potentially add a new parameter that describes NIC functionality?","commit_id":"e19356a769ac75bf14d698541b6c8bd8da6b9f4b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eb998b64cdd78b4bcd686e8f1122f1852870b0d3","unresolved":true,"context_lines":[{"line_number":90,"context_line":"        self.addCleanup(self.networks_client.delete_network,"},{"line_number":91,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        self._create_sriov_subnet(net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":94,"context_line":"        return net"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def _create_sriov_subnet(self, network_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"8559cce7_2b6b3452","line":93,"updated":"2021-03-30 15:01:17.000000000","message":"Extreme nit: it\u0027d be cleaner if this wasn\u0027t called from _create_sriov_net, in order to stick to one method doing one thing. So the called would do something like:\n\n  net \u003d _create_sriov_net()\n  subnet \u003d _create_sriov_subnet()","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"004e7d28d6cc453d400b657ca36ed58f376dc574","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.addCleanup(self.networks_client.delete_network,"},{"line_number":91,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        self._create_sriov_subnet(net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":94,"context_line":"        return net"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def _create_sriov_subnet(self, network_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac4548a_e4c93c28","line":93,"in_reply_to":"8559cce7_2b6b3452","updated":"2021-03-30 16:02:52.000000000","message":"Ack","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eb998b64cdd78b4bcd686e8f1122f1852870b0d3","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        by neutron ports client"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        if vnic_type is None:"},{"line_number":125,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":126,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":127,"context_line":"                                             **vnic_type)"},{"line_number":128,"context_line":"        self.addCleanup(self.ports_client.delete_port,"}],"source_content_type":"text/x-python","patch_set":3,"id":"4165db4c_f1b200e9","line":125,"range":{"start_line":125,"start_character":59,"end_line":125,"end_character":73},"updated":"2021-03-30 15:01:17.000000000","message":"Hrmpf, so we\u0027ll explode if this is not set and we didn\u0027t get passed a vnic_type. I think this is technically left over from a previous change, and I\u0027m just noticing it now, but yeah, not ideal. But we can leave this for and fix later - I suspect each individual test will need to know whether it\u0027s always passing a vnic_type, or whether it needs to skip because it\u0027s using the default in CONF.network.port_vnic_type.","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"631f3cd8b0a0f6a4f68274ff62f2929a2ddd66c8","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        by neutron ports client"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        if vnic_type is None:"},{"line_number":125,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":126,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":127,"context_line":"                                             **vnic_type)"},{"line_number":128,"context_line":"        self.addCleanup(self.ports_client.delete_port,"}],"source_content_type":"text/x-python","patch_set":3,"id":"b28d74b3_46bdabf0","line":125,"range":{"start_line":125,"start_character":59,"end_line":125,"end_character":73},"in_reply_to":"2da6ac1b_d68e5dd3","updated":"2021-03-30 16:59:58.000000000","message":"Yeah, that sounds like the smartest way forward. Remove the default, the test methods themselves pass CONF.network.port_vnic_type to _create_sriov_port(), and then the test method (or entire class) can have the skip check in them.","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"004e7d28d6cc453d400b657ca36ed58f376dc574","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        by neutron ports client"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        if vnic_type is None:"},{"line_number":125,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":126,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":127,"context_line":"                                             **vnic_type)"},{"line_number":128,"context_line":"        self.addCleanup(self.ports_client.delete_port,"}],"source_content_type":"text/x-python","patch_set":3,"id":"2da6ac1b_d68e5dd3","line":125,"range":{"start_line":125,"start_character":59,"end_line":125,"end_character":73},"in_reply_to":"4165db4c_f1b200e9","updated":"2021-03-30 16:02:52.000000000","message":"I could remove the default of None and the associated logic for setting vnic_type and put a skip check for the SRIOVNumaAffinity class to confirm that a value is present in tempest.conf.  The live migration tests already pass an explicit type so that could remain the same. wdyt?","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"06b24ee566b26e8de7e753528a4266f16df37dbc","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        by neutron ports client"},{"line_number":123,"context_line":"        \"\"\""},{"line_number":124,"context_line":"        if vnic_type is None:"},{"line_number":125,"context_line":"            vnic_type \u003d {\u0027binding:vnic_type\u0027: CONF.network.port_vnic_type}"},{"line_number":126,"context_line":"        port \u003d self.ports_client.create_port(network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":127,"context_line":"                                             **vnic_type)"},{"line_number":128,"context_line":"        self.addCleanup(self.ports_client.delete_port,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5e94c572_9f3f11eb","line":125,"range":{"start_line":125,"start_character":59,"end_line":125,"end_character":73},"in_reply_to":"b28d74b3_46bdabf0","updated":"2021-04-07 15:52:22.000000000","message":"Quick correction L239 effectively does the skip check already for the NUMAAffinity class, but I will still remove the defaults and make sure everything is passed explicitly.","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eb998b64cdd78b4bcd686e8f1122f1852870b0d3","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            \u0027tag %s but instead it is tagged with %s\u0027 %"},{"line_number":167,"context_line":"            (expected_vlan, interface_vlan))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _get_port_attribute(self, port_id, attribute, default\u003dNone):"},{"line_number":170,"context_line":"        \"\"\"Get a specific attribute for provided port id"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        :param port_id: str the port id to search for"}],"source_content_type":"text/x-python","patch_set":3,"id":"2cb76b30_64a5282d","line":169,"range":{"start_line":169,"start_character":54,"end_line":169,"end_character":61},"updated":"2021-03-30 15:01:17.000000000","message":"You\u0027re not actually using the default anywhere that I can see, do we really need it?","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"004e7d28d6cc453d400b657ca36ed58f376dc574","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            \u0027tag %s but instead it is tagged with %s\u0027 %"},{"line_number":167,"context_line":"            (expected_vlan, interface_vlan))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _get_port_attribute(self, port_id, attribute, default\u003dNone):"},{"line_number":170,"context_line":"        \"\"\"Get a specific attribute for provided port id"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        :param port_id: str the port id to search for"}],"source_content_type":"text/x-python","patch_set":3,"id":"ca53eb4c_09c15aa1","line":169,"range":{"start_line":169,"start_character":54,"end_line":169,"end_character":61},"in_reply_to":"2cb76b30_64a5282d","updated":"2021-03-30 16:02:52.000000000","message":"This was suggested couple patch sets ago, and correct in its current usage default is never used.  I do like the design pattern though, especially if a method like this needs to be utilized in additional test cases.  I\u0027m ok with either leaving it or waiting to add it when it actually needs to be leveraged though.","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"631f3cd8b0a0f6a4f68274ff62f2929a2ddd66c8","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            \u0027tag %s but instead it is tagged with %s\u0027 %"},{"line_number":167,"context_line":"            (expected_vlan, interface_vlan))"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _get_port_attribute(self, port_id, attribute, default\u003dNone):"},{"line_number":170,"context_line":"        \"\"\"Get a specific attribute for provided port id"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        :param port_id: str the port id to search for"}],"source_content_type":"text/x-python","patch_set":3,"id":"d2489283_87b32d38","line":169,"range":{"start_line":169,"start_character":54,"end_line":169,"end_character":61},"in_reply_to":"ca53eb4c_09c15aa1","updated":"2021-03-30 16:59:58.000000000","message":"I\u0027m a fan of not adding stuff unless we need to - I hope I\u0027m not contradicting my own review from earlier :P","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eb998b64cdd78b4bcd686e8f1122f1852870b0d3","unresolved":true,"context_lines":[{"line_number":178,"context_line":"        port \u003d body[\u0027port\u0027]"},{"line_number":179,"context_line":"        return port.get(attribute, default)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def _get_pci_device_info(self, column, value):"},{"line_number":182,"context_line":"        \"\"\"Returns all pci_device\u0027s address, status, and dev_type that match"},{"line_number":183,"context_line":"        query criteria."},{"line_number":184,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"917cd536_60c0603a","line":181,"updated":"2021-03-30 15:01:17.000000000","message":"nit: I\u0027d rename this to something like _search_pci_devices() Currently it gives the impression that you\u0027re getting column/value\u0027s pci_device_info or something.","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"004e7d28d6cc453d400b657ca36ed58f376dc574","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        port \u003d body[\u0027port\u0027]"},{"line_number":179,"context_line":"        return port.get(attribute, default)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def _get_pci_device_info(self, column, value):"},{"line_number":182,"context_line":"        \"\"\"Returns all pci_device\u0027s address, status, and dev_type that match"},{"line_number":183,"context_line":"        query criteria."},{"line_number":184,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"c073a18f_cb555a36","line":181,"in_reply_to":"917cd536_60c0603a","updated":"2021-03-30 16:02:52.000000000","message":"Ack","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"eb998b64cdd78b4bcd686e8f1122f1852870b0d3","unresolved":true,"context_lines":[{"line_number":447,"context_line":"    @classmethod"},{"line_number":448,"context_line":"    def skip_checks(cls):"},{"line_number":449,"context_line":"        super(SRIOVMigration, cls).skip_checks()"},{"line_number":450,"context_line":"        if (CONF.compute.min_compute_nodes \u003c 2 or"},{"line_number":451,"context_line":"                CONF.whitebox.max_compute_nodes \u003e 2):"},{"line_number":452,"context_line":"            raise cls.skipException(\u0027Exactly 2 compute nodes required.\u0027)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"145775ec_a0bc2470","line":450,"updated":"2021-03-30 15:01:17.000000000","message":"In retrospect this isn\u0027t a great way to do it, but until we come up with something better, this can stay.","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"004e7d28d6cc453d400b657ca36ed58f376dc574","unresolved":false,"context_lines":[{"line_number":447,"context_line":"    @classmethod"},{"line_number":448,"context_line":"    def skip_checks(cls):"},{"line_number":449,"context_line":"        super(SRIOVMigration, cls).skip_checks()"},{"line_number":450,"context_line":"        if (CONF.compute.min_compute_nodes \u003c 2 or"},{"line_number":451,"context_line":"                CONF.whitebox.max_compute_nodes \u003e 2):"},{"line_number":452,"context_line":"            raise cls.skipException(\u0027Exactly 2 compute nodes required.\u0027)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"0106e9f2_2f1587a3","line":450,"in_reply_to":"145775ec_a0bc2470","updated":"2021-03-30 16:02:52.000000000","message":"Ack","commit_id":"54898cb57388d34eaf4dbc4ed198d73bc6ab4cf0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4dedcd0e61e8c5b1c097c1eb1770c1f0e682c088","unresolved":true,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    @classmethod"},{"line_number":47,"context_line":"    def setup_clients(cls):"},{"line_number":48,"context_line":"        super(SRIOVBase, cls).setup_clients()"},{"line_number":49,"context_line":"        cls.networks_client \u003d cls.os_admin.networks_client"},{"line_number":50,"context_line":"        cls.subnets_client \u003d cls.os_admin.subnets_client"},{"line_number":51,"context_line":"        cls.ports_client \u003d cls.os_admin.ports_client"}],"source_content_type":"text/x-python","patch_set":4,"id":"bbbae19f_fab0b3dd","line":48,"updated":"2021-04-14 21:39:13.000000000","message":"Given the pain that this kind of \"admin override\" has caused with the Barbican job, we should probably not repeat that mistake. I think we should explicitly use the admin client when we need them.","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3c0e8dfe41e55f8688a2f0fb9fb95442ddf63933","unresolved":true,"context_lines":[{"line_number":112,"context_line":"        return subnet"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _create_sriov_port(self, net, vnic_type):"},{"line_number":115,"context_line":"        \"\"\"Create an sr-iov port with a vnic_type provided by tempest config"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        :param net: dictionary with network details"},{"line_number":118,"context_line":"        :param vnic_type: str, representing the vnic type to use with creating"}],"source_content_type":"text/x-python","patch_set":4,"id":"581aa2f8_1798652d","line":115,"range":{"start_line":115,"start_character":50,"end_line":115,"end_character":76},"updated":"2021-04-14 14:50:30.000000000","message":"This isn\u0027t true in the context of this method - from this method\u0027s POV, it gets passed a vnic_type, and doesn\u0027t care where it comes from.","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"b479e9c1db40f6e8a4b658f1ed1c01835438276d","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        return subnet"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _create_sriov_port(self, net, vnic_type):"},{"line_number":115,"context_line":"        \"\"\"Create an sr-iov port with a vnic_type provided by tempest config"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        :param net: dictionary with network details"},{"line_number":118,"context_line":"        :param vnic_type: str, representing the vnic type to use with creating"}],"source_content_type":"text/x-python","patch_set":4,"id":"a74f8fe2_949a8c34","line":115,"range":{"start_line":115,"start_character":50,"end_line":115,"end_character":76},"in_reply_to":"581aa2f8_1798652d","updated":"2021-04-14 15:04:55.000000000","message":"Ack","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3c0e8dfe41e55f8688a2f0fb9fb95442ddf63933","unresolved":true,"context_lines":[{"line_number":190,"context_line":"            result \u003d cursor.execute("},{"line_number":191,"context_line":"                \u0027SELECT address,status,dev_type FROM \u0027"},{"line_number":192,"context_line":"                \u0027pci_devices WHERE %s \u003d \"%s\"\u0027 % (column, value))"},{"line_number":193,"context_line":"            self.assertGreater(result, 0)"},{"line_number":194,"context_line":"            data \u003d cursor.fetchall()"},{"line_number":195,"context_line":"        return data"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d56d94d2_32114f90","line":193,"updated":"2021-04-14 14:50:30.000000000","message":"Is it sane to fail if we get nothing back? I guess it is, if we expect the caller to give us existing values...","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"37734edbc6a3fd7662c3c60436a46e952738f527","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            result \u003d cursor.execute("},{"line_number":191,"context_line":"                \u0027SELECT address,status,dev_type FROM \u0027"},{"line_number":192,"context_line":"                \u0027pci_devices WHERE %s \u003d \"%s\"\u0027 % (column, value))"},{"line_number":193,"context_line":"            self.assertGreater(result, 0)"},{"line_number":194,"context_line":"            data \u003d cursor.fetchall()"},{"line_number":195,"context_line":"        return data"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"8e4b7992_0d34cc99","line":193,"in_reply_to":"954ebf92_bd920f07","updated":"2021-04-14 19:14:20.000000000","message":"There isn\u0027t really precedence for this in WB [1], so I will go ahead and remove it.\n\n\n[1] https://opendev.org/openstack/whitebox-tempest-plugin/src/branch/master/whitebox_tempest_plugin/api/compute/test_cpu_pinning.py#L115","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"b479e9c1db40f6e8a4b658f1ed1c01835438276d","unresolved":true,"context_lines":[{"line_number":190,"context_line":"            result \u003d cursor.execute("},{"line_number":191,"context_line":"                \u0027SELECT address,status,dev_type FROM \u0027"},{"line_number":192,"context_line":"                \u0027pci_devices WHERE %s \u003d \"%s\"\u0027 % (column, value))"},{"line_number":193,"context_line":"            self.assertGreater(result, 0)"},{"line_number":194,"context_line":"            data \u003d cursor.fetchall()"},{"line_number":195,"context_line":"        return data"},{"line_number":196,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"954ebf92_bd920f07","line":193,"in_reply_to":"d56d94d2_32114f90","updated":"2021-04-14 15:04:55.000000000","message":"It doesn\u0027t have to necessarily, it will just fail later on in the test.  I am ok with updating it out or leaving it.","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3c0e8dfe41e55f8688a2f0fb9fb95442ddf63933","unresolved":true,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"class SRIOVMigration(SRIOVBase):"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":"    min_microversion \u003d \u00272.74\u0027"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def setUp(self):"},{"line_number":447,"context_line":"        super(SRIOVMigration, self).setUp()"}],"source_content_type":"text/x-python","patch_set":4,"id":"ea4410dc_d5eedfb3","line":444,"updated":"2021-04-14 14:50:30.000000000","message":"nit: we should make a habit of commenting why we need a particular microversion","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"b479e9c1db40f6e8a4b658f1ed1c01835438276d","unresolved":true,"context_lines":[{"line_number":441,"context_line":""},{"line_number":442,"context_line":"class SRIOVMigration(SRIOVBase):"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":"    min_microversion \u003d \u00272.74\u0027"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def setUp(self):"},{"line_number":447,"context_line":"        super(SRIOVMigration, self).setUp()"}],"source_content_type":"text/x-python","patch_set":4,"id":"0f759e64_368c10fe","line":444,"in_reply_to":"ea4410dc_d5eedfb3","updated":"2021-04-14 15:04:55.000000000","message":"I\u0027ll update accordingly with why.","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3c0e8dfe41e55f8688a2f0fb9fb95442ddf63933","unresolved":true,"context_lines":[{"line_number":541,"context_line":""},{"line_number":542,"context_line":"        # Confirm total port allocations still remains one after final"},{"line_number":543,"context_line":"        # migration"},{"line_number":544,"context_line":"        allocation_count \u003d self._get_pci_status_count(\u0027allocated\u0027)"},{"line_number":545,"context_line":"        self.assertEqual(allocation_count, 1, \u0027Total allocated pci devices \u0027"},{"line_number":546,"context_line":"                         \u0027after second migration should be 1 but instead \u0027"},{"line_number":547,"context_line":"                         \u0027is %s\u0027 % allocation_count)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dbe69ae8_8a75569d","line":544,"range":{"start_line":544,"start_character":8,"end_line":544,"end_character":24},"updated":"2021-04-14 14:50:30.000000000","message":"nit: this is Placement terminology, could we rename this to perhaps `pci_allocated_count`?","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"b479e9c1db40f6e8a4b658f1ed1c01835438276d","unresolved":false,"context_lines":[{"line_number":541,"context_line":""},{"line_number":542,"context_line":"        # Confirm total port allocations still remains one after final"},{"line_number":543,"context_line":"        # migration"},{"line_number":544,"context_line":"        allocation_count \u003d self._get_pci_status_count(\u0027allocated\u0027)"},{"line_number":545,"context_line":"        self.assertEqual(allocation_count, 1, \u0027Total allocated pci devices \u0027"},{"line_number":546,"context_line":"                         \u0027after second migration should be 1 but instead \u0027"},{"line_number":547,"context_line":"                         \u0027is %s\u0027 % allocation_count)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac8b8af_35228430","line":544,"range":{"start_line":544,"start_character":8,"end_line":544,"end_character":24},"in_reply_to":"dbe69ae8_8a75569d","updated":"2021-04-14 15:04:55.000000000","message":"Ack","commit_id":"36212cb2f852e5494c68076206442c8add4315bb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1f11b10e2c8bb169a56b219096778d3775ea2eab","unresolved":true,"context_lines":[{"line_number":63,"context_line":"        elif vnic_type \u003d\u003d \u0027macvtap\u0027:"},{"line_number":64,"context_line":"            return \u0027direct\u0027"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _create_sriov_net(self):"},{"line_number":67,"context_line":"        \"\"\"Create an IPv4 L2 vlan network.  Physical network provider comes"},{"line_number":68,"context_line":"        from sriov_physnet provided in tempest config"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        :return net A dictionary describing details about the created network"},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        name_net \u003d data_utils.rand_name(self.__class__.__name__)"},{"line_number":73,"context_line":"        vlan_id \u003d \\"},{"line_number":74,"context_line":"            CONF.network_feature_enabled.provider_net_base_segmentation_id"},{"line_number":75,"context_line":"        physical_net \u003d CONF.whitebox_hardware.sriov_physnet"},{"line_number":76,"context_line":"        net_dict \u003d {"},{"line_number":77,"context_line":"            \u0027provider:network_type\u0027: \u0027vlan\u0027,"},{"line_number":78,"context_line":"            \u0027provider:physical_network\u0027: physical_net,"},{"line_number":79,"context_line":"            \u0027provider:segmentation_id\u0027: vlan_id"},{"line_number":80,"context_line":"        }"},{"line_number":81,"context_line":"        net \u003d self.os_admin.networks_client.create_network("},{"line_number":82,"context_line":"            name\u003dname_net,"},{"line_number":83,"context_line":"            **net_dict)"},{"line_number":84,"context_line":"        self.addCleanup(self.os_admin.networks_client.delete_network,"},{"line_number":85,"context_line":"                        net[\u0027network\u0027][\u0027id\u0027])"},{"line_number":86,"context_line":"        return net"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def _create_sriov_subnet(self, network_id):"},{"line_number":89,"context_line":"        \"\"\"Create an IPv4 L2 vlan network.  Physical network provider comes"},{"line_number":90,"context_line":"        from sriov_physnet provided in tempest config"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        :param network_id: str, network id subnet will be associated with"},{"line_number":93,"context_line":"        :return net A dictionary describing details about the created network"},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":"        name_subnet \u003d data_utils.rand_name(self.__class__.__name__)"},{"line_number":96,"context_line":"        subnet \u003d self.os_admin.subnets_client.create_subnet("},{"line_number":97,"context_line":"            name\u003dname_subnet,"},{"line_number":98,"context_line":"            network_id\u003dnetwork_id,"},{"line_number":99,"context_line":"            cidr\u003dCONF.network.project_network_cidr,"},{"line_number":100,"context_line":"            ip_version\u003d4"},{"line_number":101,"context_line":"        )"},{"line_number":102,"context_line":"        self.addCleanup("},{"line_number":103,"context_line":"            self.os_admin.subnets_client.delete_subnet,"},{"line_number":104,"context_line":"            subnet[\u0027subnet\u0027][\u0027id\u0027]"},{"line_number":105,"context_line":"        )"},{"line_number":106,"context_line":"        return subnet"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _create_sriov_port(self, net, vnic_type):"},{"line_number":109,"context_line":"        \"\"\"Create an sr-iov port based on the provided vnic type"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"        :param net: dictionary with network details"},{"line_number":112,"context_line":"        :param vnic_type: str, representing the vnic type to use with creating"},{"line_number":113,"context_line":"        the sriov port, e.g. direct, macvtap, etc."},{"line_number":114,"context_line":"        :return port: dictionary with details about newly created port provided"},{"line_number":115,"context_line":"        by neutron ports client"},{"line_number":116,"context_line":"        \"\"\""},{"line_number":117,"context_line":"        vnic_params \u003d {\u0027binding:vnic_type\u0027: vnic_type}"},{"line_number":118,"context_line":"        port \u003d self.os_admin.ports_client.create_port("},{"line_number":119,"context_line":"            network_id\u003dnet[\u0027network\u0027][\u0027id\u0027],"},{"line_number":120,"context_line":"            **vnic_params)"},{"line_number":121,"context_line":"        self.addCleanup(self.os_admin.ports_client.delete_port,"},{"line_number":122,"context_line":"                        port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":123,"context_line":"        return port"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def _get_xml_interface_device(self, server_id, port_id):"},{"line_number":126,"context_line":"        \"\"\"Returns xml interface element that matches provided port mac"}],"source_content_type":"text/x-python","patch_set":5,"id":"e243aff9_1eea1d5c","line":123,"range":{"start_line":66,"start_character":1,"end_line":123,"end_character":19},"updated":"2021-05-11 11:46:36.000000000","message":"these work but i would really like to see us centralise these in the future into a helper module.","commit_id":"4c6f3b688126f7ec9ef447611dfe693b23e5046f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1f11b10e2c8bb169a56b219096778d3775ea2eab","unresolved":true,"context_lines":[{"line_number":159,"context_line":"            \u0027tag %s but instead it is tagged with %s\u0027 %"},{"line_number":160,"context_line":"            (expected_vlan, interface_vlan))"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _get_port_attribute(self, port_id, attribute):"},{"line_number":163,"context_line":"        \"\"\"Get a specific attribute for provided port id"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        :param port_id: str the port id to search for"},{"line_number":166,"context_line":"        :param attribute: str the attribute or key to check from the returned"},{"line_number":167,"context_line":"        port dictionary"},{"line_number":168,"context_line":"        :return port_attribute: the requested port attribute value"},{"line_number":169,"context_line":"        \"\"\""},{"line_number":170,"context_line":"        body \u003d self.os_admin.ports_client.show_port(port_id)"},{"line_number":171,"context_line":"        port \u003d body[\u0027port\u0027]"},{"line_number":172,"context_line":"        return port.get(attribute)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def _search_pci_devices(self, column, value):"},{"line_number":175,"context_line":"        \"\"\"Returns all pci_device\u0027s address, status, and dev_type that match"}],"source_content_type":"text/x-python","patch_set":5,"id":"19fddf4f_2e27acf2","line":172,"range":{"start_line":162,"start_character":0,"end_line":172,"end_character":34},"updated":"2021-05-11 11:46:36.000000000","message":"this really does not add much value","commit_id":"4c6f3b688126f7ec9ef447611dfe693b23e5046f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1f11b10e2c8bb169a56b219096778d3775ea2eab","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        :param port_id str, the port id to request from the ports client"},{"line_number":196,"context_line":"        :param server_id str, the guest id to check"},{"line_number":197,"context_line":"        \"\"\""},{"line_number":198,"context_line":"        binding_profile \u003d self._get_port_attribute(port_id, \u0027binding:profile\u0027)"},{"line_number":199,"context_line":"        vnic_type \u003d self._get_port_attribute(port_id, \u0027binding:vnic_type\u0027)"},{"line_number":200,"context_line":"        pci_info \u003d self._search_pci_devices(\u0027instance_uuid\u0027, server_id)"},{"line_number":201,"context_line":"        for pci_device in pci_info:"},{"line_number":202,"context_line":"            self.assertEqual("}],"source_content_type":"text/x-python","patch_set":5,"id":"4d2c1248_29f64196","line":199,"range":{"start_line":198,"start_character":7,"end_line":199,"end_character":74},"updated":"2021-05-11 11:46:36.000000000","message":"this is making a seperate request for the port every time you invoke it.\nthats nto the end of the world but its pretty inefficent.\n\ni would just do\n\nport \u003d self.os_admin.ports_client.show_port(port_id)\nbinding_profile \u003d port[\u0027binding:profile\u0027]\nvnic_type \u003d port[\u0027binding:vnic_type\u0027]\n\nim not going to -1 for this but i would prefer if we removed the _get_port_attribute function in a followup","commit_id":"4c6f3b688126f7ec9ef447611dfe693b23e5046f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1f11b10e2c8bb169a56b219096778d3775ea2eab","unresolved":true,"context_lines":[{"line_number":456,"context_line":"        \"\"\""},{"line_number":457,"context_line":"        db_client \u003d clients.DatabaseClient()"},{"line_number":458,"context_line":"        db \u003d CONF.whitebox_database.nova_cell1_db_name"},{"line_number":459,"context_line":"        with db_client.cursor(db) as cursor:"},{"line_number":460,"context_line":"            cursor.execute(\u0027select COUNT(*) from pci_devices WHERE \u0027"},{"line_number":461,"context_line":"                           \u0027status \u003d \"%s\"\u0027 % status)"},{"line_number":462,"context_line":"            data \u003d cursor.fetchall()"},{"line_number":463,"context_line":"        return data[0][\u0027COUNT(*)\u0027]"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"    def _base_test_live_migration(self, vnic_type):"},{"line_number":466,"context_line":"        \"\"\"Parent test class that perform sr-iov live migration"}],"source_content_type":"text/x-python","patch_set":5,"id":"352d2c50_d737752d","line":463,"range":{"start_line":459,"start_character":5,"end_line":463,"end_character":34},"updated":"2021-05-11 11:46:36.000000000","message":"i kind of feel like we shoudl not use raw sql and should use sqlalchamey at somepoint.\ncan we look at removing the raw sql in a followup.\n\nI wont block on this for now since we already have an instance of doing this \nhttps://opendev.org/openstack/whitebox-tempest-plugin/src/commit/12eca6f75e2b966ddae0c1fe11d8bed6992c71fd/whitebox_tempest_plugin/api/compute/test_cpu_pinning.py#L172-L174\n\nbut i would prefer not to add any more.","commit_id":"4c6f3b688126f7ec9ef447611dfe693b23e5046f"}]}