)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"9a58affe0a2ae1694f6faa9105813be3f94a795d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9b1da58c_2c3bf326","updated":"2024-08-21 13:38:57.000000000","message":"As far as I understand it, it looks good to me.\nOf course the point raised by haixin is valid and must be managed.","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"2d15318a7dab799e32b2906a30cc487a1772c482","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"751ee12b_70a9385d","updated":"2024-08-19 14:06:04.000000000","message":"Thank you very much for the review. Please take a look at the reply to the question inline :)","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"ace05787dd2c0d0521194894643f397ffe360cab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3db81a22_53878666","updated":"2024-08-20 01:26:57.000000000","message":"code looks good, just one comment inline.","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"23d5c22368537f748ae3abb73f5cd8c4bd916073","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"80d4c899_629e0a95","updated":"2024-08-19 09:43:29.000000000","message":"thanks for your change. i have a question.\n\nIf want to realize that any share with access rule is not allowed to be deleted, why not directly delete the share at the api layer, query the access rule(active) associated with share, once found out, directly return 403(forbidden:has access rule).\n\nThis patch can also be implemented using resource lock, which seems less convenient than the above method.what do you think?","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"2d15318a7dab799e32b2906a30cc487a1772c482","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d709198e_e0bd2a72","in_reply_to":"5de7571c_8b6aae8a","updated":"2024-08-19 14:06:04.000000000","message":"Hey, haixin! Thank you for bringing it up, and also thank you very much for adding your thoughts into the nova change. Really nice to see your interaction there.\nThe main reason behind this is that we would alter the behavior we had in Manila since the very beginning: when we delete the shares, we will drop the access rules alongside and won\u0027t check it. If we change the behavior for everyone, we might break some automation.\n\nSo some reasons behind an extra lock to the share are:\n- Avoiding to change the share deletion behavior in case there are access rules and breaking automation and the previous workflow.\n- Reusing the lock mechanism.\n- Taking the assumption that if an access rule is locked (you don\u0027t want it to go away and get disconnected), you also don\u0027t want the share to go away.\n\nAll of that, reusing the lock mechanism.\nI agree that this approach is going to be more costly in terms of performance, but I think that\u0027s the approach that will help us to remain consistent with our APIs and do not break many workflows.","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"d1271641fe9a31d331bb04a3825db0fd3fee6f67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5de7571c_8b6aae8a","in_reply_to":"80d4c899_629e0a95","updated":"2024-08-19 09:58:56.000000000","message":"https://review.opendev.org/c/openstack/nova/+/833090/52/nova/compute/manager.py#4646\nFor the nova patch above, need to disable the deletion of the share that has access rule associated with the nova compute host.","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"ace05787dd2c0d0521194894643f397ffe360cab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3f9c805f_436a7826","in_reply_to":"d709198e_e0bd2a72","updated":"2024-08-20 01:26:57.000000000","message":"hi, Carlos Eduardo. Thank you very much for the detailed explanation.\nFrom this point of view, this way should be a better solution at present.","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ae0a26cc67ba09e71b00ca4ae5db2ae5d1077ae1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"75c5c80b_a09005b6","updated":"2024-08-28 10:39:22.000000000","message":"I tested this patch together with the series https://review.opendev.org/c/openstack/nova/+/871642/41\n\nIt works as expected. \n\n```\nstack@gibi-devstack-aio-jammy:~$ openstack share lock list --all\n+--------------------------------------+--------------------------------------+---------------+-----------------+\n| ID                                   | Resource Id                          | Resource Type | Resource Action |\n+--------------------------------------+--------------------------------------+---------------+-----------------+\n| 711d8a24-dc38-4243-a0b3-b9a1f2330f3e | b4140a8a-fcdb-4e6b-a74e-7c0babdec3fc | access_rule   | show            |\n| 8e576091-22ff-4082-bf71-aa645862ac4e | a731660c-8764-497b-b553-3dd947365ba4 | share         | delete          |\n| 884bcf2d-c29b-4026-8daf-b157f2c6e485 | b4140a8a-fcdb-4e6b-a74e-7c0babdec3fc | access_rule   | delete          |\n+--------------------------------------+--------------------------------------+---------------+-----------------+\nstack@gibi-devstack-aio-jammy:~$ openstack share lock show 711d8a24-dc38-4243-a0b3-b9a1f2330f3e\n+-----------------+--------------------------------------+\n| Field           | Value                                |\n+-----------------+--------------------------------------+\n| ID              | 711d8a24-dc38-4243-a0b3-b9a1f2330f3e |\n| Resource Id     | b4140a8a-fcdb-4e6b-a74e-7c0babdec3fc |\n| Resource Type   | access_rule                          |\n| Resource Action | show                                 |\n| Lock Context    | admin                                |\n| User Id         | 9eeb0cc2992c4af0aecaa9ae195218b8     |\n| Project Id      | 0bad8a3008c548709fe8115edb0b437d     |\n| Created At      | 2024-08-28T10:29:19.027556           |\n| Updated At      | None                                 |\n| Lock Reason     | Lock by nova                         |\n+-----------------+--------------------------------------+\nstack@gibi-devstack-aio-jammy:~$ openstack share lock show 8e576091-22ff-4082-bf71-aa645862ac4e\n+-----------------+-------------------------------------------------------------+\n| Field           | Value                                                       |\n+-----------------+-------------------------------------------------------------+\n| ID              | 8e576091-22ff-4082-bf71-aa645862ac4e                        |\n| Resource Id     | a731660c-8764-497b-b553-3dd947365ba4                        |\n| Resource Type   | share                                                       |\n| Resource Action | delete                                                      |\n| Lock Context    | admin                                                       |\n| User Id         | 9eeb0cc2992c4af0aecaa9ae195218b8                            |\n| Project Id      | 0bad8a3008c548709fe8115edb0b437d                            |\n| Created At      | 2024-08-28T10:29:19.012950                                  |\n| Updated At      | None                                                        |\n| Lock Reason     | Locked by access lock: 884bcf2d-c29b-4026-8daf-b157f2c6e485 |\n+-----------------+-------------------------------------------------------------+\nstack@gibi-devstack-aio-jammy:~$ openstack share lock show 884bcf2d-c29b-4026-8daf-b157f2c6e485\n+-----------------+--------------------------------------+\n| Field           | Value                                |\n+-----------------+--------------------------------------+\n| ID              | 884bcf2d-c29b-4026-8daf-b157f2c6e485 |\n| Resource Id     | b4140a8a-fcdb-4e6b-a74e-7c0babdec3fc |\n| Resource Type   | access_rule                          |\n| Resource Action | delete                               |\n| Lock Context    | admin                                |\n| User Id         | 9eeb0cc2992c4af0aecaa9ae195218b8     |\n| Project Id      | 0bad8a3008c548709fe8115edb0b437d     |\n| Created At      | 2024-08-28T10:29:18.991647           |\n| Updated At      | None                                 |\n| Lock Reason     | Lock by nova                         |\n+-----------------+--------------------------------------+\nstack@gibi-devstack-aio-jammy:~$ \n\n```\n\nAnd with the demo user I cannot delete the share now:\n```\nstack@gibi-devstack-aio-jammy:~$ openstack share delete a731660c-8764-497b-b553-3dd947365ba4\nFailed to delete share with name or ID \u0027a731660c-8764-497b-b553-3dd947365ba4\u0027: Resource lock/s [8e576091-22ff-4082-bf71-aa645862ac4e] prevent delete action. (HTTP 403) (Request-ID: req-67fd379d-d79d-4894-8f29-f65dba60c5cb)\n1 of 1 shares failed to delete.\n\n```\n\n\nThanks for the fix the behavior looks good to me. (I did not reviewed the implementation itself)","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"8503d26336b6145f45722bc25b80512feb5f510c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b7848f9c_dc07589b","updated":"2024-08-23 01:31:39.000000000","message":"LGTM. thanks!","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"334c27db61cace55e42ef3a523be102e2d28cdcb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9256c846_65780241","updated":"2024-09-04 06:27:59.000000000","message":"Thanks Carlos! LGTM","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"d861318ebabadee061e167dc1911b9c9776fcc9b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e203ab46_ef966d46","updated":"2024-08-22 19:27:22.000000000","message":"Thanks for the reviews, PTAL","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0e3622bf8864ab5df79d4bb20a2069cbfa1ae10f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"418f5e96_af685c4b","updated":"2024-08-28 13:31:52.000000000","message":"are we waiting for sometihng to add +w?\n\ngiven this is being treated as a bug i also assume this will be backproted to the release that adde the lock api after this is merged.","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e323fc6ce283b64480abc289dee3da52ad4ed2c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9e52e2cb_c8cf65f5","updated":"2024-08-28 13:32:54.000000000","message":"by the way if this will not be backported then this need a new api microverison if manilla uses thsoe as we need to be able to detect this in nova.","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"32596853fbf275bd94e44b7b748261ff1e9dbd3a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6371e815_be941efe","in_reply_to":"9e52e2cb_c8cf65f5","updated":"2024-08-28 15:06:28.000000000","message":"Hey, Sean! Thanks for the review.\nYes, we are planning to backport this change.\nFor merging/reviews, I am hoping that we can have @gouthampravi@gmail.com eyes on this change too, as he also worked on the resource locks feature :)","commit_id":"36f6ec0426dc186654c690503ea2ea2ca5a38ef4"}],"manila/api/v1/shares.py":[{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"ace05787dd2c0d0521194894643f397ffe360cab","unresolved":true,"context_lines":[{"line_number":515,"context_line":"                    resource_type\u003d\u0027share\u0027,"},{"line_number":516,"context_line":"                    resource_action\u003dconstants.RESOURCE_ACTION_DELETE,"},{"line_number":517,"context_line":"                    lock_reason\u003dshare_lock_reason)"},{"line_number":518,"context_line":"            except Exception:"},{"line_number":519,"context_line":"                raise_lock_failed("},{"line_number":520,"context_line":"                    access[\u0027share_id\u0027], constants.RESOURCE_ACTION_DELETE,"},{"line_number":521,"context_line":"                    resource_type\u003d\u0027share\u0027"},{"line_number":522,"context_line":"                )"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        if lock_visibility:"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ef4d99f4_258629fd","line":522,"range":{"start_line":518,"start_character":11,"end_line":522,"end_character":17},"updated":"2024-08-20 01:26:57.000000000","message":"if access lock added successed. but share lock failed.\ni think we should roll back access lock. self.resource_locks_api.delete().\nBecause, if the user tries to lock the access rule again, the previous access lock will remain.  Need to manually delete it, we can automatically roll it back here,\n\n    except Exception:\n        if access_deletion_lock:\n            self.resource_locks_api.delete(context, access_deletion_lock[\u0027id\u0027])\n        raise_lock_failed(\n            access[\u0027share_id\u0027], constants.RESOURCE_ACTION_DELETE,\n            resource_type\u003d\u0027share\u0027)","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"3c6c3175151f949823790e98fdd204d02554471a","unresolved":false,"context_lines":[{"line_number":515,"context_line":"                    resource_type\u003d\u0027share\u0027,"},{"line_number":516,"context_line":"                    resource_action\u003dconstants.RESOURCE_ACTION_DELETE,"},{"line_number":517,"context_line":"                    lock_reason\u003dshare_lock_reason)"},{"line_number":518,"context_line":"            except Exception:"},{"line_number":519,"context_line":"                raise_lock_failed("},{"line_number":520,"context_line":"                    access[\u0027share_id\u0027], constants.RESOURCE_ACTION_DELETE,"},{"line_number":521,"context_line":"                    resource_type\u003d\u0027share\u0027"},{"line_number":522,"context_line":"                )"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        if lock_visibility:"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"c1329d57_365eef6f","line":522,"range":{"start_line":518,"start_character":11,"end_line":522,"end_character":17},"in_reply_to":"54f94871_7244ae37","updated":"2024-08-22 19:27:12.000000000","message":"Done","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"9a97c6259083682be10e8bc431a7a4b8cf098b39","unresolved":true,"context_lines":[{"line_number":515,"context_line":"                    resource_type\u003d\u0027share\u0027,"},{"line_number":516,"context_line":"                    resource_action\u003dconstants.RESOURCE_ACTION_DELETE,"},{"line_number":517,"context_line":"                    lock_reason\u003dshare_lock_reason)"},{"line_number":518,"context_line":"            except Exception:"},{"line_number":519,"context_line":"                raise_lock_failed("},{"line_number":520,"context_line":"                    access[\u0027share_id\u0027], constants.RESOURCE_ACTION_DELETE,"},{"line_number":521,"context_line":"                    resource_type\u003d\u0027share\u0027"},{"line_number":522,"context_line":"                )"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        if lock_visibility:"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"54f94871_7244ae37","line":522,"range":{"start_line":518,"start_character":11,"end_line":522,"end_character":17},"in_reply_to":"ef4d99f4_258629fd","updated":"2024-08-20 10:22:09.000000000","message":"+1","commit_id":"a1f6492207b29a718a3b346eabd639fbb24c30ea"}]}