)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"a8bca9f5c265c4bc06361b8ef9929d87f2337d8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2e816352_7f779ad1","updated":"2026-08-07 16:02:24.000000000","message":"recheck","commit_id":"2d4e6e557068f6410b466a0b99800cda513e779b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"5e13cdc158751b3ed97cd816327b3d86e1cdb960","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"700238f1_a2a2a1be","updated":"2026-08-06 03:38:02.000000000","message":"recheck","commit_id":"2d4e6e557068f6410b466a0b99800cda513e779b"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"b229f56b656b67f6cc3f52c42d105cea88a10fc8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6376b00a_8a7a2748","updated":"2026-08-27 00:09:34.000000000","message":"Had a small question, but overall this LGTM.  Will +2 once I get an answer. Ping me here or on IRC if desired.","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"783f025b05cafdad42c23043bc44f7aa03669992","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5db632df_a7da19f7","updated":"2026-08-27 22:54:35.000000000","message":"LGTM and got sufficient reply to my query.","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"27dbf1b4d7fc4f8918c56581ee93a31a229431be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"607ba058_cfe61d09","updated":"2026-08-10 20:52:10.000000000","message":"recheck","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"}],"cinder/volume/drivers/pure.py":[{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"d44b20f63b437a95fe0d7836aa0c3f7d85090f56","unresolved":true,"context_lines":[{"line_number":2049,"context_line":"                \u0027replication_status\u0027: fields.ReplicationStatus.ERROR}"},{"line_number":2050,"context_line":"            LOG.error(\"Failed to enable replication for group %(group)s: \""},{"line_number":2051,"context_line":"                      \"%(err)s\","},{"line_number":2052,"context_line":"                      {\"group\": group.id, \"err\": res.errors[0].message})"},{"line_number":2053,"context_line":"        return model_update, None"},{"line_number":2054,"context_line":""},{"line_number":2055,"context_line":"    @pure_driver_debug_trace"}],"source_content_type":"text/x-python","patch_set":1,"id":"3540db69_6618d677","line":2052,"updated":"2026-08-04 05:13:31.000000000","message":"Patch tests include one error response with ErrorResponse(400, [DotNotation({\u0027message\u0027: \u0027does not exist\u0027})], {}), and the code likely logs res.errors[0].message. If the Pure SDK returns:\n\nerrors\u003d[]\nerrors\u003dNone\nan exception instead of response\na response body without .message\n\nthen the error path raises IndexError or AttributeError, masking the real backend failure","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"b6eafda7137cfb0217f6059107e6ef9d7e758484","unresolved":false,"context_lines":[{"line_number":2049,"context_line":"                \u0027replication_status\u0027: fields.ReplicationStatus.ERROR}"},{"line_number":2050,"context_line":"            LOG.error(\"Failed to enable replication for group %(group)s: \""},{"line_number":2051,"context_line":"                      \"%(err)s\","},{"line_number":2052,"context_line":"                      {\"group\": group.id, \"err\": res.errors[0].message})"},{"line_number":2053,"context_line":"        return model_update, None"},{"line_number":2054,"context_line":""},{"line_number":2055,"context_line":"    @pure_driver_debug_trace"}],"source_content_type":"text/x-python","patch_set":1,"id":"7eee3fb6_8b3c7a64","line":2052,"in_reply_to":"3540db69_6618d677","updated":"2026-08-06 00:24:01.000000000","message":"Good catch - fixed","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bb8b86124bff19769110cc7f12332d9e38a4b66c","unresolved":true,"context_lines":[{"line_number":2127,"context_line":"            # volumes on the original primary are stale because async"},{"line_number":2128,"context_line":"            # replication is not bi-directional, so they are reported as"},{"line_number":2129,"context_line":"            # errored pending an admin resync."},{"line_number":2130,"context_line":"            current_array.patch_protection_groups("},{"line_number":2131,"context_line":"                names\u003d[pgroup_name],"},{"line_number":2132,"context_line":"                protection_group\u003dflasharray.ProtectionGroup("},{"line_number":2133,"context_line":"                    replication_schedule\u003dflasharray.ReplicationSchedule("}],"source_content_type":"text/x-python","patch_set":1,"id":"88e8cbd0_31cd17fb","line":2130,"updated":"2026-07-24 18:55:27.000000000","message":"In this async failback path, patch_protection_groups is called to re-enable the replication schedule but the return value is not checked. If the call fails, the method still returns replication_status: ENABLED, which could be misleading.\n\nCompare with enable_replication (line 2042) which checks res.status_code !\u003d 200 and returns ERROR on failure. The failback path should be consistent.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fff13f6038d8d070c25f32e78f1dacbbd827c6b2","unresolved":false,"context_lines":[{"line_number":2127,"context_line":"            # volumes on the original primary are stale because async"},{"line_number":2128,"context_line":"            # replication is not bi-directional, so they are reported as"},{"line_number":2129,"context_line":"            # errored pending an admin resync."},{"line_number":2130,"context_line":"            current_array.patch_protection_groups("},{"line_number":2131,"context_line":"                names\u003d[pgroup_name],"},{"line_number":2132,"context_line":"                protection_group\u003dflasharray.ProtectionGroup("},{"line_number":2133,"context_line":"                    replication_schedule\u003dflasharray.ReplicationSchedule("}],"source_content_type":"text/x-python","patch_set":1,"id":"b0e03b97_3a282a3d","line":2130,"updated":"2026-07-24 18:45:27.000000000","message":"In this async failback path, patch_protection_groups is called to re-enable the replication schedule but the return value is not checked. If the call fails, the method still returns replication_status: ENABLED, which could be misleading.\n\nCompare with enable_replication (line 2042) which checks res.status_code !\u003d 200 and returns ERROR on failure. The failback path should be consistent.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"b6eafda7137cfb0217f6059107e6ef9d7e758484","unresolved":false,"context_lines":[{"line_number":2127,"context_line":"            # volumes on the original primary are stale because async"},{"line_number":2128,"context_line":"            # replication is not bi-directional, so they are reported as"},{"line_number":2129,"context_line":"            # errored pending an admin resync."},{"line_number":2130,"context_line":"            current_array.patch_protection_groups("},{"line_number":2131,"context_line":"                names\u003d[pgroup_name],"},{"line_number":2132,"context_line":"                protection_group\u003dflasharray.ProtectionGroup("},{"line_number":2133,"context_line":"                    replication_schedule\u003dflasharray.ReplicationSchedule("}],"source_content_type":"text/x-python","patch_set":1,"id":"179e3fde_c5b8cdf5","line":2130,"in_reply_to":"88e8cbd0_31cd17fb","updated":"2026-08-06 00:24:01.000000000","message":"Agreed - fixed","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bb8b86124bff19769110cc7f12332d9e38a4b66c","unresolved":true,"context_lines":[{"line_number":2145,"context_line":"            # ActiveCluster volumes already exist and are live on the"},{"line_number":2146,"context_line":"            # secondary array via the stretched pod, so failover only needs to"},{"line_number":2147,"context_line":"            # update status - the same logic used for host failover."},{"line_number":2148,"context_line":"            secondary_array \u003d self._find_sync_failover_target()"},{"line_number":2149,"context_line":"            if not secondary_array:"},{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""}],"source_content_type":"text/x-python","patch_set":1,"id":"754c47d7_39bc54a1","line":2148,"updated":"2026-07-24 18:55:27.000000000","message":"When is_sync\u003dTrue and secondary_backend_id is provided (not \"default\"), _find_sync_failover_target() does its own auto-discovery, silently ignoring the user-specified secondary_backend_id. In contrast, the async path at line 2157 respects it via _get_secondary().\n\nThis matches how failover_host works (sync always auto-discovers), so it may be intentional. If so, consider adding a LOG.debug noting that secondary_backend_id is ignored for ActiveCluster groups, to avoid operator confusion.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fff13f6038d8d070c25f32e78f1dacbbd827c6b2","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            # ActiveCluster volumes already exist and are live on the"},{"line_number":2146,"context_line":"            # secondary array via the stretched pod, so failover only needs to"},{"line_number":2147,"context_line":"            # update status - the same logic used for host failover."},{"line_number":2148,"context_line":"            secondary_array \u003d self._find_sync_failover_target()"},{"line_number":2149,"context_line":"            if not secondary_array:"},{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""}],"source_content_type":"text/x-python","patch_set":1,"id":"baf04020_6e521438","line":2148,"updated":"2026-07-24 18:45:27.000000000","message":"When is_sync\u003dTrue and secondary_backend_id is provided (not \"default\"), _find_sync_failover_target() does its own auto-discovery, silently ignoring the user-specified secondary_backend_id. In contrast, the async path at line 2157 respects it via _get_secondary().\n\nThis matches how failover_host works (sync always auto-discovers), so it may be intentional. If so, consider adding a LOG.debug noting that secondary_backend_id is ignored for ActiveCluster groups, to avoid operator confusion.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"b6eafda7137cfb0217f6059107e6ef9d7e758484","unresolved":false,"context_lines":[{"line_number":2145,"context_line":"            # ActiveCluster volumes already exist and are live on the"},{"line_number":2146,"context_line":"            # secondary array via the stretched pod, so failover only needs to"},{"line_number":2147,"context_line":"            # update status - the same logic used for host failover."},{"line_number":2148,"context_line":"            secondary_array \u003d self._find_sync_failover_target()"},{"line_number":2149,"context_line":"            if not secondary_array:"},{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""}],"source_content_type":"text/x-python","patch_set":1,"id":"c4734dc1_10cfd81e","line":2148,"in_reply_to":"754c47d7_39bc54a1","updated":"2026-08-06 00:24:01.000000000","message":"Yes, it is intentional: for an ActiveCluster group the peer is whichever array is serving the stretched pod, so there is no meaningful operator choice to honour, and this matches how failover_host() behaves for sync targets.\n\nAdded the LOG.debug you suggested","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"d44b20f63b437a95fe0d7836aa0c3f7d85090f56","unresolved":true,"context_lines":[{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""},{"line_number":2152,"context_line":"                             \"array for group failover.\"))"},{"line_number":2153,"context_line":"            host_updates \u003d self._sync_failover_host(volumes, secondary_array)"},{"line_number":2154,"context_line":"        else:"},{"line_number":2155,"context_line":"            # Determine which secondary array to fail over to."},{"line_number":2156,"context_line":"            if secondary_backend_id:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fc4c9e80_5c75fe46","line":2153,"updated":"2026-08-04 05:13:31.000000000","message":"Tiramisu’s purpose is tenant granular failover without failing over the whole backend. The spec explicitly contrasts group-level failover with backend-wide Cheesecake failover.group failover must be group-scoped, not host/backend-scoped. If _sync_failover_host() changes any driver-wide state such as active array, current array, replication target state, or backend active_backend_id assumptions, it can affect volumes outside the group.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e3e42a7096ce4dcf3b5f1a8dbc327ba6fb4b46c9","unresolved":true,"context_lines":[{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""},{"line_number":2152,"context_line":"                             \"array for group failover.\"))"},{"line_number":2153,"context_line":"            host_updates \u003d self._sync_failover_host(volumes, secondary_array)"},{"line_number":2154,"context_line":"        else:"},{"line_number":2155,"context_line":"            # Determine which secondary array to fail over to."},{"line_number":2156,"context_line":"            if secondary_backend_id:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8206dcdf_c3a3707d","line":2153,"in_reply_to":"16920515_7eb92dfe","updated":"2026-08-08 16:15:41.000000000","message":"Following up now that PS4 is up — my earlier reply said I had verified and documented this rather than changed behaviour, and that is no longer the whole story.\n\nThe core of my answer stands: failover_replication() still does not call failover_completed(), so _swap_replication_state() never runs and the driver\u0027s current array, active_backend_id and replication target lists are untouched. A group failover does not move the backend.\n\nBut you were right that leaving it there was not enough. Because the current array does not move, nothing was routing operations on the failed over volumes to the array now serving them — Cinder would mark the group FAILED_OVER while the driver carried on talking to the primary. PS3/PS4 fix that per volume rather than per backend:\n\nfailover_replication() records the serving array\u0027s backend_id in each volume\u0027s replication_driver_data (and clears it on failback). The volume manager already persists whatever the driver returns in volumes_model_update, so this needs no core change.\n_get_current_array() takes an optional volume\u003d, resolving through the new _get_array_for_volume(). Volumes with no recorded backend behave exactly as before, so volumes outside the group are unaffected.\nDelete, extend, snapshot create/delete, revert, and attach/detach on all three protocols now follow the volume to the secondary. Clone, create-from-snapshot and retype are rejected while failed over, since they would place a new volume on the secondary outside the replication topology.\nOne limitation remains, documented in the release note: operations on the array protection group itself (delete group, add/remove volumes, group snapshot) do not work while an async group is failed over, because the protection group only exists on the original primary. That gap is shared with backend-wide failover_host and I have not tried to close it here.\n\ntest_failover_replication_leaves_backend_state_alone asserts the backend state is untouched, and PureGroupFailoverRoutingTestCase covers the routing. Please push back if you think the split between what routes and what is rejected is drawn in the wrong place.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"fb26c944bd7cef09a803f33b1424a0ecd9f00099","unresolved":false,"context_lines":[{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""},{"line_number":2152,"context_line":"                             \"array for group failover.\"))"},{"line_number":2153,"context_line":"            host_updates \u003d self._sync_failover_host(volumes, secondary_array)"},{"line_number":2154,"context_line":"        else:"},{"line_number":2155,"context_line":"            # Determine which secondary array to fail over to."},{"line_number":2156,"context_line":"            if secondary_backend_id:"}],"source_content_type":"text/x-python","patch_set":1,"id":"f0ecb6ca_53c86489","line":2153,"in_reply_to":"8206dcdf_c3a3707d","updated":"2026-08-17 15:36:05.000000000","message":"failover_completed() / _swap_replication_state(),\nand volumes outside the group are untouched. I agree","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"b6eafda7137cfb0217f6059107e6ef9d7e758484","unresolved":true,"context_lines":[{"line_number":2150,"context_line":"                raise PureDriverException("},{"line_number":2151,"context_line":"                    reason\u003d_(\"Unable to find viable ActiveCluster secondary \""},{"line_number":2152,"context_line":"                             \"array for group failover.\"))"},{"line_number":2153,"context_line":"            host_updates \u003d self._sync_failover_host(volumes, secondary_array)"},{"line_number":2154,"context_line":"        else:"},{"line_number":2155,"context_line":"            # Determine which secondary array to fail over to."},{"line_number":2156,"context_line":"            if secondary_backend_id:"}],"source_content_type":"text/x-python","patch_set":1,"id":"16920515_7eb92dfe","line":2153,"in_reply_to":"fc4c9e80_5c75fe46","updated":"2026-08-06 00:24:01.000000000","message":"Agree completely with the principle, and I believe the implementation already meets it — so I have verified and documented this rather than changed behaviour. Please do push back if you see a path I have missed.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"bb8b86124bff19769110cc7f12332d9e38a4b66c","unresolved":true,"context_lines":[{"line_number":2158,"context_line":"            else:"},{"line_number":2159,"context_line":"                secondary_array \u003d None"},{"line_number":2160,"context_line":"                for array in self._replication_target_arrays:"},{"line_number":2161,"context_line":"                    if array.replication_type \u003d\u003d REPLICATION_TYPE_ASYNC:"},{"line_number":2162,"context_line":"                        secondary_array \u003d array"},{"line_number":2163,"context_line":"                        break"},{"line_number":2164,"context_line":"            if not secondary_array:"}],"source_content_type":"text/x-python","patch_set":1,"id":"545d014b_4affce29","line":2161,"updated":"2026-07-24 18:55:27.000000000","message":"When secondary_backend_id is not provided (it\u0027s optional per the API schema) and the group is trisync, is_sync is False (because REPLICATION_TYPE_SYNC not in {\u0027trisync\u0027}), so the code falls through to the else branch here. This loop only matches REPLICATION_TYPE_ASYNC, so a trisync target (array.replication_type \u003d\u003d REPLICATION_TYPE_TRISYNC) is never matched — secondary_array stays None and the method raises PureDriverException.\n\nFailover with an explicit secondary_backend_id works fine (goes through _get_secondary() which matches on backend_id regardless of type), but the auto-discovery path is broken for trisync groups.\n\nSuggested fix:\n\n  if array.replication_type in [REPLICATION_TYPE_ASYNC,\n                                REPLICATION_TYPE_TRISYNC]:\n\nNote: the existing _find_async_failover_target() in failover_host has the same ASYNC-only filter — that may also be a pre-existing gap, but it\u0027s out of scope for this patch.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fff13f6038d8d070c25f32e78f1dacbbd827c6b2","unresolved":false,"context_lines":[{"line_number":2158,"context_line":"            else:"},{"line_number":2159,"context_line":"                secondary_array \u003d None"},{"line_number":2160,"context_line":"                for array in self._replication_target_arrays:"},{"line_number":2161,"context_line":"                    if array.replication_type \u003d\u003d REPLICATION_TYPE_ASYNC:"},{"line_number":2162,"context_line":"                        secondary_array \u003d array"},{"line_number":2163,"context_line":"                        break"},{"line_number":2164,"context_line":"            if not secondary_array:"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c3e4073_e5b8389f","line":2161,"updated":"2026-07-24 18:45:27.000000000","message":"When secondary_backend_id is not provided (it\u0027s optional per the API schema) and the group is trisync, is_sync is False (because REPLICATION_TYPE_SYNC not in {\u0027trisync\u0027}), so the code falls through to the else branch here. This loop only matches REPLICATION_TYPE_ASYNC, so a trisync target (array.replication_type \u003d\u003d REPLICATION_TYPE_TRISYNC) is never matched — secondary_array stays None and the method raises PureDriverException.\n\nFailover with an explicit secondary_backend_id works fine (goes through _get_secondary() which matches on backend_id regardless of type), but the auto-discovery path is broken for trisync groups.\n\nSuggested fix:\n\n  if array.replication_type in [REPLICATION_TYPE_ASYNC,\n                                REPLICATION_TYPE_TRISYNC]:\n\nNote: the existing _find_async_failover_target() in failover_host has the same ASYNC-only filter — that may also be a pre-existing gap, but it\u0027s out of scope for this patch.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"b6eafda7137cfb0217f6059107e6ef9d7e758484","unresolved":true,"context_lines":[{"line_number":2158,"context_line":"            else:"},{"line_number":2159,"context_line":"                secondary_array \u003d None"},{"line_number":2160,"context_line":"                for array in self._replication_target_arrays:"},{"line_number":2161,"context_line":"                    if array.replication_type \u003d\u003d REPLICATION_TYPE_ASYNC:"},{"line_number":2162,"context_line":"                        secondary_array \u003d array"},{"line_number":2163,"context_line":"                        break"},{"line_number":2164,"context_line":"            if not secondary_array:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a7ecc4a8_d301fa9b","line":2161,"in_reply_to":"545d014b_4affce29","updated":"2026-08-06 00:24:01.000000000","message":"I have taken your suggested fix in PS2, but I do not think the failure mode is reachable today, so I want to record the reasoning.\n\nNo replication target ever has replication_type \u003d\u003d REPLICATION_TYPE_TRISYNC. Trisync is a topology, not a device type: a trisync deployment configures exactly two replication_device entries, one type\u003dsync and one type\u003dasync, and do_setup_trisync() explicitly rejects any other combination (\"Replication devices provided must be one each of sync and async\"). REPLICATION_TYPE_TRISYNC is only ever a volume-type/group-level replication type.\n\nFor a trisync group it is the async leg that holds the replicated snapshots of the group\u0027s protection group — which is exactly what _get_latest_replicated_pg_snap() needs here — so the ASYNC-only filter was already selecting the correct array, and auto-discovery works.\n\nThat said, \"type\" in replication_device is free-form configuration text, so accepting TRISYNC as well is cheap insurance against a misconfiguration slipping through. PS2 makes the change with a comment explaining that it is defensive rather than a live bug fix, and adds test_failover_replication_no_secondary_id, which covers a trisync group auto-discovering past the sync target to the async one (plus test_failover_replication_no_secondary_id_no_target for the no-viable-target case).\n\nAgreed that the same observation applies to _find_async_failover_target() and is out of scope here.","commit_id":"b95347fcd04754884e2e500b33296ee2387717bf"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"b229f56b656b67f6cc3f52c42d105cea88a10fc8","unresolved":true,"context_lines":[{"line_number":2120,"context_line":"        return model_update, None"},{"line_number":2121,"context_line":""},{"line_number":2122,"context_line":"    @pure_driver_debug_trace"},{"line_number":2123,"context_line":"    def failover_replication(self, context, group, volumes,"},{"line_number":2124,"context_line":"                             secondary_backend_id\u003dNone):"},{"line_number":2125,"context_line":"        \"\"\"Failover replication for a group. (Tiramisu)"},{"line_number":2126,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0886035e_a42b61d7","line":2123,"updated":"2026-08-27 00:09:34.000000000","message":"I will not neg for this reason and there is no need to change it (in fact, I wouldn\u0027t if it were me unless someone else asks you for changes), but I do feel like this method should be broken down into 2-4 smaller chunks for readability.","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"b229f56b656b67f6cc3f52c42d105cea88a10fc8","unresolved":true,"context_lines":[{"line_number":2189,"context_line":"            else:"},{"line_number":2190,"context_line":"                # The schedule was not re-enabled, so the group is not"},{"line_number":2191,"context_line":"                # replicating again - do not report it as enabled."},{"line_number":2192,"context_line":"                repl_status \u003d fields.ReplicationStatus.ERROR"},{"line_number":2193,"context_line":"                LOG.error(\"Failed to re-enable replication for group \""},{"line_number":2194,"context_line":"                          \"%(group)s on failback: %(err)s\","},{"line_number":2195,"context_line":"                          {\"group\": group.id, \"err\": _get_error_details(res)})"}],"source_content_type":"text/x-python","patch_set":4,"id":"cd845fe5_9d30e586","line":2192,"updated":"2026-08-27 00:09:34.000000000","message":"Is there a reason why the admin resync/replication schedule doesn\u0027t occur/enable automatically?","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"783f025b05cafdad42c23043bc44f7aa03669992","unresolved":false,"context_lines":[{"line_number":2189,"context_line":"            else:"},{"line_number":2190,"context_line":"                # The schedule was not re-enabled, so the group is not"},{"line_number":2191,"context_line":"                # replicating again - do not report it as enabled."},{"line_number":2192,"context_line":"                repl_status \u003d fields.ReplicationStatus.ERROR"},{"line_number":2193,"context_line":"                LOG.error(\"Failed to re-enable replication for group \""},{"line_number":2194,"context_line":"                          \"%(group)s on failback: %(err)s\","},{"line_number":2195,"context_line":"                          {\"group\": group.id, \"err\": _get_error_details(res)})"}],"source_content_type":"text/x-python","patch_set":4,"id":"8395dedd_7a24d705","line":2192,"in_reply_to":"cd845fe5_9d30e586","updated":"2026-08-27 22:54:35.000000000","message":"From Simon over IRC:\n\n\u003e re: your q on 996497 (pure.py:2192) - the replication schedule *is*\n\u003e re-enabled automatically, that\u0027s the patch_protection_groups call just\n\u003e above at 2182, and PS2 added a status check on it so a failure reports\n\u003e ERROR rather than claiming ENABLED. The \"pending an admin resync\"\n\u003e comment above it is about the data, not the schedule - my wording is\n\u003e misleading there, happy to reword it.\n\u003e \n\u003e What\u0027s not automatic is resyncing the data. FA async replication is\n\u003e one-directional (primary-\u003esecondary), so after a group failover the\n\u003e current data is on the secondary and the volumes back on the primary\n\u003e are stale. Resyncing means picking which copy wins and overwriting the\n\u003e other, and I didn\u0027t want a status-change API call to silently do a\n\u003e destructive data movement - if the failover was a mistake, an automatic\n\u003e overwrite destroys the option of going back. So the volumes come back\n\u003e as \u0027error\u0027 to make the staleness visible instead of quietly serving old\n\u003e data.\n\u003e \n\u003e It also matches what the existing backend-wide failback already does -\n\u003e _swap_replication_state(failback\u003dTrue) swaps state and re-adds the old\n\u003e primary as a target, but moves no data either. So it\u0027s consistent with\n\u003e the driver rather than a new gap.\n\u003e \n\u003e The array *can* replicate the other way, so a real reverse-sync-then-\n\u003e flip failback is doable as a follow-up if people want it - just felt\n\u003e like more than this patch should take on.","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"},{"author":{"_account_id":38081,"name":"Anthony Galica","display_name":"agalica","email":"anthony.galica@hitachivantara.com","username":"agalica","status":"Hitachi Vantara"},"change_message_id":"b229f56b656b67f6cc3f52c42d105cea88a10fc8","unresolved":false,"context_lines":[{"line_number":4193,"context_line":"        for array in self._replication_target_arrays:"},{"line_number":4194,"context_line":"            if array.backend_id \u003d\u003d backend_id:"},{"line_number":4195,"context_line":"                return array"},{"line_number":4196,"context_line":"        # The recorded array is no longer a configured replication target,"},{"line_number":4197,"context_line":"        # which most likely means replication_device has been changed since"},{"line_number":4198,"context_line":"        # the group was failed over. There is nothing better to do than use"},{"line_number":4199,"context_line":"        # the current array, but the operation may well fail."}],"source_content_type":"text/x-python","patch_set":4,"id":"6513bf63_865760cc","line":4196,"updated":"2026-08-27 00:09:34.000000000","message":"Gotta love users!","commit_id":"9f08ae2885ddbf0e7d6f698312e0f78468478f95"}]}