)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9df587cfe8bb1e667e4a4a0c14356b9f2c8fb35d","unresolved":true,"context_lines":[{"line_number":11,"context_line":"nova need to correctly clean up the volume connection on"},{"line_number":12,"context_line":"the host when the last instance is removed."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"currently we do not have a volume level lock to garud the"},{"line_number":15,"context_line":"critical section that determins if the current disconenct is"},{"line_number":16,"context_line":"removing the final usage of the volume."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3024861a_c02ab634","line":14,"range":{"start_line":14,"start_character":48,"end_line":14,"end_character":53},"updated":"2024-08-30 08:52:00.000000000","message":"guard","commit_id":"0a15fa8e650d5a981163b9234dca0e116e163326"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d78cc38db93add6388d8eae7af3d9e1ff7d6342","unresolved":false,"context_lines":[{"line_number":11,"context_line":"nova need to correctly clean up the volume connection on"},{"line_number":12,"context_line":"the host when the last instance is removed."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"currently we do not have a volume level lock to garud the"},{"line_number":15,"context_line":"critical section that determins if the current disconenct is"},{"line_number":16,"context_line":"removing the final usage of the volume."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8d8e0e31_f8a4c995","line":14,"range":{"start_line":14,"start_character":48,"end_line":14,"end_character":53},"in_reply_to":"3024861a_c02ab634","updated":"2025-11-12 10:35:02.000000000","message":"Done","commit_id":"0a15fa8e650d5a981163b9234dca0e116e163326"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9df587cfe8bb1e667e4a4a0c14356b9f2c8fb35d","unresolved":true,"context_lines":[{"line_number":12,"context_line":"the host when the last instance is removed."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"currently we do not have a volume level lock to garud the"},{"line_number":15,"context_line":"critical section that determins if the current disconenct is"},{"line_number":16,"context_line":"removing the final usage of the volume."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This can lead to leakign the volume or other issues as"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"06609f35_1a9ac3d8","line":15,"range":{"start_line":15,"start_character":47,"end_line":15,"end_character":57},"updated":"2024-08-30 08:52:00.000000000","message":"disconnect","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d78cc38db93add6388d8eae7af3d9e1ff7d6342","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the host when the last instance is removed."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"currently we do not have a volume level lock to garud the"},{"line_number":15,"context_line":"critical section that determins if the current disconenct is"},{"line_number":16,"context_line":"removing the final usage of the volume."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This can lead to leakign the volume or other issues as"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"2c1b320a_cea31baf","line":15,"range":{"start_line":15,"start_character":47,"end_line":15,"end_character":57},"in_reply_to":"06609f35_1a9ac3d8","updated":"2025-11-12 10:35:02.000000000","message":"Done","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9df587cfe8bb1e667e4a4a0c14356b9f2c8fb35d","unresolved":true,"context_lines":[{"line_number":15,"context_line":"critical section that determins if the current disconenct is"},{"line_number":16,"context_line":"removing the final usage of the volume."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This can lead to leakign the volume or other issues as"},{"line_number":19,"context_line":"noted in bug: #2048837"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This change introduces a FairLockGuard to ensure we acquire"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"b94f4958_fed5296a","line":18,"range":{"start_line":18,"start_character":17,"end_line":18,"end_character":24},"updated":"2024-08-30 08:52:00.000000000","message":"leaking","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d78cc38db93add6388d8eae7af3d9e1ff7d6342","unresolved":false,"context_lines":[{"line_number":15,"context_line":"critical section that determins if the current disconenct is"},{"line_number":16,"context_line":"removing the final usage of the volume."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This can lead to leakign the volume or other issues as"},{"line_number":19,"context_line":"noted in bug: #2048837"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This change introduces a FairLockGuard to ensure we acquire"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"94a57f8f_4d008e27","line":18,"range":{"start_line":18,"start_character":17,"end_line":18,"end_character":24},"in_reply_to":"b94f4958_fed5296a","updated":"2025-11-12 10:35:02.000000000","message":"Done","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d4fa228440ceb8ed3d155314f9f2a993987d0bec","unresolved":true,"context_lines":[{"line_number":27,"context_line":"in parallel but if we are disconnecting the same volume in multiple"},{"line_number":28,"context_line":"greenthread concurrently they will be serialised."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Closes-Bug: #2048837"},{"line_number":31,"context_line":"Change-Id: I67e10cace451259127a5d7da8fbdf7739afe3e51"},{"line_number":32,"context_line":"Signed-off-by: Sean Mooney \u003cwork@seanmooney.info\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"a4ef1bc3_6afcf5c3","line":30,"updated":"2025-11-13 22:08:13.000000000","message":"this proaably shoudl have a release note too so ill add that in the next version as well","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c4014ec5ebe89af0adda8c4a3def068c794b7016","unresolved":false,"context_lines":[{"line_number":27,"context_line":"in parallel but if we are disconnecting the same volume in multiple"},{"line_number":28,"context_line":"greenthread concurrently they will be serialised."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Closes-Bug: #2048837"},{"line_number":31,"context_line":"Change-Id: I67e10cace451259127a5d7da8fbdf7739afe3e51"},{"line_number":32,"context_line":"Signed-off-by: Sean Mooney \u003cwork@seanmooney.info\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"7b3a9f77_94f55d0e","line":30,"in_reply_to":"a4ef1bc3_6afcf5c3","updated":"2025-11-17 11:34:26.000000000","message":"Done","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"607e194e83293ce9f31003afec709b824fc16ab2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"809bdea6_9bd7aa00","updated":"2024-09-02 14:21:55.000000000","message":"Dropping hard -1 and lock sorting prevents the deadlock.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"289822a4db385f3eb66628546001608c56c0c17c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"400f3142_6829bb84","updated":"2024-09-02 08:15:35.000000000","message":"I\u0027m hard -1 due to a potential deadlock.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8f32ff098db42f4ad00421398bbef6eff39774d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4351376b_3012edd2","updated":"2024-08-01 14:54:15.000000000","message":"just a few nits","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a900f17d3754e8c15efe3e06b750224f1ed0d8ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"7af34406_cc585922","updated":"2024-09-04 10:56:19.000000000","message":"lgtm","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"df4b598bfa523cb432411302451c69c295aa01db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"2c49cf0b_b99e63cd","updated":"2024-05-27 15:57:48.000000000","message":"ok tempest-integrated-compute times out but ill hold the recheck until this has been reviewed","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da656e3cd7f669b8108851d83f2f952f4d625f44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0c3c85ed_d3e0fc7c","updated":"2024-07-20 06:15:06.000000000","message":"recheck new results","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"747974fc7907d4b5a0af7e87106df29dd239fdbc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fb5eb5d9_324045b3","updated":"2024-06-10 12:04:26.000000000","message":"recheck timeout","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d8bd4d8d99c9a270f0073bb1bc4ccfaddd2c5b9b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"bacfe7fa_caa955fc","updated":"2024-09-03 15:06:57.000000000","message":"we are pas","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"956ea56b_9cea422e","updated":"2025-11-14 22:16:32.000000000","message":"Almost looks good but there are some cases where \u0027fair\u0027ness of lock is not honored and also it making the things serial which could go in parallel.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"72b7ce5c_c394c5a2","updated":"2025-11-13 22:07:29.000000000","message":"ill do another revision of this to address the pending feedback but i really dont want to keep expanding the scope of the FairLockGuard more.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0fe9dadbd64c4fa95ff933c74321d00ed9846c58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"2d9517ac_d187fa56","updated":"2025-11-13 23:24:48.000000000","message":"teim-ci: manual","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"353c190964e18d6df43e86a15877b71bc14147a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"15d215ab_c9675127","updated":"2025-11-17 13:38:55.000000000","message":"Looks good to me. Thanks","commit_id":"22012360c40045ac1bf6dd0dc95aea3e15fed0cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"14c76b3c30bdb1c3ce76cdbbba179510dd7f29f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"de036d54_5cdddf12","updated":"2025-12-04 01:57:47.000000000","message":"recheck","commit_id":"22012360c40045ac1bf6dd0dc95aea3e15fed0cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"de4c01c21de8d0a3c0a093d4cc7a2852551587a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"d184544f_d9a16099","updated":"2025-11-24 14:22:21.000000000","message":"recheck nova alt config failed ot install ectd \n\n++ functions:get_extra_file:68              :   wget --progress\u003ddot:giga -t 2 -c https://github.com/etcd-io/etcd/releases/download/v3.5.21/etcd-v3.5.21-linux-amd64.tar.gz -O /opt/stack/devstack/files/etcd-v3.5.21-linux-amd64.tar.gz\n--2025-11-20 19:01:29--  https://github.com/etcd-io/etcd/releases/download/v3.5.21/etcd-v3.5.21-linux-amd64.tar.gz\nResolving github.com (github.com)... 140.82.121.3\nConnecting to github.com (github.com)|140.82.121.3|:443... connected.\nHTTP request sent, awaiting response... 503 Service Unavailable","commit_id":"22012360c40045ac1bf6dd0dc95aea3e15fed0cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0bc787f3d3001a5806207dad195207d647fca452","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"a695297f_fe0b5be3","updated":"2025-11-20 18:35:58.000000000","message":"recheck test_resize_volume_backed_server_confirm ssh timeout in grenade which is not related to this change","commit_id":"22012360c40045ac1bf6dd0dc95aea3e15fed0cd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"96d4013d0e274651ee6a6f1fa3c05c276baaa11e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"bd9ec9ca_f3a94a5e","updated":"2025-11-24 18:17:40.000000000","message":"retry etcd download error again.\n\nim going to do a poc to add a retry on that in devstack\n\nlooking at opensarch we had 32 other failure of this todya so it looks liek a tempory issues.\n\ni was able to download the tar locally just fine.","commit_id":"22012360c40045ac1bf6dd0dc95aea3e15fed0cd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"cee09d88259db71defd7f8c995dc13aff0b4e3c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"04c3b5ef_b2fe4239","updated":"2025-11-20 18:35:40.000000000","message":"thanks for updates, lgtm now.","commit_id":"22012360c40045ac1bf6dd0dc95aea3e15fed0cd"}],"nova/compute/manager.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8f32ff098db42f4ad00421398bbef6eff39774d3","unresolved":true,"context_lines":[{"line_number":22,"context_line":"building a disk image, launching it via the underlying virtualization driver,"},{"line_number":23,"context_line":"responding to calls to check its state, attaching persistent storage, and"},{"line_number":24,"context_line":"terminating it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import base64"}],"source_content_type":"text/x-python","patch_set":7,"id":"d6c0e5e7_e43791cc","side":"PARENT","line":25,"updated":"2024-08-01 14:54:15.000000000","message":"unrelated change","commit_id":"2cdf0e4fb544a65aefa8435c22441c01e9bde549"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d78cc38db93add6388d8eae7af3d9e1ff7d6342","unresolved":false,"context_lines":[{"line_number":22,"context_line":"building a disk image, launching it via the underlying virtualization driver,"},{"line_number":23,"context_line":"responding to calls to check its state, attaching persistent storage, and"},{"line_number":24,"context_line":"terminating it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import base64"}],"source_content_type":"text/x-python","patch_set":7,"id":"04994bb0_c04549c0","side":"PARENT","line":25,"in_reply_to":"b4fc3b5b_67fdecc0","updated":"2025-11-12 10:35:02.000000000","message":"Acknowledged","commit_id":"2cdf0e4fb544a65aefa8435c22441c01e9bde549"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c8a28f36a8ee59cc11d983ec0f10b5b18f606fe7","unresolved":true,"context_lines":[{"line_number":22,"context_line":"building a disk image, launching it via the underlying virtualization driver,"},{"line_number":23,"context_line":"responding to calls to check its state, attaching persistent storage, and"},{"line_number":24,"context_line":"terminating it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import base64"}],"source_content_type":"text/x-python","patch_set":7,"id":"b4fc3b5b_67fdecc0","side":"PARENT","line":25,"in_reply_to":"d6c0e5e7_e43791cc","updated":"2024-09-03 15:27:15.000000000","message":"i believe this  pep 257 guidelines recommend that docstring should not end in an empty new line\n\nhttps://peps.python.org/pep-0257/#multi-line-docstrings\n\nthis was done by autopep8 if i recall correctly and tools like docutils rely on the convetions in that pep.\n\nhttps://peps.python.org/pep-0008/#documentation-strings even show the example where there is no blank line before the closing triple quote\n\nthe imporant part is the last sentence here\nhttps://peps.python.org/pep-0257/#handling-docstring-indentation\n```\nDocstring processing tools will strip a uniform amount of indentation from the second and further lines of the docstring, equal to the minimum indentation of all non-blank lines after the first line.\nAny indentation in the first line of the docstring (i.e., up to the first newline) is insignificant and removed.\nRelative indentation of later lines in the docstring is retained.\nBlank lines should be removed from the beginning and end of the docstring.\n```","commit_id":"2cdf0e4fb544a65aefa8435c22441c01e9bde549"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8f32ff098db42f4ad00421398bbef6eff39774d3","unresolved":true,"context_lines":[{"line_number":3282,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3283,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3284,"context_line":"        with utils.FairLockGuard("},{"line_number":3285,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3286,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3287,"context_line":""},{"line_number":3288,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":7,"id":"3000a0ea_d9a43d6b","line":3285,"updated":"2024-08-01 14:54:15.000000000","message":"please add another tab","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5d78cc38db93add6388d8eae7af3d9e1ff7d6342","unresolved":false,"context_lines":[{"line_number":3282,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3283,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3284,"context_line":"        with utils.FairLockGuard("},{"line_number":3285,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3286,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3287,"context_line":""},{"line_number":3288,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a9408a5_c70be1b3","line":3285,"in_reply_to":"207130b0_283a9add","updated":"2025-11-12 10:35:02.000000000","message":"Acknowledged","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9df587cfe8bb1e667e4a4a0c14356b9f2c8fb35d","unresolved":true,"context_lines":[{"line_number":3282,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3283,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3284,"context_line":"        with utils.FairLockGuard("},{"line_number":3285,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3286,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3287,"context_line":""},{"line_number":3288,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":7,"id":"c0336266_8f670100","line":3285,"in_reply_to":"3000a0ea_d9a43d6b","updated":"2024-08-30 08:52:00.000000000","message":"+1","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"289822a4db385f3eb66628546001608c56c0c17c","unresolved":true,"context_lines":[{"line_number":3282,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3283,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3284,"context_line":"        with utils.FairLockGuard("},{"line_number":3285,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3286,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3287,"context_line":""},{"line_number":3288,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":7,"id":"c2c5c8cf_aa4c629b","line":3285,"in_reply_to":"c0336266_8f670100","updated":"2024-09-02 08:15:35.000000000","message":"or move the `):` to a new line\n\n// Can we have black please to avoid these kind of debates?","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c8a28f36a8ee59cc11d983ec0f10b5b18f606fe7","unresolved":true,"context_lines":[{"line_number":3282,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3283,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3284,"context_line":"        with utils.FairLockGuard("},{"line_number":3285,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3286,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3287,"context_line":""},{"line_number":3288,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":7,"id":"207130b0_283a9add","line":3285,"in_reply_to":"c2c5c8cf_aa4c629b","updated":"2024-09-03 15:27:15.000000000","message":"annother tab would be a pep8 issues and would require the moving of ): to a new line to be corect\n\nand i know dan does not like when i do that\n\nbut i agree with gibi im really want to stop having this converation and just us a tool\n\nblack, ruff format, i dont actully care. \n\ni ahve already added autopep8 which fixes pep 8 issues for us and this passes flake8/hacking checks so this type of comemnt is an example of something we should avoid doing.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":true,"context_lines":[{"line_number":22,"context_line":"building a disk image, launching it via the underlying virtualization driver,"},{"line_number":23,"context_line":"responding to calls to check its state, attaching persistent storage, and"},{"line_number":24,"context_line":"terminating it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import base64"}],"source_content_type":"text/x-python","patch_set":8,"id":"94222892_b0715de7","side":"PARENT","line":25,"updated":"2025-11-12 11:33:54.000000000","message":"whitespace damage","commit_id":"fac1a4d9de977cbc5579c51ff8c21f587ced8e22"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bdeea17a1f1809484457725799491d1b689fa822","unresolved":true,"context_lines":[{"line_number":22,"context_line":"building a disk image, launching it via the underlying virtualization driver,"},{"line_number":23,"context_line":"responding to calls to check its state, attaching persistent storage, and"},{"line_number":24,"context_line":"terminating it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import base64"}],"source_content_type":"text/x-python","patch_set":8,"id":"d55536c2_ee57106c","side":"PARENT","line":25,"in_reply_to":"94222892_b0715de7","updated":"2025-11-12 12:23:21.000000000","message":"i kept this intentionally as noted in the previous revsion.\n\nhttps://review.opendev.org/c/openstack/nova/+/916322/comment/d6c0e5e7_e43791cc/\n\nthis is actully required or at least recommend by pep8","commit_id":"fac1a4d9de977cbc5579c51ff8c21f587ced8e22"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":false,"context_lines":[{"line_number":22,"context_line":"building a disk image, launching it via the underlying virtualization driver,"},{"line_number":23,"context_line":"responding to calls to check its state, attaching persistent storage, and"},{"line_number":24,"context_line":"terminating it."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import base64"}],"source_content_type":"text/x-python","patch_set":8,"id":"fb771012_86b6a530","side":"PARENT","line":25,"in_reply_to":"d55536c2_ee57106c","updated":"2025-11-13 08:17:29.000000000","message":"Acknowledged","commit_id":"fac1a4d9de977cbc5579c51ff8c21f587ced8e22"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":true,"context_lines":[{"line_number":3355,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3356,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3357,"context_line":"        with utils.FairLockGuard("},{"line_number":3358,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3359,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":8,"id":"02b9742b_a291afd1","line":3358,"updated":"2025-11-12 11:33:54.000000000","message":"* for readability I would put the `):` to the next line to visually separate the context manager code from the code under the context.\n\n* I think it worth to note that even if the order of the bdms in the list is not stable it does not matter as the FairLockGuard will take care of ordering.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bdeea17a1f1809484457725799491d1b689fa822","unresolved":true,"context_lines":[{"line_number":3355,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3356,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3357,"context_line":"        with utils.FairLockGuard("},{"line_number":3358,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3359,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":8,"id":"1b4eec29_53114f97","line":3358,"in_reply_to":"02b9742b_a291afd1","updated":"2025-11-12 12:23:21.000000000","message":"when i do that i normally get asked not to so i didnt change this intentionaly","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[{"line_number":3355,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3356,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3357,"context_line":"        with utils.FairLockGuard("},{"line_number":3358,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)):"},{"line_number":3359,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"            # NOTE(vish): We have already deleted the instance, so we have"}],"source_content_type":"text/x-python","patch_set":8,"id":"7656885a_7f45d864","line":3358,"in_reply_to":"1b4eec29_53114f97","updated":"2025-11-13 22:07:29.000000000","message":"Done","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"92de8a7fd745d2d4bb8d3d426c468c81386cfa4d","unresolved":false,"context_lines":[{"line_number":3355,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3356,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3357,"context_line":"        with utils.FairLockGuard("},{"line_number":3358,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)"},{"line_number":3359,"context_line":"        ):"},{"line_number":3360,"context_line":"            self._shutdown_instance(context, instance, bdms)"},{"line_number":3361,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"6e926d5c_71223976","line":3358,"range":{"start_line":3358,"start_character":57,"end_line":3358,"end_character":61},"updated":"2025-11-14 22:22:09.000000000","message":"other more optimized solution I was thinking to aquire lock per multiattach volume in bdms and let parallel call to each volume cleanup unstead of per delete instance lock. But that might be too complex and for that we need to inject the lock at more deeper level. I think current way is good improvement but if later we want to make it more parallel then lock per vol in bdms is something to try.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"92de8a7fd745d2d4bb8d3d426c468c81386cfa4d","unresolved":true,"context_lines":[{"line_number":3376,"context_line":""},{"line_number":3377,"context_line":"            self._cleanup_volumes(context, instance, bdms,"},{"line_number":3378,"context_line":"                    raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":3379,"context_line":"            # if a delete task succeeded, always update vm state and task"},{"line_number":3380,"context_line":"            # state without expecting task state to be DELETING"},{"line_number":3381,"context_line":"            instance.vm_state \u003d vm_states.DELETED"},{"line_number":3382,"context_line":"            instance.task_state \u003d None"},{"line_number":3383,"context_line":"            instance.power_state \u003d power_state.NOSTATE"},{"line_number":3384,"context_line":"            instance.terminated_at \u003d timeutils.utcnow()"},{"line_number":3385,"context_line":"            instance.save()"},{"line_number":3386,"context_line":""},{"line_number":3387,"context_line":"            self._complete_deletion(context, instance)"},{"line_number":3388,"context_line":"            # only destroy the instance in the db if the _complete_deletion"},{"line_number":3389,"context_line":"            # doesn\u0027t raise and therefore allocation is successfully"},{"line_number":3390,"context_line":"            # deleted in placement"},{"line_number":3391,"context_line":"            instance.destroy()"},{"line_number":3392,"context_line":""},{"line_number":3393,"context_line":"        self._notify_about_instance_usage(context, instance, \"delete.end\")"},{"line_number":3394,"context_line":"        compute_utils.notify_about_instance_action(context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"a206df11_ae6901f6","line":3391,"range":{"start_line":3379,"start_character":0,"end_line":3391,"end_character":30},"updated":"2025-11-14 22:22:09.000000000","message":"do we need to keep these also under lock?","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7eb346f9d3e68238aba04fae5c2a7d09763a7333","unresolved":false,"context_lines":[{"line_number":3376,"context_line":""},{"line_number":3377,"context_line":"            self._cleanup_volumes(context, instance, bdms,"},{"line_number":3378,"context_line":"                    raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":3379,"context_line":"            # if a delete task succeeded, always update vm state and task"},{"line_number":3380,"context_line":"            # state without expecting task state to be DELETING"},{"line_number":3381,"context_line":"            instance.vm_state \u003d vm_states.DELETED"},{"line_number":3382,"context_line":"            instance.task_state \u003d None"},{"line_number":3383,"context_line":"            instance.power_state \u003d power_state.NOSTATE"},{"line_number":3384,"context_line":"            instance.terminated_at \u003d timeutils.utcnow()"},{"line_number":3385,"context_line":"            instance.save()"},{"line_number":3386,"context_line":""},{"line_number":3387,"context_line":"            self._complete_deletion(context, instance)"},{"line_number":3388,"context_line":"            # only destroy the instance in the db if the _complete_deletion"},{"line_number":3389,"context_line":"            # doesn\u0027t raise and therefore allocation is successfully"},{"line_number":3390,"context_line":"            # deleted in placement"},{"line_number":3391,"context_line":"            instance.destroy()"},{"line_number":3392,"context_line":""},{"line_number":3393,"context_line":"        self._notify_about_instance_usage(context, instance, \"delete.end\")"},{"line_number":3394,"context_line":"        compute_utils.notify_about_instance_action(context, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ea3046f0_e1f7917a","line":3391,"range":{"start_line":3379,"start_character":0,"end_line":3391,"end_character":30},"in_reply_to":"a206df11_ae6901f6","updated":"2025-11-17 10:56:05.000000000","message":"the instance.save() need to be included\n\nself._complete_deletion(context, instance)\n\nensure the resouce tracker if fully up to date but that and the destroy are not required","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"406b5fa43b89e30ba417425fa66f3b8e9c5abd46","unresolved":true,"context_lines":[{"line_number":3360,"context_line":"                self.host, action\u003dfields.NotificationAction.DELETE,"},{"line_number":3361,"context_line":"                phase\u003dfields.NotificationPhase.START, bdms\u003dbdms)"},{"line_number":3362,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3363,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3364,"context_line":"        with utils.FairLockGuard("},{"line_number":3365,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)"},{"line_number":3366,"context_line":"        ):"}],"source_content_type":"text/x-python","patch_set":12,"id":"1527a500_4d10255d","line":3363,"updated":"2025-11-17 12:08:34.000000000","message":"Sorry that I\u0027m just noted this. Before the change delete_arqs_if_needed() was celled after self._shutdown_instance. So we were sure that the ARQs are not used any more by the instance when we signal cyborg that the devices in the ARQs are now free. \nHowever in the new code ARQ deletion happen before shutdown_instance. So this can cause race condition where cyborg thinks a device is free but the instance is still using it as shutdown is not yet called. I think this is a bug.","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fd217125651921add3850e6412042933d46e60e5","unresolved":true,"context_lines":[{"line_number":3360,"context_line":"                self.host, action\u003dfields.NotificationAction.DELETE,"},{"line_number":3361,"context_line":"                phase\u003dfields.NotificationPhase.START, bdms\u003dbdms)"},{"line_number":3362,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3363,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3364,"context_line":"        with utils.FairLockGuard("},{"line_number":3365,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)"},{"line_number":3366,"context_line":"        ):"}],"source_content_type":"text/x-python","patch_set":12,"id":"ecb8a315_977fe84c","line":3363,"in_reply_to":"1527a500_4d10255d","updated":"2025-11-17 13:16:10.000000000","message":"oh yep let me fix that quickly\n\nthat was not intentionaly i think it was jsut a rebase issue","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6efdd4b41dd3470a3cc5de23924283ba6dc7a388","unresolved":false,"context_lines":[{"line_number":3360,"context_line":"                self.host, action\u003dfields.NotificationAction.DELETE,"},{"line_number":3361,"context_line":"                phase\u003dfields.NotificationPhase.START, bdms\u003dbdms)"},{"line_number":3362,"context_line":"        # Delete Cyborg ARQs if the instance has a device profile."},{"line_number":3363,"context_line":"        compute_utils.delete_arqs_if_needed(context, instance)"},{"line_number":3364,"context_line":"        with utils.FairLockGuard("},{"line_number":3365,"context_line":"            self._get_multiattach_volume_lock_names_bdms(bdms)"},{"line_number":3366,"context_line":"        ):"}],"source_content_type":"text/x-python","patch_set":12,"id":"8843f247_7ff91da1","line":3363,"in_reply_to":"ecb8a315_977fe84c","updated":"2025-11-17 13:28:17.000000000","message":"Done","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"}],"nova/objects/block_device.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f918e87557dbf17873e442d587af2ea79b76f17","unresolved":true,"context_lines":[{"line_number":323,"context_line":"            return False"},{"line_number":324,"context_line":"        if \u0027connection_info\u0027 not in self:"},{"line_number":325,"context_line":"            return False"},{"line_number":326,"context_line":"        info \u003d jsonutils.loads(self.connection_info)"},{"line_number":327,"context_line":"        return info.get(\"multiattach\", False)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":6,"id":"174b346d_831045e4","line":326,"updated":"2024-05-22 00:32:40.000000000","message":"i also need to return false if self.connection_info is expictly set to None…","commit_id":"12703d9e10a1d9f6d739fb25620708934629437a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"18b1e9a5d4e0d0da67f8445036377c7bd87bca92","unresolved":false,"context_lines":[{"line_number":323,"context_line":"            return False"},{"line_number":324,"context_line":"        if \u0027connection_info\u0027 not in self:"},{"line_number":325,"context_line":"            return False"},{"line_number":326,"context_line":"        info \u003d jsonutils.loads(self.connection_info)"},{"line_number":327,"context_line":"        return info.get(\"multiattach\", False)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":6,"id":"d80c8836_f7e97efa","line":326,"in_reply_to":"174b346d_831045e4","updated":"2024-05-27 15:57:59.000000000","message":"Done","commit_id":"12703d9e10a1d9f6d739fb25620708934629437a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"10ebfd220de148b3cc52d3637f290f47fa0776e9","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                    fields.BlockDeviceDestinationType.VOLUME)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    @property"},{"line_number":321,"context_line":"    def is_multiattach(self):"},{"line_number":322,"context_line":"        \"\"\"Return whether the volume is multiattach."},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        Note that this property is only valid for volumes that are attached to"}],"source_content_type":"text/x-python","patch_set":11,"id":"e461548a_bdcf652f","line":321,"in_reply_to":"48e18c52_8c6cc6a7","updated":"2025-11-14 00:32:55.000000000","message":"```\n[severity: suggestion] Consider early return in is_multiattach property - Confidence: 0.8\nLocation: nova/objects/block_device.py:321-338\nBenefit: Improves readability and reduces nesting\nRecommendation: Flatten the if-else structure to return early on each condition\n```\n\nwell i am already doing early return\n\ni could combine the 3 ifs and have on ereturn false.\n\nso instead of \n\n```\n        if not self.is_volume:\n            return False\n        if \u0027connection_info\u0027 not in self:\n            return False\n        if not self.connection_info:\n            return False\n```\n\ni coudl do\n\n```\n        if (not self.is_volume or \u0027connection_info\u0027 not in self\n            or not self.connection_info):\n            return False\n```\ni guess i could\n\ni feel liek there are effectivy 2 checcks\n\n\n```\n        if not self.is_volume:\n            return False\n        if \u0027connection_info\u0027 not in self or not self.connection_info:\n            return False\n```\n\nbut honestly all 3 of these are personal tasty and i tend to graviate to the last but\ni dont think this really imporved readblity.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"10ebfd220de148b3cc52d3637f290f47fa0776e9","unresolved":false,"context_lines":[{"line_number":334,"context_line":"            return False"},{"line_number":335,"context_line":"        if not self.connection_info:"},{"line_number":336,"context_line":"            return False"},{"line_number":337,"context_line":"        info \u003d jsonutils.loads(self.connection_info)"},{"line_number":338,"context_line":"        return info.get(\"multiattach\", False)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":11,"id":"1645e278_e6a1ffb2","line":337,"in_reply_to":"e0be7b14_dbe40317","updated":"2025-11-14 00:32:55.000000000","message":"```\n[severity: warning] Potential JSON parsing error in is_multiattach property - Confidence: 0.8\nLocation: nova/objects/block_device.py:337\nImpact: Could cause  if connection_info is malformed JSON\nSuggestion: Add try/except around jsonutils.loads() to handle malformed JSON gracefully\n```\nits not going to raise an AttributeErroror  here or on the return line but it could raise JSONDecodeError or UnicodeDecodeError\n\ni coudl catch thos and returhn false but i actully think just letting this bunnle up might be better.\n\nif we cant deserialist this it means theat there is currption in the our bdms.\n\nwe now heal that on reboot so this is vanishingly unlikely to happen","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"}],"nova/tests/functional/regressions/test_bug_2048837.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        self.lock.acquire()"},{"line_number":96,"context_line":"        self.api.delete_server(self.server_a[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.api.delete_server(self.server_b[\u0027id\u0027])"},{"line_number":98,"context_line":"        self.disconnect_volume_mock.assert_not_called()"},{"line_number":99,"context_line":"        self.lock.release()"},{"line_number":100,"context_line":"        self._wait_until_deleted(self.server_a)"},{"line_number":101,"context_line":"        self._wait_until_deleted(self.server_b)"}],"source_content_type":"text/x-python","patch_set":8,"id":"470ef961_08d169fa","line":98,"updated":"2025-11-12 11:33:54.000000000","message":"I think it worth a note why the lock is used. \nI think it is used to queue up both server delete to call _should_disconnect(). But at the same time it forces the serialization of the two calls to _should_disconnect() too so I\u0027m not sure how that is not breaking the goal to have truly concurrent server deletes here.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bdeea17a1f1809484457725799491d1b689fa822","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        self.lock.acquire()"},{"line_number":96,"context_line":"        self.api.delete_server(self.server_a[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.api.delete_server(self.server_b[\u0027id\u0027])"},{"line_number":98,"context_line":"        self.disconnect_volume_mock.assert_not_called()"},{"line_number":99,"context_line":"        self.lock.release()"},{"line_number":100,"context_line":"        self._wait_until_deleted(self.server_a)"},{"line_number":101,"context_line":"        self._wait_until_deleted(self.server_b)"}],"source_content_type":"text/x-python","patch_set":8,"id":"c7f38829_f9d722da","line":98,"in_reply_to":"470ef961_08d169fa","updated":"2025-11-12 12:23:21.000000000","message":"yes your correct the loc is there to create the concurrent delete sequnce. hence the comment on line 94.\n\ni can make that more explcit.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        self.lock.acquire()"},{"line_number":96,"context_line":"        self.api.delete_server(self.server_a[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.api.delete_server(self.server_b[\u0027id\u0027])"},{"line_number":98,"context_line":"        self.disconnect_volume_mock.assert_not_called()"},{"line_number":99,"context_line":"        self.lock.release()"},{"line_number":100,"context_line":"        self._wait_until_deleted(self.server_a)"},{"line_number":101,"context_line":"        self._wait_until_deleted(self.server_b)"}],"source_content_type":"text/x-python","patch_set":8,"id":"f542f3a5_fb3c577a","line":98,"in_reply_to":"c7f38829_f9d722da","updated":"2025-11-13 22:07:29.000000000","message":"Done","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        self.lock.acquire()"},{"line_number":96,"context_line":"        self.api.delete_server(self.server_a[\u0027id\u0027])"},{"line_number":97,"context_line":"        self.api.delete_server(self.server_b[\u0027id\u0027])"},{"line_number":98,"context_line":"        self.disconnect_volume_mock.assert_not_called()"},{"line_number":99,"context_line":"        self.lock.release()"},{"line_number":100,"context_line":"        self._wait_until_deleted(self.server_a)"},{"line_number":101,"context_line":"        self._wait_until_deleted(self.server_b)"}],"source_content_type":"text/x-python","patch_set":8,"id":"14fb1264_58fd84dd","line":98,"in_reply_to":"c7f38829_f9d722da","updated":"2025-11-13 08:17:29.000000000","message":"thanks. It make sense to me now.\n\n(not something to change here but a higher level primitive would have made the intention clearer. E.g. a Barrier of 3 to wait all 3 threads to reach L76 and L113 before we let all three continue over the barrier. Or an Event that is waited on L76 and set on L113)","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":true,"context_lines":[{"line_number":90,"context_line":"        self._delete_server(self.server_a)"},{"line_number":91,"context_line":"        self.should_disconnect_mock.assert_called()"},{"line_number":92,"context_line":"        self.disconnect_volume_mock.assert_not_called()"},{"line_number":93,"context_line":"        self._delete_server(self.server_b)"},{"line_number":94,"context_line":"        self.disconnect_volume_mock.assert_called()"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_concurrent_server_delete(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"976d4978_b1fde01e","line":93,"range":{"start_line":93,"start_character":41,"end_line":93,"end_character":42},"updated":"2025-11-14 22:16:32.000000000","message":"we can check self.should_disconnect_mock.assert_called() just to make sure 2nd delete also go through the self.should_disconnect method.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7eb346f9d3e68238aba04fae5c2a7d09763a7333","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self._delete_server(self.server_a)"},{"line_number":91,"context_line":"        self.should_disconnect_mock.assert_called()"},{"line_number":92,"context_line":"        self.disconnect_volume_mock.assert_not_called()"},{"line_number":93,"context_line":"        self._delete_server(self.server_b)"},{"line_number":94,"context_line":"        self.disconnect_volume_mock.assert_called()"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    def test_concurrent_server_delete(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"5410b1dd_9594af32","line":93,"range":{"start_line":93,"start_character":41,"end_line":93,"end_character":42},"in_reply_to":"976d4978_b1fde01e","updated":"2025-11-17 10:56:05.000000000","message":"Done","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        self.lock.release_write_lock()"},{"line_number":114,"context_line":"        self._wait_until_deleted(self.server_a)"},{"line_number":115,"context_line":"        self._wait_until_deleted(self.server_b)"},{"line_number":116,"context_line":"        self.should_disconnect_mock.assert_called()"},{"line_number":117,"context_line":"        # this validates bug 2048837"},{"line_number":118,"context_line":"        self.disconnect_volume_mock.assert_called()"}],"source_content_type":"text/x-python","patch_set":11,"id":"980668ff_de349d8a","line":116,"range":{"start_line":116,"start_character":35,"end_line":116,"end_character":51},"updated":"2025-11-14 22:16:32.000000000","message":"we can assert here that it is called twice so that we know both instance delete tried to check should_disconnect but endup in race.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7eb346f9d3e68238aba04fae5c2a7d09763a7333","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        self.lock.release_write_lock()"},{"line_number":114,"context_line":"        self._wait_until_deleted(self.server_a)"},{"line_number":115,"context_line":"        self._wait_until_deleted(self.server_b)"},{"line_number":116,"context_line":"        self.should_disconnect_mock.assert_called()"},{"line_number":117,"context_line":"        # this validates bug 2048837"},{"line_number":118,"context_line":"        self.disconnect_volume_mock.assert_called()"}],"source_content_type":"text/x-python","patch_set":11,"id":"1dfba20d_2f51718f","line":116,"range":{"start_line":116,"start_character":35,"end_line":116,"end_character":51},"in_reply_to":"980668ff_de349d8a","updated":"2025-11-17 10:56:05.000000000","message":"Done","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"}],"nova/tests/unit/test_utils.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8f32ff098db42f4ad00421398bbef6eff39774d3","unresolved":true,"context_lines":[{"line_number":1456,"context_line":"        for lock in test_locks:"},{"line_number":1457,"context_line":"            self.assertFalse(lock.is_writer())"},{"line_number":1458,"context_line":""},{"line_number":1459,"context_line":"    def test_partial_aquire(self):"},{"line_number":1460,"context_line":"        lock_names \u003d [\u0027test_partial_aquire1\u0027, \u0027test_partial_aquire2\u0027]"},{"line_number":1461,"context_line":"        test_locks \u003d [utils.NOVA_FAIR_LOCKS.get("},{"line_number":1462,"context_line":"            lock_name) for lock_name in lock_names]"}],"source_content_type":"text/x-python","patch_set":7,"id":"b4af5b01_8644dfa2","line":1459,"range":{"start_line":1459,"start_character":8,"end_line":1459,"end_character":27},"updated":"2024-08-01 14:54:15.000000000","message":"nit : acquire","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bdeea17a1f1809484457725799491d1b689fa822","unresolved":false,"context_lines":[{"line_number":1456,"context_line":"        for lock in test_locks:"},{"line_number":1457,"context_line":"            self.assertFalse(lock.is_writer())"},{"line_number":1458,"context_line":""},{"line_number":1459,"context_line":"    def test_partial_aquire(self):"},{"line_number":1460,"context_line":"        lock_names \u003d [\u0027test_partial_aquire1\u0027, \u0027test_partial_aquire2\u0027]"},{"line_number":1461,"context_line":"        test_locks \u003d [utils.NOVA_FAIR_LOCKS.get("},{"line_number":1462,"context_line":"            lock_name) for lock_name in lock_names]"}],"source_content_type":"text/x-python","patch_set":7,"id":"dae69f2f_04ed9db0","line":1459,"range":{"start_line":1459,"start_character":8,"end_line":1459,"end_character":27},"in_reply_to":"67c195bf_2beb26fd","updated":"2025-11-12 12:23:21.000000000","message":"Done","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c8a28f36a8ee59cc11d983ec0f10b5b18f606fe7","unresolved":true,"context_lines":[{"line_number":1456,"context_line":"        for lock in test_locks:"},{"line_number":1457,"context_line":"            self.assertFalse(lock.is_writer())"},{"line_number":1458,"context_line":""},{"line_number":1459,"context_line":"    def test_partial_aquire(self):"},{"line_number":1460,"context_line":"        lock_names \u003d [\u0027test_partial_aquire1\u0027, \u0027test_partial_aquire2\u0027]"},{"line_number":1461,"context_line":"        test_locks \u003d [utils.NOVA_FAIR_LOCKS.get("},{"line_number":1462,"context_line":"            lock_name) for lock_name in lock_names]"}],"source_content_type":"text/x-python","patch_set":7,"id":"67c195bf_2beb26fd","line":1459,"range":{"start_line":1459,"start_character":8,"end_line":1459,"end_character":27},"in_reply_to":"b4af5b01_8644dfa2","updated":"2024-09-03 15:27:15.000000000","message":"looking at the code review feedback i think this is the only comment that need to be adressed.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":true,"context_lines":[{"line_number":1786,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1787,"context_line":"        lock_acquired.wait()"},{"line_number":1788,"context_line":"        # Note the first lock is still held by the worker thread"},{"line_number":1789,"context_line":"        # so this will block until the worker releases it"},{"line_number":1790,"context_line":"        with lock_guard:"},{"line_number":1791,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1792,"context_line":"            self.assertTrue(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":9,"id":"4e3b0ecc_10d45102","line":1789,"updated":"2025-11-13 08:17:29.000000000","message":"We should assert that. \nYou can have another event set by the worker after relase_write_lock() and you can assert in below under the context manager that such event is set.","commit_id":"923ca552780a04b99d6d7ec41a7ef46ae1d5cc83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7eb346f9d3e68238aba04fae5c2a7d09763a7333","unresolved":false,"context_lines":[{"line_number":1786,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1787,"context_line":"        lock_acquired.wait()"},{"line_number":1788,"context_line":"        # Note the first lock is still held by the worker thread"},{"line_number":1789,"context_line":"        # so this will block until the worker releases it"},{"line_number":1790,"context_line":"        with lock_guard:"},{"line_number":1791,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1792,"context_line":"            self.assertTrue(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":9,"id":"34144106_4c011e1c","line":1789,"in_reply_to":"4e3b0ecc_10d45102","updated":"2025-11-17 10:56:05.000000000","message":"Done","commit_id":"923ca552780a04b99d6d7ec41a7ef46ae1d5cc83"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":true,"context_lines":[{"line_number":1794,"context_line":"        thread.join()"},{"line_number":1795,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1796,"context_line":"        self.assertFalse(test_locks[0].is_writer())"},{"line_number":1797,"context_line":"        self.assertFalse(test_locks[1].is_writer())"}],"source_content_type":"text/x-python","patch_set":11,"id":"4f2dedc2_bc548f23","line":1797,"range":{"start_line":1797,"start_character":50,"end_line":1797,"end_character":51},"updated":"2025-11-14 22:16:32.000000000","message":"also can we add a tests which can show the thread comes first get the all their required lock first and thread came later (even they want subset of lock what T1 want) will be going after T1. Basically for my comment in utils file.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7eb346f9d3e68238aba04fae5c2a7d09763a7333","unresolved":false,"context_lines":[{"line_number":1794,"context_line":"        thread.join()"},{"line_number":1795,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1796,"context_line":"        self.assertFalse(test_locks[0].is_writer())"},{"line_number":1797,"context_line":"        self.assertFalse(test_locks[1].is_writer())"}],"source_content_type":"text/x-python","patch_set":11,"id":"8ab856e8_2b4c29fe","line":1797,"range":{"start_line":1797,"start_character":50,"end_line":1797,"end_character":51},"in_reply_to":"4f2dedc2_bc548f23","updated":"2025-11-17 10:56:05.000000000","message":"im not really sure i wnat to make that a guarentee.\n\nyes that shoudl technically happen but all we really need to have as our garentee is forward progress ideally with some level of fairness to prevent starvation of specific calls so that they do not hit timeouts.\n\nwith that said sure ill add a test for this.\n\nthe behaviour of the fairness and how that is defied is a atirbute of fastener and oslo that is out of scope of nova. if they had a more optimal defieniton of fiarness for example somethign that minimesed over all wait tiem ranter then perseving the intiall thread request order that would still be fair form our perspective.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"406b5fa43b89e30ba417425fa66f3b8e9c5abd46","unresolved":true,"context_lines":[{"line_number":1799,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1800,"context_line":"            self.assertTrue(test_locks[0].is_writer())"},{"line_number":1801,"context_line":"            self.assertTrue(test_locks[1].is_writer())"},{"line_number":1802,"context_line":"            self.assertTrue(lock_released.wait(timeout\u003d1))"},{"line_number":1803,"context_line":"        thread.join()"},{"line_number":1804,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1805,"context_line":"        self.assertFalse(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":12,"id":"be49bc6b_e89e5168","line":1802,"updated":"2025-11-17 12:08:34.000000000","message":"I would reduce the timeout here to \u003c 1 second as the sleep in the worker is also 1 second. We should not have to wait here that much as we know that test_lock[0] is released as lock_guard allowed to enter. If there is a bug and the lock guard enters too early then the 1 second sleep actually masks that we are entered too early.","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6efdd4b41dd3470a3cc5de23924283ba6dc7a388","unresolved":false,"context_lines":[{"line_number":1799,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1800,"context_line":"            self.assertTrue(test_locks[0].is_writer())"},{"line_number":1801,"context_line":"            self.assertTrue(test_locks[1].is_writer())"},{"line_number":1802,"context_line":"            self.assertTrue(lock_released.wait(timeout\u003d1))"},{"line_number":1803,"context_line":"        thread.join()"},{"line_number":1804,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1805,"context_line":"        self.assertFalse(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":12,"id":"2222285f_67ccf9af","line":1802,"in_reply_to":"6f7bed97_e178fdc9","updated":"2025-11-17 13:28:17.000000000","message":"actully im just going to use is_set as we shoudl not need to wait at all.\nits a bug if this is not set and we are here.","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fd217125651921add3850e6412042933d46e60e5","unresolved":true,"context_lines":[{"line_number":1799,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1800,"context_line":"            self.assertTrue(test_locks[0].is_writer())"},{"line_number":1801,"context_line":"            self.assertTrue(test_locks[1].is_writer())"},{"line_number":1802,"context_line":"            self.assertTrue(lock_released.wait(timeout\u003d1))"},{"line_number":1803,"context_line":"        thread.join()"},{"line_number":1804,"context_line":"        self.assertFalse(lock_guard.is_locked())"},{"line_number":1805,"context_line":"        self.assertFalse(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":12,"id":"6f7bed97_e178fdc9","line":1802,"in_reply_to":"be49bc6b_e89e5168","updated":"2025-11-17 13:16:10.000000000","message":"right we shoudl not need to to wait for this at all.\n\nbut ok i im respining for the odering issue with cyborg so ill reduce htis.","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"406b5fa43b89e30ba417425fa66f3b8e9c5abd46","unresolved":true,"context_lines":[{"line_number":1932,"context_line":"        lock_guard2 \u003d utils.FairLockGuard(lock_names)"},{"line_number":1933,"context_line":""},{"line_number":1934,"context_line":"        # on any one thread we can acquire the same locks]"},{"line_number":1935,"context_line":"        # multiple times provided we use separate context managers."},{"line_number":1936,"context_line":"        with lock_guard:"},{"line_number":1937,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1938,"context_line":"            self.assertTrue(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":12,"id":"8fc2a8a1_5905f938","line":1935,"updated":"2025-11-17 12:08:34.000000000","message":"Does it mean that the lock behind the guard are re-entrant locks?","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"353c190964e18d6df43e86a15877b71bc14147a9","unresolved":false,"context_lines":[{"line_number":1932,"context_line":"        lock_guard2 \u003d utils.FairLockGuard(lock_names)"},{"line_number":1933,"context_line":""},{"line_number":1934,"context_line":"        # on any one thread we can acquire the same locks]"},{"line_number":1935,"context_line":"        # multiple times provided we use separate context managers."},{"line_number":1936,"context_line":"        with lock_guard:"},{"line_number":1937,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1938,"context_line":"            self.assertTrue(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":12,"id":"86e9ec4c_dd56a0e1","line":1935,"in_reply_to":"795ef546_8c39fbed","updated":"2025-11-17 13:38:55.000000000","message":"Acknowledged","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fd217125651921add3850e6412042933d46e60e5","unresolved":true,"context_lines":[{"line_number":1932,"context_line":"        lock_guard2 \u003d utils.FairLockGuard(lock_names)"},{"line_number":1933,"context_line":""},{"line_number":1934,"context_line":"        # on any one thread we can acquire the same locks]"},{"line_number":1935,"context_line":"        # multiple times provided we use separate context managers."},{"line_number":1936,"context_line":"        with lock_guard:"},{"line_number":1937,"context_line":"            self.assertTrue(lock_guard.is_locked())"},{"line_number":1938,"context_line":"            self.assertTrue(test_locks[0].is_writer())"}],"source_content_type":"text/x-python","patch_set":12,"id":"795ef546_8c39fbed","line":1935,"in_reply_to":"8fc2a8a1_5905f938","updated":"2025-11-17 13:16:10.000000000","message":"yes they are althoguh again this is an implemtion detail fo the fastners locks\nhttps://github.com/harlowja/fasteners/blob/06c3f06cab4e135b8d921932019a231c180eb9f4/fasteners/lock.py#L199-L227\n\ni dont nessisarly want use to depend on that as this is really an  instance of \nhttps://www.hyrumslaw.com/\n\noslo does not say they are rentrent \n\nhttps://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/lockutils.py#L89-L143\n\nbut they are.\n\nagain you shoudl not really be doin gthis but im just demonstrating that even if you end up nesting these the impelmeation does something sane today,","commit_id":"3bc523837f2a4d11bda7c51206e5e99f7938c717"}],"nova/utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"289822a4db385f3eb66628546001608c56c0c17c","unresolved":true,"context_lines":[{"line_number":1172,"context_line":"        self.locks \u003d []"},{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"3c10e55a_9145f697","line":1175,"updated":"2024-09-02 08:15:35.000000000","message":"Hm. I feel a potential deadlock looming here. \n\nThis code takes the locks one by one. But this code can be called in parallel.\n\nImagine three parallel threads trying to take locks\n* T1 wants to take L1, L3, L2\n* T2 wants to take L2, L3, L1\n* T3 wants to take L3\n\nThen look at the following execution order:\n1. T3 enters the ctx manager and takes L3 and goes doing its long running business\n2. T2 enters the ctx manager and takes L2.\n3. T2 tries to take the L3 but it is taken so it starts to wait on L3\n4. T1 enters the ctx manager and takes L1.\n5. T1 tries to take the L3 but it is taken so it starts to wait on L3\n6. T3 finishes its business and releases L3\n7. T2 takes L3\n8. T1 tries to take L3 but it is taken so it waits on L3\n9. T2 tries to take L1 but it is taken so it waits on L1\n\n(when we switch from eventlet to native threads, due to preemtiveness 2 threads T1 and T2 are enough to create the same deadlock)","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"607e194e83293ce9f31003afec709b824fc16ab2","unresolved":true,"context_lines":[{"line_number":1172,"context_line":"        self.locks \u003d []"},{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"4e1172f3_ddf58a0d","line":1175,"in_reply_to":"3c10e55a_9145f697","updated":"2024-09-02 14:21:55.000000000","message":"Hehe you thought about this and sorted the locks. That indeed removes this deadlock possibility. I would emphasize that nova does the sorting to avoid deadlocks.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d8bd4d8d99c9a270f0073bb1bc4ccfaddd2c5b9b","unresolved":true,"context_lines":[{"line_number":1172,"context_line":"        self.locks \u003d []"},{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9d3b5fe0_d9da3e37","line":1175,"in_reply_to":"4e1172f3_ddf58a0d","updated":"2024-09-03 15:06:57.000000000","message":"ya so i didn\u0027t call this out but I am doing the sorting precisely to remove the possibility of deadlocks\n\nI watch a lot of c++ confrence session on youtube and one of the many presentaiton on locks laid out this exact deadlock problem and how to solve it.\n\nthe c++ std::lock_guard does not catere for this mutlipel mutex case so in c++17 \nstd::scoped_lock was added https://en.cppreference.com/w/cpp/thread/scoped_lock\n\nthe problem is know as the Dining philosophers problem\n\nhttps://en.wikipedia.org/wiki/Dining_philosophers_problem\n\ni opted for the simpletes solution which is the ordered solution.\nhttp://howardhinnant.github.io/dining_philosophers.html","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":false,"context_lines":[{"line_number":1172,"context_line":"        self.locks \u003d []"},{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"eabd17ca_d4a8586f","line":1175,"in_reply_to":"9d3b5fe0_d9da3e37","updated":"2025-11-12 11:33:54.000000000","message":"Done","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9df587cfe8bb1e667e4a4a0c14356b9f2c8fb35d","unresolved":true,"context_lines":[{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1179,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"2722ecf9_446aeec9","line":1176,"range":{"start_line":1176,"start_character":25,"end_line":1176,"end_character":40},"updated":"2024-08-30 08:52:00.000000000","message":"I think name(FairLockGuard and NOVA_FAIR) represent something here, but its not clear to me.\n\nsynonym like valid/charge/fairAmtOfTime-lock-guard does not make sense.\n\nfrom commit-msg it seems, the operation is called in parallel and we want to handle these calls one-by-one\n\nI get the logic, but name is not clear.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d8bd4d8d99c9a270f0073bb1bc4ccfaddd2c5b9b","unresolved":true,"context_lines":[{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1179,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"cdf353b7_f90aa85b","line":1176,"range":{"start_line":1176,"start_character":25,"end_line":1176,"end_character":40},"in_reply_to":"2722ecf9_446aeec9","updated":"2024-09-03 15:06:57.000000000","message":"in computer science there are may types of lock and when designing a lock implemtion on of the characteristic you need to consider is fi two threads try to acquire the lock when its already locked by a third who should you provdie that lock to when its released.\n\nthis is known generally as the fairness characteristic of the lock.\n\nthe are several choices a priority lock, which a reader writer lock can be an exmaple of may release the lock to the reader or writer preferencially or based on a thread priority\n\na fair lock is one that remebers the order in which a thread asked for the lock\nadn release the lock in that order.\n\nIt\u0027s effectively a FIFO queue or first come first served.\n\nby calling it a FairLockGuard I am signalling to others that this class internally uses fair locks rather then priority locks \n\nthis is in an effort to ensure if we have 2 parallel delete request that both share a lock (via a shared multi attached volume) then we will delete the instances in teh order we received the requests in.\n\n\nfair lock try to prevent \"starvation\" of thread by ensuring the receive the lock in a deterministic manner to ensure forward progress.\n\nhttps://read.seas.harvard.edu/cs161/2019/lectures/lecture18/\n\nin this case im using the fair loc form oslo which is actully wrapping the fasteres reader writer lock which is a fair lock\n\nhttps://github.com/harlowja/fasteners/blob/06c3f06cab4e135b8d921932019a231c180eb9f4/fasteners/lock.py#L199-L213","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a900f17d3754e8c15efe3e06b750224f1ed0d8ba","unresolved":false,"context_lines":[{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"    def __enter__(self):"},{"line_number":1175,"context_line":"        for name in self.names:"},{"line_number":1176,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1177,"context_line":"            self.locks.append(named_lock)"},{"line_number":1178,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1179,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"180507bf_ece43083","line":1176,"range":{"start_line":1176,"start_character":25,"end_line":1176,"end_character":40},"in_reply_to":"cdf353b7_f90aa85b","updated":"2024-09-04 10:56:19.000000000","message":"ack, thanks Sean for detailed explanation.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8f32ff098db42f4ad00421398bbef6eff39774d3","unresolved":true,"context_lines":[{"line_number":1182,"context_line":"            lock.release_write_lock()"},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"    def is_locked(self):"},{"line_number":1185,"context_line":"        return self.locks and all(lock.is_writer() for lock in self.locks)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5ae25850_f9c07d93","line":1185,"range":{"start_line":1185,"start_character":30,"end_line":1185,"end_character":33},"updated":"2024-08-01 14:54:15.000000000","message":"maybe stupid question, but shouldn\u0027t it be any() instead ?\n\nI mean, the contextmanager can have multiple locks but at least if one of them is still writing, then we couldn\u0027t accept to write another one.","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d8bd4d8d99c9a270f0073bb1bc4ccfaddd2c5b9b","unresolved":true,"context_lines":[{"line_number":1182,"context_line":"            lock.release_write_lock()"},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"    def is_locked(self):"},{"line_number":1185,"context_line":"        return self.locks and all(lock.is_writer() for lock in self.locks)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5c23bd4a_62869238","line":1185,"range":{"start_line":1185,"start_character":30,"end_line":1185,"end_character":33},"in_reply_to":"5ae25850_f9c07d93","updated":"2024-09-03 15:06:57.000000000","message":"by convention, a lock guard that supports multiple locks is only considered locked if it has acquired all of them. \n\nyou coudl defin it otherwise but that would be surprising to those familar with the concept in other languages like c++","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            lock.release_write_lock()"},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"    def is_locked(self):"},{"line_number":1185,"context_line":"        return self.locks and all(lock.is_writer() for lock in self.locks)"}],"source_content_type":"text/x-python","patch_set":7,"id":"40ce7d3b_6ee3cc0d","line":1185,"range":{"start_line":1185,"start_character":30,"end_line":1185,"end_character":33},"in_reply_to":"5c23bd4a_62869238","updated":"2025-11-13 22:07:29.000000000","message":"Done","commit_id":"486927213d4bf2a5a8ba10ffcc6634c36ebf3743"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":true,"context_lines":[{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def __enter__(self):"},{"line_number":1194,"context_line":"        for name in self.names:"},{"line_number":1195,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1196,"context_line":"            self.locks.append(named_lock)"},{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"eef89ce7_1279cff2","line":1195,"updated":"2025-11-12 11:33:54.000000000","message":"It worth noting that even though this is a global variable the .get() calls has a lock so we don\u0027t need to lock it explicitly here.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def __enter__(self):"},{"line_number":1194,"context_line":"        for name in self.names:"},{"line_number":1195,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1196,"context_line":"            self.locks.append(named_lock)"},{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"f7dd4300_32015a24","line":1195,"in_reply_to":"eef89ce7_1279cff2","updated":"2025-11-13 22:07:29.000000000","message":"Acknowledged","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"    def __enter__(self):"},{"line_number":1194,"context_line":"        for name in self.names:"},{"line_number":1195,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1196,"context_line":"            self.locks.append(named_lock)"},{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"}],"source_content_type":"text/x-python","patch_set":8,"id":"38cb7b4e_8d1278eb","line":1196,"updated":"2025-11-12 11:33:54.000000000","message":"hm, Can it be that two threads reaches the contenxt manager `__enter__` concurrently and therefore tries to manipulate this list concurrent? If so then we need to guard the list with a lock.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bdeea17a1f1809484457725799491d1b689fa822","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"    def __enter__(self):"},{"line_number":1194,"context_line":"        for name in self.names:"},{"line_number":1195,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1196,"context_line":"            self.locks.append(named_lock)"},{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"}],"source_content_type":"text/x-python","patch_set":8,"id":"534b5d09_c89a715e","line":1196,"in_reply_to":"38cb7b4e_8d1278eb","updated":"2025-11-12 12:23:21.000000000","message":"if i reorder this and the next line the named_lock will work for that.\nit not strictly required as while i may muteate the list in memory we woudl still blcok in one of the two trhead on teh aquire before exiting this fucntion.\n\nhowever i was not planning to supprot that at all and i dont see a usecase that reuiqre us to supprot sharing a single instnace of this between threads.\n\ni think it could work if that was done but the use case for thsi is there are 2 parallel rpc calls happening in seperate threads or greenthread both of which are deleting different vms\n\neach callstack will have its own instance fo the context manager with its own internal list of locks so there wotn be any corss thread sharing.\n\nto have the isturation you are descibing i woudl ahve to create the context manager and then pass it into 2 threads by passing it to a fucntiona dn spawning that on a thread or threadpool. that fucntion woudl then have to invoke the context manage via a with statement. that not really a good pattern in general to do.\n\nall the locks are using oslo\u0027s fair locks whchi are actully interprocess lock internally meaning they write a file to the file sytem\n\nthat mean that if we try to aquire the same named lock form tow diffent process or thread only one of the two independent isntance of the context manager will be able to aquire it.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"    def __enter__(self):"},{"line_number":1194,"context_line":"        for name in self.names:"},{"line_number":1195,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1196,"context_line":"            self.locks.append(named_lock)"},{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"}],"source_content_type":"text/x-python","patch_set":8,"id":"41dcfaa5_67a5e0a1","line":1196,"in_reply_to":"534b5d09_c89a715e","updated":"2025-11-13 08:17:29.000000000","message":"\u003e all the locks are using oslo\u0027s fair locks whchi are actully interprocess lock internally meaning they write a file to the file sytem\n\u003e that mean that if we try to aquire the same named lock form tow diffent process or thread only one of the two independent isntance of the context manager will be able to aquire it.\n\nDo we really need that heavy weight of a lock here? Compute is single process multiple (green) threads service. So there won\u0027t ever be another process on the same host trying to manage the same fs lock. I thin simple threading.Locks would be enough for our use case.\nDo I miss something?","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[{"line_number":1193,"context_line":"    def __enter__(self):"},{"line_number":1194,"context_line":"        for name in self.names:"},{"line_number":1195,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1196,"context_line":"            self.locks.append(named_lock)"},{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"}],"source_content_type":"text/x-python","patch_set":8,"id":"c23d7269_34c71de9","line":1196,"in_reply_to":"534b5d09_c89a715e","updated":"2025-11-13 22:07:29.000000000","message":"Acknowledged","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"200ff156da9d41b9566ef9864d52d14c421060a5","unresolved":true,"context_lines":[{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1200,"context_line":"        for lock in self.locks:"},{"line_number":1201,"context_line":"            lock.release_write_lock()"},{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":"    def is_locked(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"2c6c3193_1ca04196","line":1200,"updated":"2025-11-12 11:33:54.000000000","message":"here, if `__exit__` and `__enter__` can overlap between threads then one thread might add things to the list while the other thread iterating it that is not good and we need a lock","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bdeea17a1f1809484457725799491d1b689fa822","unresolved":true,"context_lines":[{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1200,"context_line":"        for lock in self.locks:"},{"line_number":1201,"context_line":"            lock.release_write_lock()"},{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":"    def is_locked(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3d1fe05d_d231b850","line":1200,"in_reply_to":"2c6c3193_1ca04196","updated":"2025-11-12 12:23:21.000000000","message":"as noted above they should not, i would consider that a missus of the class.\n\nif you really want me to support that usecase i could add a lock but that not really how you shoudl use context managers in my opinion.","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1200,"context_line":"        for lock in self.locks:"},{"line_number":1201,"context_line":"            lock.release_write_lock()"},{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":"    def is_locked(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3952993b_da5fc0a5","line":1200,"in_reply_to":"3d1fe05d_d231b850","updated":"2025-11-13 08:17:29.000000000","message":"Acknowledged","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"            named_lock.acquire_write_lock()"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1200,"context_line":"        for lock in self.locks:"},{"line_number":1201,"context_line":"            lock.release_write_lock()"},{"line_number":1202,"context_line":""},{"line_number":1203,"context_line":"    def is_locked(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5ef4ecf1_43693bf0","line":1200,"in_reply_to":"3d1fe05d_d231b850","updated":"2025-11-13 22:07:29.000000000","message":"Acknowledged","commit_id":"48d5a3e4349daebf15b43a9ebd2de0e281ece05b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":true,"context_lines":[{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    The intended usage model for this context manager is to"},{"line_number":1184,"context_line":"    mediate access to a set of locks by name, not by pre-creating"},{"line_number":1185,"context_line":"    the locks and passing them in. lock Creation and management"},{"line_number":1186,"context_line":"    is handled entirely internally."},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"    If you are using this between threads, Thread-A and Thread-B"}],"source_content_type":"text/x-python","patch_set":9,"id":"dc697081_1c35ab13","line":1185,"updated":"2025-11-13 08:17:29.000000000","message":"nit: Lock creation and management","commit_id":"923ca552780a04b99d6d7ec41a7ef46ae1d5cc83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7eb346f9d3e68238aba04fae5c2a7d09763a7333","unresolved":false,"context_lines":[{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    The intended usage model for this context manager is to"},{"line_number":1184,"context_line":"    mediate access to a set of locks by name, not by pre-creating"},{"line_number":1185,"context_line":"    the locks and passing them in. lock Creation and management"},{"line_number":1186,"context_line":"    is handled entirely internally."},{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"    If you are using this between threads, Thread-A and Thread-B"}],"source_content_type":"text/x-python","patch_set":9,"id":"51537641_77e8a556","line":1185,"in_reply_to":"dc697081_1c35ab13","updated":"2025-11-17 10:56:05.000000000","message":"Done","commit_id":"923ca552780a04b99d6d7ec41a7ef46ae1d5cc83"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"364a62c980f58088088e39fd9aafb0b5424a80cb","unresolved":false,"context_lines":[{"line_number":1187,"context_line":""},{"line_number":1188,"context_line":"    If you are using this between threads, Thread-A and Thread-B"},{"line_number":1189,"context_line":"    should both create there own context manager instead of sharing"},{"line_number":1190,"context_line":"    a single context manager between treads."},{"line_number":1191,"context_line":"    \"\"\""},{"line_number":1192,"context_line":""},{"line_number":1193,"context_line":"    def __init__(self, names):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3d5a450b_55ff1a4f","line":1190,"updated":"2025-11-13 08:17:29.000000000","message":"thanks!","commit_id":"923ca552780a04b99d6d7ec41a7ef46ae1d5cc83"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"10ebfd220de148b3cc52d3637f290f47fa0776e9","unresolved":false,"context_lines":[{"line_number":1173,"context_line":"    return outer_wrapper"},{"line_number":1174,"context_line":""},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"class FairLockGuard:"},{"line_number":1177,"context_line":"    \"\"\"A lock guard context manager"},{"line_number":1178,"context_line":""},{"line_number":1179,"context_line":"    This class support acquiring multiple locks safely by name"}],"source_content_type":"text/x-python","patch_set":11,"id":"b72b1214_7dad0574","line":1176,"in_reply_to":"6db6fd6b_03e0f806","updated":"2025-11-14 00:32:55.000000000","message":"```\n[severity: suggestion] Add docstring examples for FairLockGuard - Confidence: 0.6\nLocation: nova/utils.py:1176-1191\nBenefit: Improves usability and understanding of the new utility class\nRecommendation: Add usage examples showing single and multiple lock scenarios\n```\n\ni could but no this sinot a bad suggestion btu the usage in this patch already demonstrates how to use it properly.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"5be47f01458c541fc1c298990f2599a53404a371","unresolved":true,"context_lines":[{"line_number":1199,"context_line":"        # https://howardhinnant.github.io/dining_philosophers.html"},{"line_number":1200,"context_line":"        self.names \u003d sorted(names)"},{"line_number":1201,"context_line":"        self.locks \u003d []"},{"line_number":1202,"context_line":"        # NOTE(sean-k-mooney): this is technically not required"},{"line_number":1203,"context_line":"        # but it protect from incorrect usage where a context manager"},{"line_number":1204,"context_line":"        # is shared between threads."},{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"}],"source_content_type":"text/x-python","patch_set":11,"id":"a9950903_b2917e9b","line":1205,"range":{"start_line":1202,"start_character":0,"end_line":1205,"end_character":42},"updated":"2025-11-13 20:04:09.000000000","message":"That is what I was thinking that why we need it? FairLocks -\u003eacquire_write_lock() does wait for other writers [1]. and it does waits for the pending writer[2] and differentiate them via current_thread[3] so thinkgs will work even thread sharing the contest manager? \n\nI am not against not having a lock here, which is good safet,y but I think ReaderWriterLock internally take care of it (maybe adding soem test can be good here in case soemthing ReaderWriterLock is missing and this lock is needed?).\n\n[1] https://github.com/harlowja/fasteners/blob/ea6fce24e8e396813d53d3599f35a22162fb9280/fasteners/lock.py#L186\n[2] https://github.com/harlowja/fasteners/blob/ea6fce24e8e396813d53d3599f35a22162fb9280/fasteners/lock.py#L187\n[3] https://github.com/harlowja/fasteners/blob/ea6fce24e8e396813d53d3599f35a22162fb9280/fasteners/lock.py#L209","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":false,"context_lines":[{"line_number":1199,"context_line":"        # https://howardhinnant.github.io/dining_philosophers.html"},{"line_number":1200,"context_line":"        self.names \u003d sorted(names)"},{"line_number":1201,"context_line":"        self.locks \u003d []"},{"line_number":1202,"context_line":"        # NOTE(sean-k-mooney): this is technically not required"},{"line_number":1203,"context_line":"        # but it protect from incorrect usage where a context manager"},{"line_number":1204,"context_line":"        # is shared between threads."},{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ddebf6b6_05270088","line":1205,"range":{"start_line":1202,"start_character":0,"end_line":1205,"end_character":42},"in_reply_to":"13a49303_ec2b7435","updated":"2025-11-14 22:16:32.000000000","message":"yeah, I think it is fine to protect the  self.locks modification. no need to any test on this.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":true,"context_lines":[{"line_number":1199,"context_line":"        # https://howardhinnant.github.io/dining_philosophers.html"},{"line_number":1200,"context_line":"        self.names \u003d sorted(names)"},{"line_number":1201,"context_line":"        self.locks \u003d []"},{"line_number":1202,"context_line":"        # NOTE(sean-k-mooney): this is technically not required"},{"line_number":1203,"context_line":"        # but it protect from incorrect usage where a context manager"},{"line_number":1204,"context_line":"        # is shared between threads."},{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"}],"source_content_type":"text/x-python","patch_set":11,"id":"cecd9d6d_a105a973","line":1205,"range":{"start_line":1202,"start_character":0,"end_line":1205,"end_character":42},"in_reply_to":"a9950903_b2917e9b","updated":"2025-11-13 22:07:29.000000000","message":"so this is here because gibi asked about the idea of creating an instance of the context manager and passing that single \n\ni.e somethign like this\n```\n\nlg \u003d FairLockGuard(\"lock-name\")\n\nwork \u003d functools.partial(real_work, lg)\nthreads \u003d []\nthreads.append(threading.Thread(target\u003dwork))\nthreads.append(threading.Thread(target\u003dwork))\n\n# Start each thread\nfor t in threads:\n    t.start()\n\n# Wait for all threads to finish\nfor t in threads:\n    t.join()\n\n```\n\nnow you shoudl not do that you shoudl create the FairLockGuard in real_work\nbut this menas that only one of the two FairLockGuard will aquire the locs at a time.\n\na ReaderWriterLock woudl be better hear because i actully only need a reader lock for is_locked.\n\ni can try and add a test for this case but its kind fo hard to prove a negetive i.e. that this wont break but i also dont want to supprot this way of using it somy motivation to make it bullte proff for bad usage is low vs just reviewign and makign sure we dont do that pateren.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"10ebfd220de148b3cc52d3637f290f47fa0776e9","unresolved":true,"context_lines":[{"line_number":1199,"context_line":"        # https://howardhinnant.github.io/dining_philosophers.html"},{"line_number":1200,"context_line":"        self.names \u003d sorted(names)"},{"line_number":1201,"context_line":"        self.locks \u003d []"},{"line_number":1202,"context_line":"        # NOTE(sean-k-mooney): this is technically not required"},{"line_number":1203,"context_line":"        # but it protect from incorrect usage where a context manager"},{"line_number":1204,"context_line":"        # is shared between threads."},{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"}],"source_content_type":"text/x-python","patch_set":11,"id":"13a49303_ec2b7435","line":1205,"range":{"start_line":1202,"start_character":0,"end_line":1205,"end_character":42},"in_reply_to":"cecd9d6d_a105a973","updated":"2025-11-14 00:32:55.000000000","message":"by the wya the concer with having two thead share the context manager is that if both of the invoke __enter__\n\nthe would be try to modify   self.locks \u003d []\n\nso this is here to make sure that that does nto happen.\n\nthe locks we get fomr FairLockGuard will handel multipel tread properly as you said.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"10ebfd220de148b3cc52d3637f290f47fa0776e9","unresolved":false,"context_lines":[{"line_number":1204,"context_line":"        # is shared between threads."},{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"},{"line_number":1209,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1210,"context_line":"            with self.locks_lock:"}],"source_content_type":"text/x-python","patch_set":11,"id":"cad700f1_deffde21","line":1207,"in_reply_to":"4a8f521f_f59d5e22","updated":"2025-11-14 00:32:55.000000000","message":"```\n[severity: suggestion] Improve exception handling in FairLockGuard.enter - Confidence: 0.7\nLocation: nova/utils.py:1207-1215\nBenefit: Better error handling and lock state management\nRecommendation: Add try/except to ensure proper cleanup if acquire_write_lock fails\n```\n```\ndef acquire_write_lock(self):\n        \"\"\"Acquire a write lock.\n\n        Will wait until no active readers. Blocks readers after acquiring.\n\n        Guaranteed for locks to be processed in fair order (FIFO).\n\n        Raises:\n            RuntimeError: if an active reader attempts to acquire a lock.\n        \"\"\"\n        me \u003d self._current_thread()\n        if self._writer \u003d\u003d me:\n            self._writer_entries +\u003d 1\n        else:\n            self._acquire_write_lock(me)\n\n    def release_write_lock(self):\n        \"\"\"Release a write lock.\n\n        Raises:\n            RuntimeError: if the current thread does not own a write lock.\n        \"\"\"\n        me \u003d self._current_thread()\n        if self._writer \u003d\u003d me:\n            self._writer_entries -\u003d 1\n            if self._writer_entries \u003d\u003d 0:\n                self._release_write_lock(me)\n        else:\n            raise RuntimeError(f\"Thread {me} does not own a write lock\")\n\n```\nhttps://github.com/harlowja/fasteners/blob/06c3f06cab4e135b8d921932019a231c180eb9f4/fasteners/lock.py#L199-L213 \n\nthis shoudl not raise unless you start sharign it between thread but also that is not inteded to be supproted so this is fine as is.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":true,"context_lines":[{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"},{"line_number":1209,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1210,"context_line":"            with self.locks_lock:"},{"line_number":1211,"context_line":"                self.locks.append(named_lock)"},{"line_number":1212,"context_line":"                # we ensure we add it to the list"},{"line_number":1213,"context_line":"                # before we acquire the lock to make sure"},{"line_number":1214,"context_line":"                # we release it if there is an exception"},{"line_number":1215,"context_line":"                named_lock.acquire_write_lock()"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1218,"context_line":"        with self.locks_lock:"}],"source_content_type":"text/x-python","patch_set":11,"id":"179514b3_63d6a6a7","line":1215,"range":{"start_line":1208,"start_character":8,"end_line":1215,"end_character":47},"updated":"2025-11-14 22:16:32.000000000","message":"I read your and gibi discussion[1] on possible deadlock and ordered solution (sorted names) solves the problem of deadlock but that leads to some delay in threads cleaning up different volumes.\n\nFor example, \nT1 (lock-for-v1, lock-for-v2) - thread deleting instance1 having v1, v2 multiattach vol in bdms\nT2 (lock-for-v1) - thread deleting instance2 having v1 multiattach vol in bdms\nT3 (lock-for-v2) - thread deleting instance3 having v2 multiattach vol in bdms\n\nIf execution happens in below order:\n \nstep1: T1 call __enter__ and aquired lock-for-v1 but before T1 aquires the lock-for-v2, T3 call __enter__ and acquires the lock-for-v2 and start doing things. *issue1:* This is breaking the logic of \u0027fair\u0027 lock where T3 came after T1 got the chance to execute the things.\n\nste2: T1 did not get lock-for-v2 and is in wait but *holding the lock-for-v1*. \n\n*Step3: T2 comes now but T2 will be blocked because T1 is holding its lock-for-v1. *issue2:* This is making T2 delay until T3 and T1 are finished; instead, T2 and T3 can go in parallel because they are cleaning up the different volumes, which no one else is cleaning in parallel.\n\nStep4: T3 finished, T1 get lock-for-v2 and starts doing things, T1 finished, T2 gets chance to start.\n\nOne easy solution for both issues is to put L1209 (named_lock \u003d NOVA_FAIR_LOCKS.get(name)) also under self.locks_lock: so that T1 can get all their required locks in one shot. T3, T2 wait because they are after T1 (as it is fair lock) and once T1 finishes, we can still let T3 and T2 execute in parallel.\n\n[1] https://review.opendev.org/c/openstack/nova/+/916322/comment/3c10e55a_9145f697/","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"92de8a7fd745d2d4bb8d3d426c468c81386cfa4d","unresolved":true,"context_lines":[{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"},{"line_number":1209,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1210,"context_line":"            with self.locks_lock:"},{"line_number":1211,"context_line":"                self.locks.append(named_lock)"},{"line_number":1212,"context_line":"                # we ensure we add it to the list"},{"line_number":1213,"context_line":"                # before we acquire the lock to make sure"},{"line_number":1214,"context_line":"                # we release it if there is an exception"},{"line_number":1215,"context_line":"                named_lock.acquire_write_lock()"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1218,"context_line":"        with self.locks_lock:"}],"source_content_type":"text/x-python","patch_set":11,"id":"73569e14_0385a6d6","line":1215,"range":{"start_line":1208,"start_character":8,"end_line":1215,"end_character":47},"in_reply_to":"179514b3_63d6a6a7","updated":"2025-11-14 22:22:09.000000000","message":"NOTE: I do disagree with the \u0027Ordered\u0027 solution and not asking to change it but this is just a more improvement in that","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"eaf64cd8f5f6ffbdd81bbe2cfcd9a3712d247aea","unresolved":true,"context_lines":[{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"},{"line_number":1209,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1210,"context_line":"            with self.locks_lock:"},{"line_number":1211,"context_line":"                self.locks.append(named_lock)"},{"line_number":1212,"context_line":"                # we ensure we add it to the list"},{"line_number":1213,"context_line":"                # before we acquire the lock to make sure"},{"line_number":1214,"context_line":"                # we release it if there is an exception"},{"line_number":1215,"context_line":"                named_lock.acquire_write_lock()"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1218,"context_line":"        with self.locks_lock:"}],"source_content_type":"text/x-python","patch_set":11,"id":"f5e29684_28ab6230","line":1215,"range":{"start_line":1208,"start_character":8,"end_line":1215,"end_character":47},"in_reply_to":"73569e14_0385a6d6","updated":"2025-11-15 01:49:06.000000000","message":"\u003e One easy solution for both issues is to put L1209 (named_lock \u003d NOVA_FAIR_LOCKS.get(name)) also under self.locks_lock:.... \n\nI think we need to put whole for loop inside lock, otherwise there is still a chance that one thread can be interrupted while it gets all its required locks","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"21746c73df4a84940bfd1255da07f6214c8d4a8c","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"        self.locks_lock \u003d threading.Lock()"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    def __enter__(self):"},{"line_number":1208,"context_line":"        for name in self.names:"},{"line_number":1209,"context_line":"            named_lock \u003d NOVA_FAIR_LOCKS.get(name)"},{"line_number":1210,"context_line":"            with self.locks_lock:"},{"line_number":1211,"context_line":"                self.locks.append(named_lock)"},{"line_number":1212,"context_line":"                # we ensure we add it to the list"},{"line_number":1213,"context_line":"                # before we acquire the lock to make sure"},{"line_number":1214,"context_line":"                # we release it if there is an exception"},{"line_number":1215,"context_line":"                named_lock.acquire_write_lock()"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def __exit__(self, exc_type, exc_value, traceback):"},{"line_number":1218,"context_line":"        with self.locks_lock:"}],"source_content_type":"text/x-python","patch_set":11,"id":"6ec61b92_3f8169a2","line":1215,"range":{"start_line":1208,"start_character":8,"end_line":1215,"end_character":47},"in_reply_to":"f5e29684_28ab6230","updated":"2025-11-17 10:59:15.000000000","message":"i have done this but i didnt want ot supprot sharign this between thread at all and if you dont do that you do not need a lock for the list of locks at all.\n\ni would have prefered not to add this complexity or expand the critical section to the full loop but have have doen it in the latest version and took a belt and braces approch to prventing missues by  nesting ectra including more tests.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8a9beb73275eed24a3a345afad5d2a84512f1857","unresolved":true,"context_lines":[{"line_number":1221,"context_line":"                # we add the lock to the list before we"},{"line_number":1222,"context_line":"                # acquire it check anyway."},{"line_number":1223,"context_line":"                if lock.is_writer():"},{"line_number":1224,"context_line":"                    lock.release_write_lock()"},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"    def is_locked(self):"},{"line_number":1227,"context_line":"        # NOTE(sean-k-mooney): LockGuards exist in several programming"}],"source_content_type":"text/x-python","patch_set":11,"id":"3801a4dc_9d4d195e","line":1224,"updated":"2025-11-13 22:07:29.000000000","message":"if i reall want to make the sharign between thread work i shold also clear self.locks here. out sid ethe fore but inside the locks_lock.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d604bfcfaaa5831392040a31e2886dab08e462a7","unresolved":true,"context_lines":[{"line_number":1221,"context_line":"                # we add the lock to the list before we"},{"line_number":1222,"context_line":"                # acquire it check anyway."},{"line_number":1223,"context_line":"                if lock.is_writer():"},{"line_number":1224,"context_line":"                    lock.release_write_lock()"},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"    def is_locked(self):"},{"line_number":1227,"context_line":"        # NOTE(sean-k-mooney): LockGuards exist in several programming"}],"source_content_type":"text/x-python","patch_set":11,"id":"999c3004_837a80d5","line":1224,"in_reply_to":"3801a4dc_9d4d195e","updated":"2025-11-14 22:16:32.000000000","message":"Well, even in context sharing between threads, it is fine not to clear. The condition `lock.is_writer()` ensures that only the same thread that acquired the lock can only call the release. For instance, if Thread T1 tries to release the lock \u0027L2\u0027 that was acquired by Thread T2, the method `L2.is_writer()` will return false for T1. \n\nNow, consider the case where `L2.is_writer()` could return true for T1. This might happen if T1 is waiting for the L2 lock, which is held by T2. But in that case, T1 will not be calling __exit__ because it is waiting in __enter__ on one of the locks it needs.","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"21746c73df4a84940bfd1255da07f6214c8d4a8c","unresolved":false,"context_lines":[{"line_number":1221,"context_line":"                # we add the lock to the list before we"},{"line_number":1222,"context_line":"                # acquire it check anyway."},{"line_number":1223,"context_line":"                if lock.is_writer():"},{"line_number":1224,"context_line":"                    lock.release_write_lock()"},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"    def is_locked(self):"},{"line_number":1227,"context_line":"        # NOTE(sean-k-mooney): LockGuards exist in several programming"}],"source_content_type":"text/x-python","patch_set":11,"id":"026e880a_3345ff52","line":1224,"in_reply_to":"999c3004_837a80d5","updated":"2025-11-17 10:59:15.000000000","message":"Acknowledged","commit_id":"4c9c6b9b7d8dbebb65baa2d870f4fa141cfe3bbf"}]}