)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2559d4fdeb3cb6d6014f0492857e37d3e59a44f9","unresolved":false,"context_lines":[{"line_number":13,"context_line":"passed to the claim call."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: If469762b22d687151198468f0291821cebdf26b2"},{"line_number":16,"context_line":"Closes-Bug: #1893221"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_cbfc8c0e","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":10},"updated":"2020-08-31 18:04:15.000000000","message":"i suspect this should really be partial-bug as i think there is still some issues here however the current patch is more correct then what we were doing before it so im +1 but as i said i think we will need to do more then this.\n\n\nspecifically i dont think the new numa toplogy took the pci request into account but i think that can be adressed separately so im ok with this for now. even if they are taking the pci requests into account since we claim them later there is a posiblity for a race with other vms.\n\nthis is still better then not taking numa into account at all.","commit_id":"442981e16f9d7e331c2a3bf8ccdaa3d1ca79728c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49cda0be84fc5b1c30a28d0a978b697c1306d95e","unresolved":false,"context_lines":[{"line_number":13,"context_line":"passed to the claim call."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: If469762b22d687151198468f0291821cebdf26b2"},{"line_number":16,"context_line":"Closes-Bug: #1893221"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_22127d0a","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":10},"in_reply_to":"9f560f44_cbfc8c0e","updated":"2020-09-01 15:25:07.000000000","message":"I cannot prove it but I think the new numa topology took the pci requests into account.\n\nSo during live migration the scheduling is basically the same process than during boot so there the pci request is considered the same way as during boot.\n\nThen the claim happens at [1] by calling _live_migration_claim() that is basically a MoveClaim [2] and the MoveClaim does take care of the instance pci requests [3].\n\n\n[1] https://github.com/openstack/nova/blob/662398a1a4bb69c11a08fa5fa7196a89ed1acc87/nova/compute/manager.py#L7695\n[2] https://github.com/openstack/nova/blob/662398a1a4bb69c11a08fa5fa7196a89ed1acc87/nova/compute/resource_tracker.py#L240\n[3] https://github.com/openstack/nova/blob/662398a1a4bb69c11a08fa5fa7196a89ed1acc87/nova/compute/resource_tracker.py#L285-L297","commit_id":"442981e16f9d7e331c2a3bf8ccdaa3d1ca79728c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"773d2b70ba7c30c2e10dd0ddd8787faa7979c28b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Make PCI claim NUMA aware during live migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"NUMA aware live migration and SRIOV live migration was implemented as"},{"line_number":10,"context_line":"two separate feature. As a consequence the case when both SRIOV and NUMA"},{"line_number":11,"context_line":"is present in the instance was missed. When the PCI device is claimed on"},{"line_number":12,"context_line":"the destination host the NUMA topology of the instance needs to be"},{"line_number":13,"context_line":"passed to the claim call."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9f560f44_cad44fec","line":10,"range":{"start_line":10,"start_character":13,"end_line":10,"end_character":20},"updated":"2020-09-03 14:27:47.000000000","message":"nit: features","commit_id":"fdeae19875a7a068cedd5fdd3fcb817b1c21a1ec"}],"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a445fc36165d8d4e75ab74b5d9f5464ffe295b69","unresolved":false,"context_lines":[{"line_number":10397,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            claimed_pci_devices_objs \u003d self.rt.claim_pci_devices("},{"line_number":10400,"context_line":"                ctxt, vif_pci_requests, instance.numa_topology)"},{"line_number":10401,"context_line":""},{"line_number":10402,"context_line":"            # Update VIFMigrateData profile with the newly claimed PCI"},{"line_number":10403,"context_line":"            # device"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_3c078284","line":10400,"range":{"start_line":10400,"start_character":40,"end_line":10400,"end_character":62},"updated":"2020-08-27 15:23:23.000000000","message":"is this the source numa toplogy or the dest numa topology?\ngiven the numa toplogy can change as part of the migration and the vaild topogoies on the dest depend on the vf avaiable we kind of need ot generate teh new toplogy at the same time we are claiming the sriov devices so im not sure if this is correct.","commit_id":"d759ca3af6123e2444653bc0b43c1117abe30b49"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a50c62ab456da0e438c42071f317c4a306ca4300","unresolved":false,"context_lines":[{"line_number":10397,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            claimed_pci_devices_objs \u003d self.rt.claim_pci_devices("},{"line_number":10400,"context_line":"                ctxt, vif_pci_requests, instance.numa_topology)"},{"line_number":10401,"context_line":""},{"line_number":10402,"context_line":"            # Update VIFMigrateData profile with the newly claimed PCI"},{"line_number":10403,"context_line":"            # device"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_ccf5ebce","line":10400,"range":{"start_line":10400,"start_character":40,"end_line":10400,"end_character":62},"in_reply_to":"9f560f44_3c078284","updated":"2020-08-28 11:40:59.000000000","message":"this is most probably the source topo, but you and Artom helped me on IRC finding the dest topo saved in the instance.migration_context. \n\nSo I fixed this up.","commit_id":"d759ca3af6123e2444653bc0b43c1117abe30b49"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2559d4fdeb3cb6d6014f0492857e37d3e59a44f9","unresolved":false,"context_lines":[{"line_number":10396,"context_line":"                requests\u003dpci_reqs,"},{"line_number":10397,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            # if we are called during the live migration wiht NUMA topology"},{"line_number":10400,"context_line":"            # support the the PCI claim needs to consider the destination NUMA"},{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_a31fe559","line":10399,"range":{"start_line":10399,"start_character":56,"end_line":10399,"end_character":61},"updated":"2020-08-31 18:04:15.000000000","message":"nit: with","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49cda0be84fc5b1c30a28d0a978b697c1306d95e","unresolved":false,"context_lines":[{"line_number":10396,"context_line":"                requests\u003dpci_reqs,"},{"line_number":10397,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            # if we are called during the live migration wiht NUMA topology"},{"line_number":10400,"context_line":"            # support the the PCI claim needs to consider the destination NUMA"},{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_a2ee8db1","line":10399,"range":{"start_line":10399,"start_character":56,"end_line":10399,"end_character":61},"in_reply_to":"9f560f44_a31fe559","updated":"2020-09-01 15:25:07.000000000","message":"Done","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"600de029ece70e4aa2925a92940448b2ba4dc2bd","unresolved":false,"context_lines":[{"line_number":10397,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            # if we are called during the live migration wiht NUMA topology"},{"line_number":10400,"context_line":"            # support the the PCI claim needs to consider the destination NUMA"},{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"},{"line_number":10403,"context_line":"            if instance.migration_context:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_34ee69ed","line":10400,"updated":"2020-08-28 13:50:29.000000000","message":"pep8: N343: Doubled word \u0027the\u0027 typo found","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2559d4fdeb3cb6d6014f0492857e37d3e59a44f9","unresolved":false,"context_lines":[{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            # if we are called during the live migration wiht NUMA topology"},{"line_number":10400,"context_line":"            # support the the PCI claim needs to consider the destination NUMA"},{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"},{"line_number":10403,"context_line":"            if instance.migration_context:"},{"line_number":10404,"context_line":"                dest_topo \u003d instance.migration_context.new_numa_topology"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_23e4753c","line":10401,"range":{"start_line":10401,"start_character":15,"end_line":10401,"end_character":67},"updated":"2020-08-31 18:04:15.000000000","message":"the toplogoy may not be stored at this point and even if it is it may be incorrect. but its the most correct toplogy we have. acess to currently.\n\ni think we will still need to revisit this with a different solution at some other point.","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49cda0be84fc5b1c30a28d0a978b697c1306d95e","unresolved":false,"context_lines":[{"line_number":10398,"context_line":""},{"line_number":10399,"context_line":"            # if we are called during the live migration wiht NUMA topology"},{"line_number":10400,"context_line":"            # support the the PCI claim needs to consider the destination NUMA"},{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"},{"line_number":10403,"context_line":"            if instance.migration_context:"},{"line_number":10404,"context_line":"                dest_topo \u003d instance.migration_context.new_numa_topology"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_42e5f1db","line":10401,"range":{"start_line":10401,"start_character":15,"end_line":10401,"end_character":67},"in_reply_to":"9f560f44_23e4753c","updated":"2020-09-01 15:25:07.000000000","message":"Why do you think that the new_numa_topology stored in the migration context is incorrect?","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2559d4fdeb3cb6d6014f0492857e37d3e59a44f9","unresolved":false,"context_lines":[{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"},{"line_number":10403,"context_line":"            if instance.migration_context:"},{"line_number":10404,"context_line":"                dest_topo \u003d instance.migration_context.new_numa_topology"},{"line_number":10405,"context_line":""},{"line_number":10406,"context_line":"            claimed_pci_devices_objs \u003d self.rt.claim_pci_devices("},{"line_number":10407,"context_line":"                ctxt, vif_pci_requests, dest_topo)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_e3ab9dd4","line":10404,"range":{"start_line":10404,"start_character":55,"end_line":10404,"end_character":72},"updated":"2020-08-31 18:04:15.000000000","message":"by the way im not 100% sure this too account of the pci devices when it was generated there is a race hear in anycase since we have not claimed them yet.\n\ni need to look at the ordering of the calls to see if this makes sense.","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49cda0be84fc5b1c30a28d0a978b697c1306d95e","unresolved":false,"context_lines":[{"line_number":10401,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10402,"context_line":"            dest_topo \u003d None"},{"line_number":10403,"context_line":"            if instance.migration_context:"},{"line_number":10404,"context_line":"                dest_topo \u003d instance.migration_context.new_numa_topology"},{"line_number":10405,"context_line":""},{"line_number":10406,"context_line":"            claimed_pci_devices_objs \u003d self.rt.claim_pci_devices("},{"line_number":10407,"context_line":"                ctxt, vif_pci_requests, dest_topo)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_62f47588","line":10404,"range":{"start_line":10404,"start_character":55,"end_line":10404,"end_character":72},"in_reply_to":"9f560f44_e3ab9dd4","updated":"2020-09-01 15:25:07.000000000","message":"One interesting thing is that the MoveClaim that generated the new_numa_topology is also seems to be claiming pci devices. (See the links in my reply in the commit message). So one possible thing is that we might double claim.","commit_id":"b511751eb4168a9510640f8a134e2ab59fe5f27d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5b9432b10eb6cf07031310d41f4456a3edc605df","unresolved":true,"context_lines":[{"line_number":10602,"context_line":"            # topology that is then stored in the migration_context"},{"line_number":10603,"context_line":"            dest_topo \u003d None"},{"line_number":10604,"context_line":"            if instance.migration_context:"},{"line_number":10605,"context_line":"                dest_topo \u003d instance.migration_context.new_numa_topology"},{"line_number":10606,"context_line":""},{"line_number":10607,"context_line":"            claimed_pci_devices_objs \u003d self.rt.claim_pci_devices("},{"line_number":10608,"context_line":"                ctxt, vif_pci_requests, dest_topo)"}],"source_content_type":"text/x-python","patch_set":10,"id":"78253085_b6085388","line":10605,"updated":"2021-01-20 12:17:46.000000000","message":"Nit: Part of me hoped to see a test that ensured this was populated with the new_numa_topology at this point in the live-migration process. But I have no idea how I would go about testing that, so I am not to require that test here.","commit_id":"1273c5ee0b18974d9837e9221fc9270429d428bf"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"773d2b70ba7c30c2e10dd0ddd8787faa7979c28b","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                                 new_pci_requests, migration, limits\u003dlimits)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"        claimed_pci_devices_objs \u003d []"},{"line_number":303,"context_line":"        # TODO(artom) The second part of this condition should not be"},{"line_number":304,"context_line":"        # necessary, but since SRIOV live migration is currently handled"},{"line_number":305,"context_line":"        # elsewhere - see for example _claim_pci_for_instance_vifs() in the"},{"line_number":306,"context_line":"        # compute manager - we don\u0027t do any PCI claims if this is a live"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_0a1c8775","line":303,"updated":"2020-09-03 14:27:47.000000000","message":"If I suggest addressing this TODO in order to support NUMA + SRIOV live migration I think I\u0027ll end up in a fight with Sean, but I still think it\u0027d the right way to go. Currently we have PCI live migration done one way, and SRIOV live migration done another. It\u0027s just confusing and needs extra code for no reason other than \"they were implemented by different people at different times\"","commit_id":"fdeae19875a7a068cedd5fdd3fcb817b1c21a1ec"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"91b28ab01bd9cac2c05338ebcaf8354b94249577","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                                 new_pci_requests, migration, limits\u003dlimits)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"        claimed_pci_devices_objs \u003d []"},{"line_number":303,"context_line":"        # TODO(artom) The second part of this condition should not be"},{"line_number":304,"context_line":"        # necessary, but since SRIOV live migration is currently handled"},{"line_number":305,"context_line":"        # elsewhere - see for example _claim_pci_for_instance_vifs() in the"},{"line_number":306,"context_line":"        # compute manager - we don\u0027t do any PCI claims if this is a live"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_d5ed10b8","line":303,"in_reply_to":"9f560f44_0a1c8775","updated":"2020-09-03 14:28:40.000000000","message":"Err, \"NUMA live migration done one way, and SRIOV live migration done another.\"","commit_id":"fdeae19875a7a068cedd5fdd3fcb817b1c21a1ec"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"eb075000a12292e470c1724183eaafcd7c058bda","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                                 new_pci_requests, migration, limits\u003dlimits)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"        claimed_pci_devices_objs \u003d []"},{"line_number":303,"context_line":"        # TODO(artom) The second part of this condition should not be"},{"line_number":304,"context_line":"        # necessary, but since SRIOV live migration is currently handled"},{"line_number":305,"context_line":"        # elsewhere - see for example _claim_pci_for_instance_vifs() in the"},{"line_number":306,"context_line":"        # compute manager - we don\u0027t do any PCI claims if this is a live"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_0c565007","line":303,"in_reply_to":"9f560f44_d5ed10b8","updated":"2020-09-07 08:54:43.000000000","message":"Let\u0027s try to remove this technical debt in a separate patch.","commit_id":"fdeae19875a7a068cedd5fdd3fcb817b1c21a1ec"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"773d2b70ba7c30c2e10dd0ddd8787faa7979c28b","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"        :returns: a list of nova.objects.PciDevice objects"},{"line_number":1879,"context_line":"        \"\"\""},{"line_number":1880,"context_line":"        result \u003d self.pci_tracker.claim_instance("},{"line_number":1881,"context_line":"            context, pci_requests, instance_numa_topology)"},{"line_number":1882,"context_line":"        self.pci_tracker.save(context)"},{"line_number":1883,"context_line":"        return result"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_956398dd","line":1881,"range":{"start_line":1881,"start_character":35,"end_line":1881,"end_character":57},"updated":"2020-09-03 14:27:47.000000000","message":"So this arg was already present, you\u0027re just actually using it now for live migration.","commit_id":"fdeae19875a7a068cedd5fdd3fcb817b1c21a1ec"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"eb075000a12292e470c1724183eaafcd7c058bda","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"        :returns: a list of nova.objects.PciDevice objects"},{"line_number":1879,"context_line":"        \"\"\""},{"line_number":1880,"context_line":"        result \u003d self.pci_tracker.claim_instance("},{"line_number":1881,"context_line":"            context, pci_requests, instance_numa_topology)"},{"line_number":1882,"context_line":"        self.pci_tracker.save(context)"},{"line_number":1883,"context_line":"        return result"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_8c49c061","line":1881,"range":{"start_line":1881,"start_character":35,"end_line":1881,"end_character":57},"in_reply_to":"9f560f44_956398dd","updated":"2020-09-07 08:54:43.000000000","message":"yes.","commit_id":"fdeae19875a7a068cedd5fdd3fcb817b1c21a1ec"}]}
