)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"43f4605c5879c35594ac4b0d4fbfb04fa80af86f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4023bd88_d03bd893","updated":"2026-06-19 10:12:43.000000000","message":"few comments inline","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"b1a5c3f760982b8f14fe24f718c72693c506fe32","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"94e3fc30_df1fa460","updated":"2026-06-19 05:31:54.000000000","message":"recheck","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"c8357622aa398666d7791005294a6e7ed4fc3bd1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a6c083a1_2d25a1a2","updated":"2026-06-19 08:25:20.000000000","message":"recheck","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"7c0308f19215dc1bf38137baf482e16726cd0ad1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c9b3b473_d3c95c21","updated":"2026-06-18 18:27:49.000000000","message":"run-DellEMC PowerFlex-v4 CI","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"e1d4756b64f1dd7d557236392ffe216171926c84","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f1a5db87_54c2a53a","updated":"2026-06-19 11:11:17.000000000","message":"@rajatdhasmana@gmail.com thanks for the review, update the code, please review it when you get a time.","commit_id":"9fb2d9936cfaf41c249e7302a35f88174a8a1840"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f6621bc796088dca434cb1918c9773761fb63ec9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3cfc876f_43578437","updated":"2026-06-19 18:50:54.000000000","message":"My comments are addressed and powerflex CI is passing.\nOne small issue I\u0027ve with tests is that they look redundant but that\u0027s not a blocked for this.","commit_id":"9fb2d9936cfaf41c249e7302a35f88174a8a1840"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"9f82f2fbe127fd4040ce4dfde4c2a3b6a2670f8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"49406bc9_1e61a242","updated":"2026-06-19 11:34:01.000000000","message":"run-DellEMC PowerFlex-v4 CI","commit_id":"9fb2d9936cfaf41c249e7302a35f88174a8a1840"}],"cinder/tests/unit/volume/drivers/dell_emc/powerflex/test_create_cloned_volume.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f6621bc796088dca434cb1918c9773761fb63ec9","unresolved":true,"context_lines":[{"line_number":318,"context_line":"            \u0027test_vtree_id\u0027)"},{"line_number":319,"context_line":"        mock_create.assert_called_once_with(self.new_volume, self.src_volume)"},{"line_number":320,"context_line":""},{"line_number":321,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"},{"line_number":322,"context_line":"                \u0027PowerFlexDriver._create_volume_from_source\u0027)"},{"line_number":323,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"},{"line_number":324,"context_line":"                \u0027PowerFlexDriver._get_client\u0027)"},{"line_number":325,"context_line":"    def test_create_cloned_volume_image_cache_excludes_root("},{"line_number":326,"context_line":"            self, mock_client, mock_create):"},{"line_number":327,"context_line":"        \"\"\"Test that the root cache volume itself is not counted."},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        The root volume has ancestorVolumeId\u003dNone (or missing) and should"},{"line_number":330,"context_line":"        not be included in the direct children count."},{"line_number":331,"context_line":"        \"\"\""},{"line_number":332,"context_line":"        self.set_https_response_mode(self.RESPONSE_MODE.Valid)"},{"line_number":333,"context_line":"        mock_create.return_value \u003d {}"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        # Set clone limit to 5"},{"line_number":336,"context_line":"        self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 5,"},{"line_number":337,"context_line":"                             configuration.SHARED_CONF_GROUP)"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        mock_rest_client \u003d mock.Mock()"},{"line_number":340,"context_line":"        mock_client.return_value \u003d mock_rest_client"},{"line_number":341,"context_line":"        mock_rest_client.query_volume.return_value \u003d {"},{"line_number":342,"context_line":"            \u0027vtreeId\u0027: \u0027test_vtree_id\u0027"},{"line_number":343,"context_line":"        }"},{"line_number":344,"context_line":"        # Root volume + 3 direct children"},{"line_number":345,"context_line":"        src_provider_id \u003d self.src_volume.provider_id"},{"line_number":346,"context_line":"        mock_rest_client.query_vtree_volumes.return_value \u003d ["},{"line_number":347,"context_line":"            {\u0027id\u0027: src_provider_id, \u0027ancestorVolumeId\u0027: None},"},{"line_number":348,"context_line":"            {\u0027id\u0027: \u0027child_1\u0027, \u0027ancestorVolumeId\u0027: src_provider_id},"},{"line_number":349,"context_line":"            {\u0027id\u0027: \u0027child_2\u0027, \u0027ancestorVolumeId\u0027: src_provider_id},"},{"line_number":350,"context_line":"            {\u0027id\u0027: \u0027child_3\u0027, \u0027ancestorVolumeId\u0027: src_provider_id},"},{"line_number":351,"context_line":"        ]"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        with mock.patch(\u0027cinder.volume.volume_utils.is_image_cache_entry\u0027,"},{"line_number":354,"context_line":"                        return_value\u003dTrue):"},{"line_number":355,"context_line":"            # Count should be 3, not 4 (root excluded)"},{"line_number":356,"context_line":"            self.driver.create_cloned_volume(self.new_volume, self.src_volume)"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        mock_rest_client.query_vtree_volumes.assert_called_once_with("},{"line_number":359,"context_line":"            \u0027test_vtree_id\u0027)"},{"line_number":360,"context_line":"        mock_create.assert_called_once_with(self.new_volume, self.src_volume)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"},{"line_number":363,"context_line":"                \u0027PowerFlexDriver._create_volume_from_source\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"821af3d6_dc866d24","line":360,"range":{"start_line":321,"start_character":0,"end_line":360,"end_character":77},"updated":"2026-06-19 18:50:54.000000000","message":"the test above (test_create_cloned_volume_image_cache_excludes_grandchildren) tests both excluding grandchildren and also root right? I\u0027m not against this extra test but just thinking about the value add it brings","commit_id":"9fb2d9936cfaf41c249e7302a35f88174a8a1840"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f6621bc796088dca434cb1918c9773761fb63ec9","unresolved":true,"context_lines":[{"line_number":393,"context_line":"            \u0027test_vtree_id\u0027)"},{"line_number":394,"context_line":"        mock_create.assert_called_once_with(self.new_volume, self.src_volume)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"},{"line_number":397,"context_line":"                \u0027PowerFlexDriver._create_volume_from_source\u0027)"},{"line_number":398,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"},{"line_number":399,"context_line":"                \u0027PowerFlexDriver._get_client\u0027)"},{"line_number":400,"context_line":"    def test_create_cloned_volume_image_cache_mixed_descendants("},{"line_number":401,"context_line":"            self, mock_client, mock_create):"},{"line_number":402,"context_line":"        \"\"\"Test correct count with mixed direct children and descendants."},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        vTree has 3 direct children + 5 grandchildren + 2 great-grandchildren."},{"line_number":405,"context_line":"        Only the 3 direct children should be counted."},{"line_number":406,"context_line":"        \"\"\""},{"line_number":407,"context_line":"        self.set_https_response_mode(self.RESPONSE_MODE.Valid)"},{"line_number":408,"context_line":"        mock_create.return_value \u003d {}"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        self.override_config(options.POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE, 5,"},{"line_number":411,"context_line":"                             configuration.SHARED_CONF_GROUP)"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        mock_rest_client \u003d mock.Mock()"},{"line_number":414,"context_line":"        mock_client.return_value \u003d mock_rest_client"},{"line_number":415,"context_line":"        mock_rest_client.query_volume.return_value \u003d {"},{"line_number":416,"context_line":"            \u0027vtreeId\u0027: \u0027test_vtree_id\u0027"},{"line_number":417,"context_line":"        }"},{"line_number":418,"context_line":"        src_provider_id \u003d self.src_volume.provider_id"},{"line_number":419,"context_line":"        mock_rest_client.query_vtree_volumes.return_value \u003d ["},{"line_number":420,"context_line":"            {\u0027id\u0027: \u0027root\u0027, \u0027ancestorVolumeId\u0027: None},"},{"line_number":421,"context_line":"            # 3 direct children"},{"line_number":422,"context_line":"            {\u0027id\u0027: \u0027child_1\u0027, \u0027ancestorVolumeId\u0027: src_provider_id},"},{"line_number":423,"context_line":"            {\u0027id\u0027: \u0027child_2\u0027, \u0027ancestorVolumeId\u0027: src_provider_id},"},{"line_number":424,"context_line":"            {\u0027id\u0027: \u0027child_3\u0027, \u0027ancestorVolumeId\u0027: src_provider_id},"},{"line_number":425,"context_line":"            # 5 grandchildren (children of child_1 and child_2)"},{"line_number":426,"context_line":"            {\u0027id\u0027: \u0027gc_1\u0027, \u0027ancestorVolumeId\u0027: \u0027child_1\u0027},"},{"line_number":427,"context_line":"            {\u0027id\u0027: \u0027gc_2\u0027, \u0027ancestorVolumeId\u0027: \u0027child_1\u0027},"},{"line_number":428,"context_line":"            {\u0027id\u0027: \u0027gc_3\u0027, \u0027ancestorVolumeId\u0027: \u0027child_2\u0027},"},{"line_number":429,"context_line":"            {\u0027id\u0027: \u0027gc_4\u0027, \u0027ancestorVolumeId\u0027: \u0027child_2\u0027},"},{"line_number":430,"context_line":"            {\u0027id\u0027: \u0027gc_5\u0027, \u0027ancestorVolumeId\u0027: \u0027child_2\u0027},"},{"line_number":431,"context_line":"            # 2 great-grandchildren"},{"line_number":432,"context_line":"            {\u0027id\u0027: \u0027ggc_1\u0027, \u0027ancestorVolumeId\u0027: \u0027gc_1\u0027},"},{"line_number":433,"context_line":"            {\u0027id\u0027: \u0027ggc_2\u0027, \u0027ancestorVolumeId\u0027: \u0027gc_3\u0027},"},{"line_number":434,"context_line":"        ]"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"        with mock.patch(\u0027cinder.volume.volume_utils.is_image_cache_entry\u0027,"},{"line_number":437,"context_line":"                        return_value\u003dTrue):"},{"line_number":438,"context_line":"            # Count \u003d 3 direct children, which is \u003c 5 limit"},{"line_number":439,"context_line":"            self.driver.create_cloned_volume(self.new_volume, self.src_volume)"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"        mock_rest_client.query_vtree_volumes.assert_called_once_with("},{"line_number":442,"context_line":"            \u0027test_vtree_id\u0027)"},{"line_number":443,"context_line":"        mock_create.assert_called_once_with(self.new_volume, self.src_volume)"},{"line_number":444,"context_line":""},{"line_number":445,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"},{"line_number":446,"context_line":"                \u0027PowerFlexDriver._create_volume_from_source\u0027)"},{"line_number":447,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.dell_emc.powerflex.driver.\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"5658a2bd_ddc373c7","line":444,"range":{"start_line":396,"start_character":0,"end_line":444,"end_character":0},"updated":"2026-06-19 18:50:54.000000000","message":"I feel all of these tests could be part of one test with all the different cases being called by different test methods like\n\n    def _test_create_cloned_volume_image_cache(...):\n        ....\n        \n    def test_create_cloned_volume_image_cache_mixed_descendants(...):\n        call _test_create_cloned_volume_image_cache with different data dict","commit_id":"9fb2d9936cfaf41c249e7302a35f88174a8a1840"}],"cinder/volume/drivers/dell_emc/powerflex/driver.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"43f4605c5879c35594ac4b0d4fbfb04fa80af86f","unresolved":true,"context_lines":[{"line_number":891,"context_line":"            volume_info \u003d client.query_volume(src_vref.provider_id)"},{"line_number":892,"context_line":"            vtree_id \u003d volume_info[\"vtreeId\"]"},{"line_number":893,"context_line":"            vtree_volumes \u003d client.query_vtree_volumes(vtree_id)"},{"line_number":894,"context_line":"            direct_children \u003d ["},{"line_number":895,"context_line":"                v for v in vtree_volumes"},{"line_number":896,"context_line":"                if v.get(\"ancestorVolumeId\") \u003d\u003d src_vref.provider_id"},{"line_number":897,"context_line":"            ]"},{"line_number":898,"context_line":"            if len(direct_children) \u003e\u003d max_size:"},{"line_number":899,"context_line":"                raise exception.SnapshotLimitReached(set_limit\u003dmax_size)"},{"line_number":900,"context_line":"        except exception.VolumeBackendAPIException:"},{"line_number":901,"context_line":"            LOG.warning(\"Failed to query vTree volumes for image \""},{"line_number":902,"context_line":"                        \"cache volume %(vol_id)s. Proceeding with clone.\","}],"source_content_type":"text/x-python","patch_set":1,"id":"067513b0_f3c365aa","line":899,"range":{"start_line":894,"start_character":0,"end_line":899,"end_character":72},"updated":"2026-06-19 10:12:43.000000000","message":"This code is inefficient in the following manner,\nlet\u0027s suppose we have max_size set to 10 and number of direct volumes in vtree are 1000\nwe first iterate over 1000 volumes just to evaluate 1000 \u003e 10 and then raise the exception, this can be better optimized as follows\n\n    count_direct_children \u003d 0\n    for v in vtree_volumes:\n        if v.get(\"ancestorVolumeId\") \u003d\u003d src_vref.provider_id:\n            count_direct_children +\u003d 1\n        if len(count_direct_children) \u003e\u003d max_size:\n            raise exception.SnapshotLimitReached(set_limit\u003dmax_size)\n\nWith this logic, we are not wasting memory (storing in direct_children variable) and failing fast by evaluating count in every iteration","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"e1d4756b64f1dd7d557236392ffe216171926c84","unresolved":false,"context_lines":[{"line_number":891,"context_line":"            volume_info \u003d client.query_volume(src_vref.provider_id)"},{"line_number":892,"context_line":"            vtree_id \u003d volume_info[\"vtreeId\"]"},{"line_number":893,"context_line":"            vtree_volumes \u003d client.query_vtree_volumes(vtree_id)"},{"line_number":894,"context_line":"            direct_children \u003d ["},{"line_number":895,"context_line":"                v for v in vtree_volumes"},{"line_number":896,"context_line":"                if v.get(\"ancestorVolumeId\") \u003d\u003d src_vref.provider_id"},{"line_number":897,"context_line":"            ]"},{"line_number":898,"context_line":"            if len(direct_children) \u003e\u003d max_size:"},{"line_number":899,"context_line":"                raise exception.SnapshotLimitReached(set_limit\u003dmax_size)"},{"line_number":900,"context_line":"        except exception.VolumeBackendAPIException:"},{"line_number":901,"context_line":"            LOG.warning(\"Failed to query vTree volumes for image \""},{"line_number":902,"context_line":"                        \"cache volume %(vol_id)s. Proceeding with clone.\","}],"source_content_type":"text/x-python","patch_set":1,"id":"de2114ad_0d2e41ed","line":899,"range":{"start_line":894,"start_character":0,"end_line":899,"end_character":72},"in_reply_to":"067513b0_f3c365aa","updated":"2026-06-19 11:11:17.000000000","message":"Good point, Fixed.","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"}],"cinder/volume/drivers/dell_emc/powerflex/options.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"43f4605c5879c35594ac4b0d4fbfb04fa80af86f","unresolved":true,"context_lines":[{"line_number":161,"context_line":"                    \u0027timeout value (in seconds) for rest call.\u0027),"},{"line_number":162,"context_line":"    cfg.IntOpt(POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE,"},{"line_number":163,"context_line":"               default\u003d0, min\u003d0, max\u003drest_client.MAX_SNAPS_IN_VTREE,"},{"line_number":164,"context_line":"               help\u003d\u0027Maximum size of the vTree associated with an entry in \u0027"},{"line_number":165,"context_line":"                    \u0027the image volume cache. When the size is exceeded, \u0027"},{"line_number":166,"context_line":"                    \u0027the cache entry will be replaced with one created from \u0027"},{"line_number":167,"context_line":"                    \u0027a new vTree. A value of 0 means the size is limited by \u0027"},{"line_number":168,"context_line":"                    \u0027the PowerFlex vTree snapshot limit.\u0027)"},{"line_number":169,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"65fdd372_74037db0","line":168,"range":{"start_line":164,"start_character":0,"end_line":168,"end_character":58},"updated":"2026-06-19 10:12:43.000000000","message":"should we update this to following? please check and update the snapshot limit if 126 is not correct\n\n             help\u003d\u0027Maximum number of direct child volumes allowed for an \u0027\n                  \u0027image volume cache entry. Only immediate clones are counted; \u0027\n                  \u0027grandchildren and deeper descendants are excluded. When the \u0027\n                  \u0027limit is reached, the cache entry will be replaced with one \u0027\n                  \u0027created from a new vTree. A value of 0 means the size is \u0027\n                  \u0027limited by the PowerFlex vTree snapshot limit (126).\u0027)","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"},{"author":{"_account_id":36725,"name":"Nilesh Thathagar","display_name":"Nilesh Thathagar","email":"nilesh.thathagar@dell.com","username":"NileshT"},"change_message_id":"e1d4756b64f1dd7d557236392ffe216171926c84","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                    \u0027timeout value (in seconds) for rest call.\u0027),"},{"line_number":162,"context_line":"    cfg.IntOpt(POWERFLEX_MAX_IMAGE_CACHE_VTREE_SIZE,"},{"line_number":163,"context_line":"               default\u003d0, min\u003d0, max\u003drest_client.MAX_SNAPS_IN_VTREE,"},{"line_number":164,"context_line":"               help\u003d\u0027Maximum size of the vTree associated with an entry in \u0027"},{"line_number":165,"context_line":"                    \u0027the image volume cache. When the size is exceeded, \u0027"},{"line_number":166,"context_line":"                    \u0027the cache entry will be replaced with one created from \u0027"},{"line_number":167,"context_line":"                    \u0027a new vTree. A value of 0 means the size is limited by \u0027"},{"line_number":168,"context_line":"                    \u0027the PowerFlex vTree snapshot limit.\u0027)"},{"line_number":169,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"5348ac72_664d6505","line":168,"range":{"start_line":164,"start_character":0,"end_line":168,"end_character":58},"in_reply_to":"65fdd372_74037db0","updated":"2026-06-19 11:11:17.000000000","message":"Done, we can\u0027t add limit size 126 as PowerFlex v5 has limit size of 1024.","commit_id":"18605da7b7c05f89ebfdc5e06813e3d523099d70"}]}