)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"7ab9b74c7b2821cb1b4a7bc815796108181c97cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5ff3985a_a9f7d96d","updated":"2026-05-05 20:39:44.000000000","message":"Example of CI failures where the race condition manifested itself:\nhttps://review.opendev.org/c/openstack/ansible-collections-openstack/+/987384","commit_id":"f49d9e1efc043b56d1393080ae716e0d19f99fd7"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"e1bb330d672ba664b1bde3faab3dc41c68777bb4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"78581762_40375e74","updated":"2026-05-06 12:45:43.000000000","message":"Hi Mathieu thanks for helping us with that. For us to have a better understanding of the problem you are hitting, can you please open a bug in launchpad[1], and provide as much information on how to reproduce the issue and logs you have (before and after the fix preferably)?\n\n\n[1] https://bugs.launchpad.net/cinder","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"49301a61a59642edce9c039eb3f0310ff3e657ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1a175a33_0484c8a0","updated":"2026-05-06 07:35:48.000000000","message":"The change itself looks good and is simple and harmless.\n\nbut I am soft -1, as we do not have clear bug report to show race condition actually occured for this, can you please create a bug and add in commit msg.\n\nI checked the CI logs for  diff failing jobs for `987384` bu can\u0027t find the bug traceback, could only see cliet-side 500 err - which is unrealted to this change\nhttps://zuul.opendev.org/t/openstack/builds?change\u003d987384\u0026skip\u003d0\n\nso I might be missing it, is this fail frequent? can you direct us to cinder logs containing traceback.\n\nI have a nit, tat this could be fixed at called func itself `lv_is_snapshot` but that could be fixed in next PS.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"eeb5fc82990b8a04a3ff10e3381d281d0d130801","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"27784b21_d03cafc8","in_reply_to":"1a175a33_0484c8a0","updated":"2026-05-06 15:37:40.000000000","message":"I opened a bug report with the relevant logs and timing demonstrating the race condition: https://bugs.launchpad.net/cinder/+bug/2151569\nI will open this change and commit message to mention the bug once we have an agreement on the implementation.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"49b0c30abdd563400c50b3304c0316f0af0724ce","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dc404e57_802bb181","in_reply_to":"27784b21_d03cafc8","updated":"2026-05-07 10:23:20.000000000","message":"so I saw the change 987384 and its unclear - why was manageable_volumes being called during a loadbalancer quota tests!!\n\nthanks for the bug report, can see the race happened in logs, but I\u0027m unclear on how to reproduce this.\n\ngiven race conditions are hard to reproduce.\nif this is a real race condition that happens in CI and frequent enough to patch, please share - specific CI test/scenario which triggers this.\n\nhow frequently this occurs once-multiple ?","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"eeb5fc82990b8a04a3ff10e3381d281d0d130801","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ef6e3296_8ad6c52b","in_reply_to":"78581762_40375e74","updated":"2026-05-06 15:37:40.000000000","message":"I opened a bug report with relevant logs and timing. [1]\n\nI won\u0027t be able to provide logs after the fix as I don\u0027t have enough experience/knowledge to pull the fix into the other change I\u0027m working on. [2]\n\nIt is also difficult to prove a race condition has been fixed due to the nature of the issue: it\u0027s a race condition. Hopefully the logs are enough to demonstrate the root cause.\n\n[1] https://bugs.launchpad.net/cinder/+bug/2151569\n[2] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/987384","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ade125cc36300b84488bd6c8327c1162807bf6f3","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0444d35d_d3b78280","in_reply_to":"ef6e3296_8ad6c52b","updated":"2026-05-07 13:40:21.000000000","message":"The race condition makes sense - it would happen if listing manageable resources while another operation simultaneously deletes LVM objects.  LVs can definitely disappear between line 691 and 696.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"}],"cinder/volume/drivers/lvm.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"e874b51eeca6e3e28e0b6f2888e55d28943f38d0","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"33e7a599_3f6adb60","line":701,"updated":"2026-05-06 07:41:30.000000000","message":"\"not found\" is kind of ambigous, could match other erros! - is there a more specific pattern or way to detect this specific error.\n\na tracback might help here.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"49301a61a59642edce9c039eb3f0310ff3e657ce","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"93a0dfbd_140aa09b","line":701,"updated":"2026-05-06 07:35:48.000000000","message":"if its frequent, would it be better to fix this inside `lv_is_snapshot()` itself? to correct all caller, instead of needing try-excpot at every caller.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"eeb5fc82990b8a04a3ff10e3381d281d0d130801","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5efd31d9_e0d08f78","line":701,"in_reply_to":"33e7a599_3f6adb60","updated":"2026-05-06 15:37:40.000000000","message":"The implementation was copied from get_lv_info which inspects the exception message.\nhttps://opendev.org/openstack/cinder/src/commit/28326c3edbe2ba4f49ce21a7ac96d2edd339d825/cinder/brick/local_dev/lvm.py#L330","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ade125cc36300b84488bd6c8327c1162807bf6f3","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5922fa6a_b7085d73","line":701,"in_reply_to":"5164e196_9dc8a4c3","updated":"2026-05-07 13:40:21.000000000","message":"I would not try to put this logic into lv_is_snapshot() itself ... it throwing an exception is better than it trying to guess at a True/False answer which will not be reliable.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"1cb6946dd8a561d61ef5a8e1ae6fad51b019808a","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fae0c8b9_7facd11e","line":701,"in_reply_to":"5922fa6a_b7085d73","updated":"2026-05-13 08:21:50.000000000","message":"agreed, yes, throwing an exception is better then guessing true/false.\n\nI\u0027m suggesting lv_is_snapshot() still throw an exception (not return False) in this scenario, just catch ProcessExecutionError and re-raise appropriate exceptions \nVolumeGroupNotFound for \"not found\" and \nVolumeNotFound for \"Failed to find\".\n\nthis centralize that check instead of repeating it at each call\nso our current string matching will be in one place and all callers get clean exception.\n\nhere it can be look like\n\n```\ntry:\n      is_snap \u003d self.vg.lv_is_snapshot(lv[\u0027name\u0027])\nexcept (VGNotFound, (LVNotFound or volumeNotFound) ):\n                # LVNotFound does not exists!\n      continue  # scenerio due to race\n\n```","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ade125cc36300b84488bd6c8327c1162807bf6f3","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ea2750c6_6f3547ba","line":701,"in_reply_to":"5efd31d9_e0d08f78","updated":"2026-05-07 13:40:21.000000000","message":"\"not found\" matches the case when the VG is not present, \"Failed to find\" matches when the LV doesn\u0027t exist. This seems to be reliable enough.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"eeb5fc82990b8a04a3ff10e3381d281d0d130801","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d080c130_08c01c21","line":701,"in_reply_to":"93a0dfbd_140aa09b","updated":"2026-05-06 15:37:40.000000000","message":"I\u0027m opened to alternative implementation that can fixes other callers.\n\nMost functions found in cinder/brick/local_dev/lvm.py just log and reraise the ProcessExecutionError exception, except some functions which raise specialized exceptions:\n- exception.VolumeNotDeactivated\n- exception.VolumeGroupCreationFailed\n- exception.VolumeGroupNotFound\n\nHow do you suggest moving the validation to lv_is_snapshot() ? Should the function raise a different Exception or return None?\n\nPlease keep in mind that I\u0027m an operator trying to contribute to Ansible and I\u0027m now fixing race condition in an unrelated project. I therefore have limited bandwidth if a major refactor is needed.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"49b0c30abdd563400c50b3304c0316f0af0724ce","unresolved":true,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5164e196_9dc8a4c3","line":701,"in_reply_to":"d080c130_08c01c21","updated":"2026-05-07 10:23:20.000000000","message":"thanks jumping in to fix this - even though it\u0027s outside your main project.\n\nbefore we dive into implementation/fix details, let\u0027s understand what\u0027s triggering this. Since this code has been stable for 10 years, knowing the scenario and frequency will help us with the right approach","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"1cb6946dd8a561d61ef5a8e1ae6fad51b019808a","unresolved":false,"context_lines":[{"line_number":698,"context_line":"                # Ignore missing volume due to a race condition between the"},{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"43ece3c6_22129dfa","line":701,"in_reply_to":"ea2750c6_6f3547ba","updated":"2026-05-13 08:21:50.000000000","message":"ack, makes sense to distinguish between VG-level and LV-level errors, not blocking on it here.\n\nthough it\u0027s simple and works, I still feel string matching stderr is kind of fragile (version-dependent may be), would be nice if LVM provided exit codes we could check, or if we had a more structured way to parse these errors. Could be worth improving project-wide later.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"49b0c30abdd563400c50b3304c0316f0af0724ce","unresolved":true,"context_lines":[{"line_number":699,"context_line":"                # get_volumes() call and the lv_is_snapshot() call, where"},{"line_number":700,"context_line":"                # the volume can get deleted or renamed between the two calls."},{"line_number":701,"context_line":"                if \"not found\" in exc.stderr or \"Failed to find\" in exc.stderr:"},{"line_number":702,"context_line":"                    continue"},{"line_number":703,"context_line":"                raise"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"            if ((resource_type \u003d\u003d \u0027volume\u0027 and is_snap) or"}],"source_content_type":"text/x-python","patch_set":2,"id":"ad69764d_d8c29e4f","line":702,"updated":"2026-05-07 10:23:20.000000000","message":"also should we log this too, like may be target should not have renamed or deleted! logging help.","commit_id":"ee581f902757103ed5e4025e7911290b214d09fd"}]}