)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"38ef3e14_d50cdf67","updated":"2025-03-25 17:44:26.000000000","message":"I will continue reviewing tomorrow. But so far it looks good","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"58544b4ef78d978af896651a9a1ab34766165354","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"94fb7df1_28a46f88","updated":"2025-03-27 11:03:59.000000000","message":"I think the two assert functions are good. I have a suggestion for the explicit NoValidHost checking inline. If you can use that then I think we can land this.","commit_id":"eb4b3e240df9feff9c48d8e99a368557cc3d54a6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b6c4229147f4d5559fdbd1254803a36a43a95fde","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4b9e2143_e658bb04","updated":"2025-03-27 15:44:57.000000000","message":"Looks good to me. Thanks Rene!","commit_id":"9947dac7aed5103400e654d5b24d90ab0c4422a3"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ff444e47ae93496b5aa4470f5275fc6b1ea69627","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4707bf05_ef17f9b2","updated":"2025-04-24 13:52:10.000000000","message":"recheck timeout on resize","commit_id":"9947dac7aed5103400e654d5b24d90ab0c4422a3"}],"nova/tests/functional/libvirt/test_pci_sriov_servers.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d8c4202cf29ebb7e1e7c58ff73c9f550a62459df","unresolved":true,"context_lines":[{"line_number":773,"context_line":"            {"},{"line_number":774,"context_line":"                \"vendor_id\": fakelibvirt.PCI_VEND_ID,"},{"line_number":775,"context_line":"                \"product_id\": fakelibvirt.VF_PROD_ID,"},{"line_number":776,"context_line":"                \"name\": f\"{self.VFS_ALIAS_NAME}:1\","},{"line_number":777,"context_line":"                \"device_type\": fields.PciDeviceType.SRIOV_VF,"},{"line_number":778,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":779,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":2,"id":"3179e2a4_c1659c74","line":776,"updated":"2025-03-13 09:52:38.000000000","message":"wait a sec, why do we need to change the pci alias name here ?\nI guess this is because you\u0027d prefer to check the names by a number suffix, right ? If so, you\u0027re changing a lot of tests while it\u0027s not needed 😞","commit_id":"95c6703f1e1dd91cf64b7c3a89db66c7517e2048"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"8fb9010033101e465a40cebc3a698e9fcf92f953","unresolved":true,"context_lines":[{"line_number":773,"context_line":"            {"},{"line_number":774,"context_line":"                \"vendor_id\": fakelibvirt.PCI_VEND_ID,"},{"line_number":775,"context_line":"                \"product_id\": fakelibvirt.VF_PROD_ID,"},{"line_number":776,"context_line":"                \"name\": f\"{self.VFS_ALIAS_NAME}:1\","},{"line_number":777,"context_line":"                \"device_type\": fields.PciDeviceType.SRIOV_VF,"},{"line_number":778,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":779,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":2,"id":"742cb898_123e73e0","line":776,"in_reply_to":"3179e2a4_c1659c74","updated":"2025-03-13 10:38:01.000000000","message":"Gibi did not like having a qty list separated.\nSo I had the quantity as a name suffix. And yes I had to change a lot of tests due to that.","commit_id":"95c6703f1e1dd91cf64b7c3a89db66c7517e2048"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"e20fcada600e0f015da107fc92143307fb134991","unresolved":false,"context_lines":[{"line_number":773,"context_line":"            {"},{"line_number":774,"context_line":"                \"vendor_id\": fakelibvirt.PCI_VEND_ID,"},{"line_number":775,"context_line":"                \"product_id\": fakelibvirt.VF_PROD_ID,"},{"line_number":776,"context_line":"                \"name\": f\"{self.VFS_ALIAS_NAME}:1\","},{"line_number":777,"context_line":"                \"device_type\": fields.PciDeviceType.SRIOV_VF,"},{"line_number":778,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":779,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":2,"id":"666b608b_d673e44d","line":776,"in_reply_to":"40bcdb2f_4a1da5cd","updated":"2025-03-21 16:17:57.000000000","message":"Done","commit_id":"95c6703f1e1dd91cf64b7c3a89db66c7517e2048"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a5fe174299a5914c63fa39b0e40d788091c601b7","unresolved":true,"context_lines":[{"line_number":773,"context_line":"            {"},{"line_number":774,"context_line":"                \"vendor_id\": fakelibvirt.PCI_VEND_ID,"},{"line_number":775,"context_line":"                \"product_id\": fakelibvirt.VF_PROD_ID,"},{"line_number":776,"context_line":"                \"name\": f\"{self.VFS_ALIAS_NAME}:1\","},{"line_number":777,"context_line":"                \"device_type\": fields.PciDeviceType.SRIOV_VF,"},{"line_number":778,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":779,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":2,"id":"40bcdb2f_4a1da5cd","line":776,"in_reply_to":"742cb898_123e73e0","updated":"2025-03-14 12:50:23.000000000","message":"wait the alias itself should not have a quantity that is part of the flavor extra spec.\n\nSo this is still mixing concepts.\n\nI would like a test structure where the test case itself clearly and separately defines:\n* what devices the compute has via the HostPCIDevicesInfo struct\n* what is the device_spec config for the compute\n* what is the alias config\n* what is the flavor extra_spec\n\nplease do not try to combine / hide these","commit_id":"95c6703f1e1dd91cf64b7c3a89db66c7517e2048"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"8fb9010033101e465a40cebc3a698e9fcf92f953","unresolved":true,"context_lines":[{"line_number":1649,"context_line":"            },"},{"line_number":1650,"context_line":"        ]"},{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        def customize_config(src_pci_info, dst_pci_info, extra_spec, networks):"},{"line_number":1653,"context_line":"            src_pci_info.add_device("},{"line_number":1654,"context_line":"                dev_type\u003d\u0027PF\u0027,"},{"line_number":1655,"context_line":"                prod_id\u003d\u00278888\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"d9786a32_f16fa27a","line":1652,"updated":"2025-03-13 10:38:01.000000000","message":"I forgot to \"factor\" this for all test cases that use it.\nI\u0027m going to do it in a new iteration.","commit_id":"95c6703f1e1dd91cf64b7c3a89db66c7517e2048"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"e20fcada600e0f015da107fc92143307fb134991","unresolved":false,"context_lines":[{"line_number":1649,"context_line":"            },"},{"line_number":1650,"context_line":"        ]"},{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"        def customize_config(src_pci_info, dst_pci_info, extra_spec, networks):"},{"line_number":1653,"context_line":"            src_pci_info.add_device("},{"line_number":1654,"context_line":"                dev_type\u003d\u0027PF\u0027,"},{"line_number":1655,"context_line":"                prod_id\u003d\u00278888\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"46bf1c5e_f82b91a5","line":1652,"in_reply_to":"d9786a32_f16fa27a","updated":"2025-03-21 16:17:57.000000000","message":"Done","commit_id":"95c6703f1e1dd91cf64b7c3a89db66c7517e2048"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a5fe174299a5914c63fa39b0e40d788091c601b7","unresolved":true,"context_lines":[{"line_number":790,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d1, free\u003d0)"},{"line_number":791,"context_line":"        self._assertDeviceAddressesMapped(src_xml, dst_xml)"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"    def test_live_migrate_VF_fails_lm_requested_no_lm_dev(self):"},{"line_number":794,"context_line":"        \"\"\"We should fail to create the instance because we request a"},{"line_number":795,"context_line":"        live migratable PCI device and there is only a non live migratable one."},{"line_number":796,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"7b2e672b_86fd6a6b","line":793,"range":{"start_line":793,"start_character":8,"end_line":793,"end_character":57},"updated":"2025-03-14 12:50:23.000000000","message":"this is not a live migrate test just a test that tries to create a VM","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d1, free\u003d0)"},{"line_number":791,"context_line":"        self._assertDeviceAddressesMapped(src_xml, dst_xml)"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"    def test_live_migrate_VF_fails_lm_requested_no_lm_dev(self):"},{"line_number":794,"context_line":"        \"\"\"We should fail to create the instance because we request a"},{"line_number":795,"context_line":"        live migratable PCI device and there is only a non live migratable one."},{"line_number":796,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"2ff3718a_3d2e701a","line":793,"range":{"start_line":793,"start_character":8,"end_line":793,"end_character":57},"in_reply_to":"04126058_239ad2fe","updated":"2025-03-25 17:44:26.000000000","message":"Acknowledged","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"e20fcada600e0f015da107fc92143307fb134991","unresolved":true,"context_lines":[{"line_number":790,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d1, free\u003d0)"},{"line_number":791,"context_line":"        self._assertDeviceAddressesMapped(src_xml, dst_xml)"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"    def test_live_migrate_VF_fails_lm_requested_no_lm_dev(self):"},{"line_number":794,"context_line":"        \"\"\"We should fail to create the instance because we request a"},{"line_number":795,"context_line":"        live migratable PCI device and there is only a non live migratable one."},{"line_number":796,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"04126058_239ad2fe","line":793,"range":{"start_line":793,"start_character":8,"end_line":793,"end_character":57},"in_reply_to":"7b2e672b_86fd6a6b","updated":"2025-03-21 16:17:57.000000000","message":"I have not changed the test names because they are, in my opinion, part of the same test \"family,\" btw I can execute all of them by supplying the \"test_live_migrate_VF\" prefix.\nAlso, I see migration as the scenario goal. In that case, we cannot build because our entries are incorrect.","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a5fe174299a5914c63fa39b0e40d788091c601b7","unresolved":true,"context_lines":[{"line_number":865,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d1, free\u003d1)"},{"line_number":866,"context_line":"        self._wait_for_state_change(server, \u0027ACTIVE\u0027)"},{"line_number":867,"context_line":""},{"line_number":868,"context_line":"    def test_live_migrate_VF_fails_non_lm_requested_only_lm_dev(self):"},{"line_number":869,"context_line":"        \"\"\"We requested a non live migratable PCI device and there is only a"},{"line_number":870,"context_line":"        live migratable one. Instance creation should fail."},{"line_number":871,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"4b683859_95f29978","line":868,"range":{"start_line":868,"start_character":8,"end_line":868,"end_character":63},"updated":"2025-03-14 12:50:23.000000000","message":"this is not a live_migrate test just a test trying to creating a VM","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ef53fe416c0bd8eaf77a4cb66f8cf4cdfaba4781","unresolved":false,"context_lines":[{"line_number":865,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d1, free\u003d1)"},{"line_number":866,"context_line":"        self._wait_for_state_change(server, \u0027ACTIVE\u0027)"},{"line_number":867,"context_line":""},{"line_number":868,"context_line":"    def test_live_migrate_VF_fails_non_lm_requested_only_lm_dev(self):"},{"line_number":869,"context_line":"        \"\"\"We requested a non live migratable PCI device and there is only a"},{"line_number":870,"context_line":"        live migratable one. Instance creation should fail."},{"line_number":871,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"355e7097_f07ac0cf","line":868,"range":{"start_line":868,"start_character":8,"end_line":868,"end_character":63},"in_reply_to":"4b683859_95f29978","updated":"2025-03-26 09:09:27.000000000","message":"Done","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a5fe174299a5914c63fa39b0e40d788091c601b7","unresolved":true,"context_lines":[{"line_number":932,"context_line":"        )"},{"line_number":933,"context_line":"        self.assertPCIDeviceCounts(self.comp0, total\u003d1, free\u003d1)"},{"line_number":934,"context_line":""},{"line_number":935,"context_line":"    def test_live_migrate_VF_fails_non_lm_requested_dev_unspecified(self):"},{"line_number":936,"context_line":"        \"\"\"We requested a non live migratable PCI device and there is only a"},{"line_number":937,"context_line":"        device with live migratable not specified. Instance creation should"},{"line_number":938,"context_line":"        fail."}],"source_content_type":"text/x-python","patch_set":3,"id":"268489cd_3a3642b3","line":935,"range":{"start_line":935,"start_character":8,"end_line":935,"end_character":67},"updated":"2025-03-14 12:50:23.000000000","message":"this is not a live_migration test case","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ef53fe416c0bd8eaf77a4cb66f8cf4cdfaba4781","unresolved":false,"context_lines":[{"line_number":932,"context_line":"        )"},{"line_number":933,"context_line":"        self.assertPCIDeviceCounts(self.comp0, total\u003d1, free\u003d1)"},{"line_number":934,"context_line":""},{"line_number":935,"context_line":"    def test_live_migrate_VF_fails_non_lm_requested_dev_unspecified(self):"},{"line_number":936,"context_line":"        \"\"\"We requested a non live migratable PCI device and there is only a"},{"line_number":937,"context_line":"        device with live migratable not specified. Instance creation should"},{"line_number":938,"context_line":"        fail."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff3862bf_f77c4aad","line":935,"range":{"start_line":935,"start_character":8,"end_line":935,"end_character":67},"in_reply_to":"268489cd_3a3642b3","updated":"2025-03-26 09:09:27.000000000","message":"Done","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a5fe174299a5914c63fa39b0e40d788091c601b7","unresolved":true,"context_lines":[{"line_number":2051,"context_line":"            devices,  one for each product ID."},{"line_number":2052,"context_line":"            - The first set is assigned to slot 0, and each subsequent set"},{"line_number":2053,"context_line":"            is assigned to the next slot incrementally."},{"line_number":2054,"context_line":"        :param customize_config: (function) A function to customize the"},{"line_number":2055,"context_line":"            environment.  This function will be called with the"},{"line_number":2056,"context_line":"            following parameters:"},{"line_number":2057,"context_line":"            - src_pci_info (fakelibvirt.HostPCIDevicesInfo):"}],"source_content_type":"text/x-python","patch_set":3,"id":"62195e65_6a36b3ae","line":2054,"updated":"2025-03-14 12:50:23.000000000","message":"please don\u0027t. Instead of hooking, let the caller pass in the inputs that needed. It is OK to have duplication in test code to help the readability. It is totally fine if each test case looks the following:\n* start source compute with a HostPCIDevicesInfo\n* start dest compute with a HostPCIDevicesInfo\n* set device_spec config\n* set alias config\n* create flavor(s) with pci extra spec(s)\n* try create a VM\n* try to migrate the VM\n\ndon\u0027t try to hide the scenario into a helper, we want to understand a high level scenario and important parameters of single test case mostly by just looking at the test case itself and not the content of the helpers. (yes test case code deduplication governed by different rules than normal code deduplication)","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"623ef357ef635f5d14836c5602b6e2d1a6a89afa","unresolved":true,"context_lines":[{"line_number":2051,"context_line":"            devices,  one for each product ID."},{"line_number":2052,"context_line":"            - The first set is assigned to slot 0, and each subsequent set"},{"line_number":2053,"context_line":"            is assigned to the next slot incrementally."},{"line_number":2054,"context_line":"        :param customize_config: (function) A function to customize the"},{"line_number":2055,"context_line":"            environment.  This function will be called with the"},{"line_number":2056,"context_line":"            following parameters:"},{"line_number":2057,"context_line":"            - src_pci_info (fakelibvirt.HostPCIDevicesInfo):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2f9712_a39d95c0","line":2054,"in_reply_to":"62195e65_6a36b3ae","updated":"2025-03-14 13:44:46.000000000","message":"ya i was not a fan of this refactoring and actully stop reviewing this patch becasue if it\n\nbasiclly i didnt have the mental bandwith left to see if this was sane but i didnt like what it was doing to the tests.\n\nso +1 to what gibi said","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"e20fcada600e0f015da107fc92143307fb134991","unresolved":false,"context_lines":[{"line_number":2051,"context_line":"            devices,  one for each product ID."},{"line_number":2052,"context_line":"            - The first set is assigned to slot 0, and each subsequent set"},{"line_number":2053,"context_line":"            is assigned to the next slot incrementally."},{"line_number":2054,"context_line":"        :param customize_config: (function) A function to customize the"},{"line_number":2055,"context_line":"            environment.  This function will be called with the"},{"line_number":2056,"context_line":"            following parameters:"},{"line_number":2057,"context_line":"            - src_pci_info (fakelibvirt.HostPCIDevicesInfo):"}],"source_content_type":"text/x-python","patch_set":3,"id":"40864006_7a3b56e1","line":2054,"in_reply_to":"7a2f9712_a39d95c0","updated":"2025-03-21 16:17:57.000000000","message":"Done","commit_id":"eb9ec0af3499c3cbe38123b108dda3285cdebba2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":true,"context_lines":[{"line_number":902,"context_line":"        # The AssertionError means the server failed to be created"},{"line_number":903,"context_line":"        # it fails on the assertion in _wait_for_state_change"},{"line_number":904,"context_line":"        self.assertRaises("},{"line_number":905,"context_line":"            AssertionError,"},{"line_number":906,"context_line":"            self._create_server,"},{"line_number":907,"context_line":"            host\u003dself.comp0,"},{"line_number":908,"context_line":"            flavor_id\u003dflavor_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9c7746da_b9b3da3d","line":905,"updated":"2025-03-25 17:44:26.000000000","message":"Can we assert that this fails with NoValidHost? (here and below in test cases where the create fails)","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"d03b49751e3b44ec41675db07d1db9f86c9383ca","unresolved":true,"context_lines":[{"line_number":902,"context_line":"        # The AssertionError means the server failed to be created"},{"line_number":903,"context_line":"        # it fails on the assertion in _wait_for_state_change"},{"line_number":904,"context_line":"        self.assertRaises("},{"line_number":905,"context_line":"            AssertionError,"},{"line_number":906,"context_line":"            self._create_server,"},{"line_number":907,"context_line":"            host\u003dself.comp0,"},{"line_number":908,"context_line":"            flavor_id\u003dflavor_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"b1a68277_a497bdaa","line":905,"in_reply_to":"07628300_43129bd2","updated":"2025-03-27 15:12:46.000000000","message":"I have fixed all of those with the snippet suggested by Gibi.\nI think that\u0027s ok now. Please let me know if it is not the case.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7e72fdba260723c93d7070c373dfa4513de0ecb4","unresolved":true,"context_lines":[{"line_number":902,"context_line":"        # The AssertionError means the server failed to be created"},{"line_number":903,"context_line":"        # it fails on the assertion in _wait_for_state_change"},{"line_number":904,"context_line":"        self.assertRaises("},{"line_number":905,"context_line":"            AssertionError,"},{"line_number":906,"context_line":"            self._create_server,"},{"line_number":907,"context_line":"            host\u003dself.comp0,"},{"line_number":908,"context_line":"            flavor_id\u003dflavor_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"caef8545_e11f6259","line":905,"in_reply_to":"9c7746da_b9b3da3d","updated":"2025-03-26 09:13:20.000000000","message":"If I recall correctly, NoValidHost is raised at some point, but that\u0027s probably not the last exception. Hence, I\u0027m not sure it can be used.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"58544b4ef78d978af896651a9a1ab34766165354","unresolved":true,"context_lines":[{"line_number":902,"context_line":"        # The AssertionError means the server failed to be created"},{"line_number":903,"context_line":"        # it fails on the assertion in _wait_for_state_change"},{"line_number":904,"context_line":"        self.assertRaises("},{"line_number":905,"context_line":"            AssertionError,"},{"line_number":906,"context_line":"            self._create_server,"},{"line_number":907,"context_line":"            host\u003dself.comp0,"},{"line_number":908,"context_line":"            flavor_id\u003dflavor_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"f96ca61d_1075a1db","line":905,"in_reply_to":"caef8545_e11f6259","updated":"2025-03-27 11:03:59.000000000","message":"This would be the way to explicitly test for the No valid host result.\n```python\n        server \u003d self._create_server(\n            host\u003dself.comp0,\n            flavor_id\u003dflavor_id,\n            networks\u003dnetworks,\n            expected_state\u003d\"ERROR\")\n        self.assertIn(\"No valid host was found.\", server[\"fault\"][\"message\"])\n```","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a2ccd6f98d0b3a719fe93864bf63498322a3f142","unresolved":true,"context_lines":[{"line_number":902,"context_line":"        # The AssertionError means the server failed to be created"},{"line_number":903,"context_line":"        # it fails on the assertion in _wait_for_state_change"},{"line_number":904,"context_line":"        self.assertRaises("},{"line_number":905,"context_line":"            AssertionError,"},{"line_number":906,"context_line":"            self._create_server,"},{"line_number":907,"context_line":"            host\u003dself.comp0,"},{"line_number":908,"context_line":"            flavor_id\u003dflavor_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"07628300_43129bd2","line":905,"in_reply_to":"f96ca61d_1075a1db","updated":"2025-03-27 13:47:03.000000000","message":"This is definitely better than checking for the assertion, IMHO. It also clearly shows we\u0027re expecting the thing to go to ERROR state, in addition to checking for the actual reason why.\n\nThere are other instances of asserting that AssertionError is raised in this file, which really feels wrong to me. You\u0027re asserting that some part of the test infrastructure asserted something instead of asserting something about the runtime code you\u0027re supposed to be testing.\n\nPerhaps those others could be cleaned up as well?","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":true,"context_lines":[{"line_number":1486,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":1487,"context_line":"                \"address\": {"},{"line_number":1488,"context_line":"                    \"domain\": \"00\","},{"line_number":1489,"context_line":"                    \"bus\": \"8[1-2]\","},{"line_number":1490,"context_line":"                    \"slot\": \"00\","},{"line_number":1491,"context_line":"                    \"function\": \"[1-3]\","},{"line_number":1492,"context_line":"                },"}],"source_content_type":"text/x-python","patch_set":8,"id":"622b5747_02481a10","line":1489,"updated":"2025-03-25 17:44:26.000000000","message":"Is this just to have a single spec that works on both compute?","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"63795dfc5ff56b0459379bbaeb5302d560af6fa6","unresolved":false,"context_lines":[{"line_number":1486,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":1487,"context_line":"                \"address\": {"},{"line_number":1488,"context_line":"                    \"domain\": \"00\","},{"line_number":1489,"context_line":"                    \"bus\": \"8[1-2]\","},{"line_number":1490,"context_line":"                    \"slot\": \"00\","},{"line_number":1491,"context_line":"                    \"function\": \"[1-3]\","},{"line_number":1492,"context_line":"                },"}],"source_content_type":"text/x-python","patch_set":8,"id":"1db7661b_d7f0f64e","line":1489,"in_reply_to":"27ff962e_51c833b1","updated":"2025-03-26 12:38:19.000000000","message":"Acknowledged","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"999503cd57c2df3ab9c236317ac89e0415f6f753","unresolved":true,"context_lines":[{"line_number":1486,"context_line":"                \"live_migratable\": \"yes\","},{"line_number":1487,"context_line":"                \"address\": {"},{"line_number":1488,"context_line":"                    \"domain\": \"00\","},{"line_number":1489,"context_line":"                    \"bus\": \"8[1-2]\","},{"line_number":1490,"context_line":"                    \"slot\": \"00\","},{"line_number":1491,"context_line":"                    \"function\": \"[1-3]\","},{"line_number":1492,"context_line":"                },"}],"source_content_type":"text/x-python","patch_set":8,"id":"27ff962e_51c833b1","line":1489,"in_reply_to":"622b5747_02481a10","updated":"2025-03-26 09:08:02.000000000","message":"Yes","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":true,"context_lines":[{"line_number":1571,"context_line":"        # method, which only verifies that a similar device — for"},{"line_number":1572,"context_line":"        # example, one with slot 0x82 — exists in the destination XML,"},{"line_number":1573,"context_line":"        # regardless of order."},{"line_number":1574,"context_line":"        self._assertDeviceAddressesPresent(src_xml, dst_xml)"},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"    def test_live_migrate_VF_fails_dest_no_lm_dev(self):"},{"line_number":1577,"context_line":"        \"\"\"Live migrate an instance with 3 x PCI VF."}],"source_content_type":"text/x-python","patch_set":8,"id":"7d11b51a_dc2d1368","line":1574,"updated":"2025-03-25 17:44:26.000000000","message":"note to myself: I will have to get back to this tomorrow with a fresh brain.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"58544b4ef78d978af896651a9a1ab34766165354","unresolved":true,"context_lines":[{"line_number":1571,"context_line":"        # method, which only verifies that a similar device — for"},{"line_number":1572,"context_line":"        # example, one with slot 0x82 — exists in the destination XML,"},{"line_number":1573,"context_line":"        # regardless of order."},{"line_number":1574,"context_line":"        self._assertDeviceAddressesPresent(src_xml, dst_xml)"},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"    def test_live_migrate_VF_fails_dest_no_lm_dev(self):"},{"line_number":1577,"context_line":"        \"\"\"Live migrate an instance with 3 x PCI VF."}],"source_content_type":"text/x-python","patch_set":8,"id":"c15fdc41_229054dc","line":1574,"in_reply_to":"7d11b51a_dc2d1368","updated":"2025-03-27 11:03:59.000000000","message":"OK I see it now. \nWhen there is a single possible device for a given alias on the destination then the _assertDeviceAddressesMapped() can use the assumption that that only change happened is the bus from 0x81 to 0x82 (because the slot numbering is symmetric and only one device is available per bus). But when there are multiple possible devices available for a given alias on the dest then top of the bus change 0x81 -\u003e 0x82 it is up to the pci tracker to decide which available device (which slot) to assign as each is a good target as each matching the alias.\n\nI think this is fine how you tested it. And we can land this as is.\n\nIf you are interested what could be a full assert then I think what we could do is to pass a list of possible (equivalent and alias matching) PCI addresses to the assert function for each type or xmlalias and then the assert function would check that the dest address is one of those possible addresses.\n\nThe most complex case there would be:\n* source \n  * pci alias1 matching 2 devs (slot1,2)\n  * pci alias2 matching 2 devs (slot3,4)\n  * flavor asks alias1:2 alias2:2\n* dest\n  * pci alias1 matching 4 devs (slot1,2,3,4)\n  * pci alias2 matching 4 devs (slot5,6,7,8)\n\nThen the assert needs input from the test case to know that for slot1 (and slot2) on the src any of slot1,2,3,4 on the dest are valid. And for slot3 (slot4) on the src any of the slot5,6,7,8 on the dest are valid as target.\n\nI think it does not worth the complexity to cover this here as what that would test on top of what we already testing is that the pci tracker (and placement if enabled) properly selects target dev to match the pci alias. But that is tested elsewhere already.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":true,"context_lines":[{"line_number":1971,"context_line":"        dst_xml \u003d self._get_xml(self.comp1, server)"},{"line_number":1972,"context_line":"        self.assertPCIDeviceCounts(self.comp0, total\u003d8, free\u003d8)"},{"line_number":1973,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d8, free\u003d6)"},{"line_number":1974,"context_line":"        self._assertDeviceAddressesMapped(src_xml, dst_xml)"},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"    def test_live_migrate_VF_success_with_pci_in_placement(self):"},{"line_number":1977,"context_line":"        PCI_DEVICE_SPEC \u003d [jsonutils.dumps(x) for x in ("}],"source_content_type":"text/x-python","patch_set":8,"id":"1cd60a00_71b07120","line":1974,"updated":"2025-03-25 17:44:26.000000000","message":"note to myself: use this to trace assert logic to compare with the 3 equivalent VFs case.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"58544b4ef78d978af896651a9a1ab34766165354","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"        dst_xml \u003d self._get_xml(self.comp1, server)"},{"line_number":1972,"context_line":"        self.assertPCIDeviceCounts(self.comp0, total\u003d8, free\u003d8)"},{"line_number":1973,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d8, free\u003d6)"},{"line_number":1974,"context_line":"        self._assertDeviceAddressesMapped(src_xml, dst_xml)"},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"    def test_live_migrate_VF_success_with_pci_in_placement(self):"},{"line_number":1977,"context_line":"        PCI_DEVICE_SPEC \u003d [jsonutils.dumps(x) for x in ("}],"source_content_type":"text/x-python","patch_set":8,"id":"2c6bccb7_0c50211c","line":1974,"in_reply_to":"1cd60a00_71b07120","updated":"2025-03-27 11:03:59.000000000","message":"Done","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":false,"context_lines":[{"line_number":1973,"context_line":"        self.assertPCIDeviceCounts(self.comp1, total\u003d8, free\u003d6)"},{"line_number":1974,"context_line":"        self._assertDeviceAddressesMapped(src_xml, dst_xml)"},{"line_number":1975,"context_line":""},{"line_number":1976,"context_line":"    def test_live_migrate_VF_success_with_pci_in_placement(self):"},{"line_number":1977,"context_line":"        PCI_DEVICE_SPEC \u003d [jsonutils.dumps(x) for x in ("},{"line_number":1978,"context_line":"            {"},{"line_number":1979,"context_line":"                \"vendor_id\": fakelibvirt.PCI_VEND_ID,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1b8584c4_b4e8cc10","line":1976,"updated":"2025-03-25 17:44:26.000000000","message":"thanks for having specific test coverage for PCI in Placement.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":true,"context_lines":[{"line_number":2210,"context_line":"        # method, which only verifies that a similar device — for"},{"line_number":2211,"context_line":"        # example, one with slot 0x82 — exists in the destination XML,"},{"line_number":2212,"context_line":"        # regardless of order."},{"line_number":2213,"context_line":"        self._assertDeviceAddressesPresent(src_xml, dst_xml)"},{"line_number":2214,"context_line":""},{"line_number":2215,"context_line":"    def test_live_migrate_VF_success_with_pip_2_aliases(self):"},{"line_number":2216,"context_line":"        PCI_DEVICE_SPEC \u003d [jsonutils.dumps(x) for x in ("}],"source_content_type":"text/x-python","patch_set":8,"id":"c2aae78b_2929fdab","line":2213,"updated":"2025-03-25 17:44:26.000000000","message":"ditto I need to look at it tomorrow.","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b6c4229147f4d5559fdbd1254803a36a43a95fde","unresolved":false,"context_lines":[{"line_number":2210,"context_line":"        # method, which only verifies that a similar device — for"},{"line_number":2211,"context_line":"        # example, one with slot 0x82 — exists in the destination XML,"},{"line_number":2212,"context_line":"        # regardless of order."},{"line_number":2213,"context_line":"        self._assertDeviceAddressesPresent(src_xml, dst_xml)"},{"line_number":2214,"context_line":""},{"line_number":2215,"context_line":"    def test_live_migrate_VF_success_with_pip_2_aliases(self):"},{"line_number":2216,"context_line":"        PCI_DEVICE_SPEC \u003d [jsonutils.dumps(x) for x in ("}],"source_content_type":"text/x-python","patch_set":8,"id":"b5bf485d_015961dc","line":2213,"in_reply_to":"c2aae78b_2929fdab","updated":"2025-03-27 15:44:57.000000000","message":"Done","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e0778ed1dbf73c3b06f46fa64a5f3315b63a054","unresolved":true,"context_lines":[{"line_number":2550,"context_line":"        )"},{"line_number":2551,"context_line":""},{"line_number":2552,"context_line":"        # By mocking threading.Event.wait we prevent the test to wait until the"},{"line_number":2553,"context_line":"        # timeout happens."},{"line_number":2554,"context_line":"        # We return True signaling that the event is set, i.e. the libvirt"},{"line_number":2555,"context_line":"        # event the caller is waiting for has been received."},{"line_number":2556,"context_line":"        # Note: This mock behavior cannot be added to the fixture because"}],"source_content_type":"text/x-python","patch_set":8,"id":"ed05d5a4_78d8f798","line":2553,"updated":"2025-03-25 17:44:26.000000000","message":"nit: ... during the device detach used for the neutron requested PCI device before the live migration. (or is it for the attach step that happens after the live migration?)","commit_id":"e5ed5569d970eb9d41e876db43ab9095076e62f7"}]}