)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ff5dba20045e3d6cae728a916ad8655fd1c8dd6f","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related to blueprint one-time-use-devices"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Depends-On: https://review.opendev.org/c/openstack/nova/+/944054"},{"line_number":18,"context_line":"Change-Id: Idfe8a746a97d68cd4eae30afb7d22f4e3af80327"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"e3fda9e9_23909160","line":17,"updated":"2025-03-26 15:36:38.000000000","message":"Oops, this should be the placement traits patch. Must have been a mis-paste.","commit_id":"c8f7ee56dfbb644c4dbd9a8c81074d1ff9744001"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related to blueprint one-time-use-devices"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Depends-On: https://review.opendev.org/c/openstack/nova/+/944054"},{"line_number":18,"context_line":"Change-Id: Idfe8a746a97d68cd4eae30afb7d22f4e3af80327"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"6b7a1d02_bcb50a59","line":17,"in_reply_to":"e3fda9e9_23909160","updated":"2025-03-28 16:32:08.000000000","message":"Done","commit_id":"c8f7ee56dfbb644c4dbd9a8c81074d1ff9744001"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5dfb362059d480a198a73a3664e4a83e025c4b00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"be8cd0f8_eceff392","updated":"2025-03-07 18:40:35.000000000","message":"Obviously this needs cleanup and tests, but it actually works for me locally. Since I\u0027m using my qemu (first level guest)\u0027s QXL video device to test with, and since deleting an instance that has that passed through crashes the host shortly later, I\u0027ve also confirmed that restarting nova leaves the reserved count\u003d1 even when the instance is gone.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ec00200dba61fe285970eeec2b7bc242e834da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"37e707ff_5bcee8ed","updated":"2025-03-10 18:00:07.000000000","message":"Thanks gibi!","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b768c2704766c8bc33f7df8ab83341b1caa63741","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fac2189a_9a687a26","updated":"2025-03-11 18:49:40.000000000","message":"This is closer to fleshed out, but still needs docs and I haven\u0027t yet re-tested this on a real system (or even run functional tests against it) yet.","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d0fe99158dda04a307e9ca804166a5139b219a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2c2fcbc3_35f57698","updated":"2025-03-12 09:00:10.000000000","message":"This is going to the right direction. I left some small suggestions inline. I haven\u0027t looked at the tests yet.","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9899cd2ad5c85244d857587a41327523ced229a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9e5f8248_bfde4a4a","updated":"2025-03-13 13:49:03.000000000","message":"recheck zuul issue reportedly fixed","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"018277b85a2233179ed9cc8afd526ed50448a755","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9b322fda_d569d3ad","updated":"2025-03-13 13:48:22.000000000","message":"zuul issue reportedly fixed","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5cdc4cfd_27d1c318","updated":"2025-03-28 16:11:54.000000000","message":"I have some ideas to refactor. It magically solves the mypy issues as well https://review.opendev.org/c/openstack/nova/+/945856","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5cb505a9a5106b87f8621bbf02d456fc65692bbf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"7b0ac1ad_28f992bb","updated":"2025-04-01 14:48:49.000000000","message":"Looks good. I still think we can drop the code that copies the the otu tag from the dev_spec to the dev dict becasue there are no code depeding on that tag being in the dev dict or in the dev ovo","commit_id":"e80dfcc4ba3fdd80b0514d06d6092fcf6dc343f3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"22393d2ccb1ba97e108b15618e5bf42e0d107787","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"0e3af5e4_0389bc2d","updated":"2025-04-02 11:59:36.000000000","message":"I did manual testing of this series in a local multinode devstack with IGB PFs. It works well. What I verified:\n* VM boot, delete with multiple PFs one being OTU succeeds. OTU is reserved during boot and kept reserved after delete\n* Shelve offload and ushelve works. OTU dev is kept reserved after offload. And needed external unreservaton before unshelving could be done to the same dev.\n* new traits managed properly on RPs (add / delete)\n* input validation works, i.e.no OTU VF allowed.\n* marking an in-use device as OTU and restarting nova-compute triggers the reservation of the in-use device.","commit_id":"9b9103d900bb5151fbc74310a37aa0534f94f409"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4831997d4c01f3399124a9ef9a9c13aa548c7d74","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"ee951517_fb663d2f","in_reply_to":"0e3af5e4_0389bc2d","updated":"2025-04-02 13:34:02.000000000","message":"Sweet, thanks!","commit_id":"9b9103d900bb5151fbc74310a37aa0534f94f409"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"537d11af61c370b582258efbc6cfa88e5ab2c124","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"dd885582_2c03aae6","updated":"2025-04-02 13:51:03.000000000","message":"Looks good. Holding +2 until the os-traits change is released and bumped in our requirements.txt.","commit_id":"a852b19ee2699919d69275f272ee16464da0a3b7"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e39f0f0968c7dbc2c28ae9e317684fd07e1e651a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"8487a89e_8d22e298","updated":"2025-04-03 07:23:25.000000000","message":"This is good to land now as os-triats lib is released and bumped.","commit_id":"28a266461a1eadfe9c3b268123b14da7005ed165"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cdea37d4022d23e291c80cae66b45f5cd23a73a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"a9927088_13208e0e","updated":"2025-04-07 15:10:24.000000000","message":"recheck tempest fix merged","commit_id":"28a266461a1eadfe9c3b268123b14da7005ed165"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"94642cbb1808c21d616a3f71965681734ba83003","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"1deec187_800eab4f","updated":"2025-04-07 20:07:36.000000000","message":"recheck unrelated post failure on migration job","commit_id":"28a266461a1eadfe9c3b268123b14da7005ed165"}],"nova/compute/pci_placement_translator.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fbc5a8b72051befb44dfe40e4792c6e2499e7415","unresolved":true,"context_lines":[{"line_number":249,"context_line":"            \"total\": len(self.devs),"},{"line_number":250,"context_line":"            \"max_unit\": len(self.devs),"},{"line_number":251,"context_line":"        }"},{"line_number":252,"context_line":"        # Determine if we are a top-level device and if we are currently"},{"line_number":253,"context_line":"        # allocated"},{"line_number":254,"context_line":"        is_allocated \u003d ("},{"line_number":255,"context_line":"            self.parent_dev and"},{"line_number":256,"context_line":"            \u0027instance_uuid\u0027 in self.parent_dev and"},{"line_number":257,"context_line":"            self.parent_dev.instance_uuid)"},{"line_number":258,"context_line":"        # Determine if we are marked as one_time_use\u003dtrue"},{"line_number":259,"context_line":"        is_otu \u003d strutils.bool_from_string(self.parent_dev.extra_info.get("},{"line_number":260,"context_line":"            \u0027one_time_use\u0027, \u0027false\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"cb826fb7_6451c604","line":257,"range":{"start_line":252,"start_character":0,"end_line":257,"end_character":42},"updated":"2025-03-10 15:11:21.000000000","message":"The self.parent_dev is set if this RP only represents a single PF/PCI device and then parent_dev points to that PCIDevice object. If this RP represents a set of interchangeable VFs under the same PF then self.children_devs stores the list of PCIDevice objects that is represented by this RP. \n\nThe pci in placement code enforces that both the PF and its children VFs cannot be configured for nova\u0027s consumption at the same time. Just either or. (This is called dependent device handling and the non pci in placement code allows it. And reserved the PF when the first child VF is consumed and reserved all the children VFs when the PF is consumed)\n\nI understand that we only want to support OTU for full PFs / PCI devs and not VFs. So I guess this is why you filter for RPs here by checking that self.parent_dev is set. If so then when the OTU flag is set for VFs in the dev_spec then that should be rejected as wrong configuration. We should try to catch that early if possible during the dev_spec config parsing. If we don\u0027t have enough info there then as a last resort we can catch it here and raise but then we need to double check if raising here can stop the nova-compute startup.\n\nIf we ever want to support OTU for VFs then the placement model will not help as it does not represent each VFs as individual RPs so a specific VF cannot be reserved. So if we go with the current approach that means later if VF support is needed we need to implement a parallel support for VFs in the PCI tracker instead.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ec00200dba61fe285970eeec2b7bc242e834da","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            \"total\": len(self.devs),"},{"line_number":250,"context_line":"            \"max_unit\": len(self.devs),"},{"line_number":251,"context_line":"        }"},{"line_number":252,"context_line":"        # Determine if we are a top-level device and if we are currently"},{"line_number":253,"context_line":"        # allocated"},{"line_number":254,"context_line":"        is_allocated \u003d ("},{"line_number":255,"context_line":"            self.parent_dev and"},{"line_number":256,"context_line":"            \u0027instance_uuid\u0027 in self.parent_dev and"},{"line_number":257,"context_line":"            self.parent_dev.instance_uuid)"},{"line_number":258,"context_line":"        # Determine if we are marked as one_time_use\u003dtrue"},{"line_number":259,"context_line":"        is_otu \u003d strutils.bool_from_string(self.parent_dev.extra_info.get("},{"line_number":260,"context_line":"            \u0027one_time_use\u0027, \u0027false\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"c1dc4e23_594123c9","line":257,"range":{"start_line":252,"start_character":0,"end_line":257,"end_character":42},"in_reply_to":"cb826fb7_6451c604","updated":"2025-03-10 18:00:07.000000000","message":"\u003e The self.parent_dev is set if this RP only represents a single PF/PCI device and then parent_dev points to that PCIDevice object. If this RP represents a set of interchangeable VFs under the same PF then self.children_devs stores the list of PCIDevice objects that is represented by this RP. \n\nThis was my understanding, so thanks for confirming.\n\n\u003e The pci in placement code enforces that both the PF and its children VFs cannot be configured for nova\u0027s consumption at the same time. Just either or. (This is called dependent device handling and the non pci in placement code allows it. And reserved the PF when the first child VF is consumed and reserved all the children VFs when the PF is consumed)\n\nCool.\n\n\u003e I understand that we only want to support OTU for full PFs / PCI devs and not VFs. So I guess this is why you filter for RPs here by checking that self.parent_dev is set. If so then when the OTU flag is set for VFs in the dev_spec then that should be rejected as wrong configuration. We should try to catch that early if possible during the dev_spec config parsing. If we don\u0027t have enough info there then as a last resort we can catch it here and raise but then we need to double check if raising here can stop the nova-compute startup.\n\nAck, I\u0027m not sure if I think we should abort startup for that case or not. I just figure this code should never allow reserving an (unnamed fungible) VF but I\u0027m not sure it needs to fail deep here. If we do other validation of the PCI config at startup, then it makes sense to add this to the list of things we check.\n\n\u003e If we ever want to support OTU for VFs then the placement model will not help as it does not represent each VFs as individual RPs so a specific VF cannot be reserved. So if we go with the current approach that means later if VF support is needed we need to implement a parallel support for VFs in the PCI tracker instead.\n\nAs we discussed on IRC (recording here for posterity), I\u0027m not sure when/why OTU VFs would ever make sense as those devices *should* be sort of virt-native anyway. If we did, we\u0027ll need all manner of more work, including somewhere to track them and some interface for an operator to un-reserve them when cleaned. I think it\u0027s probably best that we limit the use of this (both now, and in our heads for the future) to only full devices, tracked in placement and leave any more complex situations to cyborg.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5dfb362059d480a198a73a3664e4a83e025c4b00","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            # not if we are not allocated. These devices are intended to go"},{"line_number":267,"context_line":"            # from unallocated to allocated AND reserved. They may be"},{"line_number":268,"context_line":"            # unreserved by an external entity, but never nova."},{"line_number":269,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":270,"context_line":"            LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":271,"context_line":"                     \u0027resource provider %s (%s)\u0027,"},{"line_number":272,"context_line":"                     len(self.devs), rp_uuid, self.name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e181b9a_e3a825a3","line":269,"updated":"2025-03-07 18:40:35.000000000","message":"Note for reviewers: this is not really where I wanted this to be done, as I was hoping to put this into the resource tracker right near the PCI claim (in `instance_claim`). However, this seems to be the least invasive place to do this, since we\u0027re already updating inventory. Basically, here:\n\nhttps://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/resource_tracker.py#L204\n\nIf I did this in the resource tracker, I need to dig through the provider tree, update it there, which will trigger an update, 409, refresh, and then another attempt. Then, the obligatory `update_provider_tree` we do after the claim will basically do the same thing again (including the conflicts) just a few lines later, when we call:\n\nhttps://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/resource_tracker.py#L215\n\nThus putting it here, means the above `_update()` will do the sync-to-placement for us in basically the same place.\n\nIt\u0027s a little less explicit than I was hoping, but this also has the side effect of any PCI device that we discover is allocated and marked for one-time-use will get reserved. Meaning, an instance that already exists with a PCI device, and the operator adds the one-time-use flag to the devspec, will get updated during the periodic and reserved.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fbc5a8b72051befb44dfe40e4792c6e2499e7415","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            # not if we are not allocated. These devices are intended to go"},{"line_number":267,"context_line":"            # from unallocated to allocated AND reserved. They may be"},{"line_number":268,"context_line":"            # unreserved by an external entity, but never nova."},{"line_number":269,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":270,"context_line":"            LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":271,"context_line":"                     \u0027resource provider %s (%s)\u0027,"},{"line_number":272,"context_line":"                     len(self.devs), rp_uuid, self.name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd01e6a5_3c6b2c08","line":269,"in_reply_to":"7e181b9a_e3a825a3","updated":"2025-03-10 15:11:21.000000000","message":"I think the location of the change is OK. \n\nI guess we are OK to fail on the side of over-reserve. So if the VM boot fails after this point (e.g. in the driver.spawn call) then even though the allocation will be reverted by the compute manager we will not revert the reservation here and let the any external tool handle that case.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ec00200dba61fe285970eeec2b7bc242e834da","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            # not if we are not allocated. These devices are intended to go"},{"line_number":267,"context_line":"            # from unallocated to allocated AND reserved. They may be"},{"line_number":268,"context_line":"            # unreserved by an external entity, but never nova."},{"line_number":269,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":270,"context_line":"            LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":271,"context_line":"                     \u0027resource provider %s (%s)\u0027,"},{"line_number":272,"context_line":"                     len(self.devs), rp_uuid, self.name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"42e9225c_201adb42","line":269,"in_reply_to":"cd01e6a5_3c6b2c08","updated":"2025-03-10 18:00:07.000000000","message":"Failing in a way that we burn more devices than we need is safer than a case where we think it\u0027s safe to un-burn a device and get it wrong. What I don\u0027t want to do is have a case where some scheduling code doesn\u0027t know if we\u0027re booting, rescheduling a boot, doing a migrate, evacuate, etc and un-reserves a device incorrectly. I\u0027d rather tell operators we\u0027re erring on the side of caution and they\u0027re responsible for cleaning up things that get burned, regardless of why.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fbc5a8b72051befb44dfe40e4792c6e2499e7415","unresolved":true,"context_lines":[{"line_number":633,"context_line":"    # trigger an update on the device pools in the tracker to get the device"},{"line_number":634,"context_line":"    # RP UUID mapped to the device pools"},{"line_number":635,"context_line":"    pci_tracker.stats.populate_pools_metadata_from_assigned_devices()"},{"line_number":636,"context_line":"    updated \u003d pv.update_allocations(allocations, provider_tree)"},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"    if updated:"},{"line_number":639,"context_line":"        LOG.debug("},{"line_number":640,"context_line":"            \"Placement PCI view needs allocation healing. This should only \""},{"line_number":641,"context_line":"            \"happen if [filter_scheduler]pci_in_placement is still disabled. \""},{"line_number":642,"context_line":"            \"Original allocations: %s New allocations: %s\","},{"line_number":643,"context_line":"            old_alloc,"},{"line_number":644,"context_line":"            allocations,"},{"line_number":645,"context_line":"        )"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    return updated"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e3f0c5f_a873bcda","line":646,"range":{"start_line":636,"start_character":0,"end_line":646,"end_character":0},"updated":"2025-03-10 15:11:21.000000000","message":"This is the temporary allocation healing code path that will go away. But the current proposal only changed the path starting above at update_provider_tree() which expected to remain as that does the inventory generation. So the location of the proposed change looks good to me.\n\nWe also discussed over IRC that this code is called at the end of the instance_claim synchronously (not just during periodic update_available_resources) so there is no race window between device allocation and reservation.\n\nSo this looks good.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ec00200dba61fe285970eeec2b7bc242e834da","unresolved":false,"context_lines":[{"line_number":633,"context_line":"    # trigger an update on the device pools in the tracker to get the device"},{"line_number":634,"context_line":"    # RP UUID mapped to the device pools"},{"line_number":635,"context_line":"    pci_tracker.stats.populate_pools_metadata_from_assigned_devices()"},{"line_number":636,"context_line":"    updated \u003d pv.update_allocations(allocations, provider_tree)"},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"    if updated:"},{"line_number":639,"context_line":"        LOG.debug("},{"line_number":640,"context_line":"            \"Placement PCI view needs allocation healing. This should only \""},{"line_number":641,"context_line":"            \"happen if [filter_scheduler]pci_in_placement is still disabled. \""},{"line_number":642,"context_line":"            \"Original allocations: %s New allocations: %s\","},{"line_number":643,"context_line":"            old_alloc,"},{"line_number":644,"context_line":"            allocations,"},{"line_number":645,"context_line":"        )"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    return updated"}],"source_content_type":"text/x-python","patch_set":1,"id":"de3b5ce0_0ed12d22","line":646,"range":{"start_line":636,"start_character":0,"end_line":646,"end_character":0},"in_reply_to":"7e3f0c5f_a873bcda","updated":"2025-03-10 18:00:07.000000000","message":"Acknowledged","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d0fe99158dda04a307e9ca804166a5139b219a3","unresolved":true,"context_lines":[{"line_number":174,"context_line":"        if \u0027one_time_use\u0027 in dev.extra_info:"},{"line_number":175,"context_line":"            # Child devices cannot be OTU. Do not even tolerate setting \u003dfalse"},{"line_number":176,"context_line":"            raise exception.PlacementPciException("},{"line_number":177,"context_line":"                error\u003d\u0027Only type-PCI and type-PF devices may set one_time_use\u0027)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        self.children_devs.append(dev)"},{"line_number":180,"context_line":"        self.resource_class \u003d rc"}],"source_content_type":"text/x-python","patch_set":2,"id":"35fc7a1d_0b266218","line":177,"updated":"2025-03-12 09:00:10.000000000","message":"I would include the device to the exception message to point out which device causes the config issue","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ac946d53e3fd660343249267b15aba5f02e9fa8a","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        if \u0027one_time_use\u0027 in dev.extra_info:"},{"line_number":175,"context_line":"            # Child devices cannot be OTU. Do not even tolerate setting \u003dfalse"},{"line_number":176,"context_line":"            raise exception.PlacementPciException("},{"line_number":177,"context_line":"                error\u003d\u0027Only type-PCI and type-PF devices may set one_time_use\u0027)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        self.children_devs.append(dev)"},{"line_number":180,"context_line":"        self.resource_class \u003d rc"}],"source_content_type":"text/x-python","patch_set":2,"id":"571857c4_6f0fa2a0","line":177,"in_reply_to":"35fc7a1d_0b266218","updated":"2025-03-12 18:49:02.000000000","message":"Done","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d0fe99158dda04a307e9ca804166a5139b219a3","unresolved":true,"context_lines":[{"line_number":272,"context_line":"            # from unallocated to allocated AND reserved. They may be"},{"line_number":273,"context_line":"            # unreserved by an external entity, but never nova."},{"line_number":274,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":275,"context_line":"            LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":276,"context_line":"                     \u0027resource provider %s (%s)\u0027,"},{"line_number":277,"context_line":"                     len(self.devs), rp_uuid, self.name)"},{"line_number":278,"context_line":"        if is_otu:"}],"source_content_type":"text/x-python","patch_set":2,"id":"0864c63d_3895aa7d","line":275,"updated":"2025-03-12 09:00:10.000000000","message":"nit: this might be a repeated log line for each update_available_resources periodic as we log this even if the reserved is already set.","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ac946d53e3fd660343249267b15aba5f02e9fa8a","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            # from unallocated to allocated AND reserved. They may be"},{"line_number":273,"context_line":"            # unreserved by an external entity, but never nova."},{"line_number":274,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":275,"context_line":"            LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":276,"context_line":"                     \u0027resource provider %s (%s)\u0027,"},{"line_number":277,"context_line":"                     len(self.devs), rp_uuid, self.name)"},{"line_number":278,"context_line":"        if is_otu:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b94793e6_f0cc4075","line":275,"in_reply_to":"0864c63d_3895aa7d","updated":"2025-03-12 18:49:02.000000000","message":"Done","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8d0fe99158dda04a307e9ca804166a5139b219a3","unresolved":true,"context_lines":[{"line_number":281,"context_line":"            self.traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":282,"context_line":"        else:"},{"line_number":283,"context_line":"            self.traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        provider_tree.update_inventory("},{"line_number":286,"context_line":"            self.name,"},{"line_number":287,"context_line":"            # NOTE(gibi): The rest of the inventory fields (allocation_ratio,"}],"source_content_type":"text/x-python","patch_set":2,"id":"f975682f_c4ad4621","line":284,"updated":"2025-03-12 09:00:10.000000000","message":"I would pull the OTU handling here out to a separate function something like:\n```python\n        rp_uuid \u003d provider_tree.data(self.name).uuid\n        inventory \u003d {\n            \"total\": len(self.devs),\n            \"max_unit\": len(self.devs),\n        }\n        \n        self.handle_one_time_use(inventory)\n        \n        provider_tree.update_inventory(\n            self.name,\n            ...\n```","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"354d99596c850175b8f3c9c5316c28a1165d7b66","unresolved":false,"context_lines":[{"line_number":281,"context_line":"            self.traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":282,"context_line":"        else:"},{"line_number":283,"context_line":"            self.traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        provider_tree.update_inventory("},{"line_number":286,"context_line":"            self.name,"},{"line_number":287,"context_line":"            # NOTE(gibi): The rest of the inventory fields (allocation_ratio,"}],"source_content_type":"text/x-python","patch_set":2,"id":"a6ba415d_52e9eac9","line":284,"in_reply_to":"f975682f_c4ad4621","updated":"2025-03-12 13:25:39.000000000","message":"Acknowledged","commit_id":"59d8a79b7cbdc07bfc54d2ad0350b52ad439a4ed"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"de214eacf36797268982d99119612d461b58f8c8","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # But, mypy needs a lot of convincing, so we\u0027ll put this in the runtime"},{"line_number":228,"context_line":"        # code just to make it happy :("},{"line_number":229,"context_line":"        dev: pci_device.PciDevice \u003d self.parent_dev"},{"line_number":230,"context_line":"        traits: set \u003d self.traits or set()  # This can never happen"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        is_allocated \u003d \u0027instance_uuid\u0027 in dev and dev.instance_uuid"},{"line_number":233,"context_line":"        is_otu \u003d strutils.bool_from_string(dev.extra_info.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"a53d713d_5ac511db","line":230,"updated":"2025-03-12 18:53:00.000000000","message":"I spent way (way) too long trying to avoid the above nonsensical thing to make mypy happy and failed. Happy (well, willing) to replace this with the right anti-duck-typing-fu out of respect to the rest of this code, if someone can point me in the right direction.","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        # But, mypy needs a lot of convincing, so we\u0027ll put this in the runtime"},{"line_number":228,"context_line":"        # code just to make it happy :("},{"line_number":229,"context_line":"        dev: pci_device.PciDevice \u003d self.parent_dev"},{"line_number":230,"context_line":"        traits: set \u003d self.traits or set()  # This can never happen"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        is_allocated \u003d \u0027instance_uuid\u0027 in dev and dev.instance_uuid"},{"line_number":233,"context_line":"        is_otu \u003d strutils.bool_from_string(dev.extra_info.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"f1a22d0c_6d6636c2","line":230,"in_reply_to":"006dca7c_4078f95c","updated":"2025-03-31 14:41:59.000000000","message":"Acknowledged","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d36bc99542281f7966f8074880860195243c0bb2","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # But, mypy needs a lot of convincing, so we\u0027ll put this in the runtime"},{"line_number":228,"context_line":"        # code just to make it happy :("},{"line_number":229,"context_line":"        dev: pci_device.PciDevice \u003d self.parent_dev"},{"line_number":230,"context_line":"        traits: set \u003d self.traits or set()  # This can never happen"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        is_allocated \u003d \u0027instance_uuid\u0027 in dev and dev.instance_uuid"},{"line_number":233,"context_line":"        is_otu \u003d strutils.bool_from_string(dev.extra_info.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"006dca7c_4078f95c","line":230,"in_reply_to":"17f55bf0_d0c0f7be","updated":"2025-03-26 14:17:09.000000000","message":"Sorry maybe I\u0027m confused about what you\u0027re suggesting, since you commented on two different patch sets. Just making the change you describe here results in the same error that I get with no extra type dance here. My `or set()` is what convinces it that we\u0027re not going to be setting None into the `traits` local variable.\n\nWhat I have here makes mypy happy.. Are you suggesting there\u0027s a better thing to do (if so, I don\u0027t think it is what you\u0027ve got here) than what I have working here currently?","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"62c785ea06382ad3c0ddc5e306327fc503b2419f","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # But, mypy needs a lot of convincing, so we\u0027ll put this in the runtime"},{"line_number":228,"context_line":"        # code just to make it happy :("},{"line_number":229,"context_line":"        dev: pci_device.PciDevice \u003d self.parent_dev"},{"line_number":230,"context_line":"        traits: set \u003d self.traits or set()  # This can never happen"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        is_allocated \u003d \u0027instance_uuid\u0027 in dev and dev.instance_uuid"},{"line_number":233,"context_line":"        is_otu \u003d strutils.bool_from_string(dev.extra_info.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"17f55bf0_d0c0f7be","line":230,"in_reply_to":"39c5dd9a_ee65134a","updated":"2025-03-26 13:57:13.000000000","message":"We won\u0027t ever get here until we\u0027ve already set these things, so mypy is not correct, it just doesn\u0027t know how the code gets used (of course).\n\nI also don\u0027t see the point of just declaring the optional type (and further marking up the code with all that) just to make it happy when no extra safety is actually gained. But, I\u0027d rather do that than change what actually happens at runtime (i.e. change the initialization from None).","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"122e89d01869a72b65f593c518e5ff93618a1166","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # But, mypy needs a lot of convincing, so we\u0027ll put this in the runtime"},{"line_number":228,"context_line":"        # code just to make it happy :("},{"line_number":229,"context_line":"        dev: pci_device.PciDevice \u003d self.parent_dev"},{"line_number":230,"context_line":"        traits: set \u003d self.traits or set()  # This can never happen"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        is_allocated \u003d \u0027instance_uuid\u0027 in dev and dev.instance_uuid"},{"line_number":233,"context_line":"        is_otu \u003d strutils.bool_from_string(dev.extra_info.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"39c5dd9a_ee65134a","line":230,"in_reply_to":"a53d713d_5ac511db","updated":"2025-03-26 12:52:13.000000000","message":"i proposed one fix about but the other fix would be to declare the type here to be\n```suggestion\n        traits: ty.Optional[ty.Set[str]] \u003d self.traits\n```\n\nto match the type decalred in  __init__\nits actuly initalising it to None explcitly so it can happen if we cunstoct an instance of the class an dnever add traits to it. but you are proably correct that in practice we never would fall back to the set but it possible the way the class is defiend so mypy is rightfuly compaining based on how we have sepcfied the class","commit_id":"bb492e0da550f43309e48aa4399ba6c607fb1533"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"122e89d01869a72b65f593c518e5ff93618a1166","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        self.parent_dev \u003d None"},{"line_number":135,"context_line":"        self.children_devs: ty.List[pci_device.PciDevice] \u003d []"},{"line_number":136,"context_line":"        self.resource_class: ty.Optional[str] \u003d None"},{"line_number":137,"context_line":"        self.traits: ty.Optional[ty.Set[str]] \u003d None"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def devs(self) -\u003e ty.List[pci_device.PciDevice]:"}],"source_content_type":"text/x-python","patch_set":4,"id":"c8a72747_12d6a4e3","line":137,"updated":"2025-03-26 12:52:13.000000000","message":"@dan so you issue is because its declared as nullable here\n\nthere are a few ways to fix this.\n\ni have not tired it bue i belive this will make it happy \n\n\n```suggestion\n        self.traits: ty.Set[str] \u003d set()\n```","commit_id":"c8f7ee56dfbb644c4dbd9a8c81074d1ff9744001"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        self.parent_dev \u003d None"},{"line_number":135,"context_line":"        self.children_devs: ty.List[pci_device.PciDevice] \u003d []"},{"line_number":136,"context_line":"        self.resource_class: ty.Optional[str] \u003d None"},{"line_number":137,"context_line":"        self.traits: ty.Optional[ty.Set[str]] \u003d None"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def devs(self) -\u003e ty.List[pci_device.PciDevice]:"}],"source_content_type":"text/x-python","patch_set":4,"id":"25a2161a_3386de0e","line":137,"in_reply_to":"ad65a010_112c4900","updated":"2025-03-31 14:41:59.000000000","message":"Acknowledged","commit_id":"c8f7ee56dfbb644c4dbd9a8c81074d1ff9744001"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"62c785ea06382ad3c0ddc5e306327fc503b2419f","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        self.parent_dev \u003d None"},{"line_number":135,"context_line":"        self.children_devs: ty.List[pci_device.PciDevice] \u003d []"},{"line_number":136,"context_line":"        self.resource_class: ty.Optional[str] \u003d None"},{"line_number":137,"context_line":"        self.traits: ty.Optional[ty.Set[str]] \u003d None"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def devs(self) -\u003e ty.List[pci_device.PciDevice]:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ad65a010_112c4900","line":137,"in_reply_to":"c8a72747_12d6a4e3","updated":"2025-03-26 13:57:13.000000000","message":"Yeah I know this could be a solution, but I think it\u0027s really terrible to have to make the *runtime* code different in order to make the static checker happy. But then again, I\u0027m not in favor of any of this :)","commit_id":"c8f7ee56dfbb644c4dbd9a8c81074d1ff9744001"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        self.parent_dev \u003d dev"},{"line_number":192,"context_line":"        self.resource_class \u003d _get_rc_for_dev(dev, dev_spec_tags)"},{"line_number":193,"context_line":"        self.traits \u003d _get_traits_for_dev(dev_spec_tags)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def remove_child(self, dev: pci_device.PciDevice) -\u003e None:"},{"line_number":196,"context_line":"        # Nothing to do here. The update_provider_tree will handle the"}],"source_content_type":"text/x-python","patch_set":6,"id":"aa6f060c_a7c06d22","line":193,"updated":"2025-03-28 16:11:54.000000000","message":"as add_child \"handles\" OTU It would make sense to me if add_parent would do as well.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        self.parent_dev \u003d dev"},{"line_number":192,"context_line":"        self.resource_class \u003d _get_rc_for_dev(dev, dev_spec_tags)"},{"line_number":193,"context_line":"        self.traits \u003d _get_traits_for_dev(dev_spec_tags)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def remove_child(self, dev: pci_device.PciDevice) -\u003e None:"},{"line_number":196,"context_line":"        # Nothing to do here. The update_provider_tree will handle the"}],"source_content_type":"text/x-python","patch_set":6,"id":"09b8eeb8_f96e2a6f","line":193,"in_reply_to":"a84e2c95_5c7288be","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        self.parent_dev \u003d dev"},{"line_number":192,"context_line":"        self.resource_class \u003d _get_rc_for_dev(dev, dev_spec_tags)"},{"line_number":193,"context_line":"        self.traits \u003d _get_traits_for_dev(dev_spec_tags)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def remove_child(self, dev: pci_device.PciDevice) -\u003e None:"},{"line_number":196,"context_line":"        # Nothing to do here. The update_provider_tree will handle the"}],"source_content_type":"text/x-python","patch_set":6,"id":"a84e2c95_5c7288be","line":193,"in_reply_to":"aa6f060c_a7c06d22","updated":"2025-03-28 16:32:08.000000000","message":"You\u0027re asking to move the call to handle OTU here instead of in `update_provider_tree()`? AFAIK, we don\u0027t have a way to affect the inventory that is being built there without a lot more refactoring, since it was intended to be mostly static. Am I wrong? Or are you suggesting to do that refactoring?\n\nThe `add_child` path doesn\u0027t really \"handle\" it, it just sanity checks that we\u0027re not trying to mark one as OTU. That felt like the wrong place to do that, but I could also put that sanity check as an `else` on the `if self.parent_dev` down below for symmetry if you think it\u0027s important.\n\n(later)\n\nOh I see you *do* want that refactor. Alright.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":243,"context_line":"            if inventory.get(\u0027reserved\u0027, 0) \u003d\u003d 0:"},{"line_number":244,"context_line":"                LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":245,"context_line":"                         \u0027resource provider %s\u0027,"},{"line_number":246,"context_line":"                         len(self.devs), self.name)"},{"line_number":247,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if is_otu:"}],"source_content_type":"text/x-python","patch_set":6,"id":"6d1ae40a_2d6c3362","line":246,"updated":"2025-03-28 16:11:54.000000000","message":"I guess you wanted to limit the logging for the case when reserved moves from 0 to 1 and skip logging when it goes from 1 to 1. But the translator code always re-translate the input dev_specs and devs to RPs, inventories, and allocations from scratch. So the inventory.reserved will never be set before we hit this code.\n\nI think in this code there is no way to know if this is the first time we will ask placement to set reserved to 1 for a device because this code does not depend on the state of placement. Or in other words we never really update the inventory but we generate the expected inventory from scratch in each run.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":243,"context_line":"            if inventory.get(\u0027reserved\u0027, 0) \u003d\u003d 0:"},{"line_number":244,"context_line":"                LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":245,"context_line":"                         \u0027resource provider %s\u0027,"},{"line_number":246,"context_line":"                         len(self.devs), self.name)"},{"line_number":247,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if is_otu:"}],"source_content_type":"text/x-python","patch_set":6,"id":"d3b7767a_4c9e9b57","line":246,"in_reply_to":"6d1ae40a_2d6c3362","updated":"2025-03-28 16:32:08.000000000","message":"Yep, I\u0027ve probably not had a device allocated long enough to see this repeated in the logs for the periodic.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":243,"context_line":"            if inventory.get(\u0027reserved\u0027, 0) \u003d\u003d 0:"},{"line_number":244,"context_line":"                LOG.info(\u0027Setting reserved\u003d%i for one-time-use \u0027"},{"line_number":245,"context_line":"                         \u0027resource provider %s\u0027,"},{"line_number":246,"context_line":"                         len(self.devs), self.name)"},{"line_number":247,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if is_otu:"}],"source_content_type":"text/x-python","patch_set":6,"id":"04d39901_2c418591","line":246,"in_reply_to":"d3b7767a_4c9e9b57","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        if is_otu:"},{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"6f3c1d59_b46dda5f","line":252,"range":{"start_line":252,"start_character":24,"end_line":252,"end_character":43},"updated":"2025-03-28 16:11:54.000000000","message":"as this is a standard trait it should come from os_traits.HW_PCI_ONE_TIME_USE. For that you need to land the os-traits patch[1], then release os_traits, and then bump the new lib version into both the global upper-constraints, and nova\u0027s requirements.txt.\n\n[1]https://review.opendev.org/c/openstack/os-traits/+/944049","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e39f0f0968c7dbc2c28ae9e317684fd07e1e651a","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        if is_otu:"},{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"b4e40e9c_f5171150","line":252,"range":{"start_line":252,"start_character":24,"end_line":252,"end_character":43},"in_reply_to":"47339329_267f3dcf","updated":"2025-04-03 07:23:25.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":249,"context_line":"        if is_otu:"},{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"47339329_267f3dcf","line":252,"range":{"start_line":252,"start_character":24,"end_line":252,"end_character":43},"in_reply_to":"6f3c1d59_b46dda5f","updated":"2025-03-28 16:32:08.000000000","message":"Yep, leaving this here until then so it\u0027s usable.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def update_provider_tree("}],"source_content_type":"text/x-python","patch_set":6,"id":"5a6f68cb_7c1c91d5","line":253,"updated":"2025-03-28 16:11:54.000000000","message":"I think we don\u0027t need the discard codepath. The translator code always re-translates the dev_spec and dev input to RPs, inventories and allocations. It never re-uses what is already in placement. So the traits set will never have HW_PCI_ONE_TIME_USE except if the above if branch adds it in the *current* translation run.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def update_provider_tree("}],"source_content_type":"text/x-python","patch_set":6,"id":"bc807abc_fa41e9ce","line":253,"in_reply_to":"5a6f68cb_7c1c91d5","updated":"2025-03-28 16:32:08.000000000","message":"Ack, yep, this is probably just leftover from my initial pseudocode trying to make this very tight before I was familiar with how all this worked.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def update_provider_tree("}],"source_content_type":"text/x-python","patch_set":6,"id":"10b832a7_28588b7f","line":253,"in_reply_to":"bc807abc_fa41e9ce","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":246,"context_line":"                         len(self.devs), self.name)"},{"line_number":247,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if is_otu:"},{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def update_provider_tree("},{"line_number":257,"context_line":"        self,"},{"line_number":258,"context_line":"        provider_tree: provider_tree.ProviderTree,"}],"source_content_type":"text/x-python","patch_set":6,"id":"84bc9fa7_ae95c9f1","line":255,"range":{"start_line":249,"start_character":0,"end_line":255,"end_character":0},"updated":"2025-03-28 16:11:54.000000000","message":"It feels more logical to me to handle this with the rest of the traits around _get_traits_for_dev() but I guess you want to keep the code of this feature close together","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                         len(self.devs), self.name)"},{"line_number":247,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if is_otu:"},{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def update_provider_tree("},{"line_number":257,"context_line":"        self,"},{"line_number":258,"context_line":"        provider_tree: provider_tree.ProviderTree,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ae02facb_01d46173","line":255,"range":{"start_line":249,"start_character":0,"end_line":255,"end_character":0},"in_reply_to":"0f7a6a68_a31dc997","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":246,"context_line":"                         len(self.devs), self.name)"},{"line_number":247,"context_line":"            inventory[\u0027reserved\u0027] \u003d len(self.devs)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if is_otu:"},{"line_number":250,"context_line":"            # We always decorate OTU providers with a trait so they can be"},{"line_number":251,"context_line":"            # easily found"},{"line_number":252,"context_line":"            traits.add(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":253,"context_line":"        else:"},{"line_number":254,"context_line":"            traits.discard(\u0027HW_PCI_ONE_TIME_USE\u0027)"},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    def update_provider_tree("},{"line_number":257,"context_line":"        self,"},{"line_number":258,"context_line":"        provider_tree: provider_tree.ProviderTree,"}],"source_content_type":"text/x-python","patch_set":6,"id":"0f7a6a68_a31dc997","line":255,"range":{"start_line":249,"start_character":0,"end_line":255,"end_character":0},"in_reply_to":"84bc9fa7_ae95c9f1","updated":"2025-03-28 16:32:08.000000000","message":"I mean, yeah, I think having as much of it localized together is best. But, the traits part is not complex so if you want it moved I can move it.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":281,"context_line":"                uuid\u003duuidutils.generate_uuid(dashed\u003dTrue)"},{"line_number":282,"context_line":"            )"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        rp_uuid \u003d provider_tree.data(self.name).uuid"},{"line_number":285,"context_line":"        inventory \u003d {"},{"line_number":286,"context_line":"            \"total\": len(self.devs),"},{"line_number":287,"context_line":"            \"max_unit\": len(self.devs),"}],"source_content_type":"text/x-python","patch_set":6,"id":"c0543709_c5135571","line":284,"updated":"2025-03-28 16:11:54.000000000","message":"this does not need to move from its original place as far as I see.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                uuid\u003duuidutils.generate_uuid(dashed\u003dTrue)"},{"line_number":282,"context_line":"            )"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        rp_uuid \u003d provider_tree.data(self.name).uuid"},{"line_number":285,"context_line":"        inventory \u003d {"},{"line_number":286,"context_line":"            \"total\": len(self.devs),"},{"line_number":287,"context_line":"            \"max_unit\": len(self.devs),"}],"source_content_type":"text/x-python","patch_set":6,"id":"15a74d5b_e1458788","line":284,"in_reply_to":"07500427_16010419","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":281,"context_line":"                uuid\u003duuidutils.generate_uuid(dashed\u003dTrue)"},{"line_number":282,"context_line":"            )"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":"        rp_uuid \u003d provider_tree.data(self.name).uuid"},{"line_number":285,"context_line":"        inventory \u003d {"},{"line_number":286,"context_line":"            \"total\": len(self.devs),"},{"line_number":287,"context_line":"            \"max_unit\": len(self.devs),"}],"source_content_type":"text/x-python","patch_set":6,"id":"07500427_16010419","line":284,"in_reply_to":"c0543709_c5135571","updated":"2025-03-28 16:32:08.000000000","message":"Not anymore yep.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":299,"context_line":"            # allocated one by one. We may set the reserved value to a nonzero"},{"line_number":300,"context_line":"            # amount on the provider if the operator requests it via the"},{"line_number":301,"context_line":"            # one_time_use\u003dtrue flag, but otherwise the operator controls"},{"line_number":302,"context_line":"            # reserved and nova will not override that value periodically."},{"line_number":303,"context_line":"            {"},{"line_number":304,"context_line":"                self.resource_class: inventory"},{"line_number":305,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":6,"id":"510547b4_f0bb0efc","line":302,"updated":"2025-03-28 16:11:54.000000000","message":"this comment belongs more to the definition of `inventory` that is moved so I suggests to move the comment too","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            # allocated one by one. We may set the reserved value to a nonzero"},{"line_number":300,"context_line":"            # amount on the provider if the operator requests it via the"},{"line_number":301,"context_line":"            # one_time_use\u003dtrue flag, but otherwise the operator controls"},{"line_number":302,"context_line":"            # reserved and nova will not override that value periodically."},{"line_number":303,"context_line":"            {"},{"line_number":304,"context_line":"                self.resource_class: inventory"},{"line_number":305,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":6,"id":"80834256_7481edfd","line":302,"in_reply_to":"25071a6b_355ffa02","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8bcc40b6fffc3be9296a356202db4ad821e0a1d","unresolved":true,"context_lines":[{"line_number":299,"context_line":"            # allocated one by one. We may set the reserved value to a nonzero"},{"line_number":300,"context_line":"            # amount on the provider if the operator requests it via the"},{"line_number":301,"context_line":"            # one_time_use\u003dtrue flag, but otherwise the operator controls"},{"line_number":302,"context_line":"            # reserved and nova will not override that value periodically."},{"line_number":303,"context_line":"            {"},{"line_number":304,"context_line":"                self.resource_class: inventory"},{"line_number":305,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":6,"id":"25071a6b_355ffa02","line":302,"in_reply_to":"510547b4_f0bb0efc","updated":"2025-03-28 16:32:08.000000000","message":"Happy to do that. I really (really) do not like these comments in the middle of a function call on principle anyway :)","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"}],"nova/objects/pci_device.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"441a00266a5545d34bf2175f5fa33c4045f4dd62","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        # As in the above case, we must explicitly assign to self.extra_info"},{"line_number":193,"context_line":"        # so that obj_what_changed detects the modification and triggers"},{"line_number":194,"context_line":"        # a save later."},{"line_number":195,"context_line":"        tags_to_clean \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":196,"context_line":"        for tag in tags_to_clean:"},{"line_number":197,"context_line":"            if tag not in dev_dict and tag in self.extra_info:"},{"line_number":198,"context_line":"                extra_info \u003d self.extra_info"}],"source_content_type":"text/x-python","patch_set":9,"id":"2b6f5d96_6dd79efa","line":195,"updated":"2025-04-02 07:50:14.000000000","message":"you can drop the changes from this class as now nova does not copy the field to the PCIDevice ovo in the first place.","commit_id":"9b9103d900bb5151fbc74310a37aa0534f94f409"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4831997d4c01f3399124a9ef9a9c13aa548c7d74","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        # As in the above case, we must explicitly assign to self.extra_info"},{"line_number":193,"context_line":"        # so that obj_what_changed detects the modification and triggers"},{"line_number":194,"context_line":"        # a save later."},{"line_number":195,"context_line":"        tags_to_clean \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":196,"context_line":"        for tag in tags_to_clean:"},{"line_number":197,"context_line":"            if tag not in dev_dict and tag in self.extra_info:"},{"line_number":198,"context_line":"                extra_info \u003d self.extra_info"}],"source_content_type":"text/x-python","patch_set":9,"id":"bcbdc67e_ba5ed27a","line":195,"in_reply_to":"2b6f5d96_6dd79efa","updated":"2025-04-02 13:34:02.000000000","message":"Yep, sorry. I started this patch with you telling me to go copy the live-migratable stuff and so I was doing that sort of blindly before I got my legs under me. I just pushed up the removal and am running tests locally just in case this requires a test change that I don\u0027t remember.","commit_id":"9b9103d900bb5151fbc74310a37aa0534f94f409"}],"nova/pci/devspec.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fbc5a8b72051befb44dfe40e4792c6e2499e7415","unresolved":true,"context_lines":[{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        self._normalize_device_spec_tag(\"managed\")"},{"line_number":322,"context_line":"        self._normalize_device_spec_tag(\"live_migratable\")"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self._remote_managed:"},{"line_number":325,"context_line":"            if address_obj is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"d1e3ae03_635e24a1","line":322,"updated":"2025-03-10 15:11:21.000000000","message":"* You probably want to add the OTU flag here to do the early input validation of the config value. \n\n* Also you should try to check here if the OTU flag is set for a VF in the config and reject such config by raising an exception. The remote_managed code path below uses adress_obj.is_physical_function, so I would look that direction for ideas how to check it.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1fa3c2da5bc961ab887296613223262af62e98d4","unresolved":false,"context_lines":[{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        self._normalize_device_spec_tag(\"managed\")"},{"line_number":322,"context_line":"        self._normalize_device_spec_tag(\"live_migratable\")"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self._remote_managed:"},{"line_number":325,"context_line":"            if address_obj is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3e11d6f4_864c1e46","line":322,"in_reply_to":"9781523a_fde97f53","updated":"2025-03-11 18:48:22.000000000","message":"Done","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"19514bdf12726003f440e22801fffe6950352b3e","unresolved":true,"context_lines":[{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        self._normalize_device_spec_tag(\"managed\")"},{"line_number":322,"context_line":"        self._normalize_device_spec_tag(\"live_migratable\")"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self._remote_managed:"},{"line_number":325,"context_line":"            if address_obj is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"d5589ed2_94c9f96e","line":322,"in_reply_to":"d1e3ae03_635e24a1","updated":"2025-03-11 12:10:25.000000000","message":"One more thought on input validation. The OTU feature has a hard depedency on PCI in Placement. We cannot fully validate that if OTU flag is used in the device_spec then PCI in Placement is also needs to be enabled as that feature consists of two flag to enable on different levels:\n\n* [pci]report_in_placement : this is a compute level flag so we should be able to check it here and fail if OTU is configured but report_in_placement isn\u0027t. This place of the code base runs unconditionally but the pci_placement_translator implementing OTU only runs if report_in_placement is true.\n\n* [filter_scheduler]pci_in_placement : this is a nova-api level flag so we cannot check it here we can only document it that it is needed.\n\nIf report_in_placement is enabled but pci_in_placement is not (yet) enabled then OTU will work but slightly differently than if both flag is enabled. The pci_in_placement flag enables the PCI resource request generation into the allocation candidate query and the subsequent candidate filtering in the scheduler and eventually the PCI resource allocation in placement during scheduling. So if the pci_in_placement flag is not enabled that PCI allocation of VM in placement will be missing. BUT after the claim in the compute manager the allocation healing logic in the pci_placement_translator will create the missing PCI allocation in placement. So the OTU code can still function from the inventory reservation perspective. But an external tool looking for the PCI allocations in Placement will only see that allocation after the compute claim and not right after the scheduling decision like the rest of the allocation of the VM (e.g. VCPU).\n\nBottom line: lets check for report_in_placement here and document that pci_in_placement also needs to be enabled for OTU.","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"49851122e454a9a83a914dbc6ff9786a875ff472","unresolved":true,"context_lines":[{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        self._normalize_device_spec_tag(\"managed\")"},{"line_number":322,"context_line":"        self._normalize_device_spec_tag(\"live_migratable\")"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self._remote_managed:"},{"line_number":325,"context_line":"            if address_obj is None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9781523a_fde97f53","line":322,"in_reply_to":"d5589ed2_94c9f96e","updated":"2025-03-11 13:39:32.000000000","message":"Ack, makes sense, thanks!","commit_id":"55a499a3b8c70be0eb87a6a27a4f545bd51b4b97"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":427,"context_line":"                )"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):"},{"line_number":430,"context_line":"        spec_tags \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":431,"context_line":"        for tag in spec_tags:"},{"line_number":432,"context_line":"            tag_value \u003d self.tags.get(tag)"},{"line_number":433,"context_line":"            if tag_value is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"6e3d7d0b_eca752ef","line":430,"updated":"2025-03-28 16:11:54.000000000","message":"Now that I looked that the implementation I think we don\u0027t even need to copy this flag from the device_spec to the dev dict. The implementation has access to both the dev and the matching device_spec. So we can simplify","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"102e30b6d304575e8c9509ddfc1db0ae7ccb895b","unresolved":true,"context_lines":[{"line_number":427,"context_line":"                )"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):"},{"line_number":430,"context_line":"        spec_tags \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":431,"context_line":"        for tag in spec_tags:"},{"line_number":432,"context_line":"            tag_value \u003d self.tags.get(tag)"},{"line_number":433,"context_line":"            if tag_value is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"32e27ac0_3de1eb63","line":430,"in_reply_to":"04da0533_ca6d5e86","updated":"2025-04-01 14:51:05.000000000","message":"Sorry. meant to do this despite my complaint, so I will.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5cb505a9a5106b87f8621bbf02d456fc65692bbf","unresolved":true,"context_lines":[{"line_number":427,"context_line":"                )"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):"},{"line_number":430,"context_line":"        spec_tags \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":431,"context_line":"        for tag in spec_tags:"},{"line_number":432,"context_line":"            tag_value \u003d self.tags.get(tag)"},{"line_number":433,"context_line":"            if tag_value is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"04da0533_ca6d5e86","line":430,"in_reply_to":"0abab6d9_da5b9d27","updated":"2025-04-01 14:48:49.000000000","message":"Actually the two new tags, managed and live_migratable, are the exceptions. The other existing tags like physical_network or trusted are not copied to the dev dict either. So I think it is pretty save to not copy otu either. If later an additional otu related feature wants to use the PCIDevice ovo to decide if the device is OTU or not then we can add this copying.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae68d599e25b525b9b7c2b7dbae7af591805e253","unresolved":false,"context_lines":[{"line_number":427,"context_line":"                )"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):"},{"line_number":430,"context_line":"        spec_tags \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":431,"context_line":"        for tag in spec_tags:"},{"line_number":432,"context_line":"            tag_value \u003d self.tags.get(tag)"},{"line_number":433,"context_line":"            if tag_value is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"a35fc864_afa6399c","line":430,"in_reply_to":"32e27ac0_3de1eb63","updated":"2025-04-01 17:06:54.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":true,"context_lines":[{"line_number":427,"context_line":"                )"},{"line_number":428,"context_line":""},{"line_number":429,"context_line":"    def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):"},{"line_number":430,"context_line":"        spec_tags \u003d [\"managed\", \"live_migratable\", \"one_time_use\"]"},{"line_number":431,"context_line":"        for tag in spec_tags:"},{"line_number":432,"context_line":"            tag_value \u003d self.tags.get(tag)"},{"line_number":433,"context_line":"            if tag_value is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"0abab6d9_da5b9d27","line":430,"in_reply_to":"6e3d7d0b_eca752ef","updated":"2025-03-31 14:41:59.000000000","message":"Okay I haven\u0027t really traced out all this stuff, but I guess it seems like other implementations will expect these to show up in the device if they were placed in dev_spec. Meaning, I\u0027m not sure it\u0027s simpler or cleaner to not have these known things get copied.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"}],"nova/tests/unit/compute/test_pci_placement_translator.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":295,"context_line":"            dev_type\u003dfields.PciDeviceType.SRIOV_PF,"},{"line_number":296,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":297,"context_line":"            product_id\u003d\"beef\","},{"line_number":298,"context_line":"        )"},{"line_number":299,"context_line":"        vf \u003d pci_device.PciDevice("},{"line_number":300,"context_line":"            address\u003d\"0000:74:00.0\","},{"line_number":301,"context_line":"            parent_addr\u003d\"0000:75:00.0\","}],"source_content_type":"text/x-python","patch_set":6,"id":"257729da_5d6f06c8","line":298,"updated":"2025-03-28 16:11:54.000000000","message":"I would also test with an explicit false","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":false,"context_lines":[{"line_number":295,"context_line":"            dev_type\u003dfields.PciDeviceType.SRIOV_PF,"},{"line_number":296,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":297,"context_line":"            product_id\u003d\"beef\","},{"line_number":298,"context_line":"        )"},{"line_number":299,"context_line":"        vf \u003d pci_device.PciDevice("},{"line_number":300,"context_line":"            address\u003d\"0000:74:00.0\","},{"line_number":301,"context_line":"            parent_addr\u003d\"0000:75:00.0\","}],"source_content_type":"text/x-python","patch_set":6,"id":"126eabb3_8d472e83","line":298,"in_reply_to":"257729da_5d6f06c8","updated":"2025-03-31 14:41:59.000000000","message":"Done","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0babff512a47ad175aefca7ec8d37a558337a26b","unresolved":true,"context_lines":[{"line_number":303,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":304,"context_line":"            product_id\u003d\"beef\","},{"line_number":305,"context_line":"            extra_info\u003d{\u0027one_time_use\u0027: \u0027true\u0027},"},{"line_number":306,"context_line":"        )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":309,"context_line":"        pt.new_root(\"fake-node\", uuids.compute_rp)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a87d9bfe_b98e95b2","line":306,"updated":"2025-03-28 16:11:54.000000000","message":"I would also test with an explicit false","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"552f23e06e6d36b8e69cc81f25deae853e83ea14","unresolved":true,"context_lines":[{"line_number":303,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":304,"context_line":"            product_id\u003d\"beef\","},{"line_number":305,"context_line":"            extra_info\u003d{\u0027one_time_use\u0027: \u0027true\u0027},"},{"line_number":306,"context_line":"        )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":309,"context_line":"        pt.new_root(\"fake-node\", uuids.compute_rp)"}],"source_content_type":"text/x-python","patch_set":6,"id":"aaa3e8ef_50578e62","line":306,"in_reply_to":"a87d9bfe_b98e95b2","updated":"2025-03-31 14:41:59.000000000","message":"My code (which you didn\u0027t change) will raise if you specify `one_time_use` at all for a child, even if you set \u003dFalse. I can add a false case, but it\u0027ll still raise - not sure if that was your expectation or not.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5cb505a9a5106b87f8621bbf02d456fc65692bbf","unresolved":false,"context_lines":[{"line_number":303,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":304,"context_line":"            product_id\u003d\"beef\","},{"line_number":305,"context_line":"            extra_info\u003d{\u0027one_time_use\u0027: \u0027true\u0027},"},{"line_number":306,"context_line":"        )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        pt \u003d provider_tree.ProviderTree()"},{"line_number":309,"context_line":"        pt.new_root(\"fake-node\", uuids.compute_rp)"}],"source_content_type":"text/x-python","patch_set":6,"id":"310e4817_c630e83b","line":306,"in_reply_to":"aaa3e8ef_50578e62","updated":"2025-04-01 14:48:49.000000000","message":"yeah the expectation is clear, we don\u0027t support setting the flag with any value for a VF. Thanks for adding the false case below.","commit_id":"1da1526c7c2430b2ad47e5d27ada2b281a1adb71"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5efd469c21ef11b1e72d281db02d3f485075c9cb","unresolved":true,"context_lines":[{"line_number":309,"context_line":"            product_id\u003d\"beef\","},{"line_number":310,"context_line":"        )"},{"line_number":311,"context_line":"        vf2 \u003d pci_device.PciDevice("},{"line_number":312,"context_line":"            address\u003d\"0000:74:00.0\","},{"line_number":313,"context_line":"            parent_addr\u003d\"0000:76:00.0\","},{"line_number":314,"context_line":"            dev_type\u003dfields.PciDeviceType.SRIOV_VF,"},{"line_number":315,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":316,"context_line":"            product_id\u003d\"beef\","}],"source_content_type":"text/x-python","patch_set":12,"id":"4d20bb05_6116bff7","line":313,"range":{"start_line":312,"start_character":0,"end_line":313,"end_character":39},"updated":"2025-04-04 16:20:22.000000000","message":"you reveres this.\n\nVF2 has the same address as pf3\n\nthe way its used in the test does no tdepend on this but we probably should fix this.\n\ni could be a followup but if you need to respin this we should fix it here.","commit_id":"28a266461a1eadfe9c3b268123b14da7005ed165"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70dc0bdcb214501635fed8b40810dfc29c296a3d","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            product_id\u003d\"beef\","},{"line_number":310,"context_line":"        )"},{"line_number":311,"context_line":"        vf2 \u003d pci_device.PciDevice("},{"line_number":312,"context_line":"            address\u003d\"0000:74:00.0\","},{"line_number":313,"context_line":"            parent_addr\u003d\"0000:76:00.0\","},{"line_number":314,"context_line":"            dev_type\u003dfields.PciDeviceType.SRIOV_VF,"},{"line_number":315,"context_line":"            vendor_id\u003d\"dead\","},{"line_number":316,"context_line":"            product_id\u003d\"beef\","}],"source_content_type":"text/x-python","patch_set":12,"id":"2de022de_da976825","line":313,"range":{"start_line":312,"start_character":0,"end_line":313,"end_character":39},"in_reply_to":"4d20bb05_6116bff7","updated":"2025-04-04 16:30:37.000000000","message":"Done (follow up patch in a sec)","commit_id":"28a266461a1eadfe9c3b268123b14da7005ed165"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5efd469c21ef11b1e72d281db02d3f485075c9cb","unresolved":false,"context_lines":[{"line_number":391,"context_line":"        pf.instance_uuid \u003d None"},{"line_number":392,"context_line":"        pv.update_provider_tree(pt)"},{"line_number":393,"context_line":"        assert_inventory(71, 0)"},{"line_number":394,"context_line":"        assert_inventory(72, 1)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    def test_update_provider_tree_for_pci_update_pools(self):"},{"line_number":397,"context_line":"        pt \u003d provider_tree.ProviderTree()"}],"source_content_type":"text/x-python","patch_set":12,"id":"d4a7e423_f61c0089","line":394,"updated":"2025-04-04 16:20:22.000000000","message":"+1","commit_id":"28a266461a1eadfe9c3b268123b14da7005ed165"}]}