)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"907229c3fbd4a87f858c693f2a9396acf45a7adf","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     René Ribaud \u003crribaud@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-03-20 16:08:16 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix for bug 1869804."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1ee826ced89f7947225fe678c0d416be8f111092"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"d9b8fd89_524483f4","line":7,"range":{"start_line":7,"start_character":8,"end_line":7,"end_character":19},"updated":"2023-03-21 07:04:49.000000000","message":"A brief description for current modification is necessary.\n\nAnd you can use Closes-Bug:#1869804 to trace and add the topic bug/1869804 for it.","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6300512105a62f331a9aff7623b3a062c7f9709a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     René Ribaud \u003crribaud@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-03-20 16:08:16 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix for bug 1869804."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1ee826ced89f7947225fe678c0d416be8f111092"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1c3b1fa8_ba53ecbf","line":7,"range":{"start_line":7,"start_character":8,"end_line":7,"end_character":19},"in_reply_to":"d9b8fd89_524483f4","updated":"2023-03-22 17:25:25.000000000","message":"Done","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6db0671adb0953c38c3a6383b8948f97117fdb4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"73fc6a06_3cbae1a6","updated":"2023-03-20 19:33:58.000000000","message":"I think this is good, and handles the rolling upgrades case. nit inline about adding comments, if you end up respining.","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3e17e9dffcc968ed7be4da51e16324222d7eefff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"78b74c88_7b61095a","updated":"2023-04-13 01:32:05.000000000","message":"+1 because it does the job and adds the coverage that was missing. See inline about style, I\u0027ll let others argue it, if they deem it necessary.","commit_id":"27529b215717fa058c201354c915bbb2c3ad1bcf"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"81fe1c208ccc9f6504343c4b2d01a1ef900054b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b3c03fa6_8c24f4fc","updated":"2023-11-23 19:49:56.000000000","message":"That is \"mad scientist\" level of brilliance. I agree with the aim of making this backportable, I just thought there was no way of achieving that. I\u0027d maybe only ask that we use an explicative string instead of -1 for the index, something like \"backportable_1869804_hack\".","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3d1c79cc1dad9474d7593203a62a8cc176af825e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e73aba41_dc99e382","updated":"2023-05-19 16:21:45.000000000","message":"The mocking it still weird - I think the most straightforward way is to mock the update and remove methods on each individual test, and not on the main _test_get_updated_guest_xml() method. So an example would be:\n\n    @mock.patch.object(migration, \u0027_remove_cpu_shared_set_xml\u0027)\n    @mock.patch.object(migration, \u0027_update_cpu_shared_set_xml\u0027)\n    def test_get_updated_guest_xml_dst_cpu_shared_set_info_unset(\n        self, mock_update_cpu_shared_set, mock_remove_cpu_shared_set\n    ):\n        data \u003d objects.LibvirtLiveMigrateData()\n        self._test_get_updated_guest_xml(data)\n        mock_update_cpu_shared_set.assert_not_called()\n        mock_remove_cpu_shared_set.assert_not_called()","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"639013501a91f6c27cf228c5c972f5c5e8e3315f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"5086274a_97dde0ed","updated":"2023-11-23 15:21:01.000000000","message":"We just ran into this issue when live-migrating lots of VMs to new hardware and observed that the cpu_shared_set (vcpu pinning) remained the same, which rendered 70% of the newly available physical cores completely useless.\n\nBut this is also an issue the other way around: Migration from a cpuset of a 256-core machine to a machine with less, will likely fail.\n\n\nWould really be great if we could iron out the remaining review remarks and get this fixed.","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b6864b42abfdf934cd0ece9da2d4a4db8c37278c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"105d5841_fa47962f","updated":"2023-11-23 19:54:10.000000000","message":"also, will the @property work for _setting_ it? we have the new returning a reference, not the value...","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"b1155b27b9768eb1559387fea49414725bd39d73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d52e04cf_485f86bf","updated":"2023-05-09 13:06:30.000000000","message":"recheck","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9104a4fd52e97c216649af9f31af0211e8caafcf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"c944603f_0f597b7f","in_reply_to":"105d5841_fa47962f","updated":"2023-11-23 19:54:40.000000000","message":"*have to be returning the value","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"379e6d86ec99cee26f0cdf00cf8f45074a3592aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0e1dc28e_9850b06f","in_reply_to":"5086274a_97dde0ed","updated":"2023-11-23 18:18:57.000000000","message":"ya so unfortunetlly this has been a know issue upstrema for many years.\nThe mitigation is cold migration  or  to use host aggartes to limit the hsot to only other host with the same config.\n\nTo set expectations this is not backportable as is becasue it currently requires an object change.\n\nwe coudl hack around that but we should eventually add the new field.\ni have provided some more details inline.","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6fe70b3a5339418d6d52a93377e2f979ac5eca13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0cc67735_05f6b76f","in_reply_to":"b3c03fa6_8c24f4fc","updated":"2023-11-27 11:06:13.000000000","message":"i didnt have time to check it the key was a string or an int.\nif its actuly a string then we can jsut use \u0027cpu_shared_set\u0027 as the key","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6fe70b3a5339418d6d52a93377e2f979ac5eca13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d399f46f_8b736737","in_reply_to":"c944603f_0f597b7f","updated":"2023-11-27 11:06:13.000000000","message":"you can define properties that act as setter yes\nthe example i provided is just the getter but it pretty simple\n```\nCPU_SHARED_SET_INDEX \u003d \u0027cpu_shared_set\u0027\n\n@property\ndef cpu_shared_set(self):\n    return cpu_pins.get(CPU_SHARED_SET_INDEX, None)\n\n@cpu_shared_set.setter\ndef cpu_shared_set(self, cpu_set):\n    cpu_pins[CPU_SHARED_SET_INDEX] \u003d cpu_set\n```","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"dbd1febbc8be2fd92dd64e6eda6c3dbf8bb44882","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9ade2c71_93e024cb","updated":"2023-12-14 15:50:04.000000000","message":"Use the proposed \"hack\" by Sean to allow backporting.","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"150482290b4c05463cd481005f5f2b82d1ecbd1a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"2e4171d4_e9dfe56a","updated":"2023-12-21 02:04:04.000000000","message":"this is very close but not quite correct","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"57ec5b752241ea3daa711e08378d753d1d795c51","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"d37cf3db_09309502","updated":"2024-03-12 14:18:22.000000000","message":"Couple of nits inline, but not worth respinning over.","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"615c7f1bb2b7a5f84eb3d4dd36a5dd59e19fd482","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"657a09ee_a9b33e6b","updated":"2024-03-19 17:59:38.000000000","message":"Okay, so, after a lot of conversation, I\u0027m upgrading to a -2 on this \"backportable\" hack/fix. I don\u0027t think we should do this, nor do I think we *need* to do this. We should move forward with just the proper fix with a real object change, which is the second patch in this series. It will not be trivially backportable in the way this one was aiming to be, but for good reason. If we decided to backport it either upstream or downstream, the intervening object version(s) could also be backported, which is the proper way to do.\n\nThat said, I think the consensus at this point is that the fix this hack addresses is not critical enough to justify the gymnastics implemented here, nor the lifting required to do it the right way.\n\nSo, Rene, can you just detach the second patch from this series from this one, abandon this, and get that one updated with whatever it needs to apply to master directly?","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cb5bf942ccfd0b3a77f8347837310e427091425e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"53ab0ac8_54dfd692","updated":"2024-03-18 23:09:30.000000000","message":"So I really don\u0027t like this approach. I think we should consider just doing the proper fix in the next patch and not have this extremely hackish patch in the chain. Abusing a dict in supposedly-versioned data like this is exactly what we should be avoiding in interfaces between nodes.\n\nI think we should drop this patch, just do the object bump in the next one and just deal with it. I don\u0027t think that we need to backport a fix for something that has been broken for so long, especially when it requires either a real object bump or a subversive act like this.\n\nI\u0027d like to be able to discuss this at least before we merge.","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7282efd3f483cfbf442b8dcc5c49b4d31cd37805","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"7575b1b1_0724d8b9","updated":"2024-03-12 19:37:11.000000000","message":"i think im a little to tire to review this right now ill try and look again tomorrow.\n\na quick review this looks ok to me","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"09ef18c9b5f1df26c1cce62d2ceb319a5ecc1ed3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"25c1ecd4_ab0df161","updated":"2024-03-15 08:21:55.000000000","message":"it seems there are some problems with pre_live_migration ?","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fe4e81ffdbeb54336e52053af7e5546fe56a5689","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"be8863e2_98912474","updated":"2024-03-18 15:13:12.000000000","message":"the gate will fail anyway, but please amend the regression test directly inside this change and explain in the commit msg why you need it, so then we could backport this change in C, B and A if needed.","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"}],"nova/objects/migrate_data.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcf1b75af0fcda84d06cf3aefed12f5be7d32585","unresolved":true,"context_lines":[{"line_number":277,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":278,"context_line":"        super(LibvirtLiveMigrateData, self).obj_make_compatible("},{"line_number":279,"context_line":"            primitive, target_version)"},{"line_number":280,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":281,"context_line":"        if (target_version \u003c (1, 10) and"},{"line_number":282,"context_line":"                \u0027src_supports_numa_live_migration\u0027 in primitive):"},{"line_number":283,"context_line":"            del primitive[\u0027src_supports_numa_live_migration\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"462d21ce_842ca84e","line":280,"updated":"2023-03-17 20:09:05.000000000","message":"You\u0027ll need a bit of code here to strip your new field for backwards compatibility purposes (and the corresponding unit test).","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f8f54bf37933088e55f988f13fa9d94849c72ee2","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":278,"context_line":"        super(LibvirtLiveMigrateData, self).obj_make_compatible("},{"line_number":279,"context_line":"            primitive, target_version)"},{"line_number":280,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":281,"context_line":"        if (target_version \u003c (1, 10) and"},{"line_number":282,"context_line":"                \u0027src_supports_numa_live_migration\u0027 in primitive):"},{"line_number":283,"context_line":"            del primitive[\u0027src_supports_numa_live_migration\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"e57e8b15_1f35dd02","line":280,"in_reply_to":"462d21ce_842ca84e","updated":"2023-03-20 15:08:43.000000000","message":"Sorry dumbly forget this one.","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"379e6d86ec99cee26f0cdf00cf8f45074a3592aa","unresolved":true,"context_lines":[{"line_number":143,"context_line":"        \u0027cell_pins\u0027: fields.DictOfSetOfIntegersField(),"},{"line_number":144,"context_line":"        \u0027emulator_pins\u0027: fields.SetOfIntegersField(),"},{"line_number":145,"context_line":"        \u0027sched_vcpus\u0027: fields.SetOfIntegersField(),"},{"line_number":146,"context_line":"        \u0027sched_priority\u0027: fields.IntegerField(),"},{"line_number":147,"context_line":"    }"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"b6f5d9c7_d71353f9","line":146,"updated":"2023-11-23 18:18:57.000000000","message":"if we add a new field i would genereally prefer to add\n\ncpu_shared_set: fields.SetOfIntegersField()\n\nfor backporting reasons how adding any new field means we cannot fix this on older release.\n\nif we want to backport this we need to do a hack and reuse some of the existing files.\n\ni would prefer to fix this in two commits.\n\nin this commit lets do the following.\n\ncpu_pins today is indexed by the guest logical cores starting with 0\nso we are going to use that fact to use cpu_pins[\"-1\"] to store the cpu_share_set\n\nfirst we will add a new property called cpu_shared_set\n\n@property\ndef cpu_shared_set(self):\n return cpu_pins.get(-1, set())\n\nthen we need to modify  where cpu_pins is currenlty popultated to store the cpu_shared_set.\n \nwe woudl then modify LibvirtLiveMigrateNUMAInfo is used to check \ndst_numa_info.cpu_shared_set to get the destination cpu_share_set.\n\ni.e. \n\ndest_cpu_set \u003d dst_numa_info.cpu_shared_set or Conf.compute.cpu_share_set\n\nthat will allow use to write the patch without the object change.\n\nthen in a followup patch you will replace the property with a new filed with the same name and do the object bump and updte nova to nolonger stash the info in \ncpu_pins.\n\n\nthis is a hack but its the only way we can fix this and keep it backportable.","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"dbd1febbc8be2fd92dd64e6eda6c3dbf8bb44882","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        \u0027cell_pins\u0027: fields.DictOfSetOfIntegersField(),"},{"line_number":144,"context_line":"        \u0027emulator_pins\u0027: fields.SetOfIntegersField(),"},{"line_number":145,"context_line":"        \u0027sched_vcpus\u0027: fields.SetOfIntegersField(),"},{"line_number":146,"context_line":"        \u0027sched_priority\u0027: fields.IntegerField(),"},{"line_number":147,"context_line":"    }"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"f6c4b05b_8bd309a8","line":146,"in_reply_to":"b6f5d9c7_d71353f9","updated":"2023-12-14 15:50:04.000000000","message":"Done","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"379e6d86ec99cee26f0cdf00cf8f45074a3592aa","unresolved":true,"context_lines":[{"line_number":271,"context_line":"        \u0027src_supports_numa_live_migration\u0027: fields.BooleanField(),"},{"line_number":272,"context_line":"        \u0027dst_supports_numa_live_migration\u0027: fields.BooleanField(),"},{"line_number":273,"context_line":"        \u0027dst_numa_info\u0027: fields.ObjectField(\u0027LibvirtLiveMigrateNUMAInfo\u0027),"},{"line_number":274,"context_line":"        \u0027dst_cpu_shared_set_info\u0027: fields.SetOfIntegersField(nullable\u003dTrue),"},{"line_number":275,"context_line":"    }"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1dd6d9bb_efcf7961","line":274,"updated":"2023-11-23 18:18:57.000000000","message":"-1 this should not be its own field like this IMO.\n\nsee my other comment above.","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"dbd1febbc8be2fd92dd64e6eda6c3dbf8bb44882","unresolved":false,"context_lines":[{"line_number":271,"context_line":"        \u0027src_supports_numa_live_migration\u0027: fields.BooleanField(),"},{"line_number":272,"context_line":"        \u0027dst_supports_numa_live_migration\u0027: fields.BooleanField(),"},{"line_number":273,"context_line":"        \u0027dst_numa_info\u0027: fields.ObjectField(\u0027LibvirtLiveMigrateNUMAInfo\u0027),"},{"line_number":274,"context_line":"        \u0027dst_cpu_shared_set_info\u0027: fields.SetOfIntegersField(nullable\u003dTrue),"},{"line_number":275,"context_line":"    }"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def obj_make_compatible(self, primitive, target_version):"}],"source_content_type":"text/x-python","patch_set":7,"id":"d746af6a_0eb1d9f7","line":274,"in_reply_to":"1dd6d9bb_efcf7961","updated":"2023-12-14 15:50:04.000000000","message":"Done","commit_id":"eadc301172a63b1ddffa268f4dfe164d4cc4c53a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"98eae1371bfd79d55517f4a51cd5263dbf618bd9","unresolved":true,"context_lines":[{"line_number":283,"context_line":"            return None"},{"line_number":284,"context_line":"        if \u0027cpu_pins\u0027 not in self.dst_numa_info:"},{"line_number":285,"context_line":"            return None"},{"line_number":286,"context_line":"        if self.dst_numa_info.cpu_pins is None:"},{"line_number":287,"context_line":"            return None"},{"line_number":288,"context_line":"        return self.dst_numa_info.cpu_pins.get(\"dst_cpu_shared_set_info\")"},{"line_number":289,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"c93fb825_657526a7","line":286,"updated":"2023-12-21 17:12:38.000000000","message":"At this point you can simplify this to\n\n    return self.dst_numa_info.cpus_pins.get(\u0027dst_cpu_shared_set_info\u0027)\n    \nThe None default is builtin to get()","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"020324dad8713e7e4a143c0b05069f3cbe68a41a","unresolved":false,"context_lines":[{"line_number":283,"context_line":"            return None"},{"line_number":284,"context_line":"        if \u0027cpu_pins\u0027 not in self.dst_numa_info:"},{"line_number":285,"context_line":"            return None"},{"line_number":286,"context_line":"        if self.dst_numa_info.cpu_pins is None:"},{"line_number":287,"context_line":"            return None"},{"line_number":288,"context_line":"        return self.dst_numa_info.cpu_pins.get(\"dst_cpu_shared_set_info\")"},{"line_number":289,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"25bbaf52_965c9233","line":286,"in_reply_to":"7dcfec63_59508772","updated":"2024-03-12 13:57:26.000000000","message":"Done","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7579d605c3cb1fdbd118a8a655c5d61463151ea8","unresolved":true,"context_lines":[{"line_number":283,"context_line":"            return None"},{"line_number":284,"context_line":"        if \u0027cpu_pins\u0027 not in self.dst_numa_info:"},{"line_number":285,"context_line":"            return None"},{"line_number":286,"context_line":"        if self.dst_numa_info.cpu_pins is None:"},{"line_number":287,"context_line":"            return None"},{"line_number":288,"context_line":"        return self.dst_numa_info.cpu_pins.get(\"dst_cpu_shared_set_info\")"},{"line_number":289,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7dcfec63_59508772","line":286,"in_reply_to":"c93fb825_657526a7","updated":"2023-12-21 17:21:23.000000000","message":"ya thats true you can inded remvoe this if","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"150482290b4c05463cd481005f5f2b82d1ecbd1a","unresolved":true,"context_lines":[{"line_number":297,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d set()"},{"line_number":298,"context_line":"        else:"},{"line_number":299,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d value"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":302,"context_line":"        super(LibvirtLiveMigrateData, self).obj_make_compatible("},{"line_number":303,"context_line":"            primitive, target_version)"}],"source_content_type":"text/x-python","patch_set":8,"id":"b410c701_0e53a868","line":300,"updated":"2023-12-21 02:04:04.000000000","message":"slightly more complex then i was orginaly hopping but i can see why each fo the check are needed when dealing with the possiblity of back leveled objects and unpatched nodes.","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7282efd3f483cfbf442b8dcc5c49b4d31cd37805","unresolved":false,"context_lines":[{"line_number":297,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d set()"},{"line_number":298,"context_line":"        else:"},{"line_number":299,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d value"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":302,"context_line":"        super(LibvirtLiveMigrateData, self).obj_make_compatible("},{"line_number":303,"context_line":"            primitive, target_version)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8c8e6a37_7b0e8926","line":300,"in_reply_to":"b410c701_0e53a868","updated":"2024-03-12 19:37:11.000000000","message":"Acknowledged","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"57ec5b752241ea3daa711e08378d753d1d795c51","unresolved":true,"context_lines":[{"line_number":301,"context_line":"            self.dst_numa_info \u003d LibvirtLiveMigrateNUMAInfo()"},{"line_number":302,"context_line":"        if \u0027cpu_pins\u0027 not in self.dst_numa_info:"},{"line_number":303,"context_line":"            self.dst_numa_info.cpu_pins \u003d {}"},{"line_number":304,"context_line":"        if value is None:"},{"line_number":305,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d set()"},{"line_number":306,"context_line":"        else:"},{"line_number":307,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d value"}],"source_content_type":"text/x-python","patch_set":9,"id":"bd987117_389c9be0","line":304,"updated":"2024-03-12 14:18:22.000000000","message":"nit:\n\n     self.dst_numa_info.cpu_pins[\u0027dst_cpu_shared_set_info\u0027] \u003d value() or set()","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7282efd3f483cfbf442b8dcc5c49b4d31cd37805","unresolved":false,"context_lines":[{"line_number":301,"context_line":"            self.dst_numa_info \u003d LibvirtLiveMigrateNUMAInfo()"},{"line_number":302,"context_line":"        if \u0027cpu_pins\u0027 not in self.dst_numa_info:"},{"line_number":303,"context_line":"            self.dst_numa_info.cpu_pins \u003d {}"},{"line_number":304,"context_line":"        if value is None:"},{"line_number":305,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d set()"},{"line_number":306,"context_line":"        else:"},{"line_number":307,"context_line":"            self.dst_numa_info.cpu_pins[\"dst_cpu_shared_set_info\"] \u003d value"}],"source_content_type":"text/x-python","patch_set":9,"id":"d6be15c8_dc0d8bab","line":304,"in_reply_to":"bd987117_389c9be0","updated":"2024-03-12 19:37:11.000000000","message":"yes we could make this one line but ya im fine with not respining for that","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"150482290b4c05463cd481005f5f2b82d1ecbd1a","unresolved":true,"context_lines":[{"line_number":772,"context_line":"        \u003c/domain\u003e"},{"line_number":773,"context_line":"        \"\"\""},{"line_number":774,"context_line":"        self.dst_numa_info \u003d {"},{"line_number":775,"context_line":"            \"nova_object.changes\": [\"cpu_pins\"],"},{"line_number":776,"context_line":"            \"nova_object.data\": {\"cpu_pins\": {\"dst_cpu_shared_set_info\": ()}},"},{"line_number":777,"context_line":"            \"nova_object.name\": \"LibvirtLiveMigrateNUMAInfo\","},{"line_number":778,"context_line":"            \"nova_object.namespace\": \"nova\","}],"source_content_type":"text/x-python","patch_set":8,"id":"ed948fc8_9ad3cc1b","line":775,"updated":"2023-12-21 02:04:04.000000000","message":"i mean i guess this can work but you are just manually creating the result of calling obj_to_primitiave on a LibvirtLiveMigrationNumaInfo object\n\nwhy are you not just using an object here directly.\n\nif you want to save the primitave version for comarison later htne i would constuct the objecant and call .obj_to_primitive on it here.","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"020324dad8713e7e4a143c0b05069f3cbe68a41a","unresolved":false,"context_lines":[{"line_number":772,"context_line":"        \u003c/domain\u003e"},{"line_number":773,"context_line":"        \"\"\""},{"line_number":774,"context_line":"        self.dst_numa_info \u003d {"},{"line_number":775,"context_line":"            \"nova_object.changes\": [\"cpu_pins\"],"},{"line_number":776,"context_line":"            \"nova_object.data\": {\"cpu_pins\": {\"dst_cpu_shared_set_info\": ()}},"},{"line_number":777,"context_line":"            \"nova_object.name\": \"LibvirtLiveMigrateNUMAInfo\","},{"line_number":778,"context_line":"            \"nova_object.namespace\": \"nova\","}],"source_content_type":"text/x-python","patch_set":8,"id":"0e8317b0_2a6dc07b","line":775,"in_reply_to":"ed948fc8_9ad3cc1b","updated":"2024-03-12 13:57:26.000000000","message":"Done","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"}],"nova/tests/unit/virt/libvirt/test_migration.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3e17e9dffcc968ed7be4da51e16324222d7eefff","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        mock_perf_events_xml.assert_called_once_with(mock.ANY, data)"},{"line_number":95,"context_line":"        mock_memory_backing.assert_called_once_with(mock.ANY, data)"},{"line_number":96,"context_line":"        self.assertEqual(1, mock_tostring.called)"},{"line_number":97,"context_line":"        return (mock_update_cpu_shared_set, mock_remove_cpu_shared_set)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def test_get_updated_guest_xml(self):"},{"line_number":100,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":5,"id":"9da1ee7e_af65fe23","line":97,"updated":"2023-04-13 01:32:05.000000000","message":"I\u0027ve never seen mocks returned this way. It definitely works, and covers what I mentioned in my previous comment (that we call the correct XML mangling method according to what\u0027s set in migrate_data), but it\u0027s kinda weird and lack precedent.","commit_id":"27529b215717fa058c201354c915bbb2c3ad1bcf"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"5aeb5e965fc944946df26ef90a058943f9339a03","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        mock_perf_events_xml.assert_called_once_with(mock.ANY, data)"},{"line_number":95,"context_line":"        mock_memory_backing.assert_called_once_with(mock.ANY, data)"},{"line_number":96,"context_line":"        self.assertEqual(1, mock_tostring.called)"},{"line_number":97,"context_line":"        return (mock_update_cpu_shared_set, mock_remove_cpu_shared_set)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def test_get_updated_guest_xml(self):"},{"line_number":100,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":5,"id":"df54b9e9_29c817bf","line":97,"in_reply_to":"9da1ee7e_af65fe23","updated":"2023-04-26 16:53:15.000000000","message":"I have changed that please let me know if you would rather that one.","commit_id":"27529b215717fa058c201354c915bbb2c3ad1bcf"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"dbd1febbc8be2fd92dd64e6eda6c3dbf8bb44882","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        mock_perf_events_xml.assert_called_once_with(mock.ANY, data)"},{"line_number":95,"context_line":"        mock_memory_backing.assert_called_once_with(mock.ANY, data)"},{"line_number":96,"context_line":"        self.assertEqual(1, mock_tostring.called)"},{"line_number":97,"context_line":"        return (mock_update_cpu_shared_set, mock_remove_cpu_shared_set)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def test_get_updated_guest_xml(self):"},{"line_number":100,"context_line":"        data \u003d objects.LibvirtLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":5,"id":"07dad9da_df4770d9","line":97,"in_reply_to":"df54b9e9_29c817bf","updated":"2023-12-14 15:50:04.000000000","message":"Done","commit_id":"27529b215717fa058c201354c915bbb2c3ad1bcf"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"150482290b4c05463cd481005f5f2b82d1ecbd1a","unresolved":true,"context_lines":[{"line_number":9732,"context_line":"        if instance.numa_topology:"},{"line_number":9733,"context_line":"            data.dst_supports_numa_live_migration \u003d True"},{"line_number":9734,"context_line":""},{"line_number":9735,"context_line":"        data.dst_cpu_shared_set_info \u003d hardware.get_cpu_shared_set()"},{"line_number":9736,"context_line":""},{"line_number":9737,"context_line":"        # NOTE(sean-k-mooney): The migrate_data vifs field is used to signal"},{"line_number":9738,"context_line":"        # that we are using the multiple port binding workflow so we can only"}],"source_content_type":"text/x-python","patch_set":8,"id":"4f084572_f7dace23","line":9735,"updated":"2023-12-21 02:04:04.000000000","message":"this is close but not quite what you want\n\nthis is closer\n\n```\ndata.dst_cpu_shared_set_info \u003d hardware.get_cpu_shared_set()\nif data.dst_cpu_shared_set_info is None:\n  data.dst_cpu_shared_set_info \u003d hardware.get_vcpu_pin_set()\n```\n\nbasically if [compute]cpu_shared_set is not defiend but [DEFAULT]vcpu_pin_set is\nthen the vcpu pinset defines the cores that floating vms will float over.\n\nuntil we remove vcpu_pin_set from teh codebase we still need to cater for the pre cpu_pinning in palcement usage.","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"020324dad8713e7e4a143c0b05069f3cbe68a41a","unresolved":false,"context_lines":[{"line_number":9732,"context_line":"        if instance.numa_topology:"},{"line_number":9733,"context_line":"            data.dst_supports_numa_live_migration \u003d True"},{"line_number":9734,"context_line":""},{"line_number":9735,"context_line":"        data.dst_cpu_shared_set_info \u003d hardware.get_cpu_shared_set()"},{"line_number":9736,"context_line":""},{"line_number":9737,"context_line":"        # NOTE(sean-k-mooney): The migrate_data vifs field is used to signal"},{"line_number":9738,"context_line":"        # that we are using the multiple port binding workflow so we can only"}],"source_content_type":"text/x-python","patch_set":8,"id":"55e637b0_735f3f60","line":9735,"in_reply_to":"4f084572_f7dace23","updated":"2024-03-12 13:57:26.000000000","message":"Done","commit_id":"7a57bb076b056f6d2067482ca01d837d2a9bf56f"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"57ec5b752241ea3daa711e08378d753d1d795c51","unresolved":true,"context_lines":[{"line_number":9982,"context_line":"        if instance.numa_topology:"},{"line_number":9983,"context_line":"            data.dst_supports_numa_live_migration \u003d True"},{"line_number":9984,"context_line":""},{"line_number":9985,"context_line":"        data.dst_cpu_shared_set_info \u003d hardware.get_cpu_shared_set()"},{"line_number":9986,"context_line":"        if data.dst_cpu_shared_set_info is None:"},{"line_number":9987,"context_line":"            data.dst_cpu_shared_set_info \u003d hardware.get_vcpu_pin_set()"},{"line_number":9988,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"145d4832_726488d5","line":9985,"updated":"2024-03-12 14:18:22.000000000","message":"nit:\n\n    \u003d hardware.get_cpu_shared_set() or hardware.get_vcpu_pin_set()","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcf1b75af0fcda84d06cf3aefed12f5be7d32585","unresolved":true,"context_lines":[{"line_number":66,"context_line":"    if get_vif_config is not None:"},{"line_number":67,"context_line":"        xml_doc \u003d _update_vif_xml(xml_doc, migrate_data, get_vif_config)"},{"line_number":68,"context_line":"    if ("},{"line_number":69,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":70,"context_line":"        migrate_data.dst_cpu_shared_set_info"},{"line_number":71,"context_line":"    ):"},{"line_number":72,"context_line":"        xml_doc \u003d _update_cpu_shared_set_xml(xml_doc, migrate_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0b3617fc_13728a5e","line":69,"updated":"2023-03-17 20:09:05.000000000","message":"We\u0027re going to have to be really clear on the semantics of dst_cpu_shared_set_info, especially in the rolling upgrade case, in which one compute is running the new code which sets dst_cpu_shared_set_info, and the other one isn\u0027t.\n\nIf the source is \"old\" and doesn\u0027t understand dst_cpu_shared_set_info, we get the current buggy behaviour, which is fine and expected, and there\u0027s nothing we can do about it.\n\nWe need to be carefule when the source is \"new\", there are a few possibilities:\n\n1. The dest is \"old\" and didn\u0027t set dst_cpu_shared_set_info (unset !\u003d set to None). Nothing we can do here, continue the current buggy behaviour.\n\n2. The dest is \"new\", but doesn\u0027t have cpu_shared_set. dst_cpu_shared_set_info will be set but None (not the same thing as unset). This is the only case we need to remove stuff from the XML.\n\n3. The dest is \"new\", and has cpu_shared_set. We update the XML like you do here.\n\ntl;dr unset !\u003d None !\u003d actual value","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f8f54bf37933088e55f988f13fa9d94849c72ee2","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    if get_vif_config is not None:"},{"line_number":67,"context_line":"        xml_doc \u003d _update_vif_xml(xml_doc, migrate_data, get_vif_config)"},{"line_number":68,"context_line":"    if ("},{"line_number":69,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":70,"context_line":"        migrate_data.dst_cpu_shared_set_info"},{"line_number":71,"context_line":"    ):"},{"line_number":72,"context_line":"        xml_doc \u003d _update_cpu_shared_set_xml(xml_doc, migrate_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f8125cf7_e4a3b9da","line":69,"in_reply_to":"0b3617fc_13728a5e","updated":"2023-03-20 15:08:43.000000000","message":"Done","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcf1b75af0fcda84d06cf3aefed12f5be7d32585","unresolved":true,"context_lines":[{"line_number":113,"context_line":"    return xml_doc"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"def _update_cpu_shared_set_xml(xml_doc, migrate_data):"},{"line_number":117,"context_line":"    LOG.debug(\u0027_update_cpu_shared_set_xml input xml\u003d%s\u0027,"},{"line_number":118,"context_line":"              etree.tostring(xml_doc, encoding\u003d\u0027unicode\u0027, pretty_print\u003dTrue))"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"39634540_ccbe20a5","line":116,"updated":"2023-03-17 20:09:05.000000000","message":"We\u0027ll need unit tests for this.","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f8f54bf37933088e55f988f13fa9d94849c72ee2","unresolved":false,"context_lines":[{"line_number":113,"context_line":"    return xml_doc"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"def _update_cpu_shared_set_xml(xml_doc, migrate_data):"},{"line_number":117,"context_line":"    LOG.debug(\u0027_update_cpu_shared_set_xml input xml\u003d%s\u0027,"},{"line_number":118,"context_line":"              etree.tostring(xml_doc, encoding\u003d\u0027unicode\u0027, pretty_print\u003dTrue))"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"faa464f5_ed647341","line":116,"in_reply_to":"39634540_ccbe20a5","updated":"2023-03-20 15:08:43.000000000","message":"Done","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fcf1b75af0fcda84d06cf3aefed12f5be7d32585","unresolved":true,"context_lines":[{"line_number":126,"context_line":"    return xml_doc"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def _remove_cpu_shared_set_xml(xml_doc, migrate_data):"},{"line_number":130,"context_line":"    LOG.debug(\u0027_remove_cpu_shared_set_xml input xml\u003d%s\u0027,"},{"line_number":131,"context_line":"              etree.tostring(xml_doc, encoding\u003d\u0027unicode\u0027, pretty_print\u003dTrue))"},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"0da7a2ea_a370bacb","line":129,"updated":"2023-03-17 20:09:05.000000000","message":"Ditto.","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f8f54bf37933088e55f988f13fa9d94849c72ee2","unresolved":false,"context_lines":[{"line_number":126,"context_line":"    return xml_doc"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def _remove_cpu_shared_set_xml(xml_doc, migrate_data):"},{"line_number":130,"context_line":"    LOG.debug(\u0027_remove_cpu_shared_set_xml input xml\u003d%s\u0027,"},{"line_number":131,"context_line":"              etree.tostring(xml_doc, encoding\u003d\u0027unicode\u0027, pretty_print\u003dTrue))"},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"0a813398_1ac0f649","line":129,"in_reply_to":"0da7a2ea_a370bacb","updated":"2023-03-20 15:08:43.000000000","message":"Done","commit_id":"be327b548e83b5efe3a8ddb1534aab7d0e81f6ae"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"327625948ec22ef6c3aef96218a6a0d8410fbef6","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    xml_doc \u003d _update_quota_xml(instance, xml_doc)"},{"line_number":66,"context_line":"    if get_vif_config is not None:"},{"line_number":67,"context_line":"        xml_doc \u003d _update_vif_xml(xml_doc, migrate_data, get_vif_config)"},{"line_number":68,"context_line":"    if ("},{"line_number":69,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":70,"context_line":"        migrate_data.dst_cpu_shared_set_info is not None"},{"line_number":71,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f980a9d_0d753cb4","line":68,"updated":"2023-03-21 14:26:10.000000000","message":"Actually I think we\u0027re missing a test for this bit of logic. I don\u0027t think it\u0027s worthwhile doing the same thing I did in the numa live migration series, with full functional tests simulating a rolling upgrade situation, but we should unit test that we do the right thing depending on the presence and value of dst_cpu_shared_set_info","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ee59b02ec084029e2378c8b42e63beb985d95475","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    xml_doc \u003d _update_quota_xml(instance, xml_doc)"},{"line_number":66,"context_line":"    if get_vif_config is not None:"},{"line_number":67,"context_line":"        xml_doc \u003d _update_vif_xml(xml_doc, migrate_data, get_vif_config)"},{"line_number":68,"context_line":"    if ("},{"line_number":69,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":70,"context_line":"        migrate_data.dst_cpu_shared_set_info is not None"},{"line_number":71,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":3,"id":"4ad3b907_c3b703cf","line":68,"in_reply_to":"2f980a9d_0d753cb4","updated":"2023-03-22 17:26:52.000000000","message":"Artom, can you please have a look at the new UT? Is is what you expected ?","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"2fcadcab831a9524e55d6641f9346dd19d3e46f5","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    xml_doc \u003d _update_quota_xml(instance, xml_doc)"},{"line_number":66,"context_line":"    if get_vif_config is not None:"},{"line_number":67,"context_line":"        xml_doc \u003d _update_vif_xml(xml_doc, migrate_data, get_vif_config)"},{"line_number":68,"context_line":"    if ("},{"line_number":69,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":70,"context_line":"        migrate_data.dst_cpu_shared_set_info is not None"},{"line_number":71,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":3,"id":"65d690a9_8fbba448","line":68,"in_reply_to":"4ad3b907_c3b703cf","updated":"2023-04-26 16:54:36.000000000","message":"Done","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"6db0671adb0953c38c3a6383b8948f97117fdb4d","unresolved":true,"context_lines":[{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info is None"},{"line_number":76,"context_line":"    ):"},{"line_number":77,"context_line":"        xml_doc \u003d _remove_cpu_shared_set_xml(xml_doc, migrate_data)"},{"line_number":78,"context_line":"    if \u0027dst_numa_info\u0027 in migrate_data:"},{"line_number":79,"context_line":"        xml_doc \u003d _update_numa_xml(xml_doc, migrate_data)"},{"line_number":80,"context_line":"    if new_resources:"}],"source_content_type":"text/x-python","patch_set":3,"id":"4924348a_6493f34c","line":77,"updated":"2023-03-20 19:33:58.000000000","message":"This is good, but not obvious, I would have added a comment explaining the semantics.","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6300512105a62f331a9aff7623b3a062c7f9709a","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        \u0027dst_cpu_shared_set_info\u0027 in migrate_data and"},{"line_number":75,"context_line":"        migrate_data.dst_cpu_shared_set_info is None"},{"line_number":76,"context_line":"    ):"},{"line_number":77,"context_line":"        xml_doc \u003d _remove_cpu_shared_set_xml(xml_doc, migrate_data)"},{"line_number":78,"context_line":"    if \u0027dst_numa_info\u0027 in migrate_data:"},{"line_number":79,"context_line":"        xml_doc \u003d _update_numa_xml(xml_doc, migrate_data)"},{"line_number":80,"context_line":"    if new_resources:"}],"source_content_type":"text/x-python","patch_set":3,"id":"b04772e8_f0863936","line":77,"in_reply_to":"4924348a_6493f34c","updated":"2023-03-22 17:25:25.000000000","message":"Done","commit_id":"fac85b4bde9767a308ee7f9ba6861ee05ab9ac9a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c5be5702c41affa2007c44c8d0d46129a43a4d1","unresolved":true,"context_lines":[{"line_number":188,"context_line":"          \u0027emulator_pins\u0027 in info):"},{"line_number":189,"context_line":"        for guest_id, host_ids in info.cpu_pins.items():"},{"line_number":190,"context_line":"            vcpupin \u003d xml_doc.find("},{"line_number":191,"context_line":"                \u0027./cputune/vcpupin[@vcpu\u003d\"%d\"]\u0027 % int(guest_id))"},{"line_number":192,"context_line":"            vcpupin.set(\u0027cpuset\u0027,"},{"line_number":193,"context_line":"                        hardware.format_cpu_spec(host_ids))"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5c43b459_9fa3417b","line":191,"updated":"2024-03-19 15:46:11.000000000","message":"How does the hack introduced in this patch not cause us to fail to `int(\"dst_cpu_shared_set_info\")`?\n\nI\u0027m guessing maybe we haven\u0027t tested with a situation where all three conditions are true? But this is exactly the sort of reason we shouldn\u0027t be hacking in this extra bit of data into a supposedly-versioned, supposedly-stable data structure on which our upgrades depend, IMHO.","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cd6cd9e6696e263eea7746b2561d28420d7715fb","unresolved":true,"context_lines":[{"line_number":188,"context_line":"          \u0027emulator_pins\u0027 in info):"},{"line_number":189,"context_line":"        for guest_id, host_ids in info.cpu_pins.items():"},{"line_number":190,"context_line":"            vcpupin \u003d xml_doc.find("},{"line_number":191,"context_line":"                \u0027./cputune/vcpupin[@vcpu\u003d\"%d\"]\u0027 % int(guest_id))"},{"line_number":192,"context_line":"            vcpupin.set(\u0027cpuset\u0027,"},{"line_number":193,"context_line":"                        hardware.format_cpu_spec(host_ids))"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a5a5859a_b75ed44f","line":191,"in_reply_to":"120d5fc7_87275300","updated":"2024-03-20 09:42:21.000000000","message":"for proparity we determied this was working because of the if\n\nin the non numa case \n```\nif (\u0027cpu_pins\u0027 in info and\n\u0027cell_pins\u0027 in info and\n\u0027emulator_pins\u0027 in info):\n```\nis false because we are only setting cpu_pins so we never execute the for loop\nin the numa case we discovered that the numa toplogy info is overriden and the \"dst_cpu_shared_set_info\" key is not present\n\nso this  only worked because we had mutually exlucive code paths\nnuma instance did not have the modifed object and non numa instances didn\u0027t\nuse the code path that was broken.","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"3f2d88c7573af2a7b5496847df49b8cd33d43e2e","unresolved":true,"context_lines":[{"line_number":188,"context_line":"          \u0027emulator_pins\u0027 in info):"},{"line_number":189,"context_line":"        for guest_id, host_ids in info.cpu_pins.items():"},{"line_number":190,"context_line":"            vcpupin \u003d xml_doc.find("},{"line_number":191,"context_line":"                \u0027./cputune/vcpupin[@vcpu\u003d\"%d\"]\u0027 % int(guest_id))"},{"line_number":192,"context_line":"            vcpupin.set(\u0027cpuset\u0027,"},{"line_number":193,"context_line":"                        hardware.format_cpu_spec(host_ids))"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a6644c0_2f709dda","line":191,"in_reply_to":"5c43b459_9fa3417b","updated":"2024-03-19 15:55:58.000000000","message":"I\u0027ll be honest, I don\u0027t know. https://review.opendev.org/c/openstack/whitebox-tempest-plugin/+/913433 passes, so it apparently works, but I don\u0027t understand _how_.","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7b43dfa2b72ff589781d5b5ea25b5555c768b7c9","unresolved":true,"context_lines":[{"line_number":188,"context_line":"          \u0027emulator_pins\u0027 in info):"},{"line_number":189,"context_line":"        for guest_id, host_ids in info.cpu_pins.items():"},{"line_number":190,"context_line":"            vcpupin \u003d xml_doc.find("},{"line_number":191,"context_line":"                \u0027./cputune/vcpupin[@vcpu\u003d\"%d\"]\u0027 % int(guest_id))"},{"line_number":192,"context_line":"            vcpupin.set(\u0027cpuset\u0027,"},{"line_number":193,"context_line":"                        hardware.format_cpu_spec(host_ids))"},{"line_number":194,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"120d5fc7_87275300","line":191,"in_reply_to":"9a6644c0_2f709dda","updated":"2024-03-19 19:47:18.000000000","message":"so i have looked at this a bit although i was pulled into other thing.\ni cant really ress how this is passing but i woudl speculate\nthat either we are mocking the gust numa topology in such a way as this is not set or if (\u0027cpu_pins\u0027 in info and\n          \u0027cell_pins\u0027 in info and\n          \u0027emulator_pins\u0027 in info): is false.\n          \n\n\nim leanign towards we are mocking this in the libvirt fixture \nbut its only stubbin the call to libvirt to do the migration not any of the driver code\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/functional/libvirt/base.py#L264\n\nso im not sure where that woudl be happeing\n\n\nin either case im not particalarly happy with that answer","commit_id":"f33bafa188cc65cf78e0c472e88a84b888c99a3c"}]}
