)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"126ccfd6e5237c9e4b0f449747e500ac5b75d9ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bac36381_5c482e4b","updated":"2026-02-05 15:08:37.000000000","message":"Adding netapp reviewers","commit_id":"774d718e1e8fa4896c9535d7ef702f3b193c904a"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"c48be484a5d1e0fbcd4f598a5823256d614ec5e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f935886d_397a6c4a","updated":"2026-03-05 11:38:30.000000000","message":"Please look at the comments.","commit_id":"d65131f8dc92350e3253e22585b109b9d93db8dd"}],"cinder/volume/drivers/netapp/dataontap/client/api.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"d6382a64196900d1302a407dc9f7eac3eca6286b","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        self._vserver \u003d vserver"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    @volume_utils.trace_api(filter_function\u003dna_utils.trace_filter_func_api)"},{"line_number":195,"context_line":"    def send_http_request(self, na_element, enable_tunneling\u003dFalse):"},{"line_number":196,"context_line":"        \"\"\"Invoke the API on the server.\"\"\""},{"line_number":197,"context_line":"        if not na_element or not isinstance(na_element, NaElement):"},{"line_number":198,"context_line":"            raise ValueError(\u0027NaElement must be supplied to invoke API\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"c09f27ca_283377e7","line":195,"range":{"start_line":195,"start_character":4,"end_line":195,"end_character":68},"updated":"2025-12-17 12:31:43.000000000","message":"Here is where you should be doing the retrying, and you also should use the @utils.retry() decorator. With that decorator, you will need to raise one exception if your response[\u0027error\u0027] \u003d\u003d EAPI_LOCKED_VSERVER. Also please add unit tests.","commit_id":"774d718e1e8fa4896c9535d7ef702f3b193c904a"},{"author":{"_account_id":37477,"name":"Florian","display_name":"FloD","email":"Florian.Doerheim@digits.schwarz","username":"FloD"},"change_message_id":"6eaf246743ff778861609e053c8b41fb7afb640d","unresolved":true,"context_lines":[{"line_number":192,"context_line":"        self._vserver \u003d vserver"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    @volume_utils.trace_api(filter_function\u003dna_utils.trace_filter_func_api)"},{"line_number":195,"context_line":"    def send_http_request(self, na_element, enable_tunneling\u003dFalse):"},{"line_number":196,"context_line":"        \"\"\"Invoke the API on the server.\"\"\""},{"line_number":197,"context_line":"        if not na_element or not isinstance(na_element, NaElement):"},{"line_number":198,"context_line":"            raise ValueError(\u0027NaElement must be supplied to invoke API\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"232309b8_8f23599d","line":195,"range":{"start_line":195,"start_character":4,"end_line":195,"end_character":68},"in_reply_to":"c09f27ca_283377e7","updated":"2026-02-20 15:40:52.000000000","message":"added unittests and the retry decorator","commit_id":"774d718e1e8fa4896c9535d7ef702f3b193c904a"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"c48be484a5d1e0fbcd4f598a5823256d614ec5e5","unresolved":true,"context_lines":[{"line_number":233,"context_line":"        response_element \u003d self._get_result(response_xml)"},{"line_number":234,"context_line":"        return response_element"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    @na_utils.retry(NaRetryableError, retries\u003d3, interval\u003d2)"},{"line_number":237,"context_line":"    def invoke_successfully(self, na_element, enable_tunneling\u003dFalse):"},{"line_number":238,"context_line":"        \"\"\"Invokes API and checks execution status as success."},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"2072ae29_44be2fe6","line":236,"updated":"2026-03-05 11:38:30.000000000","message":"This method is used by all outgoing REST API calls by Cinder NetApp Driver. Do we really want to enforce a common retry logic created for cloning to see if that kind of logic can address this instead. \n\nhttps://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/netapp/dataontap/block_cmode.py#L263\n\nBased on the error, you can retry a specific operation on except.","commit_id":"d65131f8dc92350e3253e22585b109b9d93db8dd"},{"author":{"_account_id":37477,"name":"Florian","display_name":"FloD","email":"Florian.Doerheim@digits.schwarz","username":"FloD"},"change_message_id":"cf703aed665891e5d7cf0cd00451d74413d09bb6","unresolved":true,"context_lines":[{"line_number":233,"context_line":"        response_element \u003d self._get_result(response_xml)"},{"line_number":234,"context_line":"        return response_element"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    @na_utils.retry(NaRetryableError, retries\u003d3, interval\u003d2)"},{"line_number":237,"context_line":"    def invoke_successfully(self, na_element, enable_tunneling\u003dFalse):"},{"line_number":238,"context_line":"        \"\"\"Invokes API and checks execution status as success."},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"45cd839e_e341c0f5","line":236,"in_reply_to":"2072ae29_44be2fe6","updated":"2026-03-11 16:16:43.000000000","message":"I see the point that placing the retry logic inside api.py isn\u0027t ideal. The API layer should just be responsible for executing the request and returning the response if it succeeded or not with the reason.\n\nMoving the retry logic up to block_cmode.py, as you mentioned here https://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/netapp/dataontap/block_cmode.py#L263, would be more correct. However, I am not really happy about simply adding another if code \u003d\u003d EAPI_LOCKED_VSERVER condition to trigger the _retry_clone_lun method again https://opendev.org/openstack/cinder/src/commit/d5f967dd35aea2ee7040b1ce2666a9c9374028b1/cinder/volume/drivers/netapp/dataontap/block_cmode.py#L263-L273. The current implementation of that method relies on a self-implemented sleep loop, which essentially does the exact same as the retry wrapper/decorator I added in the utils.py file.\n\nTo do this properly, I would like to propose an improvement to the exception handling in general. We could introduce an exception mapping for netapp API error codes, similar to how cinder handles core exceptions in cinder/exception.py https://opendev.org/openstack/cinder/src/commit/d5f967dd35aea2ee7040b1ce2666a9c9374028b1/cinder/exception.py#L62-L1044. Since the netapp API consistently returns specific status codes (which we already track in api.py https://opendev.org/openstack/cinder/src/commit/3db5e3e19d3e52da5a15fe1a55e35c599c0b5551/cinder/volume/drivers/netapp/dataontap/client/api.py#L47-L58), we could map them to specific custom exceptions like NaException or NaRetryableException. This can be done separately for ZAPI and the REST API.\n\nWith that, we could, for example, map EAPI_LOCKED_VSERVER or the \"if Device busy\" in e.message from the _retry_clone_lun method (I don\u0027t know exactly which error code that one corresponds to) to the NaRetryableException. This would then automatically trigger the retry wrapper.\nI am not sure if it makes sense to implement this approach in this specific patch, as it would be a larger architectural change.\n\nFor a quick fix right now, I could just add the if code \u003d\u003d EAPI_LOCKED_VSERVER check to trigger the _retry_clone_lun method again. Let me know what you prefer.","commit_id":"d65131f8dc92350e3253e22585b109b9d93db8dd"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"433f3005e5242b8903dd3999a95cbaca0a0df8fc","unresolved":true,"context_lines":[{"line_number":233,"context_line":"        response_element \u003d self._get_result(response_xml)"},{"line_number":234,"context_line":"        return response_element"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    @na_utils.retry(NaRetryableError, retries\u003d3, interval\u003d2)"},{"line_number":237,"context_line":"    def invoke_successfully(self, na_element, enable_tunneling\u003dFalse):"},{"line_number":238,"context_line":"        \"\"\"Invokes API and checks execution status as success."},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"4718a6c2_bbaa2f1a","line":236,"in_reply_to":"45cd839e_e341c0f5","updated":"2026-04-07 13:55:54.000000000","message":"@florian.doerheim@stackit.cloud - Agreed that the retry logic with exception handling is the correct way of doing this. We can keep it as a tech debt. But in the meanwhile, I would like to avoid any generic retries that ends up causing more issues than fix because there are customers who are already using Cinder with NetApp at scale. To avoid a generic retry loop, we can do what was done for the _retry_clone_lun method. Specifically run retry on the errors we think require retries. This way the area of impact reduces to just the clone operations. In general clone APIs are fast and since we do not split the clones, they should return immediately. For the locking it makes sense to only implement the retry when necessary.","commit_id":"d65131f8dc92350e3253e22585b109b9d93db8dd"}]}