)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fb48d95971b9f9997ac2458a5aa1119e6c132b08","unresolved":true,"context_lines":[{"line_number":15,"context_line":"becomes unusable after the transfer as any operation performed"},{"line_number":16,"context_line":"on the volume tries to find the group which doesn\u0027t exist in"},{"line_number":17,"context_line":"the destination project and errors out."},{"line_number":18,"context_line":"This patch adds a check for group_id while accepting a transfer."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-Bug: #1955670"},{"line_number":21,"context_line":"Change-Id: Ia1d7d7605686afb68900801dff8aca7be28b97d0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"cd598b57_0e2ff5c6","line":18,"range":{"start_line":18,"start_character":0,"end_line":18,"end_character":64},"updated":"2022-01-12 16:31:09.000000000","message":"Don\u0027t forget to update this if you accept my suggestion in the transfer code.","commit_id":"4468693ad02cd9e353e3da27ae6ea12f13909fd5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b0b1dcbc48ff5fb0f9ea2489d3550db748073cf8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fb549bf3_3941bce3","updated":"2022-01-13 09:10:21.000000000","message":"Hi Brian,\nthanks for review. Please find my comment inline.","commit_id":"4468693ad02cd9e353e3da27ae6ea12f13909fd5"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fb48d95971b9f9997ac2458a5aa1119e6c132b08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"af3b9aec_367a7aee","updated":"2022-01-12 16:31:09.000000000","message":"Suggestion for a more thorough fix noted inline.","commit_id":"4468693ad02cd9e353e3da27ae6ea12f13909fd5"}],"cinder/transfer/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"fb48d95971b9f9997ac2458a5aa1119e6c132b08","unresolved":true,"context_lines":[{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        volume_id \u003d transfer[\u0027volume_id\u0027]"},{"line_number":236,"context_line":"        vol_ref \u003d objects.Volume.get_by_id(context.elevated(), volume_id)"},{"line_number":237,"context_line":"        if vol_ref.consistencygroup_id or vol_ref.group_id:"},{"line_number":238,"context_line":"            msg \u003d _(\"Volume %s must not be part of a group.\") % vol_ref.id"},{"line_number":239,"context_line":"            LOG.error(msg)"},{"line_number":240,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1dea9e07_4586478e","line":237,"range":{"start_line":237,"start_character":8,"end_line":237,"end_character":59},"updated":"2022-01-12 16:31:09.000000000","message":"The only reason I can see to do the check here is to handle transfers that were already created before a deployment has been updated to a cinder containing this fix. \n\nI think it would be better to have this check in create(), where we currently do some other checks.  After all, there\u0027s nothing the acceptor can do to fix this because the group is owned by the person who created the transfer.  My suggestion is to leave this here with a TODO to remove in Z (or AA), and copy it up to the create() function where it belongs.","commit_id":"4468693ad02cd9e353e3da27ae6ea12f13909fd5"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b0b1dcbc48ff5fb0f9ea2489d3550db748073cf8","unresolved":true,"context_lines":[{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        volume_id \u003d transfer[\u0027volume_id\u0027]"},{"line_number":236,"context_line":"        vol_ref \u003d objects.Volume.get_by_id(context.elevated(), volume_id)"},{"line_number":237,"context_line":"        if vol_ref.consistencygroup_id or vol_ref.group_id:"},{"line_number":238,"context_line":"            msg \u003d _(\"Volume %s must not be part of a group.\") % vol_ref.id"},{"line_number":239,"context_line":"            LOG.error(msg)"},{"line_number":240,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b0c4a6e8_aa5e83f1","line":237,"range":{"start_line":237,"start_character":8,"end_line":237,"end_character":59},"in_reply_to":"1dea9e07_4586478e","updated":"2022-01-13 09:10:21.000000000","message":"My initial thoughts were similar that it makes sense for create transfer but due to certain reason i made changes to accept.\nThe first thought in my head was create and accept are two different operations that can be performed separately in a time difference and doesn\u0027t need to happen instantly. It could be possible that the donor is planning to do some operations on the volume before removing it from the group, so donor creates a transfer, performs it\u0027s operations, removes volume from group and notifies acceptor that you can accept the transfer now. As i didn\u0027t see any issue with creating the transfer i.e. we are not changing anything in the resource or DB, it didn\u0027t feel right to me to block that operation then.\nAlthough it doesn\u0027t make much difference if we block create or accept, the functionality will remain same for volume in groups but that\u0027s what my thoughts are on this.\nWould be happy to change if my reasoning isn\u0027t valid enough.","commit_id":"4468693ad02cd9e353e3da27ae6ea12f13909fd5"}]}