)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1f4c7c0c_f30abf3e","updated":"2025-01-14 13:04:29.000000000","message":"I have couple suggestion inline to make the change into clearer.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0fd2cd06194417cb256d7f1bf10bd7f91a71d133","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e3a7d225_f39562bc","updated":"2025-01-14 13:18:52.000000000","message":"one more thing about what we persist in the DB","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1daf1537052455aabe479849b87c1faee691f427","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4a46489a_ca70777d","updated":"2025-02-07 09:23:52.000000000","message":"We have to resolve the discussion around the pci/manager change.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f2f0ce74e7982a2916851deebc2e08dc2576e7f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7bdc25e1_67b1922c","updated":"2025-02-04 17:46:09.000000000","message":"recheck ssh timeout (ssh paramiko.ssh_exception.NoValidConnectionsError: [Errno None] Unable to connect to port 22 on 172.24.5.23)","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f989ae7e895ff0bd9d2e3247789bdaca5c8a014","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8ca0bb20_1050db10","updated":"2025-02-04 20:09:20.000000000","message":"this is incompelte by the way\n\nyou have not update the PciDeviceStats._filter_pools function filter based on the manged flag. you need add a new filter function like this.\n\n_filter_pools_for_unrequested_remote_managed_devices\nhttps://github.com/openstack/nova/blob/master/nova/pci/stats.py#L550-L569\n\nthen invoke that from _filter_pools\n\nhere https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L734\n\nso that the pci_passhtough and numa filter will schduler to a host that supprots this properly.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"71a854184036c38dd12b97cb01f80f62b2adfd26","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"abb75a8a_6e128512","in_reply_to":"0aaea932_98b1083f","updated":"2025-02-07 10:18:09.000000000","message":"Regarding the live migration, we are checking that all devices are live_migratable\u003d\u0027yes\u0027, if not we don\u0027t allow to live migrate.\n\nI will try to submit a \"clean\" patch early next week.\n\nHowever, if you wish you can have a look at the poc one https://github.com/uggla/nova/tree/poc","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd79e3bcd20dbc6a78979f10b8dcbe912e1b3f73","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d0eb6946_60f3e768","in_reply_to":"14c1870f_69b4c458","updated":"2025-02-05 15:12:33.000000000","message":"I don\u0027t think we need to schedule instances for this managed value, we just want to know whether the PCI device that has a managed value so we can modify the XML.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aa1e625a631830ab1fe2d4ca63092c21a8fc47fd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0aaea932_98b1083f","in_reply_to":"1b9bc50c_d785080f","updated":"2025-02-06 17:52:18.000000000","message":"since this is more for the live migratoin series i guess we can defer that until we get to that.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"c997c9d260d578d2406baf06a0f494b5b7b976db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"140e25e9_1b0b9272","in_reply_to":"6ca1ac6d_13124ba1","updated":"2025-02-12 13:30:57.000000000","message":"Done","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1cdf05597d026f0175862aae72330d1ec0c71900","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"afb74cc6_5fdcb57b","in_reply_to":"8ca0bb20_1050db10","updated":"2025-02-04 20:09:53.000000000","message":"sorry didnt mean to mark this as resolved","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6ca1ac6d_13124ba1","in_reply_to":"abb75a8a_6e128512","updated":"2025-02-12 13:30:29.000000000","message":"Set it as resolved because we agreed in our last meeting.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"515fb975d8257ebac9b5f53b79b85f60636688dd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"14c1870f_69b4c458","in_reply_to":"afb74cc6_5fdcb57b","updated":"2025-02-05 14:00:27.000000000","message":"Yes you are right ! 💪\nI\u0027m going to add the filter to select the appropriate node correctly.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"772796152061a5d3c2d398902e05ed6d7576f7e3","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fab14248_772b4422","in_reply_to":"d0eb6946_60f3e768","updated":"2025-02-06 16:21:47.000000000","message":"I wrote my last comment a bit too quickly and got confused with the live_migratable parameter.\n\nAccording to the spec, we defined the managed parameter only as part of device_specs.\nIn my view, this is an inherent characteristic of the device required for it to function properly, rather than something that should be requested or scheduled.\n\nTherefore, the user must define devices in device_specs with the appropriate managed parameter.\nIf PCI in placement is not used, this means that all devices across all compute nodes, identified by the same vendor ID and product ID pair, must have the same type.\n\nIf the user needs more flexibility and wants to manage multiple device types, they must enable PCI in placement.\n\nThis is what has been defined in the spec so far. If we want to change this behavior, we need to discuss it urgently, although I believe the current proposal is reasonable.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a1365575a184662e39ba3821da47d59e425f8170","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1b9bc50c_d785080f","in_reply_to":"fab14248_772b4422","updated":"2025-02-06 17:43:28.000000000","message":"as far as i am aware you canot change the managed flag when live migrating.\n\nso in this initall series you are corect its not striclty need\nhowever it will be need for the live migration work\n\nfor cold migration whether it\u0027s managed or not can vary as both domain xml are independent.\n\nif we can modify the managed flag on live migration then the filter is not required.\n\ncan you reach out to the vert folks and ask them to confirm it it would be accepted or not?","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9f3e32dd_3f81e884","updated":"2025-02-12 13:30:29.000000000","message":"I have marked as resolved the comments we agreed on, keeping only the remaining ones for clarification.","commit_id":"8adb0a9d9309f42860c65539b0c128b224fe93fc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"096fa64d4ff341b043d434d5975af7182216f7c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d9f1cb21_977fac10","updated":"2025-02-13 14:06:10.000000000","message":"I still have some requests for refactoring and a request for an extra test case.","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1243f846a74fb34bb8134fa2a48ebf750abb5fd8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7ab352a9_6f018cb0","updated":"2025-02-13 13:45:20.000000000","message":"recheck nova-next timeouth","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0db06a50e13228b044a2c9bc4928217b789ba86d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d9ff7f68_dfb924b3","updated":"2025-02-14 08:07:02.000000000","message":"One more cleanup and I\u0027m +2","commit_id":"18bb8debd845d6449a09f05e5c850d6f1a1e897c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"44652f67a5a4dbacca6d61d939848f9e9d210953","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a5c81318_05f52a84","updated":"2025-02-14 08:59:31.000000000","message":"+2 but I\u0027d still ideally want two other cores to review it too.","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"09ea0a0f325a36c3fa91ac5cc6c0f5107b02ec04","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"22ced93f_0d44ad16","updated":"2025-02-17 15:05:48.000000000","message":"I observed an additional issue with the current patch during local testing. Assume the following scenario:\n* I have a dev without managed field in the dev_spec\n* After I upgraded to the new patch I decided to set managed: false in the dev_spec\n* Then I realized I made a mistake and set false to the wrong dev_spec so I revert my config change. However the managed: false value in the PCIDevice in the nova DB is not removed.\n\nThis is because the current code is only ever additive on the object layer and never removes the managed flag when it is not defined in the config any more.\n\n@sbauza@redhat.com, @smooney@redhat.com we need to agree what should be the behavior here.\n* a) keep the current only additive behavior and document that if you ever set managed: false then the only way to go back to the managed behavior is to explicitly set managed:true (instead of just removing managed:false)\n* b) we change the patch that if the dev_dict has no managed flag then we remove that from the PCIDevice as well.","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1ec44e41c9b40c555914ff72681e95e235018e6f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ec2529da_36df7b46","updated":"2025-02-14 08:59:29.000000000","message":"Looks good.","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4b2bad9b74cee0eb7a624ebc5fdb3c00754f730a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0735a94a_8cce4b15","updated":"2025-02-18 15:28:43.000000000","message":"we discussed on the bug and on the solution, agreed with the direction","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ec650c436df57fe0a7fa8c77f5d2fbce9f638e5a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4c45cb07_bc1d31f5","in_reply_to":"12578db7_a3111cd0","updated":"2025-02-18 15:18:26.000000000","message":"agreed to fix the bug and remove the managed tag form the DB when it is not set in the config","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a98cf0d9519ca1c9c4d277111444ff5f87028d0f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"12578db7_a3111cd0","in_reply_to":"22ced93f_0d44ad16","updated":"2025-02-17 18:17:17.000000000","message":"ya so that is a bug it should be removed unless explicitly set\n\nso we shoud go with b\ni would consider the additive-only behavior to be a bug.\n\nis also mentioned this in one of our reviews or calls on this topic before.\n\nthe same applies to live-migratbale or whatever we called it for the second series","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ee09af437605783f5b0d112c007019aa2ee2dcdb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"7c8ff06e_a665e069","in_reply_to":"4c45cb07_bc1d31f5","updated":"2025-02-19 13:24:58.000000000","message":"Done","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b9bcf4372c33e5e4335aed59aa31c199d3fb5ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"4069da5a_bce92bc5","updated":"2025-02-20 15:59:32.000000000","message":"I need to run some local reconfiguration tests before I can +2 this.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b2fdb20026b106477f37180c4a4357ae39bf9e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b29572a8_c89368a8","updated":"2025-02-24 10:25:59.000000000","message":"The managed flag removal bug still exists.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7089b000005e7d8f45c9dc8e564672cf287c11ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"25042476_c5105617","updated":"2025-02-20 15:41:09.000000000","message":"recheck volume attach timeout","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd4e68ece159f2cb2df5957901d8912a3fb9ca24","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"07bcbcea_0986d6c1","in_reply_to":"850e14b3_e961cc5a","updated":"2025-02-25 16:02:56.000000000","message":"just captureing what i said on irc\n\nwe should be adding \"managed\" to the set of ignored_spec_tags and ignored_pool_tags\nhere https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L71\n\nto ensure its not commited into the pci_stats column in the nova compute nodes table and is not added to pci pools\n\nthis is because its neither not used for scheduling.\n\nif you respin this can you add it to the ignored set if not we can adress this isn a follow up.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"9007a1c5145fdc4dffd2a10715338631600b8361","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"850e14b3_e961cc5a","in_reply_to":"b29572a8_c89368a8","updated":"2025-02-25 15:03:33.000000000","message":"Thanks Gibi, this seems to be fixed now.\nhttps://paste.opendev.org/show/ba4BTb1bDKjCU052R3UP","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e5f6f16037643bfd4a910e0efddec9c4bd77e2a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"ed580e5d_6b201133","updated":"2025-02-25 16:21:14.000000000","message":"This looks good to me. The pool ignored_flags can be handled in a follow up","commit_id":"12905dade31fa5c6692026a9b9264b8059dabb13"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"049703563fbfd2d34ac703136c4fa20287b15ccb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3beedcf1_ba5363e8","updated":"2025-02-25 16:04:37.000000000","message":"just for the record the PCIPool object is cleaned too when the managed flag is removed. https://paste.opendev.org/show/bEWhDvmH5NPNozp8V4pU/","commit_id":"12905dade31fa5c6692026a9b9264b8059dabb13"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"11633de550f147fe199769eb5780b6742bc2acd6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"9ca70a8c_ea063c6c","updated":"2025-02-25 17:24:11.000000000","message":"recheck kernel panic \n\n---[ end Kernel panic - not syncing: Attempted to kill init! exitcode\u003d0x00001000 ]---\n\nlets hold +w until we are +2 on the second patch buti think this is ok.\n\ni have not had time to test this personally.\n\ni might try and do that quickly but based on the past bin check for updating the device spec and restartign the agent i think that is being done correclty.\n\n\ni was hoping ot see funtional tests that verifed that the correct content was updated in the db but that can be done in a follow up patch if other agree.\n\nthe unit tests dont relaly cover this end to end but perhaps the second patch adds that coverage?","commit_id":"12905dade31fa5c6692026a9b9264b8059dabb13"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"11d5af39167d7272830798ebcafa4e82ba0d7649","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"0368e0b7_ffdd7016","updated":"2025-02-27 09:25:40.000000000","message":"recheck multicell job timeout","commit_id":"12905dade31fa5c6692026a9b9264b8059dabb13"}],"nova/objects/pci_device.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":172,"context_line":"                # NOTE(yjiang5): extra_info.update does not update"},{"line_number":173,"context_line":"                # obj_what_changed, set it explicitly"},{"line_number":174,"context_line":"                # NOTE(ralonsoh): list of parameters currently added to"},{"line_number":175,"context_line":"                # \"extra_info\" dict:"},{"line_number":176,"context_line":"                #     - \"capabilities\": dict of (strings/list of strings)"},{"line_number":177,"context_line":"                #     - \"parent_ifname\": the netdev name of the parent (PF)"},{"line_number":178,"context_line":"                #        device of a VF"},{"line_number":179,"context_line":"                #     - \"mac_address\": the MAC address of the PF"},{"line_number":180,"context_line":"                extra_info \u003d self.extra_info"},{"line_number":181,"context_line":"                data \u003d v if isinstance(v, str) else jsonutils.dumps(v)"},{"line_number":182,"context_line":"                extra_info.update({k: data})"}],"source_content_type":"text/x-python","patch_set":2,"id":"25c80a3e_449d457b","line":179,"range":{"start_line":175,"start_character":0,"end_line":179,"end_character":64},"updated":"2025-01-14 13:04:29.000000000","message":"we probably want to update this comment with the new field","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"84818b58f8af4a82f43065df484a1208a9a2c594","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                # NOTE(yjiang5): extra_info.update does not update"},{"line_number":173,"context_line":"                # obj_what_changed, set it explicitly"},{"line_number":174,"context_line":"                # NOTE(ralonsoh): list of parameters currently added to"},{"line_number":175,"context_line":"                # \"extra_info\" dict:"},{"line_number":176,"context_line":"                #     - \"capabilities\": dict of (strings/list of strings)"},{"line_number":177,"context_line":"                #     - \"parent_ifname\": the netdev name of the parent (PF)"},{"line_number":178,"context_line":"                #        device of a VF"},{"line_number":179,"context_line":"                #     - \"mac_address\": the MAC address of the PF"},{"line_number":180,"context_line":"                extra_info \u003d self.extra_info"},{"line_number":181,"context_line":"                data \u003d v if isinstance(v, str) else jsonutils.dumps(v)"},{"line_number":182,"context_line":"                extra_info.update({k: data})"}],"source_content_type":"text/x-python","patch_set":2,"id":"1160049e_8012a8f6","line":179,"range":{"start_line":175,"start_character":0,"end_line":179,"end_character":64},"in_reply_to":"25c80a3e_449d457b","updated":"2025-02-03 14:45:41.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":178,"context_line":"                #        device of a VF"},{"line_number":179,"context_line":"                #     - \"mac_address\": the MAC address of the PF"},{"line_number":180,"context_line":"                extra_info \u003d self.extra_info"},{"line_number":181,"context_line":"                data \u003d v if isinstance(v, str) else jsonutils.dumps(v)"},{"line_number":182,"context_line":"                extra_info.update({k: data})"},{"line_number":183,"context_line":"                self.extra_info \u003d extra_info"},{"line_number":184,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e5816a50_2ab68549","line":181,"updated":"2025-01-14 13:04:29.000000000","message":"this seems to handle boolean so \"managed\": true should be OK technically but the boolean type is erased by the json dump as the PCIDevice.extra_info is a DictOfString field in the object. So probably does not make sense to try to use boolean directly.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"84818b58f8af4a82f43065df484a1208a9a2c594","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                #        device of a VF"},{"line_number":179,"context_line":"                #     - \"mac_address\": the MAC address of the PF"},{"line_number":180,"context_line":"                extra_info \u003d self.extra_info"},{"line_number":181,"context_line":"                data \u003d v if isinstance(v, str) else jsonutils.dumps(v)"},{"line_number":182,"context_line":"                extra_info.update({k: data})"},{"line_number":183,"context_line":"                self.extra_info \u003d extra_info"},{"line_number":184,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e518d7b4_9fa62979","line":181,"in_reply_to":"e5816a50_2ab68549","updated":"2025-02-03 14:45:41.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        # so make sure we replicate that behavior outside of init here"},{"line_number":305,"context_line":"        # for compatibility reasons."},{"line_number":306,"context_line":"        pci_device.obj_set_defaults(\u0027extra_info\u0027)"},{"line_number":307,"context_line":"        pci_device.update_device(dev_dict)"},{"line_number":308,"context_line":"        pci_device.status \u003d fields.PciDeviceStatus.AVAILABLE"},{"line_number":309,"context_line":"        pci_device.uuid \u003d uuidutils.generate_uuid()"},{"line_number":310,"context_line":"        pci_device._context \u003d context"}],"source_content_type":"text/x-python","patch_set":2,"id":"2c691602_5d33207b","line":307,"updated":"2025-01-14 13:04:29.000000000","message":"this is where the dev dict form the pci/manager ends up.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aac56b1cac5005f2b652ff8e4b9207b12c1fcf68","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        # so make sure we replicate that behavior outside of init here"},{"line_number":305,"context_line":"        # for compatibility reasons."},{"line_number":306,"context_line":"        pci_device.obj_set_defaults(\u0027extra_info\u0027)"},{"line_number":307,"context_line":"        pci_device.update_device(dev_dict)"},{"line_number":308,"context_line":"        pci_device.status \u003d fields.PciDeviceStatus.AVAILABLE"},{"line_number":309,"context_line":"        pci_device.uuid \u003d uuidutils.generate_uuid()"},{"line_number":310,"context_line":"        pci_device._context \u003d context"}],"source_content_type":"text/x-python","patch_set":2,"id":"849f27d0_7723ed07","line":307,"in_reply_to":"2c691602_5d33207b","updated":"2025-02-04 19:03:04.000000000","message":"yes the virt dirvers produce a driver neutal dict that is then consumed by driver indempent code via the object/db interface.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        # so make sure we replicate that behavior outside of init here"},{"line_number":305,"context_line":"        # for compatibility reasons."},{"line_number":306,"context_line":"        pci_device.obj_set_defaults(\u0027extra_info\u0027)"},{"line_number":307,"context_line":"        pci_device.update_device(dev_dict)"},{"line_number":308,"context_line":"        pci_device.status \u003d fields.PciDeviceStatus.AVAILABLE"},{"line_number":309,"context_line":"        pci_device.uuid \u003d uuidutils.generate_uuid()"},{"line_number":310,"context_line":"        pci_device._context \u003d context"}],"source_content_type":"text/x-python","patch_set":2,"id":"087fdf65_d028a8b3","line":307,"in_reply_to":"849f27d0_7723ed07","updated":"2025-02-12 13:30:29.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b2fdb20026b106477f37180c4a4357ae39bf9e7","unresolved":true,"context_lines":[{"line_number":183,"context_line":"                data \u003d v if isinstance(v, str) else jsonutils.dumps(v)"},{"line_number":184,"context_line":"                extra_info.update({k: data})"},{"line_number":185,"context_line":"                self.extra_info \u003d extra_info"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":188,"context_line":"        super(PciDevice, self).__init__(*args, **kwargs)"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"e2f64aa9_c4a6823c","line":186,"updated":"2025-02-24 10:25:59.000000000","message":"I suggest to remove the \"managed\" key from the extra_info here if it is not in the dev_dict.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4408baa01b9653c7298e5296c83b709c20860a36","unresolved":false,"context_lines":[{"line_number":183,"context_line":"                data \u003d v if isinstance(v, str) else jsonutils.dumps(v)"},{"line_number":184,"context_line":"                extra_info.update({k: data})"},{"line_number":185,"context_line":"                self.extra_info \u003d extra_info"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":188,"context_line":"        super(PciDevice, self).__init__(*args, **kwargs)"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"4b00b509_2b219727","line":186,"in_reply_to":"e2f64aa9_c4a6823c","updated":"2025-02-25 09:09:35.000000000","message":"Done","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"}],"nova/pci/devspec.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aac56b1cac5005f2b652ff8e4b9207b12c1fcf68","unresolved":true,"context_lines":[{"line_number":293,"context_line":"        self.vendor_id \u003d self.tags.pop(\"vendor_id\", ANY)"},{"line_number":294,"context_line":"        self.product_id \u003d self.tags.pop(\"product_id\", ANY)"},{"line_number":295,"context_line":"        self.dev_name \u003d self.tags.pop(\"devname\", None)"},{"line_number":296,"context_line":"        # We need to keep managed in the tags and not pop it because it"},{"line_number":297,"context_line":"        # is used later in the manager."},{"line_number":298,"context_line":"        self.managed \u003d self.tags.get(\"managed\", None)"},{"line_number":299,"context_line":"        self.address: ty.Optional[WhitelistPciAddress] \u003d None"},{"line_number":300,"context_line":"        # Note(moshele): The address attribute can be a string or a dict."},{"line_number":301,"context_line":"        # For glob syntax or specific pci it is a string and for regex syntax"}],"source_content_type":"text/x-python","patch_set":3,"id":"bd6a7418_ea5784ec","line":298,"range":{"start_line":296,"start_character":8,"end_line":298,"end_character":53},"updated":"2025-02-04 19:03:04.000000000","message":"i dont think this is correct.\n\nvendor_id, product_id, dev_name, address are all critira on which we filter the host devices.\n\nmanage is not so you shoudl not be adding ti as a new filed here.\n\nmanaged like physical_network is a tag and shoudl not be extracted as a public filed  part of this function","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"3374ed825416b5ae90e70482e13866c4632a6684","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        self.vendor_id \u003d self.tags.pop(\"vendor_id\", ANY)"},{"line_number":294,"context_line":"        self.product_id \u003d self.tags.pop(\"product_id\", ANY)"},{"line_number":295,"context_line":"        self.dev_name \u003d self.tags.pop(\"devname\", None)"},{"line_number":296,"context_line":"        # We need to keep managed in the tags and not pop it because it"},{"line_number":297,"context_line":"        # is used later in the manager."},{"line_number":298,"context_line":"        self.managed \u003d self.tags.get(\"managed\", None)"},{"line_number":299,"context_line":"        self.address: ty.Optional[WhitelistPciAddress] \u003d None"},{"line_number":300,"context_line":"        # Note(moshele): The address attribute can be a string or a dict."},{"line_number":301,"context_line":"        # For glob syntax or specific pci it is a string and for regex syntax"}],"source_content_type":"text/x-python","patch_set":3,"id":"860846df_e4e1049f","line":298,"range":{"start_line":296,"start_character":8,"end_line":298,"end_character":53},"in_reply_to":"082c3bde_105584b0","updated":"2025-02-10 15:09:23.000000000","message":"Done","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd79e3bcd20dbc6a78979f10b8dcbe912e1b3f73","unresolved":true,"context_lines":[{"line_number":293,"context_line":"        self.vendor_id \u003d self.tags.pop(\"vendor_id\", ANY)"},{"line_number":294,"context_line":"        self.product_id \u003d self.tags.pop(\"product_id\", ANY)"},{"line_number":295,"context_line":"        self.dev_name \u003d self.tags.pop(\"devname\", None)"},{"line_number":296,"context_line":"        # We need to keep managed in the tags and not pop it because it"},{"line_number":297,"context_line":"        # is used later in the manager."},{"line_number":298,"context_line":"        self.managed \u003d self.tags.get(\"managed\", None)"},{"line_number":299,"context_line":"        self.address: ty.Optional[WhitelistPciAddress] \u003d None"},{"line_number":300,"context_line":"        # Note(moshele): The address attribute can be a string or a dict."},{"line_number":301,"context_line":"        # For glob syntax or specific pci it is a string and for regex syntax"}],"source_content_type":"text/x-python","patch_set":3,"id":"082c3bde_105584b0","line":298,"range":{"start_line":296,"start_character":8,"end_line":298,"end_character":53},"in_reply_to":"bd6a7418_ea5784ec","updated":"2025-02-05 15:12:33.000000000","message":"if you don\u0027t want it public, fair, we can make it private \n\n``self._managed \u003d self.tags.get(\"managed\", None)``","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aac56b1cac5005f2b652ff8e4b9207b12c1fcf68","unresolved":true,"context_lines":[{"line_number":318,"context_line":"        # for an instance to access the control plane at the remote side (e.g."},{"line_number":319,"context_line":"        # on a DPU) for managing the PF representor corresponding to the PF."},{"line_number":320,"context_line":"        address_obj \u003d self._address_obj()"},{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("}],"source_content_type":"text/x-python","patch_set":3,"id":"6ebde9c7_544db576","line":322,"range":{"start_line":321,"start_character":7,"end_line":322,"end_character":50},"updated":"2025-02-04 19:03:04.000000000","message":"you could encode it as a private field like _remote_managed.\n\nbut it should not be accessed directly on the devsec outside fo this module.\n\nif we want a convicne funciton to make working with this simipler in other code we\ncan add a property on teh pci_device object.\nhttps://github.com/openstack/nova/blob/master/nova/objects/pci_device.py#L566-L570\n\nor on the PciDevicePool class.\nhttps://github.com/openstack/nova/blob/master/nova/objects/pci_device_pool.py#L27\n\nwhich is what is stored in the pci_stats column of the comptue nodes and used for schdulign.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        # for an instance to access the control plane at the remote side (e.g."},{"line_number":319,"context_line":"        # on a DPU) for managing the PF representor corresponding to the PF."},{"line_number":320,"context_line":"        address_obj \u003d self._address_obj()"},{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("}],"source_content_type":"text/x-python","patch_set":3,"id":"a87c476d_123ebbdd","line":322,"range":{"start_line":321,"start_character":7,"end_line":322,"end_character":50},"in_reply_to":"6ebde9c7_544db576","updated":"2025-02-12 13:30:29.000000000","message":"Done","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aac56b1cac5005f2b652ff8e4b9207b12c1fcf68","unresolved":true,"context_lines":[{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("},{"line_number":326,"context_line":"                reason\u003d_(\"Invalid value for %s\" % self.managed))"},{"line_number":327,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"4a015256_e9f64906","line":324,"range":{"start_line":324,"start_character":28,"end_line":324,"end_character":61},"updated":"2025-02-04 19:03:04.000000000","message":"we shoudl be consitent and use strutils.bool_from_string to also support True/False and Yes or NO ectra.\n\nwe generally document using true/false or yes/no but accept a wider set of boolean values.\n\nif fine with just documenting \u0027yes\u0027 and \u0027no\u0027 but we shoudl not reject it if its \u0027YES\u0027\n\nwe do the exact same thing for flavour extra specs and image properties.\nwe accpet any reasonable truthy value suing strutils.bool_from_string to normalise the value.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":true,"context_lines":[{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("},{"line_number":326,"context_line":"                reason\u003d_(\"Invalid value for %s\" % self.managed))"},{"line_number":327,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"8826d55b_9218f477","line":324,"range":{"start_line":324,"start_character":28,"end_line":324,"end_character":61},"in_reply_to":"3db94a85_f1f042af","updated":"2025-02-12 13:30:29.000000000","message":"We agreed, because the ovo object using it is a Dict of Dict[str, any].","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd79e3bcd20dbc6a78979f10b8dcbe912e1b3f73","unresolved":true,"context_lines":[{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("},{"line_number":326,"context_line":"                reason\u003d_(\"Invalid value for %s\" % self.managed))"},{"line_number":327,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff884353_ec9145b0","line":324,"range":{"start_line":324,"start_character":28,"end_line":324,"end_character":61},"in_reply_to":"4a015256_e9f64906","updated":"2025-02-05 15:12:33.000000000","message":"this sounds a fair point","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"c997c9d260d578d2406baf06a0f494b5b7b976db","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("},{"line_number":326,"context_line":"                reason\u003d_(\"Invalid value for %s\" % self.managed))"},{"line_number":327,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"94e7269d_d7a46c2f","line":324,"range":{"start_line":324,"start_character":28,"end_line":324,"end_character":61},"in_reply_to":"8826d55b_9218f477","updated":"2025-02-12 13:30:57.000000000","message":"Done","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"3374ed825416b5ae90e70482e13866c4632a6684","unresolved":true,"context_lines":[{"line_number":321,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":322,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        if self.managed and self.managed not in (\u0027yes\u0027, \u0027no\u0027):"},{"line_number":325,"context_line":"            raise exception.PciConfigInvalidSpec("},{"line_number":326,"context_line":"                reason\u003d_(\"Invalid value for %s\" % self.managed))"},{"line_number":327,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"3db94a85_f1f042af","line":324,"range":{"start_line":324,"start_character":28,"end_line":324,"end_character":61},"in_reply_to":"ff884353_ec9145b0","updated":"2025-02-10 15:09:23.000000000","message":"I use a strutils.bool_from_string to handle all possibilities.\nHowever I kept manged\u003d\u0027yes\u0027 / \u0027no\u0027 instead of a boolean.\nTBH, I\u0027m not sure it is good. Let me know if you think it is better to keep it as a boolean.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"096fa64d4ff341b043d434d5975af7182216f7c6","unresolved":true,"context_lines":[{"line_number":295,"context_line":"        self.dev_name \u003d self.tags.pop(\"devname\", None)"},{"line_number":296,"context_line":"        # We need to keep managed in the tags and not pop it because it"},{"line_number":297,"context_line":"        # is used later in the manager."},{"line_number":298,"context_line":"        self._managed \u003d self.tags.get(\"managed\", None)"},{"line_number":299,"context_line":"        self.address: ty.Optional[WhitelistPciAddress] \u003d None"},{"line_number":300,"context_line":"        # Note(moshele): The address attribute can be a string or a dict."},{"line_number":301,"context_line":"        # For glob syntax or specific pci it is a string and for regex syntax"}],"source_content_type":"text/x-python","patch_set":5,"id":"7d632026_13849bee","line":298,"updated":"2025-02-13 14:06:10.000000000","message":"* this is almost unused now so I suggest to inline it to the only place it is used\n* this will have a different value than self.tags[\"managed\"] after the below code potentially translated a \"true\" value to \"yes\"","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ca3e0102a132dba3dbd7d44fc0928ea2e165bc0b","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        self.dev_name \u003d self.tags.pop(\"devname\", None)"},{"line_number":296,"context_line":"        # We need to keep managed in the tags and not pop it because it"},{"line_number":297,"context_line":"        # is used later in the manager."},{"line_number":298,"context_line":"        self._managed \u003d self.tags.get(\"managed\", None)"},{"line_number":299,"context_line":"        self.address: ty.Optional[WhitelistPciAddress] \u003d None"},{"line_number":300,"context_line":"        # Note(moshele): The address attribute can be a string or a dict."},{"line_number":301,"context_line":"        # For glob syntax or specific pci it is a string and for regex syntax"}],"source_content_type":"text/x-python","patch_set":5,"id":"bdd3d84a_878c66e5","line":298,"in_reply_to":"7d632026_13849bee","updated":"2025-02-13 19:48:57.000000000","message":"Done","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"096fa64d4ff341b043d434d5975af7182216f7c6","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                )"},{"line_number":331,"context_line":"            except ValueError:"},{"line_number":332,"context_line":"                raise exception.PciConfigInvalidSpec("},{"line_number":333,"context_line":"                    reason\u003d_(\"Invalid value for %s\" % self._managed)"},{"line_number":334,"context_line":"                )"},{"line_number":335,"context_line":"        if self._remote_managed:"},{"line_number":336,"context_line":"            if address_obj is None:"}],"source_content_type":"text/x-python","patch_set":5,"id":"71bf46a6_c6a4b913","line":333,"updated":"2025-02-13 14:06:10.000000000","message":"nit: let express that the new exception is a direct consequences of the ValueError we caught here\n```\nraise exception.PciConfigInvalidSpec(...) from e\n```","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ca3e0102a132dba3dbd7d44fc0928ea2e165bc0b","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                )"},{"line_number":331,"context_line":"            except ValueError:"},{"line_number":332,"context_line":"                raise exception.PciConfigInvalidSpec("},{"line_number":333,"context_line":"                    reason\u003d_(\"Invalid value for %s\" % self._managed)"},{"line_number":334,"context_line":"                )"},{"line_number":335,"context_line":"        if self._remote_managed:"},{"line_number":336,"context_line":"            if address_obj is None:"}],"source_content_type":"text/x-python","patch_set":5,"id":"228ed136_8b188326","line":333,"in_reply_to":"71bf46a6_c6a4b913","updated":"2025-02-13 19:48:57.000000000","message":"This is far better. Thx.","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8143cc4b581676da6ed5a6a2fccb3ea20c1a8bc4","unresolved":true,"context_lines":[{"line_number":318,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None):"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"yes\""}],"source_content_type":"text/x-python","patch_set":7,"id":"201068d4_d240eb37","line":321,"updated":"2025-02-17 14:31:28.000000000","message":"strangely oslo.config can give us a boolean as a result for the managed field here. E.g. if the value in the config is `false` then we will get a boolean false here and the translation will not happen. But when the value in the config is `\"false\"` then we get a sting \"false\" here and the translation happens. So we need to change this to \n```\nif self.tags.get(\"managed\", None) is not None:\n```","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a98cf0d9519ca1c9c4d277111444ff5f87028d0f","unresolved":true,"context_lines":[{"line_number":318,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None):"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"yes\""}],"source_content_type":"text/x-python","patch_set":7,"id":"372d622d_a0efec80","line":321,"in_reply_to":"201068d4_d240eb37","updated":"2025-02-17 18:17:17.000000000","message":"that not oslo config\n\nI believe that is because we pass this through a JSON parser and it parses False as a bool.\n\n\nhttps://bugs.launchpad.net/nova/+bug/1915282\n\nfrom that\n```\nthe whitelist is a multiopt string field\nhttps://github.com/openstack/nova/blob/56ac9b22cfa71daaf7a452fda63bd27811a358c4/nova/conf/pci.py#L88-L173\n\nwhich is then parsed by oslo_serialization.jsonutiles.loads\n\nhttps://github.com/openstack/nova/blob/56ac9b22cfa71daaf7a452fda63bd27811a358c4/nova/pci/whitelist.py#L58\n\n dev_spec \u003d jsonutils.loads(jsonspec)\n```\n\ni did previosuly ask that we prefer using true and false instead of yes and no by the way.","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ee09af437605783f5b0d112c007019aa2ee2dcdb","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        self._remote_managed \u003d strutils.bool_from_string("},{"line_number":319,"context_line":"            self.tags.get(PCI_REMOTE_MANAGED_TAG))"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None):"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"yes\""}],"source_content_type":"text/x-python","patch_set":7,"id":"0c593e2a_6f5a4ac3","line":321,"in_reply_to":"372d622d_a0efec80","updated":"2025-02-19 13:24:58.000000000","message":"Done","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"44652f67a5a4dbacca6d61d939848f9e9d210953","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                    \"yes\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"no\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"ec2c7611_c4ce119c","line":327,"updated":"2025-02-14 08:59:31.000000000","message":"I\u0027m pretty sure dan and I don\u0027t like using parenthesis that way, but please don\u0027t modify this change just fot that 😜","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9c6ae728838490aafbb12ea91df6e58ad814e39b","unresolved":true,"context_lines":[{"line_number":324,"context_line":"                    \"yes\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"no\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"f6c6a1b7_79eab94b","line":327,"in_reply_to":"ec2c7611_c4ce119c","updated":"2025-02-20 16:15:35.000000000","message":"i also dont agree with this for what its worth\n```\n  self.tags[\"managed\"] \u003d (\n                    \"yes\"\n                    if strutils.bool_from_string(\n                        self.tags.get(\"managed\"), strict\u003dTrue\n                    )\n                    else \"no\"\n                )\n                \n ```\n \n shoudl be \n \n ```\n   self.tags[\"managed\"] \u003d (\n       \"true\" if strutils.bool_from_string(\n           self.tags.get(\"managed\"), strict\u003dTrue) else \"false\")\n ```\n \n if you need to wrap, move everyting onto the new line with one level of indetaiton and put the closeing symble on the same line.\n\n but gibi mention elsewhere that we can just do  this\n \n ```\n  self.tags[\"managed\"] \u003d str(\n      strutils.bool_from_string(self.tags.get(\"managed\"), strict\u003dTrue))\n ```\n \nalso we can avoid caling get twice if we store the tag\n```\nmanaged\u003dself.tags.get(\"managed\", None)\nif managed in not None:\n    self.tags[\"managed\"] \u003d str(\n        strutils.bool_from_string(managed, strict\u003dTrue))\n```\n\nso ^ is how i would write it","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4408baa01b9653c7298e5296c83b709c20860a36","unresolved":false,"context_lines":[{"line_number":324,"context_line":"                    \"yes\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"no\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"}],"source_content_type":"text/x-python","patch_set":7,"id":"c3cee6e0_ddb2f870","line":327,"in_reply_to":"f6c6a1b7_79eab94b","updated":"2025-02-25 09:09:35.000000000","message":"Done","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b9bcf4372c33e5e4335aed59aa31c199d3fb5ba","unresolved":true,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None) is not None:"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"true\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"false\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"},{"line_number":331,"context_line":"                raise exception.PciConfigInvalidSpec(reason\u003dstr(e))"},{"line_number":332,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":8,"id":"4f574d8f_20f81c86","line":329,"range":{"start_line":323,"start_character":0,"end_line":329,"end_character":17},"updated":"2025-02-20 15:59:32.000000000","message":"instead of the verbose ternary this can be simplified to:\n```python\nself.tags[\"managed\"] \u003d str(\n    strutils.bool_from_string(\n        self.tags.get(\"managed\"), strict\u003dTrue))\n```\nI does not worth to respin the series for this. Just push a follow up patch on top if you agree.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9c6ae728838490aafbb12ea91df6e58ad814e39b","unresolved":true,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None) is not None:"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"true\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"false\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"},{"line_number":331,"context_line":"                raise exception.PciConfigInvalidSpec(reason\u003dstr(e))"},{"line_number":332,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":8,"id":"c9adbae9_35d62e51","line":329,"range":{"start_line":323,"start_character":0,"end_line":329,"end_character":17},"in_reply_to":"4f574d8f_20f81c86","updated":"2025-02-20 16:15:35.000000000","message":"ok ya that more readable\n\nstr on bool_from_string is fun but logically it makes senes and will normalise things well.\n\nbut agree nice ot have and can be done in a followup so no need to respin","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4408baa01b9653c7298e5296c83b709c20860a36","unresolved":false,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None) is not None:"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"true\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"false\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"},{"line_number":331,"context_line":"                raise exception.PciConfigInvalidSpec(reason\u003dstr(e))"},{"line_number":332,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":8,"id":"40a9ced6_d38bea80","line":329,"range":{"start_line":323,"start_character":0,"end_line":329,"end_character":17},"in_reply_to":"bc8dda76_cfdc617b","updated":"2025-02-25 09:09:35.000000000","message":"Done","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"bc14ba27c5c7bc68d8682b03f53e16519a896e11","unresolved":true,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None) is not None:"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"true\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"false\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"},{"line_number":331,"context_line":"                raise exception.PciConfigInvalidSpec(reason\u003dstr(e))"},{"line_number":332,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":8,"id":"d90d1399_40724dd9","line":329,"range":{"start_line":323,"start_character":0,"end_line":329,"end_character":17},"in_reply_to":"c9adbae9_35d62e51","updated":"2025-02-21 22:43:40.000000000","message":"Unless if I\u0027m wrong this will normalize to \"True\" not \"true\".\n\n```\n\u003e\u003e\u003e str(strutils.bool_from_string(\"yes\", strict\u003dTrue))\n\u0027True\u0027\n```\n\nWe changed for \"yes\" -\u003e \"true\" 😓\n\nI have no pb to change that part of code to:\n```\n   self.tags[\"managed\"] \u003d (\n       \"true\" if strutils.bool_from_string(\n           self.tags.get(\"managed\"), strict\u003dTrue) else \"false\")\n```\nthat should do it.\n\nHowever, I\u0027m not inclined to change from \"true\" to \"True\" as it is \"time-consuming\" and error-prone, potentially affecting both managed and live_migratable (migration patch) by causing missed comparisons in the code or tests.\nUnless you believe it\u0027s necessary.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bc10da753b0df5ac15fe069f6906c499a5c24137","unresolved":true,"context_lines":[{"line_number":320,"context_line":""},{"line_number":321,"context_line":"        if self.tags.get(\"managed\", None) is not None:"},{"line_number":322,"context_line":"            try:"},{"line_number":323,"context_line":"                self.tags[\"managed\"] \u003d ("},{"line_number":324,"context_line":"                    \"true\""},{"line_number":325,"context_line":"                    if strutils.bool_from_string("},{"line_number":326,"context_line":"                        self.tags.get(\"managed\"), strict\u003dTrue"},{"line_number":327,"context_line":"                    )"},{"line_number":328,"context_line":"                    else \"false\""},{"line_number":329,"context_line":"                )"},{"line_number":330,"context_line":"            except ValueError as e:"},{"line_number":331,"context_line":"                raise exception.PciConfigInvalidSpec(reason\u003dstr(e))"},{"line_number":332,"context_line":"        if self._remote_managed:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bc8dda76_cfdc617b","line":329,"range":{"start_line":323,"start_character":0,"end_line":329,"end_character":17},"in_reply_to":"d90d1399_40724dd9","updated":"2025-02-24 09:42:34.000000000","message":"you are right what I proposed would produce \"True\" instead of \"true\", I missed that.\n\n\u003e Unless you believe it\u0027s necessary.\n\nIt is not a must at all. Keep it as is. We can discuss this after the deadline.\n\nIn genereal what this shows is that we have a not well defined interface here. Yeah it is nice that we don\u0027t have to change the DB schema for this feature but in the other hand we loose time on the not well defined interface.\n\nAnyhow, keep it as is for now. Does not make sense to risk a break due to the change.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fef350fbaf18abfe0a6273ba0f211da5bc5c0503","unresolved":true,"context_lines":[{"line_number":417,"context_line":"        if managed is not None:"},{"line_number":418,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":419,"context_line":"        else:"},{"line_number":420,"context_line":"            if \u0027managed\u0027 in dev:"},{"line_number":421,"context_line":"                del dev[\u0027managed\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"a3aa19d0_31d4a204","line":420,"updated":"2025-02-20 15:40:33.000000000","message":"maybe a comment explaining why we want to delete it could be nice","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4408baa01b9653c7298e5296c83b709c20860a36","unresolved":false,"context_lines":[{"line_number":417,"context_line":"        if managed is not None:"},{"line_number":418,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":419,"context_line":"        else:"},{"line_number":420,"context_line":"            if \u0027managed\u0027 in dev:"},{"line_number":421,"context_line":"                del dev[\u0027managed\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"81d71073_69f6aa43","line":420,"in_reply_to":"a3aa19d0_31d4a204","updated":"2025-02-25 09:09:35.000000000","message":"Done","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b9bcf4372c33e5e4335aed59aa31c199d3fb5ba","unresolved":true,"context_lines":[{"line_number":418,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":419,"context_line":"        else:"},{"line_number":420,"context_line":"            if \u0027managed\u0027 in dev:"},{"line_number":421,"context_line":"                del dev[\u0027managed\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"4df033f7_731c8803","line":421,"updated":"2025-02-20 15:59:32.000000000","message":"I\u0027m wondering what is the case today when this delete actually removes anything from the dev_dict. As far as I understand the dev_dict is coming from the virt driver. The virt driver does not know about the \"managed\" flag so it won\u0027t ever include it.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b2fdb20026b106477f37180c4a4357ae39bf9e7","unresolved":true,"context_lines":[{"line_number":418,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":419,"context_line":"        else:"},{"line_number":420,"context_line":"            if \u0027managed\u0027 in dev:"},{"line_number":421,"context_line":"                del dev[\u0027managed\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"cad10fe5_59af03c2","line":421,"in_reply_to":"4df033f7_731c8803","updated":"2025-02-24 10:25:59.000000000","message":"Yeah I proved that locally. So this removal code in ineffective. The removal should happen in the PCIDevice object when it is updated from the device dict.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4408baa01b9653c7298e5296c83b709c20860a36","unresolved":false,"context_lines":[{"line_number":418,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":419,"context_line":"        else:"},{"line_number":420,"context_line":"            if \u0027managed\u0027 in dev:"},{"line_number":421,"context_line":"                del dev[\u0027managed\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"55258313_bdc87d50","line":421,"in_reply_to":"cad10fe5_59af03c2","updated":"2025-02-25 09:09:35.000000000","message":"Done","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"}],"nova/pci/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        devices \u003d []"},{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    managed \u003d self.dev_filter.managed_device(dev)"},{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a5ed4cac_4e44767c","line":135,"range":{"start_line":134,"start_character":0,"end_line":135,"end_character":65},"updated":"2025-01-14 13:04:29.000000000","message":"Now both device_assignable and managed_device checks for the spec to see which one if any matches with the device. Can we refactor this to\n* get the spec that matching with the dev\n* if there is one then update the dev with the managed tag (or any other tag like live-migratable later) from the matching spec and then add the dev to the devices list\n* (then this this would also allow to have log a Warning later here if multiple spec matching with the device while still using the first match to update the dev)\n\nThis way we can avoid that the two spec matching logics start to diverge in the future.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"84818b58f8af4a82f43065df484a1208a9a2c594","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        devices \u003d []"},{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    managed \u003d self.dev_filter.managed_device(dev)"},{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ece11488_61df0d4f","line":135,"range":{"start_line":134,"start_character":0,"end_line":135,"end_character":65},"in_reply_to":"a5ed4cac_4e44767c","updated":"2025-02-03 14:45:41.000000000","message":"I\u0027m not sure this is still applicable.\nNow, I\u0027m checking the managed value upfront. At this point, we are sure that managed is \u0027no\u0027, \u0027yes\u0027, or None.\nSo the code is much simpler, and I have simply inline it.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd79e3bcd20dbc6a78979f10b8dcbe912e1b3f73","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        devices \u003d []"},{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    managed \u003d self.dev_filter.managed_device(dev)"},{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ea277909_4de9f67b","line":135,"range":{"start_line":134,"start_character":0,"end_line":135,"end_character":65},"in_reply_to":"ece11488_61df0d4f","updated":"2025-02-05 15:12:33.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    managed \u003d self.dev_filter.managed_device(dev)"},{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a1c61b98_eb9c034b","line":137,"updated":"2025-01-14 13:04:29.000000000","message":"Do we need to represent the value as a string in the PciDevice even though we use the value as a boolean? At least the _set_hvdevs below only declares the devs as `ty.Dict[str, ty.Any]` so that allow \"managed\": true.\n\n// later after reading the relevant code in PciDevice\n\nSo the ovo interface is a DictOfStrings so we should not try to use a boolean. I still would feel better to user \"true\" and \"false\" as a values as that is more natural to me. But as it is a string in any way this is becomes just a nit.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"84818b58f8af4a82f43065df484a1208a9a2c594","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    managed \u003d self.dev_filter.managed_device(dev)"},{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"}],"source_content_type":"text/x-python","patch_set":2,"id":"448f064d_2bbaa542","line":137,"in_reply_to":"a1c61b98_eb9c034b","updated":"2025-02-03 14:45:41.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":2,"id":"a30fcab4_7d69ddf0","line":139,"updated":"2025-01-14 13:04:29.000000000","message":"I would rather introduce a function something like update_dev_from_spec(dev, spec) to encapsulate the new and future logic that adds information from the spec to the device.\n\nThis will also help keeping this function high level and moving the low level details to a separate one.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0fd2cd06194417cb256d7f1bf10bd7f91a71d133","unresolved":true,"context_lines":[{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":2,"id":"12eb4c88_71a2b3d0","line":139,"updated":"2025-01-14 13:18:52.000000000","message":"Now there is an opens contradiction with a comment from @smooney@redhat.com in the spec and the impl here regarding that we might not want to persists managed:yes in the DB just assume it in the python code if the tag is not present.\n\nI remember we agreed about it for the live-migratable flag, but maybe it is applicable for the managed tag too.\n\n[1] https://review.opendev.org/c/openstack/nova-specs/+/936407/6#message-dd7a06402a56ec01ee6a7a2d6fa0f5fc471e9f5a","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3edd7622487ac6639c2a257c5f0a2bf5e49542f1","unresolved":true,"context_lines":[{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":2,"id":"c8256cd5_8bb7f6f1","line":139,"in_reply_to":"12eb4c88_71a2b3d0","updated":"2025-01-14 14:00:14.000000000","message":"we can persist it if you set it but we shoudl not add it if you dont have it in the device_spec.\n\nagain without changes to the config file i do not want there to be any need to do a data migration for devices in  the db on upgrade.\n\nif you update the config to add either the live_migratable or manged tags then those shoudl be saved into the db.\n\nif you remove either tag in the futre they should also be remvoed form the db.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":2,"id":"2adbcbbe_b9d1b39c","line":139,"in_reply_to":"89a4261d_a461d9eb","updated":"2025-02-12 13:30:29.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":2,"id":"3aeb2e44_628f0424","line":139,"in_reply_to":"a30fcab4_7d69ddf0","updated":"2025-02-12 13:30:29.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1c757badac1afa0bc3939e74c478b9ee2870aeef","unresolved":true,"context_lines":[{"line_number":136,"context_line":"                    if not managed:"},{"line_number":137,"context_line":"                        dev.update({\u0027managed\u0027: \u0027no\u0027})"},{"line_number":138,"context_line":"                    else:"},{"line_number":139,"context_line":"                        dev.update({\u0027managed\u0027: \u0027yes\u0027})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":2,"id":"89a4261d_a461d9eb","line":139,"in_reply_to":"c8256cd5_8bb7f6f1","updated":"2025-01-15 07:57:41.000000000","message":"Thanks for the clarification. Your suggestion make sense to me.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aac56b1cac5005f2b652ff8e4b9207b12c1fcf68","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"700db702_a5ad97d3","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"updated":"2025-02-04 19:03:04.000000000","message":"this does not feel like the right place to do this.\nim not actully sure that you need special handling for this at all.\n\nthe managed flag is just a tags on the object so im not sure why you are pulling it out into a top level key.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1daf1537052455aabe479849b87c1faee691f427","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"d8f64764_7107cc49","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"261da7fa_5e935ce4","updated":"2025-02-07 09:23:52.000000000","message":"I also feel bad about this change. My problem is that `.dev_filter.device_assignable(dev)` already calls spec.match(dev) and here we do it again. This calls for a refactor. I hinted for this refactor in my comments on PS2.\n\nOn Sean\u0027s comment about do we need this change or not: I think this is the place where we move the \"managed\" information from the DeviceSpec to the device dict that will become the PCIDevice ovo a bit later. I think we don\u0027t have generic code that actually moves any tag from the DeviceSpec to the extra_info field of the PCIDevice. The generic code we have in the PCIDevice object is that any field in the device_dict that is not an ovo field automatically ends up in the extra_info field of the object. So when the hypervisor adds a new field to the dict that is not an ovo field that will be in PCIDevice.extra_info. However here the source of information for the new \"managed\" flag is not the hypervisor but the DeviceSpec from the config.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"699694be58dd437c9e9caa597ba2683e19633e57","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"b081dffb_c07c54be","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"261da7fa_5e935ce4","updated":"2025-02-11 13:40:03.000000000","message":"ok i see your doing this here instead of doign it in the virt driver.\n\nfor remote manage ports we discover the extra info in the virt driver\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1432-L1474\n\n\nthis is all part of https://github.com/openstack/nova/blob/master/nova/virt/libvirt/host.py#L1563 which is the function that creates devices_json\n\nthat is called form the driver here\n\nhttps://github.com/openstack/nova/blob/e27bbe72e0d293e55c30d4f90ca0afcf47427419/nova/virt/libvirt/driver.py#L8658","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1d967ff8d052bc78ad1bb98beb0d1035a7d42c70","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"261da7fa_5e935ce4","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"422958b2_56c9f8b8","updated":"2025-02-06 17:28:20.000000000","message":"my feed back was this shoudl not be requried at all","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"096fa64d4ff341b043d434d5975af7182216f7c6","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"dd175db3_a1d7cc18","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"5561da77_f218738d","updated":"2025-02-13 14:06:10.000000000","message":"Acknowledged","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fd79e3bcd20dbc6a78979f10b8dcbe912e1b3f73","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"a942e263_e0318e64","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"700db702_a5ad97d3","updated":"2025-02-05 15:12:33.000000000","message":"AFAIK if we don\u0027t update the dev dict, then the managed value won\u0027t be persisted, hence why we need to add this check, but maybe we need to test it differently.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"5561da77_f218738d","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"911504ff_1d309d55","updated":"2025-02-12 13:30:29.000000000","message":"I have added a comment and refactored it, avoiding looping twice and respecting SRP.\nLet me know what you think about it.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"772796152061a5d3c2d398902e05ed6d7576f7e3","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"422958b2_56c9f8b8","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"a942e263_e0318e64","updated":"2025-02-06 16:21:47.000000000","message":"Hmm, to be honest, I’ve tried to place this somewhere else, but so far, I haven’t found a better spot. Maybe I missed something—please let me know if you have a specific place in mind.","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9250dd6f86fc0537ded8c1f9e2b860c3a7db2847","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        for dev in jsonutils.loads(devices_json):"},{"line_number":133,"context_line":"            try:"},{"line_number":134,"context_line":"                if self.dev_filter.device_assignable(dev):"},{"line_number":135,"context_line":"                    for spec in self.dev_filter.specs:"},{"line_number":136,"context_line":"                        if spec.match(dev):"},{"line_number":137,"context_line":"                            managed \u003d spec.tags.get(\"managed\")"},{"line_number":138,"context_line":"                            if managed is not None:"},{"line_number":139,"context_line":"                                dev.update({\u0027managed\u0027: managed})"},{"line_number":140,"context_line":"                    devices.append(dev)"},{"line_number":141,"context_line":"            except exception.PciConfigInvalidSpec as e:"},{"line_number":142,"context_line":"                # The raised exception is misleading as the problem is not with"}],"source_content_type":"text/x-python","patch_set":3,"id":"911504ff_1d309d55","line":139,"range":{"start_line":135,"start_character":20,"end_line":139,"end_character":64},"in_reply_to":"b081dffb_c07c54be","updated":"2025-02-11 14:06:57.000000000","message":"Lets make a comment that this is done here as the config is not discoverable by the virt driver.","commit_id":"972426c0af6a234103d789b4460544eecc462781"}],"nova/pci/whitelist.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":54,"context_line":"            self.specs \u003d []"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @staticmethod"},{"line_number":57,"context_line":"    def _parse_white_list_from_config("},{"line_number":58,"context_line":"        whitelists: str,"},{"line_number":59,"context_line":"    ) -\u003e ty.List[devspec.PciDeviceSpec]:"},{"line_number":60,"context_line":"        \"\"\"Parse and validate the pci whitelist from the nova config.\"\"\""},{"line_number":61,"context_line":"        specs \u003d []"},{"line_number":62,"context_line":"        for jsonspec in whitelists:"},{"line_number":63,"context_line":"            try:"},{"line_number":64,"context_line":"                dev_spec \u003d jsonutils.loads(jsonspec)"},{"line_number":65,"context_line":"            except ValueError:"},{"line_number":66,"context_line":"                raise exception.PciConfigInvalidSpec("},{"line_number":67,"context_line":"                          reason\u003d_(\"Invalid entry: \u0027%s\u0027\") % jsonspec)"},{"line_number":68,"context_line":"            if isinstance(dev_spec, dict):"},{"line_number":69,"context_line":"                dev_spec \u003d [dev_spec]"},{"line_number":70,"context_line":"            elif not isinstance(dev_spec, list):"},{"line_number":71,"context_line":"                raise exception.PciConfigInvalidSpec("},{"line_number":72,"context_line":"                          reason\u003d_(\"Invalid entry: \u0027%s\u0027; \""},{"line_number":73,"context_line":"                                   \"Expecting list or dict\") % jsonspec)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"            for ds in dev_spec:"},{"line_number":76,"context_line":"                if not isinstance(ds, dict):"},{"line_number":77,"context_line":"                    raise exception.PciConfigInvalidSpec("},{"line_number":78,"context_line":"                              reason\u003d_(\"Invalid entry: \u0027%s\u0027; \""},{"line_number":79,"context_line":"                                       \"Expecting dict\") % ds)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"                spec \u003d devspec.PciDeviceSpec(ds)"},{"line_number":82,"context_line":"                specs.append(spec)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        return specs"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def device_assignable(self, dev: ty.Dict[str, ty.Any]) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c43c1a6_685e1010","line":83,"range":{"start_line":57,"start_character":0,"end_line":83,"end_character":0},"updated":"2025-01-14 13:04:29.000000000","message":"I think this is the place (or the subsequent PciDeviecSpec constructor) where we should validate the value of the new managed tag","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1daf1537052455aabe479849b87c1faee691f427","unresolved":false,"context_lines":[{"line_number":54,"context_line":"            self.specs \u003d []"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @staticmethod"},{"line_number":57,"context_line":"    def _parse_white_list_from_config("},{"line_number":58,"context_line":"        whitelists: str,"},{"line_number":59,"context_line":"    ) -\u003e ty.List[devspec.PciDeviceSpec]:"},{"line_number":60,"context_line":"        \"\"\"Parse and validate the pci whitelist from the nova config.\"\"\""},{"line_number":61,"context_line":"        specs \u003d []"},{"line_number":62,"context_line":"        for jsonspec in whitelists:"},{"line_number":63,"context_line":"            try:"},{"line_number":64,"context_line":"                dev_spec \u003d jsonutils.loads(jsonspec)"},{"line_number":65,"context_line":"            except ValueError:"},{"line_number":66,"context_line":"                raise exception.PciConfigInvalidSpec("},{"line_number":67,"context_line":"                          reason\u003d_(\"Invalid entry: \u0027%s\u0027\") % jsonspec)"},{"line_number":68,"context_line":"            if isinstance(dev_spec, dict):"},{"line_number":69,"context_line":"                dev_spec \u003d [dev_spec]"},{"line_number":70,"context_line":"            elif not isinstance(dev_spec, list):"},{"line_number":71,"context_line":"                raise exception.PciConfigInvalidSpec("},{"line_number":72,"context_line":"                          reason\u003d_(\"Invalid entry: \u0027%s\u0027; \""},{"line_number":73,"context_line":"                                   \"Expecting list or dict\") % jsonspec)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"            for ds in dev_spec:"},{"line_number":76,"context_line":"                if not isinstance(ds, dict):"},{"line_number":77,"context_line":"                    raise exception.PciConfigInvalidSpec("},{"line_number":78,"context_line":"                              reason\u003d_(\"Invalid entry: \u0027%s\u0027; \""},{"line_number":79,"context_line":"                                       \"Expecting dict\") % ds)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"                spec \u003d devspec.PciDeviceSpec(ds)"},{"line_number":82,"context_line":"                specs.append(spec)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        return specs"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def device_assignable(self, dev: ty.Dict[str, ty.Any]) -\u003e bool:"}],"source_content_type":"text/x-python","patch_set":2,"id":"c602d1d3_3ff2b7e3","line":83,"range":{"start_line":57,"start_character":0,"end_line":83,"end_character":0},"in_reply_to":"5c43c1a6_685e1010","updated":"2025-02-07 09:23:52.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":93,"context_line":"                return True"},{"line_number":94,"context_line":"        return False"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def managed_device(self, dev: ty.Dict[str, ty.Any]) -\u003e bool:"},{"line_number":97,"context_line":"        for spec in self.specs:"},{"line_number":98,"context_line":"            if spec.match(dev):"},{"line_number":99,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"58c944ba_e0c9b26e","line":96,"updated":"2025-01-14 13:04:29.000000000","message":"I suggest to refactor this to a function that takes a dev and a spec (that is already determined to be matching) and updates the dev based on the values in the spec. This way you can keep the \"managed\" field manipulation in a single place instead of how today it is shared between this place and the caller.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"f7491ce6b31d27729881e088068704f7c4422543","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                return True"},{"line_number":94,"context_line":"        return False"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def managed_device(self, dev: ty.Dict[str, ty.Any]) -\u003e bool:"},{"line_number":97,"context_line":"        for spec in self.specs:"},{"line_number":98,"context_line":"            if spec.match(dev):"},{"line_number":99,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"634b2619_2e8da16a","line":96,"in_reply_to":"58c944ba_e0c9b26e","updated":"2025-02-12 13:30:29.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"823fff870b017f89430fcd5c5ea650e79c09e8e3","unresolved":true,"context_lines":[{"line_number":103,"context_line":"                        default\u003dTrue,"},{"line_number":104,"context_line":"                    )"},{"line_number":105,"context_line":"                except ValueError:"},{"line_number":106,"context_line":"                    raise exception.PciConfigInvalidSpec("},{"line_number":107,"context_line":"                        reason\u003d_(\"managed value: \u0027%s\u0027 is not valid.\")"},{"line_number":108,"context_line":"                        % spec.tags.get(\"managed\")"},{"line_number":109,"context_line":"                    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"6732ff86_52520104","line":106,"updated":"2025-01-14 13:04:29.000000000","message":"I think this is not a good place to raise issues about the configuration as this function is called way late when the hypevisor provided the list of devs. We should be able to validate our config without having the virt driver even started.","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"84818b58f8af4a82f43065df484a1208a9a2c594","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                        default\u003dTrue,"},{"line_number":104,"context_line":"                    )"},{"line_number":105,"context_line":"                except ValueError:"},{"line_number":106,"context_line":"                    raise exception.PciConfigInvalidSpec("},{"line_number":107,"context_line":"                        reason\u003d_(\"managed value: \u0027%s\u0027 is not valid.\")"},{"line_number":108,"context_line":"                        % spec.tags.get(\"managed\")"},{"line_number":109,"context_line":"                    )"}],"source_content_type":"text/x-python","patch_set":2,"id":"46d5aa12_b48dbef2","line":106,"in_reply_to":"6732ff86_52520104","updated":"2025-02-03 14:45:41.000000000","message":"Done","commit_id":"c9b5e4fda8879ca0b8e22656b7368c24eb53d095"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"096fa64d4ff341b043d434d5975af7182216f7c6","unresolved":true,"context_lines":[{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @staticmethod"},{"line_number":101,"context_line":"    def enhance_pci_device_with_spec_tags("},{"line_number":102,"context_line":"        dev: ty.Dict[str, ty.Any], spec: devspec.PciDeviceSpec"},{"line_number":103,"context_line":"    ):"},{"line_number":104,"context_line":"        managed \u003d spec.tags.get(\"managed\")"},{"line_number":105,"context_line":"        if managed is not None:"},{"line_number":106,"context_line":"            dev.update({\u0027managed\u0027: managed})"}],"source_content_type":"text/x-python","patch_set":5,"id":"44a18f78_361f4d4e","line":103,"range":{"start_line":100,"start_character":0,"end_line":103,"end_character":6},"updated":"2025-02-13 14:06:10.000000000","message":"I would go even one step further and move this to be a member of PciDeviceSpec object. That way the fact that the spec has a \"managed\" tag, how it is parsed, and how it is used, are all encapsulated in a single object.","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ca3e0102a132dba3dbd7d44fc0928ea2e165bc0b","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @staticmethod"},{"line_number":101,"context_line":"    def enhance_pci_device_with_spec_tags("},{"line_number":102,"context_line":"        dev: ty.Dict[str, ty.Any], spec: devspec.PciDeviceSpec"},{"line_number":103,"context_line":"    ):"},{"line_number":104,"context_line":"        managed \u003d spec.tags.get(\"managed\")"},{"line_number":105,"context_line":"        if managed is not None:"},{"line_number":106,"context_line":"            dev.update({\u0027managed\u0027: managed})"}],"source_content_type":"text/x-python","patch_set":5,"id":"f81b7ea9_68ecc1ee","line":103,"range":{"start_line":100,"start_character":0,"end_line":103,"end_character":6},"in_reply_to":"44a18f78_361f4d4e","updated":"2025-02-13 19:48:57.000000000","message":"You are right that\u0027s better. 👍️","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0db06a50e13228b044a2c9bc4928217b789ba86d","unresolved":true,"context_lines":[{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @staticmethod"},{"line_number":101,"context_line":"    def enhance_pci_device_with_spec_tags("},{"line_number":102,"context_line":"        dev: ty.Dict[str, ty.Any], spec: devspec.PciDeviceSpec"},{"line_number":103,"context_line":"    ):"},{"line_number":104,"context_line":"        managed \u003d spec.tags.get(\"managed\")"},{"line_number":105,"context_line":"        if managed is not None:"},{"line_number":106,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def get_devspec("},{"line_number":109,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"2380b798_4a0f5d7a","line":106,"range":{"start_line":100,"start_character":0,"end_line":106,"end_character":44},"updated":"2025-02-14 08:07:02.000000000","message":"This is unused now as it is moved to PCIDeviceSpec.","commit_id":"18bb8debd845d6449a09f05e5c850d6f1a1e897c"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"49da12b9456b505938972631d0af857da09afdce","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    @staticmethod"},{"line_number":101,"context_line":"    def enhance_pci_device_with_spec_tags("},{"line_number":102,"context_line":"        dev: ty.Dict[str, ty.Any], spec: devspec.PciDeviceSpec"},{"line_number":103,"context_line":"    ):"},{"line_number":104,"context_line":"        managed \u003d spec.tags.get(\"managed\")"},{"line_number":105,"context_line":"        if managed is not None:"},{"line_number":106,"context_line":"            dev.update({\u0027managed\u0027: managed})"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def get_devspec("},{"line_number":109,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"efd0b971_bf0668ea","line":106,"range":{"start_line":100,"start_character":0,"end_line":106,"end_character":44},"in_reply_to":"2380b798_4a0f5d7a","updated":"2025-02-14 08:20:28.000000000","message":"Done","commit_id":"18bb8debd845d6449a09f05e5c850d6f1a1e897c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"44652f67a5a4dbacca6d61d939848f9e9d210953","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        for spec in self.specs:"},{"line_number":96,"context_line":"            if spec.match(dev):"},{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def get_devspec("},{"line_number":101,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"41e14f8e_58777c66","line":98,"updated":"2025-02-14 08:59:31.000000000","message":"this is no longer necessary but meh.","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b9bcf4372c33e5e4335aed59aa31c199d3fb5ba","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        for spec in self.specs:"},{"line_number":96,"context_line":"            if spec.match(dev):"},{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def get_devspec("},{"line_number":101,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"71871f32_6b885c9d","line":98,"in_reply_to":"1299880e_f9b3624a","updated":"2025-02-20 15:59:32.000000000","message":"\"Explicit is better than implicit.\" https://peps.python.org/pep-0020/","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fef350fbaf18abfe0a6273ba0f211da5bc5c0503","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        for spec in self.specs:"},{"line_number":96,"context_line":"            if spec.match(dev):"},{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def get_devspec("},{"line_number":101,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1299880e_f9b3624a","line":98,"in_reply_to":"16608bd2_401154c8","updated":"2025-02-20 15:40:33.000000000","message":"not actually done but meh :)","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ee09af437605783f5b0d112c007019aa2ee2dcdb","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        for spec in self.specs:"},{"line_number":96,"context_line":"            if spec.match(dev):"},{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def get_devspec("},{"line_number":101,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"16608bd2_401154c8","line":98,"in_reply_to":"41e14f8e_58777c66","updated":"2025-02-19 13:24:58.000000000","message":"Done","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9c6ae728838490aafbb12ea91df6e58ad814e39b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        for spec in self.specs:"},{"line_number":96,"context_line":"            if spec.match(dev):"},{"line_number":97,"context_line":"                return spec"},{"line_number":98,"context_line":"        return None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def get_devspec("},{"line_number":101,"context_line":"        self, pci_dev: \u0027objects.PciDevice\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"04c7f264_6534438e","line":98,"in_reply_to":"71871f32_6b885c9d","updated":"2025-02-20 16:15:35.000000000","message":"i agree returning None is better than falling off the bottom of the function\nso this is correct even if strcitly not neesisary","commit_id":"c7da5e05e66d5e04fc44526c055f7fbc0b807daa"}],"nova/tests/unit/pci/test_manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b9bcf4372c33e5e4335aed59aa31c199d3fb5ba","unresolved":true,"context_lines":[{"line_number":344,"context_line":"        self.assertEqual("},{"line_number":345,"context_line":"            pci_addr_extra_info[\"0000:00:02.2\"][\"managed\"], \"false\""},{"line_number":346,"context_line":"        )"},{"line_number":347,"context_line":"        self.assertNotIn(\"managed\", pci_addr_extra_info[\"0000:00:02.3\"])"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    @mock.patch(\"nova.pci.manager.LOG.debug\")"},{"line_number":350,"context_line":"    @mock.patch(\"nova.pci.utils.is_physical_function\", return_value\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1faea1e5_eda79d49","line":347,"updated":"2025-02-20 15:59:32.000000000","message":"I think this might not be a proper test case as it does not account for the fact that changing the config of the service means the service is restarted and therefore the tracker is recreated from scratch so it does not have the memory of old dev_dicts.\n\nI will run a reconfiguration test in a devstack later.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"6c928c1a8b50a9f533620ddfea2598217eb75f3e","unresolved":true,"context_lines":[{"line_number":344,"context_line":"        self.assertEqual("},{"line_number":345,"context_line":"            pci_addr_extra_info[\"0000:00:02.2\"][\"managed\"], \"false\""},{"line_number":346,"context_line":"        )"},{"line_number":347,"context_line":"        self.assertNotIn(\"managed\", pci_addr_extra_info[\"0000:00:02.3\"])"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    @mock.patch(\"nova.pci.manager.LOG.debug\")"},{"line_number":350,"context_line":"    @mock.patch(\"nova.pci.utils.is_physical_function\", return_value\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7e297a3f_f2486edb","line":347,"in_reply_to":"1faea1e5_eda79d49","updated":"2025-02-21 22:54:25.000000000","message":"You are most probably correct. I think this is coming from the driver and not the DB, so it should come without any managed or live_migratable info.\n\nThe correct fix should probably be to look into the DB if this device was previously presisted with managed and or live_migratable.\n\nI will check it as soon as possible and propose a fix.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"300f694ba6dcd9f0a41969f778c31c416cb93edc","unresolved":true,"context_lines":[{"line_number":344,"context_line":"        self.assertEqual("},{"line_number":345,"context_line":"            pci_addr_extra_info[\"0000:00:02.2\"][\"managed\"], \"false\""},{"line_number":346,"context_line":"        )"},{"line_number":347,"context_line":"        self.assertNotIn(\"managed\", pci_addr_extra_info[\"0000:00:02.3\"])"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    @mock.patch(\"nova.pci.manager.LOG.debug\")"},{"line_number":350,"context_line":"    @mock.patch(\"nova.pci.utils.is_physical_function\", return_value\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8b3467ab_53d6f8ce","line":347,"in_reply_to":"7e297a3f_f2486edb","updated":"2025-02-24 09:44:33.000000000","message":"I will resume testing this feature and reviewing the live migration on today so I might have a bit better info about this in a couple of hours...","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4b2fdb20026b106477f37180c4a4357ae39bf9e7","unresolved":true,"context_lines":[{"line_number":344,"context_line":"        self.assertEqual("},{"line_number":345,"context_line":"            pci_addr_extra_info[\"0000:00:02.2\"][\"managed\"], \"false\""},{"line_number":346,"context_line":"        )"},{"line_number":347,"context_line":"        self.assertNotIn(\"managed\", pci_addr_extra_info[\"0000:00:02.3\"])"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    @mock.patch(\"nova.pci.manager.LOG.debug\")"},{"line_number":350,"context_line":"    @mock.patch(\"nova.pci.utils.is_physical_function\", return_value\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"c33b6d06_de034ce3","line":347,"in_reply_to":"8b3467ab_53d6f8ce","updated":"2025-02-24 10:25:59.000000000","message":"Yeah I have proof that the removal of the managed flag still does not work. Left suggestions above where to fix it. I also suggest to change this testcase to reproduce the original error as right now the test is missleading.","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"4408baa01b9653c7298e5296c83b709c20860a36","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        self.assertEqual("},{"line_number":345,"context_line":"            pci_addr_extra_info[\"0000:00:02.2\"][\"managed\"], \"false\""},{"line_number":346,"context_line":"        )"},{"line_number":347,"context_line":"        self.assertNotIn(\"managed\", pci_addr_extra_info[\"0000:00:02.3\"])"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"    @mock.patch(\"nova.pci.manager.LOG.debug\")"},{"line_number":350,"context_line":"    @mock.patch(\"nova.pci.utils.is_physical_function\", return_value\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bd523a0f_62552f7a","line":347,"in_reply_to":"c33b6d06_de034ce3","updated":"2025-02-25 09:09:35.000000000","message":"Done","commit_id":"d1267a0d2eea0314a6fb8ff5541f53640017cdb8"}],"nova/tests/unit/pci/test_whitelist.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1daf1537052455aabe479849b87c1faee691f427","unresolved":true,"context_lines":[{"line_number":89,"context_line":"        )"},{"line_number":90,"context_line":"        parsed \u003d whitelist.Whitelist([white_list])"},{"line_number":91,"context_line":"        self.assertEqual(1, len(parsed.specs))"},{"line_number":92,"context_line":"        self.assertTrue(parsed.device_assignable(dev_dict))"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_device_not_managed(self):"},{"line_number":95,"context_line":"        white_list \u003d ("}],"source_content_type":"text/x-python","patch_set":3,"id":"e60c3660_f28ebef2","line":92,"updated":"2025-02-07 09:23:52.000000000","message":"I suggest to assert that the managed field gets stored in the parsed[0].tags properly (and in whatever field we end up storing it)","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"3374ed825416b5ae90e70482e13866c4632a6684","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        )"},{"line_number":90,"context_line":"        parsed \u003d whitelist.Whitelist([white_list])"},{"line_number":91,"context_line":"        self.assertEqual(1, len(parsed.specs))"},{"line_number":92,"context_line":"        self.assertTrue(parsed.device_assignable(dev_dict))"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_device_not_managed(self):"},{"line_number":95,"context_line":"        white_list \u003d ("}],"source_content_type":"text/x-python","patch_set":3,"id":"bf47b19a_b8173569","line":92,"in_reply_to":"e60c3660_f28ebef2","updated":"2025-02-10 15:09:23.000000000","message":"Done","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1daf1537052455aabe479849b87c1faee691f427","unresolved":true,"context_lines":[{"line_number":97,"context_line":"        )"},{"line_number":98,"context_line":"        parsed \u003d whitelist.Whitelist([white_list])"},{"line_number":99,"context_line":"        self.assertEqual(1, len(parsed.specs))"},{"line_number":100,"context_line":"        self.assertTrue(parsed.device_assignable(dev_dict))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def test_device_managed_invalid_value(self):"},{"line_number":103,"context_line":"        white_list \u003d ("}],"source_content_type":"text/x-python","patch_set":3,"id":"ff4e1c60_f541e684","line":100,"updated":"2025-02-07 09:23:52.000000000","message":"ditto assert that we store the correct value in the devicespec","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"3374ed825416b5ae90e70482e13866c4632a6684","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        )"},{"line_number":98,"context_line":"        parsed \u003d whitelist.Whitelist([white_list])"},{"line_number":99,"context_line":"        self.assertEqual(1, len(parsed.specs))"},{"line_number":100,"context_line":"        self.assertTrue(parsed.device_assignable(dev_dict))"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def test_device_managed_invalid_value(self):"},{"line_number":103,"context_line":"        white_list \u003d ("}],"source_content_type":"text/x-python","patch_set":3,"id":"8db1a097_b286113f","line":100,"in_reply_to":"ff4e1c60_f541e684","updated":"2025-02-10 15:09:23.000000000","message":"Done","commit_id":"972426c0af6a234103d789b4460544eecc462781"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"096fa64d4ff341b043d434d5975af7182216f7c6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2e36c21c_3d5c376c","line":131,"updated":"2025-02-13 14:06:10.000000000","message":"I would add a test case where the whitelist has no managed tag specified and assert that managed tag is not added in the spec object. (I.e. to assert that we don\u0027t want to store the field with any default value if it is not provided in the config)","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"ca3e0102a132dba3dbd7d44fc0928ea2e165bc0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3c8430d0_3509a527","line":131,"in_reply_to":"2e36c21c_3d5c376c","updated":"2025-02-13 19:48:57.000000000","message":"Done","commit_id":"2e720d33a0f1527b30f53da7db980041a6c56735"}]}