)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"c5f292a58a6f97c58098a8233923ec2e3e0c9a88","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ae3242ed_91115bd2","updated":"2022-07-14 19:32:28.000000000","message":"PS1 Results, Note did not run macvtap due to [1]\n\n(overcloud) [stack@undercloud-0 tempest-dir]$ tempest run --serial --regex whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_attach_detach_port\n{0} whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_attach_detach_port [57.953429s] ... ok\n\n\u003d\u003d\u003d\u003d\u003d\u003d\nTotals\n\u003d\u003d\u003d\u003d\u003d\u003d\nRan: 1 tests in 57.9534 sec.\n - Passed: 1\n - Skipped: 0\n - Expected Fail: 0\n - Unexpected Success: 0\n - Failed: 0\nSum of execute time for each test: 57.9534 sec.\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nWorker Balance\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n - Worker 0 (1 tests) \u003d\u003e 0:00:57.953429\n\n\n[1] https://bugzilla.redhat.com/show_bug.cgi?id\u003d2093404","commit_id":"734b41f0afed45360eb9348559f34dafda6c2897"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9a8edaa143c2dc893c9184d4c3535d06f06bfd96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"71785664_003f49d7","updated":"2022-07-29 16:54:33.000000000","message":"Couple of minor things inline if you\u0027re going to respin anyways.","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"970d71a6eeb3ed3eb1ef6319916bb4ba2eed2f1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"30e1d713_e1cfed08","updated":"2022-07-29 16:03:55.000000000","message":"Talking with Sean downstream, would like to add a check that confirms the PCI address present in the XML matches what is being tracked in the DB.","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"f01e5e26b5726a41821644639d4c5514bd837b65","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6fe5eda7_475ae48c","updated":"2022-07-29 17:38:56.000000000","message":"Thanks for the review I\u0027ll incorporate these changes along with the pci db check as well.","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"f974d1fa09ec5a3961e0cf7cf32c861c0d51a676","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"11f20727_9721c28b","updated":"2022-08-03 19:48:02.000000000","message":"Downstream test results below, due to a driver issue with i40e did not run macvtap.\n(overcloud) [stack@undercloud-0 tempest-dir]$ tempest run --serial --regex whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_\n{0} whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_attach_detach_port [65.059352s] ... ok\n{0} whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_physical_attach_detach_port [73.405959s] ... ok\n\n\u003d\u003d\u003d\u003d\u003d\u003d\nTotals\n\u003d\u003d\u003d\u003d\u003d\u003d\nRan: 2 tests in 138.4735 sec.\n - Passed: 2\n - Skipped: 0\n - Expected Fail: 0\n - Unexpected Success: 0\n - Failed: 0\nSum of execute time for each test: 138.4653 sec.\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nWorker Balance\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n - Worker 0 (2 tests) \u003d\u003e 0:02:18.473515\n","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"675ae9fd279498d5c9d6dd2dbf1a382dd4e585c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2cb08fd9_7b196813","updated":"2022-08-04 08:30:09.000000000","message":"I would prefer if we change the verbs we are using for attach/detatch away form create/delete but those functions are not added in this patch so im fine to proceed with this","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"921e9f667dc7513574f74b088247858299e09497","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e206f242_12c07848","updated":"2022-08-17 19:47:09.000000000","message":"I\u0027d definitely prefer to do make the changes in this commit versus a follow-up.  Should have something up tomorrow.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"80686b8c11ef7915fe076c987bdc1c4eafaa9e00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a35e8f91_041fe739","updated":"2022-08-08 22:13:18.000000000","message":"Questions inline... not enough for a -1, but also not sure enough for a +2 :)","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"10e1f0728393fbe1bb5d1868e3e208321b1220b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f011373a_a779786f","updated":"2022-08-17 18:29:31.000000000","message":"Still a couple of things outstanding, but I don\u0027t think we need to block on them. I\u0027ll cowardly let James make the call of whether to address them here, or merge this as-is and follow up.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"dc5cad766956398ae2efedb04d6963643c9f3580","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a59a245d_b260adb6","updated":"2022-08-25 20:18:14.000000000","message":"Results for ps5:\n\n(overcloud) [stack@undercloud-0 tempest-dir]$ python3 -m testtools.run whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_physical_attach_detach_port\n/usr/lib64/python3.9/runpy.py:127: RuntimeWarning: \u0027testtools.run\u0027 found in sys.modules after import of package \u0027testtools\u0027, but prior to execution of \u0027testtools.run\u0027; this may result in unpredictable behaviour\n  warn(RuntimeWarning(msg))\nTests running...\n\nRan 1 test in 116.184s\nOK\n(overcloud) [stack@undercloud-0 tempest-dir]$ tempest run --serial --regex whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct\n{0} whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_attach_detach_port [67.049322s] ... ok\n{0} whitebox_tempest_plugin.api.compute.test_sriov.SRIOVAttachAndDetach.test_sriov_direct_physical_attach_detach_port [90.147258s] ... ok\n\n\u003d\u003d\u003d\u003d\u003d\u003d\nTotals\n\u003d\u003d\u003d\u003d\u003d\u003d\nRan: 2 tests in 157.2046 sec.\n - Passed: 2\n - Skipped: 0\n - Expected Fail: 0\n - Unexpected Success: 0\n - Failed: 0\nSum of execute time for each test: 157.1966 sec.\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nWorker Balance\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n - Worker 0 (2 tests) \u003d\u003e 0:02:37.204605\n","commit_id":"4ddccddf2ff686f4ae856c9ee24fe1760d8f8f34"}],"whitebox_tempest_plugin/api/compute/test_sriov.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ce41cc12367b0398488b66376c87a4521c43952e","unresolved":true,"context_lines":[{"line_number":836,"context_line":"        if (CONF.whitebox_hardware.sriov_nic_vendor_id is None):"},{"line_number":837,"context_line":"            msg \u003d \"CONF.whitebox_hardware.sriov_nic_vendor_id needs to be set.\""},{"line_number":838,"context_line":"            raise cls.skipException(msg)"},{"line_number":839,"context_line":"        if (CONF.whitebox_hardware.sriov_nic_device_id is None):"},{"line_number":840,"context_line":"            msg \u003d \"CONF.whitebox_hardware.sriov_nic_device_id needs to be set.\""},{"line_number":841,"context_line":"            raise cls.skipException(msg)"},{"line_number":842,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"eaefd04e_a6ede186","line":839,"updated":"2022-08-04 12:10:12.000000000","message":"Do we want to skip on missing product_id?","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"905acb55f7b395b2b713c2172d912b6a4246ab56","unresolved":true,"context_lines":[{"line_number":836,"context_line":"        if (CONF.whitebox_hardware.sriov_nic_vendor_id is None):"},{"line_number":837,"context_line":"            msg \u003d \"CONF.whitebox_hardware.sriov_nic_vendor_id needs to be set.\""},{"line_number":838,"context_line":"            raise cls.skipException(msg)"},{"line_number":839,"context_line":"        if (CONF.whitebox_hardware.sriov_nic_device_id is None):"},{"line_number":840,"context_line":"            msg \u003d \"CONF.whitebox_hardware.sriov_nic_device_id needs to be set.\""},{"line_number":841,"context_line":"            raise cls.skipException(msg)"},{"line_number":842,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bfb00614_b1f5ee65","line":839,"in_reply_to":"eaefd04e_a6ede186","updated":"2022-08-04 14:37:46.000000000","message":"Since the product ids are different for vf/pf I removed it at the class level and added more specific checks at #L1020, #L1027, #1034.","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9a8edaa143c2dc893c9184d4c3535d06f06bfd96","unresolved":true,"context_lines":[{"line_number":845,"context_line":"        cls.prepare_instance_network()"},{"line_number":846,"context_line":"        super(SRIOVAttachAndDetach, cls).setup_credentials()"},{"line_number":847,"context_line":""},{"line_number":848,"context_line":"    def wait_for_port_detach(self, port_id):"},{"line_number":849,"context_line":"        \"\"\"Waits for the port\u0027s device_id to be unset."},{"line_number":850,"context_line":"        :param port_id: The id of the port being detached."},{"line_number":851,"context_line":"        :returns: The final port dict from the show_port response."}],"source_content_type":"text/x-python","patch_set":3,"id":"78b00a86_d9b014d5","line":848,"updated":"2022-07-29 16:54:33.000000000","message":"OK, you copied this here because in tempest it\u0027s in a test class rather than a base or a lib. Ideally we\u0027d refactor tempest and use the shared helper there, but I\u0027d feel bad blocking this patch on it, so we can do in a follow up.","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"f01e5e26b5726a41821644639d4c5514bd837b65","unresolved":false,"context_lines":[{"line_number":845,"context_line":"        cls.prepare_instance_network()"},{"line_number":846,"context_line":"        super(SRIOVAttachAndDetach, cls).setup_credentials()"},{"line_number":847,"context_line":""},{"line_number":848,"context_line":"    def wait_for_port_detach(self, port_id):"},{"line_number":849,"context_line":"        \"\"\"Waits for the port\u0027s device_id to be unset."},{"line_number":850,"context_line":"        :param port_id: The id of the port being detached."},{"line_number":851,"context_line":"        :returns: The final port dict from the show_port response."}],"source_content_type":"text/x-python","patch_set":3,"id":"569c9990_66dd6cd4","line":848,"in_reply_to":"78b00a86_d9b014d5","updated":"2022-07-29 17:38:56.000000000","message":"Ack","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9a8edaa143c2dc893c9184d4c3535d06f06bfd96","unresolved":true,"context_lines":[{"line_number":925,"context_line":"        linux_client.validate_authentication()"},{"line_number":926,"context_line":"        return linux_client"},{"line_number":927,"context_line":""},{"line_number":928,"context_line":"    def create_validateable_instance(self):"},{"line_number":929,"context_line":"        \"\"\"Create a validateable instance based on provided flavor"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"        :param flavor: dict, attributes describing flavor"}],"source_content_type":"text/x-python","patch_set":3,"id":"51da2c5b_35bdab36","line":928,"updated":"2022-07-29 16:54:33.000000000","message":"Ah, so this is just a helper that return a (server, ssh_client) tuple. Could we rename it to something closer to reality? tempest already has the create_test_server() method, and that accepts a validatable kwarg, so at first glance this method looks like it\u0027s repeating work. Maybe call it create_server_and_ssh()?","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"f01e5e26b5726a41821644639d4c5514bd837b65","unresolved":false,"context_lines":[{"line_number":925,"context_line":"        linux_client.validate_authentication()"},{"line_number":926,"context_line":"        return linux_client"},{"line_number":927,"context_line":""},{"line_number":928,"context_line":"    def create_validateable_instance(self):"},{"line_number":929,"context_line":"        \"\"\"Create a validateable instance based on provided flavor"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"        :param flavor: dict, attributes describing flavor"}],"source_content_type":"text/x-python","patch_set":3,"id":"fea1d506_245c0794","line":928,"in_reply_to":"51da2c5b_35bdab36","updated":"2022-07-29 17:38:56.000000000","message":"Ack","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9a8edaa143c2dc893c9184d4c3535d06f06bfd96","unresolved":true,"context_lines":[{"line_number":952,"context_line":"        :param fixed_ip: str, expected ip address of attached iface"},{"line_number":953,"context_line":"        :param mac_addr: str, expected mac address of attached iface"},{"line_number":954,"context_line":"        \"\"\""},{"line_number":955,"context_line":"        if server_id:"},{"line_number":956,"context_line":"            iface \u003d waiters.wait_for_interface_status("},{"line_number":957,"context_line":"                self.interfaces_client, server_id, iface[\u0027port_id\u0027], \u0027ACTIVE\u0027)"},{"line_number":958,"context_line":"        if port_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"866625d0_831fd0f0","line":955,"updated":"2022-07-29 16:54:33.000000000","message":"So you never actually *not* pass the server_id here, so I feel like the code that calls this method should just do the waiting there, since, based on this method\u0027s name / docstring, it just should just verify stuff and nothing else.\n\nEdit: in fact, do we actually need the waiting? [1] looks to be synchronous, as it returns 200, so presumably if the request succeeds the interface has been attached.\n\n[1] https://docs.openstack.org/api-ref/compute/?expanded\u003dcreate-interface-detail#create-interface","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"f01e5e26b5726a41821644639d4c5514bd837b65","unresolved":false,"context_lines":[{"line_number":952,"context_line":"        :param fixed_ip: str, expected ip address of attached iface"},{"line_number":953,"context_line":"        :param mac_addr: str, expected mac address of attached iface"},{"line_number":954,"context_line":"        \"\"\""},{"line_number":955,"context_line":"        if server_id:"},{"line_number":956,"context_line":"            iface \u003d waiters.wait_for_interface_status("},{"line_number":957,"context_line":"                self.interfaces_client, server_id, iface[\u0027port_id\u0027], \u0027ACTIVE\u0027)"},{"line_number":958,"context_line":"        if port_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"d5333ea6_d1952339","line":955,"in_reply_to":"866625d0_831fd0f0","updated":"2022-07-29 17:38:56.000000000","message":"Ack","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ed75700f85fb5c906803869b7c8ef6dfe78c812","unresolved":false,"context_lines":[{"line_number":952,"context_line":"        :param fixed_ip: str, expected ip address of attached iface"},{"line_number":953,"context_line":"        :param mac_addr: str, expected mac address of attached iface"},{"line_number":954,"context_line":"        \"\"\""},{"line_number":955,"context_line":"        if server_id:"},{"line_number":956,"context_line":"            iface \u003d waiters.wait_for_interface_status("},{"line_number":957,"context_line":"                self.interfaces_client, server_id, iface[\u0027port_id\u0027], \u0027ACTIVE\u0027)"},{"line_number":958,"context_line":"        if port_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"087837ec_dfd46adb","line":955,"in_reply_to":"d5333ea6_d1952339","updated":"2022-08-01 08:54:06.000000000","message":"yepp attach_interface is an RPC call so it is synch","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ed75700f85fb5c906803869b7c8ef6dfe78c812","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c855591b_8bda17ad","line":1025,"updated":"2022-08-01 08:54:06.000000000","message":"Do we care about direct-physical ports?","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"90738332ddd975355934569f22a986e7014848cf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4d6e5f55_ab351f6a","line":1025,"in_reply_to":"c855591b_8bda17ad","updated":"2022-08-03 19:46:56.000000000","message":"Thanks, updated in the following PS.","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"675ae9fd279498d5c9d6dd2dbf1a382dd4e585c2","unresolved":true,"context_lines":[{"line_number":142,"context_line":"        pci_addr_element \u003d host_dev_xml.find(\"./source/address\")"},{"line_number":143,"context_line":"        pci_address \u003d hardware.get_pci_address_from_xml_device("},{"line_number":144,"context_line":"            pci_addr_element)"},{"line_number":145,"context_line":"        self.assertEqual("},{"line_number":146,"context_line":"            pci_address,"},{"line_number":147,"context_line":"            binding_profile[\u0027pci_slot\u0027], \u0027PCI device found in XML %s\u0027"},{"line_number":148,"context_line":"            \u0027does not match what is tracked in binding profile for port %s\u0027 %"},{"line_number":149,"context_line":"            (pci_address, binding_profile))"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def _get_xml_pf_device(self, server_id):"},{"line_number":152,"context_line":"        \"\"\"Returns xml hostdev element from the provided server id"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ebf13a8_4197492c","line":149,"range":{"start_line":145,"start_character":7,"end_line":149,"end_character":43},"updated":"2022-08-04 08:30:09.000000000","message":"ok I was wondering where this was checked. I guess that is why the port_id is passed in","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"905acb55f7b395b2b713c2172d912b6a4246ab56","unresolved":false,"context_lines":[{"line_number":142,"context_line":"        pci_addr_element \u003d host_dev_xml.find(\"./source/address\")"},{"line_number":143,"context_line":"        pci_address \u003d hardware.get_pci_address_from_xml_device("},{"line_number":144,"context_line":"            pci_addr_element)"},{"line_number":145,"context_line":"        self.assertEqual("},{"line_number":146,"context_line":"            pci_address,"},{"line_number":147,"context_line":"            binding_profile[\u0027pci_slot\u0027], \u0027PCI device found in XML %s\u0027"},{"line_number":148,"context_line":"            \u0027does not match what is tracked in binding profile for port %s\u0027 %"},{"line_number":149,"context_line":"            (pci_address, binding_profile))"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def _get_xml_pf_device(self, server_id):"},{"line_number":152,"context_line":"        \"\"\"Returns xml hostdev element from the provided server id"}],"source_content_type":"text/x-python","patch_set":4,"id":"7d3c2e2e_f583debb","line":149,"range":{"start_line":145,"start_character":7,"end_line":149,"end_character":43},"in_reply_to":"3ebf13a8_4197492c","updated":"2022-08-04 14:37:46.000000000","message":"Ack","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"80686b8c11ef7915fe076c987bdc1c4eafaa9e00","unresolved":true,"context_lines":[{"line_number":957,"context_line":"        \"\"\"Validate sr-iov interface can be attached/detached with guests"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        1. Create and sr-iov port based on the provided vnic_type"},{"line_number":960,"context_line":"        2. Launch two guests with UC access via SSH"},{"line_number":961,"context_line":"        3. Iterate over both guests doing the following steps:"},{"line_number":962,"context_line":"           3a. Attach the interface to the guest"},{"line_number":963,"context_line":"           3b. Check the return information about the attached interface"}],"source_content_type":"text/x-python","patch_set":4,"id":"7b529db3_e53617d2","line":960,"updated":"2022-08-08 22:13:18.000000000","message":"This may be a dumb question but... why 2 guests? Does something \"special\" happen when there are 2 of them? Could we get a false positive passing test if we just use one server?","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"10e1f0728393fbe1bb5d1868e3e208321b1220b9","unresolved":true,"context_lines":[{"line_number":957,"context_line":"        \"\"\"Validate sr-iov interface can be attached/detached with guests"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        1. Create and sr-iov port based on the provided vnic_type"},{"line_number":960,"context_line":"        2. Launch two guests with UC access via SSH"},{"line_number":961,"context_line":"        3. Iterate over both guests doing the following steps:"},{"line_number":962,"context_line":"           3a. Attach the interface to the guest"},{"line_number":963,"context_line":"           3b. Check the return information about the attached interface"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a0c27db_1f887e73","line":960,"in_reply_to":"09015f25_168ef024","updated":"2022-08-17 18:29:31.000000000","message":"Ah, it\u0027s the loop iteration that confused me (I know, dumb). By iterating over the two servers, with a attach-detach sequence in each iteration, we\u0027re making sure that we can attach to a different guest. Can I ask you to adjust the comment to explain that a bit more explicitly?","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"921e9f667dc7513574f74b088247858299e09497","unresolved":false,"context_lines":[{"line_number":957,"context_line":"        \"\"\"Validate sr-iov interface can be attached/detached with guests"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        1. Create and sr-iov port based on the provided vnic_type"},{"line_number":960,"context_line":"        2. Launch two guests with UC access via SSH"},{"line_number":961,"context_line":"        3. Iterate over both guests doing the following steps:"},{"line_number":962,"context_line":"           3a. Attach the interface to the guest"},{"line_number":963,"context_line":"           3b. Check the return information about the attached interface"}],"source_content_type":"text/x-python","patch_set":4,"id":"c4ad953b_133fa453","line":960,"in_reply_to":"7a0c27db_1f887e73","updated":"2022-08-17 19:47:09.000000000","message":"Sure thing I\u0027ll update it to make it clearer.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"37d2c713e967c86006d65de8e820f6ce5462c6c3","unresolved":true,"context_lines":[{"line_number":957,"context_line":"        \"\"\"Validate sr-iov interface can be attached/detached with guests"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        1. Create and sr-iov port based on the provided vnic_type"},{"line_number":960,"context_line":"        2. Launch two guests with UC access via SSH"},{"line_number":961,"context_line":"        3. Iterate over both guests doing the following steps:"},{"line_number":962,"context_line":"           3a. Attach the interface to the guest"},{"line_number":963,"context_line":"           3b. Check the return information about the attached interface"}],"source_content_type":"text/x-python","patch_set":4,"id":"09015f25_168ef024","line":960,"in_reply_to":"7b529db3_e53617d2","updated":"2022-08-09 15:27:42.000000000","message":"I wouldn\u0027t say its critical, it just ensures that attach works after a detach event with a different guest.  I\u0027m not beholden to this approach though, I could just loop over the same guest as well.  The biggest aspect to me would just be confirming the attach continues to work after a detach.  Let me know if you have a preference.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"675ae9fd279498d5c9d6dd2dbf1a382dd4e585c2","unresolved":true,"context_lines":[{"line_number":991,"context_line":"        # the attach was successful from an API, XML, and guest level and"},{"line_number":992,"context_line":"        # then detach the interface from the guest"},{"line_number":993,"context_line":"        for server, linux_client in servers:"},{"line_number":994,"context_line":"            iface \u003d self.interfaces_client.create_interface("},{"line_number":995,"context_line":"                server[\u0027id\u0027], port_id\u003dport_id)[\u0027interfaceAttachment\u0027]"},{"line_number":996,"context_line":""},{"line_number":997,"context_line":"            self.assertEqual(iface[\u0027port_id\u0027], port_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ed2d4871_0b05993f","line":994,"range":{"start_line":994,"start_character":43,"end_line":994,"end_character":49},"updated":"2022-08-04 08:30:09.000000000","message":"nit: attach?","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"905acb55f7b395b2b713c2172d912b6a4246ab56","unresolved":true,"context_lines":[{"line_number":991,"context_line":"        # the attach was successful from an API, XML, and guest level and"},{"line_number":992,"context_line":"        # then detach the interface from the guest"},{"line_number":993,"context_line":"        for server, linux_client in servers:"},{"line_number":994,"context_line":"            iface \u003d self.interfaces_client.create_interface("},{"line_number":995,"context_line":"                server[\u0027id\u0027], port_id\u003dport_id)[\u0027interfaceAttachment\u0027]"},{"line_number":996,"context_line":""},{"line_number":997,"context_line":"            self.assertEqual(iface[\u0027port_id\u0027], port_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"67b8e217_7ed9b4eb","line":994,"range":{"start_line":994,"start_character":43,"end_line":994,"end_character":49},"in_reply_to":"ed2d4871_0b05993f","updated":"2022-08-04 14:37:46.000000000","message":"I agree, I think the tempest functions [1] that were created for this were following what is in the api [2] docs in regards to naming.\n\n[1] https://github.com/openstack/tempest/blob/master/tempest/lib/services/compute/interfaces_client.py#L38\n[2] https://docs.openstack.org/api-ref/compute/?expanded\u003dcreate-interface-detail#create-interface","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"675ae9fd279498d5c9d6dd2dbf1a382dd4e585c2","unresolved":true,"context_lines":[{"line_number":1014,"context_line":"                server[\u0027id\u0027],"},{"line_number":1015,"context_line":"                port[\u0027port\u0027][\u0027id\u0027]"},{"line_number":1016,"context_line":"            )"},{"line_number":1017,"context_line":"            self.interfaces_client.delete_interface(server[\u0027id\u0027], port_id)"},{"line_number":1018,"context_line":"            self.wait_for_port_detach(port_id)"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"    @testtools.skipUnless(CONF.whitebox_hardware.sriov_vf_product_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"22bd0c80_0afc7235","line":1017,"range":{"start_line":1017,"start_character":12,"end_line":1017,"end_character":51},"updated":"2022-08-04 08:30:09.000000000","message":"this should be detach_interface\n\n\nwe use create and delete to refer to the life cycle of the neutron port\nand attach/detach or add/remove to refer to the association of the port with the vm.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"905acb55f7b395b2b713c2172d912b6a4246ab56","unresolved":true,"context_lines":[{"line_number":1014,"context_line":"                server[\u0027id\u0027],"},{"line_number":1015,"context_line":"                port[\u0027port\u0027][\u0027id\u0027]"},{"line_number":1016,"context_line":"            )"},{"line_number":1017,"context_line":"            self.interfaces_client.delete_interface(server[\u0027id\u0027], port_id)"},{"line_number":1018,"context_line":"            self.wait_for_port_detach(port_id)"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"    @testtools.skipUnless(CONF.whitebox_hardware.sriov_vf_product_id,"}],"source_content_type":"text/x-python","patch_set":4,"id":"53d91693_3e6ee12b","line":1017,"range":{"start_line":1017,"start_character":12,"end_line":1017,"end_character":51},"in_reply_to":"22bd0c80_0afc7235","updated":"2022-08-04 14:37:46.000000000","message":"Same as comment from #L994, I can see if that could be updated in tempest to better reflect the lifecycle.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"675ae9fd279498d5c9d6dd2dbf1a382dd4e585c2","unresolved":true,"context_lines":[{"line_number":1015,"context_line":"                port[\u0027port\u0027][\u0027id\u0027]"},{"line_number":1016,"context_line":"            )"},{"line_number":1017,"context_line":"            self.interfaces_client.delete_interface(server[\u0027id\u0027], port_id)"},{"line_number":1018,"context_line":"            self.wait_for_port_detach(port_id)"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"    @testtools.skipUnless(CONF.whitebox_hardware.sriov_vf_product_id,"},{"line_number":1021,"context_line":"                          \"Requires sriov NIC\u0027s VF ID\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"8541db93_f5d76dd5","line":1018,"updated":"2022-08-04 08:30:09.000000000","message":"the neutron port should still exist here but the device_id should no longer reference the vm.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"905acb55f7b395b2b713c2172d912b6a4246ab56","unresolved":false,"context_lines":[{"line_number":1015,"context_line":"                port[\u0027port\u0027][\u0027id\u0027]"},{"line_number":1016,"context_line":"            )"},{"line_number":1017,"context_line":"            self.interfaces_client.delete_interface(server[\u0027id\u0027], port_id)"},{"line_number":1018,"context_line":"            self.wait_for_port_detach(port_id)"},{"line_number":1019,"context_line":""},{"line_number":1020,"context_line":"    @testtools.skipUnless(CONF.whitebox_hardware.sriov_vf_product_id,"},{"line_number":1021,"context_line":"                          \"Requires sriov NIC\u0027s VF ID\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"f5b06bd5_5ea1697e","line":1018,"in_reply_to":"8541db93_f5d76dd5","updated":"2022-08-04 14:37:46.000000000","message":"Ack","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"80686b8c11ef7915fe076c987bdc1c4eafaa9e00","unresolved":true,"context_lines":[{"line_number":1077,"context_line":"            # The originally created port\u0027s mac address will be updated to"},{"line_number":1078,"context_line":"            # the physical device\u0027s mac address on the host. Original port mac"},{"line_number":1079,"context_line":"            # should no longer match updated host mac"},{"line_number":1080,"context_line":"            self.assertNotEqual(iface[\u0027mac_addr\u0027], port_mac_addr)"},{"line_number":1081,"context_line":"            host_dev_xml \u003d self._get_xml_pf_device(server[\u0027id\u0027])"},{"line_number":1082,"context_line":"            self._validate_pf_pci_address_in_xml(port_id, host_dev_xml)"},{"line_number":1083,"context_line":"            self._check_device_in_guest("}],"source_content_type":"text/x-python","patch_set":4,"id":"cb554e03_606b8caa","line":1080,"updated":"2022-08-08 22:13:18.000000000","message":"So most of this is repeating itself, except for the MAC address check and the PCI address check. I\u0027m weary of being \"smart\" at the cost of complexity but I wonder if there\u0027s a way to avoid copy-pasting code, and maybe just pass the different checks as a method (well, two methods) to the _base_test_attach_and_detach_sriov_port() test?","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"10e1f0728393fbe1bb5d1868e3e208321b1220b9","unresolved":true,"context_lines":[{"line_number":1077,"context_line":"            # The originally created port\u0027s mac address will be updated to"},{"line_number":1078,"context_line":"            # the physical device\u0027s mac address on the host. Original port mac"},{"line_number":1079,"context_line":"            # should no longer match updated host mac"},{"line_number":1080,"context_line":"            self.assertNotEqual(iface[\u0027mac_addr\u0027], port_mac_addr)"},{"line_number":1081,"context_line":"            host_dev_xml \u003d self._get_xml_pf_device(server[\u0027id\u0027])"},{"line_number":1082,"context_line":"            self._validate_pf_pci_address_in_xml(port_id, host_dev_xml)"},{"line_number":1083,"context_line":"            self._check_device_in_guest("}],"source_content_type":"text/x-python","patch_set":4,"id":"e21c4e12_799efaf1","line":1080,"in_reply_to":"4fd1805a_63937980","updated":"2022-08-17 18:29:31.000000000","message":"Something like that. If I\u0027ve read this correctly, the only differences are:\n\n  test_sriov_direct_physical_attach_detach_port()\n  L1080 self.assertNotEqual(iface[\u0027mac_addr\u0027], port_mac_addr)\n  L1082 self._validate_pf_pci_address_in_xml(port_id, host_dev_xml)\n\n  _base_test_attach_and_detach_sriov_port()\n  L1000 self.assertEqual(iface[\u0027mac_addr\u0027], port_mac_addr)\n  L1008 _validate_port_xml_vlan_tag\n  \nThere\u0027s a couple of extra lines around the second of each of those that are also different.\n\nIf we can figure out a way to pass those checks in as a method to the _base() test method (without being too complex/unreadable), I think it\u0027d be worth it?","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"37d2c713e967c86006d65de8e820f6ce5462c6c3","unresolved":true,"context_lines":[{"line_number":1077,"context_line":"            # The originally created port\u0027s mac address will be updated to"},{"line_number":1078,"context_line":"            # the physical device\u0027s mac address on the host. Original port mac"},{"line_number":1079,"context_line":"            # should no longer match updated host mac"},{"line_number":1080,"context_line":"            self.assertNotEqual(iface[\u0027mac_addr\u0027], port_mac_addr)"},{"line_number":1081,"context_line":"            host_dev_xml \u003d self._get_xml_pf_device(server[\u0027id\u0027])"},{"line_number":1082,"context_line":"            self._validate_pf_pci_address_in_xml(port_id, host_dev_xml)"},{"line_number":1083,"context_line":"            self._check_device_in_guest("}],"source_content_type":"text/x-python","patch_set":4,"id":"4fd1805a_63937980","line":1080,"in_reply_to":"cb554e03_606b8caa","updated":"2022-08-09 15:27:42.000000000","message":"So potentially moving #L1074-1076 and #997-999 to its own method?","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"921e9f667dc7513574f74b088247858299e09497","unresolved":false,"context_lines":[{"line_number":1077,"context_line":"            # The originally created port\u0027s mac address will be updated to"},{"line_number":1078,"context_line":"            # the physical device\u0027s mac address on the host. Original port mac"},{"line_number":1079,"context_line":"            # should no longer match updated host mac"},{"line_number":1080,"context_line":"            self.assertNotEqual(iface[\u0027mac_addr\u0027], port_mac_addr)"},{"line_number":1081,"context_line":"            host_dev_xml \u003d self._get_xml_pf_device(server[\u0027id\u0027])"},{"line_number":1082,"context_line":"            self._validate_pf_pci_address_in_xml(port_id, host_dev_xml)"},{"line_number":1083,"context_line":"            self._check_device_in_guest("}],"source_content_type":"text/x-python","patch_set":4,"id":"18a08ce5_30fba900","line":1080,"in_reply_to":"e21c4e12_799efaf1","updated":"2022-08-17 19:47:09.000000000","message":"Works for me I\u0027ll update this commit to have these checks in a shared method.","commit_id":"39ebf0da454944e529d3c3c61651b3cf9bf021c8"}],"whitebox_tempest_plugin/config.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ed75700f85fb5c906803869b7c8ef6dfe78c812","unresolved":true,"context_lines":[{"line_number":232,"context_line":"        help\u003d\u0027The vendor id of the underlying sriov nic port of the compute. \u0027"},{"line_number":233,"context_line":"             \u0027An example with Intel would be 8086\u0027),"},{"line_number":234,"context_line":"    cfg.StrOpt("},{"line_number":235,"context_line":"        \u0027sriov_nic_device_id\u0027,"},{"line_number":236,"context_line":"        default\u003dNone,"},{"line_number":237,"context_line":"        help\u003d\u0027The device id of the underlying sriov nic port of the compute. \u0027"},{"line_number":238,"context_line":"             \u0027An example with Nvidia would be 154c\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"bb6a32db_4d912c8f","line":235,"range":{"start_line":235,"start_character":9,"end_line":235,"end_character":28},"updated":"2022-08-01 08:54:06.000000000","message":"what is the standard name device_id or product_id? The nova code calls this product_id. The device_id term is a bit overaloaded as it is used in the neutron port to indicate the id of the owner of the port (i.e. the VM uuid).","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"},{"author":{"_account_id":31033,"name":"James Parker","email":"jparker@redhat.com","username":"jparker"},"change_message_id":"3c5c57fd01c6599797053ac13e15c996200fef78","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        help\u003d\u0027The vendor id of the underlying sriov nic port of the compute. \u0027"},{"line_number":233,"context_line":"             \u0027An example with Intel would be 8086\u0027),"},{"line_number":234,"context_line":"    cfg.StrOpt("},{"line_number":235,"context_line":"        \u0027sriov_nic_device_id\u0027,"},{"line_number":236,"context_line":"        default\u003dNone,"},{"line_number":237,"context_line":"        help\u003d\u0027The device id of the underlying sriov nic port of the compute. \u0027"},{"line_number":238,"context_line":"             \u0027An example with Nvidia would be 154c\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"20ba964c_1bafb309","line":235,"range":{"start_line":235,"start_character":9,"end_line":235,"end_character":28},"in_reply_to":"bb6a32db_4d912c8f","updated":"2022-08-03 19:49:44.000000000","message":"ack I updated to use product id instead","commit_id":"ca6970cabbf536cccd9fe74c29bbd8cdf22d5480"}]}