From dd0b1076df936b9cb1111567f6b4c3b747744b72 Mon Sep 17 00:00:00 2001
From: Felipe Rodrigues <felipefuty01@gmail.com>
Date: Fri, 16 Apr 2021 10:05:40 -0300
Subject: [PATCH] NetApp ONTAP: Fix sub-clone zapi call

The ONTAP documentation states that the `clone-create` ZAPI call
fails with `block-ranges` and `space-reserve` parameters sent
together. The sub-clone uses the `block-ranges` and is failing
because of that restriction.

This patch fixes the `clone-create` operation by using exactly one
of `block-ranges` or `space-reserve`.

Change-Id: I05d83d73de69c57d885e0c417e8a376f7cfb1e4f
Closes-Bug: #1924643
---
 .../dataontap/client/test_client_cmode.py     | 36 +++++++++++++++++++
 .../netapp/dataontap/client/client_cmode.py   |  7 ++--
 ...-sub-clone-operation-f42a84ab17930f24.yaml |  7 ++++
 3 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/fix-sub-clone-operation-f42a84ab17930f24.yaml

diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py
index dea528f4dad..533b90545ee 100644
--- a/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py
+++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py
@@ -823,6 +823,42 @@ class NetAppCmodeClientTestCase(test.TestCase):
             netapp_api.NaElement.create_node_with_children(
                 'clone-create', **clone_create_args), True)
 
+    @ddt.data(0, 1)
+    def test_clone_lun_is_sub_clone(self, block_count):
+
+        self.client.clone_lun(
+            'volume', 'fakeLUN', 'newFakeLUN', block_count=block_count)
+
+        clone_create_args = {
+            'volume': 'volume',
+            'source-path': 'fakeLUN',
+            'destination-path': 'newFakeLUN',
+        }
+
+        is_sub_clone = block_count > 0
+        if not is_sub_clone:
+            clone_create_args['space-reserve'] = 'true'
+
+        # build the expected request
+        expected_clone_create_request = \
+            netapp_api.NaElement.create_node_with_children(
+                'clone-create', **clone_create_args)
+
+        # add expected fields in the request if it's a sub-clone
+        if is_sub_clone:
+            block_ranges = netapp_api.NaElement("block-ranges")
+            block_range = \
+                netapp_api.NaElement.create_node_with_children(
+                    'block-range',
+                    **{'source-block-number': '0',
+                       'destination-block-number': '0',
+                       'block-count': '1'})
+            block_ranges.add_child_elem(block_range)
+            expected_clone_create_request.add_child_elem(block_ranges)
+
+        self.connection.invoke_successfully.assert_called_once_with(
+            expected_clone_create_request, True)
+
     def test_clone_lun_multiple_zapi_calls(self):
         """Test for when lun clone requires more than one zapi call."""
 
diff --git a/cinder/volume/drivers/netapp/dataontap/client/client_cmode.py b/cinder/volume/drivers/netapp/dataontap/client/client_cmode.py
index ad232fc093d..752b9f28e3c 100644
--- a/cinder/volume/drivers/netapp/dataontap/client/client_cmode.py
+++ b/cinder/volume/drivers/netapp/dataontap/client/client_cmode.py
@@ -635,12 +635,14 @@ class Client(client_base.Client):
             else:
                 block_count = zbc
 
+            is_sub_clone = block_count > 0
             zapi_args = {
                 'volume': volume,
                 'source-path': name,
                 'destination-path': new_name,
-                'space-reserve': space_reserved,
             }
+            if not is_sub_clone:
+                zapi_args['space-reserve'] = space_reserved
             if source_snapshot:
                 zapi_args['snapshot-name'] = source_snapshot
             if is_snapshot and self.features.BACKUP_CLONE_PARAM:
@@ -651,7 +653,8 @@ class Client(client_base.Client):
                 child_name = 'qos-%spolicy-group-name' % (
                     'adaptive-' if qos_policy_group_is_adaptive else '')
                 clone_create.add_new_child(child_name, qos_policy_group_name)
-            if block_count > 0:
+
+            if is_sub_clone:
                 block_ranges = netapp_api.NaElement("block-ranges")
                 segments = int(math.ceil(block_count / float(bc_limit)))
                 bc = block_count
diff --git a/releasenotes/notes/fix-sub-clone-operation-f42a84ab17930f24.yaml b/releasenotes/notes/fix-sub-clone-operation-f42a84ab17930f24.yaml
new file mode 100644
index 00000000000..a009acf9c01
--- /dev/null
+++ b/releasenotes/notes/fix-sub-clone-operation-f42a84ab17930f24.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1924643 <https://bugs.launchpad.net/cinder/+bug/1924643>`_: Fixed
+    the NetApp cinder driver sub-clone operation that might be used
+    by extend operation in case the extended size is greater than the max
+    LUN geometry.