)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":true,"context_lines":[{"line_number":9,"context_line":"The stats module uses the _find_pool() call to find a matching pool for"},{"line_number":10,"context_line":"a new device or a device that is being deallocated. If no existing pool"},{"line_number":11,"context_line":"matches with the dev then then a new pool is created for the dev. The"},{"line_number":12,"context_line":"pool matching logic was faulty as it did not removed all the metadata"},{"line_number":13,"context_line":"keys from the pool like rp_uuid. So if the dev did not have that key but"},{"line_number":14,"context_line":"the pool had then the dev did not match."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"4e165dd7_06894a83","line":12,"range":{"start_line":12,"start_character":45,"end_line":12,"end_character":52},"updated":"2025-03-14 19:10:20.000000000","message":"\"remove\"","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The stats module uses the _find_pool() call to find a matching pool for"},{"line_number":10,"context_line":"a new device or a device that is being deallocated. If no existing pool"},{"line_number":11,"context_line":"matches with the dev then then a new pool is created for the dev. The"},{"line_number":12,"context_line":"pool matching logic was faulty as it did not removed all the metadata"},{"line_number":13,"context_line":"keys from the pool like rp_uuid. So if the dev did not have that key but"},{"line_number":14,"context_line":"the pool had then the dev did not match."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"82610e51_3aaa40c1","line":12,"range":{"start_line":12,"start_character":45,"end_line":12,"end_character":52},"in_reply_to":"4e165dd7_06894a83","updated":"2025-03-17 15:40:26.000000000","message":"Acknowledged","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":true,"context_lines":[{"line_number":11,"context_line":"matches with the dev then then a new pool is created for the dev. The"},{"line_number":12,"context_line":"pool matching logic was faulty as it did not removed all the metadata"},{"line_number":13,"context_line":"keys from the pool like rp_uuid. So if the dev did not have that key but"},{"line_number":14,"context_line":"the pool had then the dev did not match."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In the other hand the PCI allocation logic (when PCI in Placement is"},{"line_number":17,"context_line":"enabled) assumed that devices from a single rp_uuid are always in a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"51c125f6_e5472853","line":14,"range":{"start_line":14,"start_character":12,"end_line":14,"end_character":13},"updated":"2025-03-14 19:10:20.000000000","message":"\"did\" would sound more natural","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[{"line_number":11,"context_line":"matches with the dev then then a new pool is created for the dev. The"},{"line_number":12,"context_line":"pool matching logic was faulty as it did not removed all the metadata"},{"line_number":13,"context_line":"keys from the pool like rp_uuid. So if the dev did not have that key but"},{"line_number":14,"context_line":"the pool had then the dev did not match."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In the other hand the PCI allocation logic (when PCI in Placement is"},{"line_number":17,"context_line":"enabled) assumed that devices from a single rp_uuid are always in a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"81eece13_23c5a9d3","line":14,"range":{"start_line":14,"start_character":12,"end_line":14,"end_character":13},"in_reply_to":"51c125f6_e5472853","updated":"2025-03-17 15:40:26.000000000","message":"Acknowledged","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":true,"context_lines":[{"line_number":13,"context_line":"keys from the pool like rp_uuid. So if the dev did not have that key but"},{"line_number":14,"context_line":"the pool had then the dev did not match."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In the other hand the PCI allocation logic (when PCI in Placement is"},{"line_number":17,"context_line":"enabled) assumed that devices from a single rp_uuid are always in a"},{"line_number":18,"context_line":"single pool. As this assumption was broken by the above bug the PCI"},{"line_number":19,"context_line":"allocation blindly tried to allocate resources for an rp_uuid from each"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"d3fe0647_0a502363","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":2},"updated":"2025-03-14 19:10:20.000000000","message":"\"On\"","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[{"line_number":13,"context_line":"keys from the pool like rp_uuid. So if the dev did not have that key but"},{"line_number":14,"context_line":"the pool had then the dev did not match."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In the other hand the PCI allocation logic (when PCI in Placement is"},{"line_number":17,"context_line":"enabled) assumed that devices from a single rp_uuid are always in a"},{"line_number":18,"context_line":"single pool. As this assumption was broken by the above bug the PCI"},{"line_number":19,"context_line":"allocation blindly tried to allocate resources for an rp_uuid from each"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"e21afe93_ba87d637","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":2},"in_reply_to":"d3fe0647_0a502363","updated":"2025-03-17 15:40:26.000000000","message":"Acknowledged","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":true,"context_lines":[{"line_number":20,"context_line":"matching pool causing overallocation."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The main fix in this patch is to ignore the metadata tags in"},{"line_number":23,"context_line":"_find_pool(). But also two safety net is added to the allocation logic."},{"line_number":24,"context_line":"The logic no asserts that the assumption is correct and if not as it"},{"line_number":25,"context_line":"founds multiple pools with the same rp_uuid, then it bails out. And also"},{"line_number":26,"context_line":"it does not blindly ever allocate the same rp_uuid request from multiple"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fe4a48a7_b9dd875c","line":23,"range":{"start_line":23,"start_character":34,"end_line":23,"end_character":40},"updated":"2025-03-14 19:10:20.000000000","message":"\"nets are\"","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[{"line_number":20,"context_line":"matching pool causing overallocation."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The main fix in this patch is to ignore the metadata tags in"},{"line_number":23,"context_line":"_find_pool(). But also two safety net is added to the allocation logic."},{"line_number":24,"context_line":"The logic no asserts that the assumption is correct and if not as it"},{"line_number":25,"context_line":"founds multiple pools with the same rp_uuid, then it bails out. And also"},{"line_number":26,"context_line":"it does not blindly ever allocate the same rp_uuid request from multiple"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"715fb2e6_8687ce8b","line":23,"range":{"start_line":23,"start_character":34,"end_line":23,"end_character":40},"in_reply_to":"fe4a48a7_b9dd875c","updated":"2025-03-17 15:40:26.000000000","message":"Acknowledged","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The main fix in this patch is to ignore the metadata tags in"},{"line_number":23,"context_line":"_find_pool(). But also two safety net is added to the allocation logic."},{"line_number":24,"context_line":"The logic no asserts that the assumption is correct and if not as it"},{"line_number":25,"context_line":"founds multiple pools with the same rp_uuid, then it bails out. And also"},{"line_number":26,"context_line":"it does not blindly ever allocate the same rp_uuid request from multiple"},{"line_number":27,"context_line":"pools."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"8fede552_d12e07a9","line":24,"range":{"start_line":24,"start_character":10,"end_line":24,"end_character":12},"updated":"2025-03-14 19:10:20.000000000","message":"\"now\"","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"The main fix in this patch is to ignore the metadata tags in"},{"line_number":23,"context_line":"_find_pool(). But also two safety net is added to the allocation logic."},{"line_number":24,"context_line":"The logic no asserts that the assumption is correct and if not as it"},{"line_number":25,"context_line":"founds multiple pools with the same rp_uuid, then it bails out. And also"},{"line_number":26,"context_line":"it does not blindly ever allocate the same rp_uuid request from multiple"},{"line_number":27,"context_line":"pools."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"8bd5c823_73380b08","line":24,"range":{"start_line":24,"start_character":10,"end_line":24,"end_character":12},"in_reply_to":"8fede552_d12e07a9","updated":"2025-03-17 15:40:26.000000000","message":"Acknowledged","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":true,"context_lines":[{"line_number":22,"context_line":"The main fix in this patch is to ignore the metadata tags in"},{"line_number":23,"context_line":"_find_pool(). But also two safety net is added to the allocation logic."},{"line_number":24,"context_line":"The logic no asserts that the assumption is correct and if not as it"},{"line_number":25,"context_line":"founds multiple pools with the same rp_uuid, then it bails out. And also"},{"line_number":26,"context_line":"it does not blindly ever allocate the same rp_uuid request from multiple"},{"line_number":27,"context_line":"pools."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7c15ff10_fbf73be8","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":6},"updated":"2025-03-14 19:10:20.000000000","message":"\"found\"","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[{"line_number":22,"context_line":"The main fix in this patch is to ignore the metadata tags in"},{"line_number":23,"context_line":"_find_pool(). But also two safety net is added to the allocation logic."},{"line_number":24,"context_line":"The logic no asserts that the assumption is correct and if not as it"},{"line_number":25,"context_line":"founds multiple pools with the same rp_uuid, then it bails out. And also"},{"line_number":26,"context_line":"it does not blindly ever allocate the same rp_uuid request from multiple"},{"line_number":27,"context_line":"pools."},{"line_number":28,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"00d4cc8a_3ab6124e","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":6},"in_reply_to":"7c15ff10_fbf73be8","updated":"2025-03-17 15:40:26.000000000","message":"Acknowledged","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a7723c16290eeb426882753380b09e773e63933","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"413cef4b_c0b2850c","updated":"2025-03-14 14:11:51.000000000","message":"Oh I better drop this to +1 because the commit message is still missing, but.. trade-able for a +2 :)","commit_id":"bccaf8d3bb50fc7728563ca2eaf4059e3bac682c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6d89684dd98c0fd4ef9044e2683eb195c2b577f5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e9deb109_52e2fe3a","updated":"2025-03-14 14:05:25.000000000","message":"Okay I get it now and I pulled down the patch to play with the test to help make sure.\n\nI do think that combining the two larger-than-the-actual-fix safety checks would be a benefit, both from a DRY perspective but also understanding which of these is the fix and which are safety checks in a year :) But +2 on this or that.","commit_id":"bccaf8d3bb50fc7728563ca2eaf4059e3bac682c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aef3c0f46b49f9908565dbaf5deccaa66836ac77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ba579904_5ef37130","updated":"2025-03-14 09:23:17.000000000","message":"thanks Dan for the review","commit_id":"bccaf8d3bb50fc7728563ca2eaf4059e3bac682c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2220bd2d48583a63dd5a2a0eb42763d09c2ecf39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a7a0fb13_c3b0ca18","in_reply_to":"413cef4b_c0b2850c","updated":"2025-03-14 15:02:20.000000000","message":"yeah I want to do couple of cleanup before I can mark this ready","commit_id":"bccaf8d3bb50fc7728563ca2eaf4059e3bac682c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6ca4ed0a_8017b10d","updated":"2025-03-14 19:10:20.000000000","message":"I picked one grammar nit and a bunch fell out. I\u0027ll update this to wordsmith the commit message with my native tongue (fingers) so we can move it along.","commit_id":"d17d9e8f2f4759312b83cc20e8175742d699bf16"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1c6a09d380038b6111a9a9e696853ffef222a161","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7b1b941d_3bca72bd","updated":"2025-03-17 14:31:50.000000000","message":"Failure is in nova-next in a secgroup test that I\u0027m sure is unrelated. I won\u0027t recheck since gibi is going to update to fix the TODOs anyway and we\u0027ll get a clean run then.","commit_id":"34fd1a8ba31cf128d73254043dd694946cd88d3c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e8971be39452597b385c79f9afd3afaad5638a2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9e8b00fb_0a569ca0","updated":"2025-03-14 19:14:35.000000000","message":"I guess we can\u0027t quite move forward with it because of your TODOs, but at least the edits are here for you. But, the fix looks good.","commit_id":"34fd1a8ba31cf128d73254043dd694946cd88d3c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bd5e319d0a02147f49a3485193864eb270d1672a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"4dbaf87c_fe278db4","updated":"2025-03-17 15:40:26.000000000","message":"thanks for the grammar fixes","commit_id":"34fd1a8ba31cf128d73254043dd694946cd88d3c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"2438d7a7_4dc34d35","updated":"2025-03-18 16:17:04.000000000","message":"there is perhasp one more check we could add but over all i think this is ok as is.\n\ncomments inline but nothign blocking.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e34567738d816d605d400ab87175c41b84945cb9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"baf4ba55_e2788c30","updated":"2025-03-19 17:34:27.000000000","message":"this just adresses a nit otherwise its the saem as v7 so looks fine to me","commit_id":"229fb3513ae39fa120167107afe42568ca85ace9"}],"nova/pci/stats.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c4d326a6d01517e855e8612ee86c633f4d6ed4","unresolved":true,"context_lines":[{"line_number":313,"context_line":"                # but if there is placement allocation then we have to follow"},{"line_number":314,"context_line":"                # it"},{"line_number":315,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":316,"context_line":"                for pool in pools:"},{"line_number":317,"context_line":"                    count \u003d requested_devs_per_pool_rp[pool[\u0027rp_uuid\u0027]]"},{"line_number":318,"context_line":"                    # We assume that devices reported to an rp_uuid are always"},{"line_number":319,"context_line":"                    # in the same pool. So we can always consume the whole"}],"source_content_type":"text/x-python","patch_set":1,"id":"07a3a8ab_935f749f","line":316,"updated":"2025-03-13 17:48:14.000000000","message":"So the original problem is that there could be the same `rp_uuid` in multiple pools, right? So we process one pool, find the `rp_uuid` we need, allocate, and then proceed to the next pool, where we find it again, and allocate again?\n\nIsn\u0027t it the case (based on your comment/error below) that one provider should only ever be in one pool? If so, isn\u0027t the problem in the pooling itself and not really here? This sort of feels like a hack to avoid the double-allocation problem instead of fixing the actual problem of why the same provider is in multiple pools. Am I reading that wrong?\n\nlater: er, wait. Maybe the change on L107 is actually fixing the pooling and you\u0027re just doing belt-and-suspenders extra checks here?","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9f7d3cff6714ad2caff2c72369652a5b0fcea61","unresolved":true,"context_lines":[{"line_number":313,"context_line":"                # but if there is placement allocation then we have to follow"},{"line_number":314,"context_line":"                # it"},{"line_number":315,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":316,"context_line":"                for pool in pools:"},{"line_number":317,"context_line":"                    count \u003d requested_devs_per_pool_rp[pool[\u0027rp_uuid\u0027]]"},{"line_number":318,"context_line":"                    # We assume that devices reported to an rp_uuid are always"},{"line_number":319,"context_line":"                    # in the same pool. So we can always consume the whole"}],"source_content_type":"text/x-python","patch_set":1,"id":"492b12cb_514fc7d7","line":316,"in_reply_to":"07a3a8ab_935f749f","updated":"2025-03-13 18:47:15.000000000","message":"\u003e later: er, wait. Maybe the change on L107 is actually fixing the pooling and you\u0027re just doing belt-and-suspenders extra checks here?\n\nEr, no, that\u0027s `_find_pool()` not `_filter_pools()` so.. yeah my original question remains.","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"2220bd2d48583a63dd5a2a0eb42763d09c2ecf39","unresolved":false,"context_lines":[{"line_number":313,"context_line":"                # but if there is placement allocation then we have to follow"},{"line_number":314,"context_line":"                # it"},{"line_number":315,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":316,"context_line":"                for pool in pools:"},{"line_number":317,"context_line":"                    count \u003d requested_devs_per_pool_rp[pool[\u0027rp_uuid\u0027]]"},{"line_number":318,"context_line":"                    # We assume that devices reported to an rp_uuid are always"},{"line_number":319,"context_line":"                    # in the same pool. So we can always consume the whole"}],"source_content_type":"text/x-python","patch_set":1,"id":"6d22ba72_7d0f32e9","line":316,"in_reply_to":"11c84a1f_b7712ed9","updated":"2025-03-14 15:02:20.000000000","message":"\u003e I think this would be more obvious which of these changes is the fix and which are \"extra safety net checking\" if you combined the checks into an obviously-named helper and called them from the two places (here and below).\n\nexactly my thinking... I will do it.","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aef3c0f46b49f9908565dbaf5deccaa66836ac77","unresolved":true,"context_lines":[{"line_number":313,"context_line":"                # but if there is placement allocation then we have to follow"},{"line_number":314,"context_line":"                # it"},{"line_number":315,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":316,"context_line":"                for pool in pools:"},{"line_number":317,"context_line":"                    count \u003d requested_devs_per_pool_rp[pool[\u0027rp_uuid\u0027]]"},{"line_number":318,"context_line":"                    # We assume that devices reported to an rp_uuid are always"},{"line_number":319,"context_line":"                    # in the same pool. So we can always consume the whole"}],"source_content_type":"text/x-python","patch_set":1,"id":"c812ac4d_af180005","line":316,"in_reply_to":"492b12cb_514fc7d7","updated":"2025-03-14 09:23:17.000000000","message":"\u003e So the original problem is that there could be the same rp_uuid in multiple pools, right? \n\nRight, the code had an assumption that one rp_uuid appears in one pool only and therefore that pool has the capacity (ensured by the pool filtering logic) to fulfill the request from that rp_uuid.\n\nThe bug in _find_pool was that even though it only wanted to compare pools by a subset of keys it was only a subset of keys in one side of the comparision and the key set size check caused inequality due to extra keys.\n\n\u003e Isn\u0027t it the case (based on your comment/error below) that one provider should only ever be in one pool? If so, isn\u0027t the problem in the pooling itself and not really here? This sort of feels like a hack to avoid the double-allocation problem instead of fixing the actual problem of why the same provider is in multiple pools. Am I reading that wrong?\n\n\u003e later: er, wait. Maybe the change on L107 is actually fixing the pooling and you\u0027re just doing belt-and-suspenders extra checks here?\n\nYou are right. The real problem is the splitting of the devices from the same rp_uuid across multiple pools. This is fixed in _find_pool(). _find_pool() is used to map devices to pools and to only create a new pool if the device does not match to an existing pool. So the bug in _find_pool() that a device did not match an existing pool caused new pool to be created.\n\nThe change in consume and _apply is a safety net, and as such debatable. If the assumption about devices form the same rp_uuid does not hold then the safety net will help a) still doing the right thing most of the time b) failing with an error log that will help troubleshooting the problem.\n\nI can reformulate the safety net checks to actually asserting the assumption directly instead of trying to work around the break or asserting a consequence of the broken assumption. That might help the reader of this code.","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6d89684dd98c0fd4ef9044e2683eb195c2b577f5","unresolved":false,"context_lines":[{"line_number":313,"context_line":"                # but if there is placement allocation then we have to follow"},{"line_number":314,"context_line":"                # it"},{"line_number":315,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":316,"context_line":"                for pool in pools:"},{"line_number":317,"context_line":"                    count \u003d requested_devs_per_pool_rp[pool[\u0027rp_uuid\u0027]]"},{"line_number":318,"context_line":"                    # We assume that devices reported to an rp_uuid are always"},{"line_number":319,"context_line":"                    # in the same pool. So we can always consume the whole"}],"source_content_type":"text/x-python","patch_set":1,"id":"11c84a1f_b7712ed9","line":316,"in_reply_to":"c812ac4d_af180005","updated":"2025-03-14 14:05:25.000000000","message":"okay I get it now.. `_find_pool()` is called from `add_device()`, which gets called in a lot of places, but at least from the \"user is done with this device\" deallocation path. If it doesn\u0027t find a pool, it allocates one, so the `_find_pool()` problem is how that happens. It seems to me that creating pools at runtime during deallocation is a bad design here (as is maybe removing devices from a pool when they\u0027re consumed and returning them/creating a new pool when they\u0027re released). But that\u0027s unrelated to the problem being fixed.\n\nI think this would be more obvious which of these changes is the fix and which are \"extra safety net checking\" if you combined the checks into an obviously-named helper and called them from the two places (here and below).","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6c4d326a6d01517e855e8612ee86c633f4d6ed4","unresolved":true,"context_lines":[{"line_number":967,"context_line":""},{"line_number":968,"context_line":"            # TODO(gibi): this is baaad but spec is a dict of string so"},{"line_number":969,"context_line":"            #  the list is serialized"},{"line_number":970,"context_line":"            return rp_uuids.split(\u0027,\u0027)"},{"line_number":971,"context_line":""},{"line_number":972,"context_line":"        # NOTE(gibi): the PCI prefilter generates RequestGroup suffixes from"},{"line_number":973,"context_line":"        # InstancePCIRequests in the form of {request_id}-{count_index}"}],"source_content_type":"text/x-python","patch_set":1,"id":"4f8ee909_ccdb006f","line":970,"updated":"2025-03-13 17:48:14.000000000","message":"It\u0027s already an unversioned dict of strings, but also encoded as a list. Ooof :(","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aef3c0f46b49f9908565dbaf5deccaa66836ac77","unresolved":true,"context_lines":[{"line_number":967,"context_line":""},{"line_number":968,"context_line":"            # TODO(gibi): this is baaad but spec is a dict of string so"},{"line_number":969,"context_line":"            #  the list is serialized"},{"line_number":970,"context_line":"            return rp_uuids.split(\u0027,\u0027)"},{"line_number":971,"context_line":""},{"line_number":972,"context_line":"        # NOTE(gibi): the PCI prefilter generates RequestGroup suffixes from"},{"line_number":973,"context_line":"        # InstancePCIRequests in the form of {request_id}-{count_index}"}],"source_content_type":"text/x-python","patch_set":1,"id":"b00900a1_af20fd4e","line":970,"in_reply_to":"4f8ee909_ccdb006f","updated":"2025-03-14 09:23:17.000000000","message":"yes I know. This was a late finding during the implementation and we opted into doing this instead of blowing up the object model. (no excuse just context)","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5d68590eee05460c6d3b805ec86cdb0419157763","unresolved":false,"context_lines":[{"line_number":967,"context_line":""},{"line_number":968,"context_line":"            # TODO(gibi): this is baaad but spec is a dict of string so"},{"line_number":969,"context_line":"            #  the list is serialized"},{"line_number":970,"context_line":"            return rp_uuids.split(\u0027,\u0027)"},{"line_number":971,"context_line":""},{"line_number":972,"context_line":"        # NOTE(gibi): the PCI prefilter generates RequestGroup suffixes from"},{"line_number":973,"context_line":"        # InstancePCIRequests in the form of {request_id}-{count_index}"}],"source_content_type":"text/x-python","patch_set":1,"id":"902406b0_1d60c73d","line":970,"in_reply_to":"b00900a1_af20fd4e","updated":"2025-03-14 19:10:20.000000000","message":"Acknowledged","commit_id":"f05fa8ede156c1e63d2e0f02efe516f9c687b2bb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":105,"context_line":"            del pool_keys[\u0027count\u0027]"},{"line_number":106,"context_line":"            del pool_keys[\u0027devices\u0027]"},{"line_number":107,"context_line":"            for tag in self.ignored_pool_tags:"},{"line_number":108,"context_line":"                pool_keys.pop(tag, None)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"            if (len(pool_keys.keys()) \u003d\u003d len(dev_pool.keys()) and"},{"line_number":111,"context_line":"                self._equal_properties(dev_pool, pool_keys, list(dev_pool))):"}],"source_content_type":"text/x-python","patch_set":7,"id":"42ef4f82_5a6b1c8a","line":108,"updated":"2025-03-18 16:17:04.000000000","message":"so this is the main part of the fix right?\neverything else is just belt extra hardening.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            del pool_keys[\u0027count\u0027]"},{"line_number":106,"context_line":"            del pool_keys[\u0027devices\u0027]"},{"line_number":107,"context_line":"            for tag in self.ignored_pool_tags:"},{"line_number":108,"context_line":"                pool_keys.pop(tag, None)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"            if (len(pool_keys.keys()) \u003d\u003d len(dev_pool.keys()) and"},{"line_number":111,"context_line":"                self._equal_properties(dev_pool, pool_keys, list(dev_pool))):"}],"source_content_type":"text/x-python","patch_set":7,"id":"a7c31e42_da4e7351","line":108,"in_reply_to":"42ef4f82_5a6b1c8a","updated":"2025-03-19 11:23:01.000000000","message":"yes.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":315,"context_line":""},{"line_number":316,"context_line":"                if not self._assert_one_pool_per_rp_uuid(pools):"},{"line_number":317,"context_line":"                    raise exception.PciDeviceRequestFailed("},{"line_number":318,"context_line":"                        requests\u003dpci_requests)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":321,"context_line":"                for pool in pools:"}],"source_content_type":"text/x-python","patch_set":7,"id":"e8920bcd_51ee4d49","line":318,"updated":"2025-03-18 16:17:04.000000000","message":"this is a nice safty check but in normal operation this should never fire.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":false,"context_lines":[{"line_number":315,"context_line":""},{"line_number":316,"context_line":"                if not self._assert_one_pool_per_rp_uuid(pools):"},{"line_number":317,"context_line":"                    raise exception.PciDeviceRequestFailed("},{"line_number":318,"context_line":"                        requests\u003dpci_requests)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"                requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":321,"context_line":"                for pool in pools:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5623af94_05538e0a","line":318,"in_reply_to":"e8920bcd_51ee4d49","updated":"2025-03-19 11:23:01.000000000","message":"you are correct. It is asserting an assumption the code makes about the relationship between the rp_uuids and the pools. By the _find_pool bugfix above this should not fire. But when it fires due to a regression in the future it will provide i) enough log to figure out what happened ii) stops the allocation to avoid inconsistent allocation to be created.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":325,"context_line":"                        pool, count, request.request_id)"},{"line_number":326,"context_line":"                    # we consumed all the requested devices for the rp_uuid"},{"line_number":327,"context_line":"                    # so we can drop that rp_uuid from the request."},{"line_number":328,"context_line":"                    requested_devs_per_pool_rp.pop(pool[\u0027rp_uuid\u0027], None)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        return alloc_devices"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"a9db355a_82b345f3","line":328,"updated":"2025-03-18 16:17:04.000000000","message":"i need to look at this more closely but taking the comment on face value i guess thats ok.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":false,"context_lines":[{"line_number":325,"context_line":"                        pool, count, request.request_id)"},{"line_number":326,"context_line":"                    # we consumed all the requested devices for the rp_uuid"},{"line_number":327,"context_line":"                    # so we can drop that rp_uuid from the request."},{"line_number":328,"context_line":"                    requested_devs_per_pool_rp.pop(pool[\u0027rp_uuid\u0027], None)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        return alloc_devices"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"a5005dfe_96284555","line":328,"in_reply_to":"a9db355a_82b345f3","updated":"2025-03-19 11:23:01.000000000","message":"It is the last resort check to prevent double allocations as happened in the original fix. I probably overcompensating here, but it is at least cheap to do.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":326,"context_line":"                    # we consumed all the requested devices for the rp_uuid"},{"line_number":327,"context_line":"                    # so we can drop that rp_uuid from the request."},{"line_number":328,"context_line":"                    requested_devs_per_pool_rp.pop(pool[\u0027rp_uuid\u0027], None)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        return alloc_devices"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    def _handle_device_dependents(self, pci_dev: \u0027objects.PciDevice\u0027) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5dc9b45b_d3a863c0","line":329,"updated":"2025-03-18 16:17:04.000000000","message":"same comment as line 889","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                    # we consumed all the requested devices for the rp_uuid"},{"line_number":327,"context_line":"                    # so we can drop that rp_uuid from the request."},{"line_number":328,"context_line":"                    requested_devs_per_pool_rp.pop(pool[\u0027rp_uuid\u0027], None)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        return alloc_devices"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    def _handle_device_dependents(self, pci_dev: \u0027objects.PciDevice\u0027) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"58a8b0da_e3ac440e","line":329,"in_reply_to":"5dc9b45b_d3a863c0","updated":"2025-03-19 11:23:01.000000000","message":"Acknowledged","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":878,"context_line":"            # but if there is placement allocation then we have to follow that"},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"            if not self._assert_one_pool_per_rp_uuid(pools):"},{"line_number":881,"context_line":"                return False"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":884,"context_line":"            for pool in filtered_pools:"}],"source_content_type":"text/x-python","patch_set":7,"id":"b059c169_2c77e244","line":881,"updated":"2025-03-18 16:17:04.000000000","message":"so in consume (on the compute manager) we raise an exception to presumably fail the isntance claim\n\nbut in the schduler, we return false to signal we could not apply the requsts and that the host should not pass.\n\nmakes sense.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":false,"context_lines":[{"line_number":878,"context_line":"            # but if there is placement allocation then we have to follow that"},{"line_number":879,"context_line":""},{"line_number":880,"context_line":"            if not self._assert_one_pool_per_rp_uuid(pools):"},{"line_number":881,"context_line":"                return False"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            requested_devs_per_pool_rp \u003d collections.Counter(rp_uuids)"},{"line_number":884,"context_line":"            for pool in filtered_pools:"}],"source_content_type":"text/x-python","patch_set":7,"id":"8d551880_1481a3fb","line":881,"in_reply_to":"b059c169_2c77e244","updated":"2025-03-19 11:23:01.000000000","message":"yeah it has a different contract, but eventually both case results in failed allocation and a LOG about why the assumption about the pools and rp_uuids are broken.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":886,"context_line":"                pool[\u0027count\u0027] -\u003d count"},{"line_number":887,"context_line":"                # we consumed all the requested devices for the rp_uuid"},{"line_number":888,"context_line":"                # so we can drop that rp_uuid from the request."},{"line_number":889,"context_line":"                requested_devs_per_pool_rp.pop(pool[\u0027rp_uuid\u0027], None)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"                if pool[\u0027count\u0027] \u003d\u003d 0:"},{"line_number":892,"context_line":"                    pools.remove(pool)"}],"source_content_type":"text/x-python","patch_set":7,"id":"961f84a6_f205584d","line":889,"updated":"2025-03-18 16:17:04.000000000","message":"as we discussed on irc ^ is not strictly needed because we are not returning this in any way so we could drop this on the floor at hte end of the cucntion in one go.\n\nby having this we could add a check on line 895 to ensure that \nrequested_devs_per_pool_rp is now empty to ensure we have processed each request.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":false,"context_lines":[{"line_number":886,"context_line":"                pool[\u0027count\u0027] -\u003d count"},{"line_number":887,"context_line":"                # we consumed all the requested devices for the rp_uuid"},{"line_number":888,"context_line":"                # so we can drop that rp_uuid from the request."},{"line_number":889,"context_line":"                requested_devs_per_pool_rp.pop(pool[\u0027rp_uuid\u0027], None)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"                if pool[\u0027count\u0027] \u003d\u003d 0:"},{"line_number":892,"context_line":"                    pools.remove(pool)"}],"source_content_type":"text/x-python","patch_set":7,"id":"f489369e_84e9546d","line":889,"in_reply_to":"961f84a6_f205584d","updated":"2025-03-19 11:23:01.000000000","message":"Acknowledged","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"}],"nova/tests/unit/pci/test_stats.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":true,"context_lines":[{"line_number":1616,"context_line":"        self.pci_stats \u003d stats.PciDeviceStats("},{"line_number":1617,"context_line":"            objects.NUMATopology(), dev_filter\u003dself.dev_filter"},{"line_number":1618,"context_line":"        )"},{"line_number":1619,"context_line":"        for pf_index in [1]:"},{"line_number":1620,"context_line":"            for vf_index in [1, 2]:"},{"line_number":1621,"context_line":"                dev \u003d objects.PciDevice("},{"line_number":1622,"context_line":"                    compute_node_id\u003d1,"}],"source_content_type":"text/x-python","patch_set":7,"id":"31ea6f85_ad1d19ba","line":1619,"range":{"start_line":1619,"start_character":6,"end_line":1619,"end_character":28},"updated":"2025-03-18 16:17:04.000000000","message":"this does not need to be a for loop it can just be \n\npf_index \u003d 1 and we can dedent the loop by one level.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6f7ff8b21ba0e860d49acc80bd1ce68b0a2d93b1","unresolved":true,"context_lines":[{"line_number":1616,"context_line":"        self.pci_stats \u003d stats.PciDeviceStats("},{"line_number":1617,"context_line":"            objects.NUMATopology(), dev_filter\u003dself.dev_filter"},{"line_number":1618,"context_line":"        )"},{"line_number":1619,"context_line":"        for pf_index in [1]:"},{"line_number":1620,"context_line":"            for vf_index in [1, 2]:"},{"line_number":1621,"context_line":"                dev \u003d objects.PciDevice("},{"line_number":1622,"context_line":"                    compute_node_id\u003d1,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fc4008c2_a12846ce","line":1619,"range":{"start_line":1619,"start_character":6,"end_line":1619,"end_character":28},"in_reply_to":"31ea6f85_ad1d19ba","updated":"2025-03-19 11:23:01.000000000","message":"true, I can will fix this.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"551c90dfad32e5ac2e068a2bc08b8199baf3ca3e","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"        self.pci_stats \u003d stats.PciDeviceStats("},{"line_number":1617,"context_line":"            objects.NUMATopology(), dev_filter\u003dself.dev_filter"},{"line_number":1618,"context_line":"        )"},{"line_number":1619,"context_line":"        for pf_index in [1]:"},{"line_number":1620,"context_line":"            for vf_index in [1, 2]:"},{"line_number":1621,"context_line":"                dev \u003d objects.PciDevice("},{"line_number":1622,"context_line":"                    compute_node_id\u003d1,"}],"source_content_type":"text/x-python","patch_set":7,"id":"a6311747_86246018","line":1619,"range":{"start_line":1619,"start_character":6,"end_line":1619,"end_character":28},"in_reply_to":"fc4008c2_a12846ce","updated":"2025-03-19 17:26:35.000000000","message":"Done","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":false,"context_lines":[{"line_number":1634,"context_line":"        # NOTE(gibi): this simulates an invalid situation where the same"},{"line_number":1635,"context_line":"        # VFs from the same PF are split across multiple pools. This should"},{"line_number":1636,"context_line":"        # not happen, but we want to check that the code rejects it"},{"line_number":1637,"context_line":"        self.pci_stats.pools.extend(copy.deepcopy(self.pci_stats.pools))"},{"line_number":1638,"context_line":""},{"line_number":1639,"context_line":"        vf_req \u003d objects.InstancePCIRequest("},{"line_number":1640,"context_line":"            count\u003d1,"}],"source_content_type":"text/x-python","patch_set":7,"id":"f7c2e541_1f0d5b1e","line":1637,"updated":"2025-03-18 16:17:04.000000000","message":"ok ya we just duplicate the pools.\nthat works","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":false,"context_lines":[{"line_number":1662,"context_line":"            \u0027same rp_uuid are in the same pool. However the following \u0027"},{"line_number":1663,"context_line":"            \u0027rp_uuids are split across multiple pools. This should not \u0027"},{"line_number":1664,"context_line":"            \u0027happen. Please file a bug report. %s\u0027,"},{"line_number":1665,"context_line":"            {uuids.pf1: self.pci_stats.pools})"},{"line_number":1666,"context_line":""},{"line_number":1667,"context_line":"    def test_consume_split_pool_rejected(self):"},{"line_number":1668,"context_line":"        self._test_split_pool_rejected("}],"source_content_type":"text/x-python","patch_set":7,"id":"ad3fc0ba_912a093b","line":1665,"updated":"2025-03-18 16:17:04.000000000","message":"+1","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":false,"context_lines":[{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"    def test_apply_split_pool_rejected(self):"},{"line_number":1672,"context_line":"        self._test_split_pool_rejected("},{"line_number":1673,"context_line":"            lambda reqs: self.pci_stats.apply_requests(reqs, {}))"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"    @ddt.data("},{"line_number":1676,"context_line":"        # no pool no problem"}],"source_content_type":"text/x-python","patch_set":7,"id":"2bb30235_b58d63c8","line":1673,"updated":"2025-03-18 16:17:04.000000000","message":"yep makes sense to reuse it like this.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"109984b496d75c6331c7c8b97c8f98c2952059f2","unresolved":false,"context_lines":[{"line_number":1684,"context_line":"        # two pools with the same rp_uuid is NOT OK"},{"line_number":1685,"context_line":"        [[{}, {\"rp_uuid\": uuids.pf1}, {\"rp_uuid\": uuids.pf2},"},{"line_number":1686,"context_line":"          {\"rp_uuid\": uuids.pf1}], False]"},{"line_number":1687,"context_line":"    )"},{"line_number":1688,"context_line":"    @ddt.unpack"},{"line_number":1689,"context_line":"    def test_assert_one_pool_per_rp_uuid(self, pools, result):"},{"line_number":1690,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":7,"id":"f9176eee_c5485e69","line":1687,"updated":"2025-03-18 16:17:04.000000000","message":"thanks for the comments it makes ddt simpler to read.","commit_id":"8d83b0617a64c4e6ce64d6e31f731817c1cbce0b"}]}