)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0339094acc44d4154d4a5706a74939ac4b153b9f","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix Infinidat driver to backup attached volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For some unknown reason, the Infinidat Cinder driver does not allow to"},{"line_number":10,"context_line":"backup the attached volume. This makes it impossible to backup for most"},{"line_number":11,"context_line":"use cases."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"To fix this issue, we can create a snapshot and a temporary volume as a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"62a75807_64288dc1","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":26},"updated":"2022-09-06 20:09:02.000000000","message":"not 100% sure but as far as i can see, we are doing attachment inside the create_volume_from_snapshot method and if the volume is already attached and isn\u0027t a multiattach volume then this looks like a reason to fail","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix Infinidat driver to backup attached volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For some unknown reason, the Infinidat Cinder driver does not allow to"},{"line_number":10,"context_line":"backup the attached volume. This makes it impossible to backup for most"},{"line_number":11,"context_line":"use cases."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"To fix this issue, we can create a snapshot and a temporary volume as a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"93a47ca7_8903e205","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":26},"in_reply_to":"62a75807_64288dc1","updated":"2022-11-25 12:38:29.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\nFixed in patch set #2:\n* Added initialize_connection_snapshot and terminate_connection_snapshot to make it possible to create a backup from a temporary snapshot. Thus, for each backup, a separate unique temporary snapshot will be created and there will be no race effect.\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c5f057dc4b8c9c469c9f625c5176f3847888deee","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Alexander Deiter \u003cadeiter@infinidat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-12-22 14:06:15 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix Infinidat driver to backup attached volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For some unknown reason, the Infinidat Cinder driver does not allow to"},{"line_number":10,"context_line":"backup the attached volume. This makes it impossible to backup for most"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"61b88360_bf15f784","line":7,"range":{"start_line":7,"start_character":24,"end_line":7,"end_character":30},"updated":"2023-01-20 14:48:45.000000000","message":":-1: Surely this is due to a lack of knowledge in the Infinity driver. But i think using the word \"backup\" here is confusing. The patch implements snapshots and clone volumes in-use. Can you explain a little better why this affects the creation of backups?","commit_id":"6bec1773519be99ff7a7e4a483a25af6eb5659ff"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"86bfd296a8ee15392bf224e1f4113457b6235051","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Alexander Deiter \u003cadeiter@infinidat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-12-22 14:06:15 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix Infinidat driver to backup attached volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For some unknown reason, the Infinidat Cinder driver does not allow to"},{"line_number":10,"context_line":"backup the attached volume. This makes it impossible to backup for most"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"7d03e990_69f9dc24","line":7,"range":{"start_line":7,"start_character":24,"end_line":7,"end_character":30},"in_reply_to":"61b88360_bf15f784","updated":"2023-01-24 13:44:46.000000000","message":"Hello Sofia,\n\nThank you very much for the review!\nI\u0027ve updated the commit message to describe all changes.\nPlease review - thank you very much!","commit_id":"6bec1773519be99ff7a7e4a483a25af6eb5659ff"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"2de574882d4bc54e16a613f19842ce15629580bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e835bca8_8e16dde1","updated":"2022-08-03 14:57:42.000000000","message":"recheck cinder-plugin-ceph-tempest ","commit_id":"cff1a847620500190c7ae9d43351a3f3965d1cfd"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"39a4759d1564ff3591a24d70858774b202ecbdea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"eb06f424_0423026b","updated":"2022-08-24 15:37:22.000000000","message":"Patch set #4: rebase and use volume name_id to resolve Infinidat volume name.","commit_id":"4e5e2c8d0c83bf257b442d8cee70cd5a036379e8"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"afb5dce39786413ce2b63f2cc434d19652c36a46","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f503ff84_e76d5ca3","updated":"2022-08-24 17:47:27.000000000","message":"recheck openstack-tox-py38","commit_id":"4e5e2c8d0c83bf257b442d8cee70cd5a036379e8"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"ec6faed00db94eb60dafd8f483a5b52ad2bcfb95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e13f4c17_0cef5aaa","updated":"2022-09-06 21:15:16.000000000","message":"I think Rajat makes a good point.  If you disagree, or if we\u0027re not understanding something about your backend that makes your approach OK, it would be good to discuss this at the weekly cinder meeting so that everyone is on the same page.","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"61b959b8fd80f58a514fc54700bab9e59e3a11b5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1829e916_b04c862c","updated":"2022-09-06 20:14:56.000000000","message":"I think the right way would be to implement get_backup_device method and return a snapshot (if we can only create a snapshot from attached volume and not clone that volume)","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"1b3d4a5a9e27bfeea06aa79cb2b72d9f06b442a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a8876660_605cd228","updated":"2022-08-24 20:40:37.000000000","message":"Patch set #5: rebase and use volume name_id to resolve Infinidat volume name.","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0339094acc44d4154d4a5706a74939ac4b153b9f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fef1a6c6_4b66dae8","updated":"2022-09-06 20:09:02.000000000","message":"The change in the create_cloned_volume method doesn\u0027t look like a reasonable fix to the issue mentioned in the commit message.","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8d694d9b_f68af564","in_reply_to":"1829e916_b04c862c","updated":"2022-11-25 12:38:29.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\nFixed in patch set #2: Added initialize_connection_snapshot,  terminate_connection_snapshot and backup_use_temp_snapshot to make it possible to create a backup from a temporary snapshot. Thus, for each backup, a separate unique temporary snapshot will be created and there will be no race effect.\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f8f43c26_3700708f","in_reply_to":"e13f4c17_0cef5aaa","updated":"2022-11-25 12:38:29.000000000","message":"Hello Brian,\n\nI think I fixed most of the comments in patch set #2.\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a00ac5bb_d290d541","in_reply_to":"fef1a6c6_4b66dae8","updated":"2022-11-25 12:38:29.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\nTo make it possible to create a backup, we can provide the ability to create a clone for the attached volume and/or the ability to attach a snapshot (please see other comments).\n\nTwo existing functions: create_cloned_volume and create_volume_from_snapshot uses the same duplicate code to attach a volume and create a full copy of the source volume. So I simplified the code for create_cloned_volume:\n\n* create_volume_from_snapshot can create a volume from snapshot\n* create_cloned_volume creates a temporary snapshot and then call create_volume_from_snapshot\n\nFor some reason InfiniBox does not yet support detached clones. And if we create a clone from some parent volume, then we will not be able to delete the parent volume while it has at least one clone. And since the InfiniBox API does not support other ways to create a clone or copy of the volume, we are forced to create a full copy of the volume on the client side (attach source and destination volumes and run dd command).\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c5f057dc4b8c9c469c9f625c5176f3847888deee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"5bb604bd_82da1169","updated":"2023-01-20 14:48:45.000000000","message":"Code looks good, just one question in line. Thanks","commit_id":"6bec1773519be99ff7a7e4a483a25af6eb5659ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"032fd7bd_52606037","updated":"2023-01-30 17:55:30.000000000","message":"Hi Alex, I\u0027ve noticed a few issues in tests inline.","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"42a0fa3ba06f08bac2630cc2a718069d0cf10d10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"10f6978e_050b34e7","in_reply_to":"032fd7bd_52606037","updated":"2023-01-31 12:53:54.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003eHi Alex, I\u0027ve noticed a few issues in tests inline.\n\nI addressed all comments in patch set #11, please review.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"79380dd83f4737169086c0e3cd47510badf7dd2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"7d64bcc9_b8eb4298","updated":"2023-01-31 15:33:04.000000000","message":"changes look good, one minor change noted in releasenote","commit_id":"aee9aafe4ad7a6075e361ddf803e80b6245df7ee"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"2c411c2f25310a7ba5c4104ed644a2359e5bfdc2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e1ca7366_7b8bd31d","updated":"2023-01-31 18:04:15.000000000","message":"All my comments are addressed, Infinidat CI is passing. LGTM. Thanks Alex!","commit_id":"907550015e0480fac3e69c5fc73f11f825723bf4"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"95aec8b77691b79c4e9357ddd0a9900e7f15087f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"82b86dd1_5b31d294","updated":"2023-01-31 18:29:56.000000000","message":"Thanks for updating this patch, looks like all comments are resolved and Infinidat CI passed","commit_id":"907550015e0480fac3e69c5fc73f11f825723bf4"}],"cinder/tests/unit/volume/drivers/test_infinidat.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":255,"context_line":"        infinibox.assert_called_once_with(self.configuration.san_ip,"},{"line_number":256,"context_line":"                                          auth\u003dauth, use_ssl\u003duse_ssl)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @ddt.data(*itertools.product((True, False), (True, False)))"},{"line_number":259,"context_line":"    @ddt.unpack"},{"line_number":260,"context_line":"    def test_system_compression(self, system_compression, config_compression):"},{"line_number":261,"context_line":"        self.override_config(\u0027infinidat_use_compression\u0027, config_compression)"},{"line_number":262,"context_line":"        self._system.compat.has_compression.return_value \u003d system_compression"},{"line_number":263,"context_line":"        if config_compression and not system_compression:"},{"line_number":264,"context_line":"            self.assertRaises(exception.VolumeDriverException,"},{"line_number":265,"context_line":"                              self.driver.do_setup, None)"},{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            self.assertIsNone(self.driver.do_setup(None))"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def test_create_export_snapshot(self):"},{"line_number":270,"context_line":"        self.assertIsNone(self.driver.create_export_snapshot("}],"source_content_type":"text/x-python","patch_set":9,"id":"a2ce818e_74325c97","line":267,"range":{"start_line":258,"start_character":0,"end_line":267,"end_character":57},"updated":"2023-01-30 17:55:30.000000000","message":"this test isn\u0027t related to the changes made in this patch?","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":255,"context_line":"        infinibox.assert_called_once_with(self.configuration.san_ip,"},{"line_number":256,"context_line":"                                          auth\u003dauth, use_ssl\u003duse_ssl)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    @ddt.data(*itertools.product((True, False), (True, False)))"},{"line_number":259,"context_line":"    @ddt.unpack"},{"line_number":260,"context_line":"    def test_system_compression(self, system_compression, config_compression):"},{"line_number":261,"context_line":"        self.override_config(\u0027infinidat_use_compression\u0027, config_compression)"},{"line_number":262,"context_line":"        self._system.compat.has_compression.return_value \u003d system_compression"},{"line_number":263,"context_line":"        if config_compression and not system_compression:"},{"line_number":264,"context_line":"            self.assertRaises(exception.VolumeDriverException,"},{"line_number":265,"context_line":"                              self.driver.do_setup, None)"},{"line_number":266,"context_line":"        else:"},{"line_number":267,"context_line":"            self.assertIsNone(self.driver.do_setup(None))"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def test_create_export_snapshot(self):"},{"line_number":270,"context_line":"        self.assertIsNone(self.driver.create_export_snapshot("}],"source_content_type":"text/x-python","patch_set":9,"id":"5da355e4_3cf95b66","line":267,"range":{"start_line":258,"start_character":0,"end_line":267,"end_character":57},"in_reply_to":"a2ce818e_74325c97","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e this test isn\u0027t related to the changes made in this patch?\n\nYes, you are right - please let me remove it and submit as another patch.\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def test_backup_use_temp_snapshot(self):"},{"line_number":278,"context_line":"        self.assertTrue(self.driver.backup_use_temp_snapshot())"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def test_initialize_connection_snapshot(self):"},{"line_number":281,"context_line":"        result \u003d self.driver.initialize_connection_snapshot("},{"line_number":282,"context_line":"            test_snapshot, test_connector)"},{"line_number":283,"context_line":"        self.assertEqual(1, result[\"data\"][\"target_lun\"])"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def test_initialize_connection(self):"},{"line_number":286,"context_line":"        self._system.hosts.safe_get.return_value \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"9061814e_112fb0cf","line":283,"range":{"start_line":279,"start_character":0,"end_line":283,"end_character":57},"updated":"2023-01-30 17:55:30.000000000","message":"this method isn\u0027t asserting anything different from the volume initialize_connection method.\nI think we should add method specific assertions like assert if these methods were called:\ninitialize_connection: _get_infinidat_volume\ninitialize_connection_snapshot: _get_infinidat_snapshot","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def test_backup_use_temp_snapshot(self):"},{"line_number":278,"context_line":"        self.assertTrue(self.driver.backup_use_temp_snapshot())"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def test_initialize_connection_snapshot(self):"},{"line_number":281,"context_line":"        result \u003d self.driver.initialize_connection_snapshot("},{"line_number":282,"context_line":"            test_snapshot, test_connector)"},{"line_number":283,"context_line":"        self.assertEqual(1, result[\"data\"][\"target_lun\"])"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def test_initialize_connection(self):"},{"line_number":286,"context_line":"        self._system.hosts.safe_get.return_value \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf6912c6_0bad1bdd","line":283,"range":{"start_line":279,"start_character":0,"end_line":283,"end_character":57},"in_reply_to":"9061814e_112fb0cf","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e I think we should add method specific assertions like assert if these methods were called:\n\nYes, it is a good idea! Fixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    def test_terminate_connection(self):"},{"line_number":353,"context_line":"        volume \u003d copy.deepcopy(test_volume)"},{"line_number":354,"context_line":"        volume.volume_attachment \u003d [test_attachment1]"},{"line_number":355,"context_line":"        self.assertFalse(self.driver.terminate_connection(volume,"},{"line_number":356,"context_line":"                                                          test_connector))"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a467cff5_1b35ad0d","line":354,"range":{"start_line":354,"start_character":8,"end_line":354,"end_character":53},"updated":"2023-01-30 17:55:30.000000000","message":"add assertion that _is_volume_multiattached and _get_infinidat_volume were called with right parameters","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    def test_terminate_connection(self):"},{"line_number":353,"context_line":"        volume \u003d copy.deepcopy(test_volume)"},{"line_number":354,"context_line":"        volume.volume_attachment \u003d [test_attachment1]"},{"line_number":355,"context_line":"        self.assertFalse(self.driver.terminate_connection(volume,"},{"line_number":356,"context_line":"                                                          test_connector))"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"8481fbc4_77f521a6","line":354,"range":{"start_line":354,"start_character":8,"end_line":354,"end_character":53},"in_reply_to":"a467cff5_1b35ad0d","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e add assertion that _is_volume_multiattached and _get_infinidat_volume were called with right parameters\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":356,"context_line":"                                                          test_connector))"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    def test_terminate_connection_snapshot(self):"},{"line_number":359,"context_line":"        self.assertIsNone(self.driver.terminate_connection_snapshot("},{"line_number":360,"context_line":"            test_snapshot, test_connector))"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def test_terminate_connection_delete_host(self):"},{"line_number":363,"context_line":"        self._mock_host.get_luns.return_value \u003d [object()]"}],"source_content_type":"text/x-python","patch_set":9,"id":"353565f0_77872b95","line":360,"range":{"start_line":359,"start_character":8,"end_line":360,"end_character":43},"updated":"2023-01-30 17:55:30.000000000","message":"this is a very weak test, apart from the return value, we aren\u0027t actually asserting anything.\n\nwe should check if _get_infinidat_snapshot and _terminate_connection were called with right parameters","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                                                          test_connector))"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    def test_terminate_connection_snapshot(self):"},{"line_number":359,"context_line":"        self.assertIsNone(self.driver.terminate_connection_snapshot("},{"line_number":360,"context_line":"            test_snapshot, test_connector))"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def test_terminate_connection_delete_host(self):"},{"line_number":363,"context_line":"        self._mock_host.get_luns.return_value \u003d [object()]"}],"source_content_type":"text/x-python","patch_set":9,"id":"97d0e7f3_4c773f52","line":360,"range":{"start_line":359,"start_character":8,"end_line":360,"end_character":43},"in_reply_to":"353565f0_77872b95","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e this is a very weak test, apart from the return value, we aren\u0027t actually asserting anything.\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":572,"context_line":"    @mock.patch(\"cinder.volume.volume_types.get_volume_type_qos_specs\")"},{"line_number":573,"context_line":"    def test_create_cloned_volume(self, *mocks):"},{"line_number":574,"context_line":"        self.driver.create_cloned_volume(test_clone, test_volume)"},{"line_number":575,"context_line":"        self.assertEqual(1, self._mock_volume.create_snapshot.call_count)"},{"line_number":576,"context_line":"        self.assertEqual(1, self._mock_volume.safe_delete.call_count)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def test_create_cloned_volume_create_fails(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"6b4dc15d_43feb910","line":575,"range":{"start_line":575,"start_character":8,"end_line":575,"end_character":73},"updated":"2023-01-30 17:55:30.000000000","message":"we should use self._mock_volume.create_snapshot.assert_called_once_with(test_clone)\n\nor maybe change test_clone to test_snapshot\n\nThis way we verify that method was called exactly once and we also validate the parameters","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":572,"context_line":"    @mock.patch(\"cinder.volume.volume_types.get_volume_type_qos_specs\")"},{"line_number":573,"context_line":"    def test_create_cloned_volume(self, *mocks):"},{"line_number":574,"context_line":"        self.driver.create_cloned_volume(test_clone, test_volume)"},{"line_number":575,"context_line":"        self.assertEqual(1, self._mock_volume.create_snapshot.call_count)"},{"line_number":576,"context_line":"        self.assertEqual(1, self._mock_volume.safe_delete.call_count)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def test_create_cloned_volume_create_fails(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"b8afd741_768a60aa","line":575,"range":{"start_line":575,"start_character":8,"end_line":575,"end_character":73},"in_reply_to":"6b4dc15d_43feb910","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e This way we verify that method was called exactly once and we also validate the parameters\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":573,"context_line":"    def test_create_cloned_volume(self, *mocks):"},{"line_number":574,"context_line":"        self.driver.create_cloned_volume(test_clone, test_volume)"},{"line_number":575,"context_line":"        self.assertEqual(1, self._mock_volume.create_snapshot.call_count)"},{"line_number":576,"context_line":"        self.assertEqual(1, self._mock_volume.safe_delete.call_count)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def test_create_cloned_volume_create_fails(self):"},{"line_number":579,"context_line":"        self._system.volumes.create.side_effect \u003d self._raise_infinisdk"}],"source_content_type":"text/x-python","patch_set":9,"id":"8a29afe7_245067a1","line":576,"range":{"start_line":576,"start_character":8,"end_line":576,"end_character":69},"updated":"2023-01-30 17:55:30.000000000","message":"same","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":573,"context_line":"    def test_create_cloned_volume(self, *mocks):"},{"line_number":574,"context_line":"        self.driver.create_cloned_volume(test_clone, test_volume)"},{"line_number":575,"context_line":"        self.assertEqual(1, self._mock_volume.create_snapshot.call_count)"},{"line_number":576,"context_line":"        self.assertEqual(1, self._mock_volume.safe_delete.call_count)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def test_create_cloned_volume_create_fails(self):"},{"line_number":579,"context_line":"        self._system.volumes.create.side_effect \u003d self._raise_infinisdk"}],"source_content_type":"text/x-python","patch_set":9,"id":"8faa1ef1_35cd0dda","line":576,"updated":"2023-01-30 17:55:30.000000000","message":"we are not asserting that create_volume_from_snapshot was called which is the main method in this flow","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":573,"context_line":"    def test_create_cloned_volume(self, *mocks):"},{"line_number":574,"context_line":"        self.driver.create_cloned_volume(test_clone, test_volume)"},{"line_number":575,"context_line":"        self.assertEqual(1, self._mock_volume.create_snapshot.call_count)"},{"line_number":576,"context_line":"        self.assertEqual(1, self._mock_volume.safe_delete.call_count)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def test_create_cloned_volume_create_fails(self):"},{"line_number":579,"context_line":"        self._system.volumes.create.side_effect \u003d self._raise_infinisdk"}],"source_content_type":"text/x-python","patch_set":9,"id":"c03c1754_c65bae8a","line":576,"range":{"start_line":576,"start_character":8,"end_line":576,"end_character":69},"in_reply_to":"8a29afe7_245067a1","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e same\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":573,"context_line":"    def test_create_cloned_volume(self, *mocks):"},{"line_number":574,"context_line":"        self.driver.create_cloned_volume(test_clone, test_volume)"},{"line_number":575,"context_line":"        self.assertEqual(1, self._mock_volume.create_snapshot.call_count)"},{"line_number":576,"context_line":"        self.assertEqual(1, self._mock_volume.safe_delete.call_count)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def test_create_cloned_volume_create_fails(self):"},{"line_number":579,"context_line":"        self._system.volumes.create.side_effect \u003d self._raise_infinisdk"}],"source_content_type":"text/x-python","patch_set":9,"id":"beeb983b_f30b69db","line":576,"in_reply_to":"8faa1ef1_35cd0dda","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e we are not asserting that create_volume_from_snapshot was called which is the main method in this flow\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"}],"cinder/volume/drivers/infinidat.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e179fe0a3725b05d2cb4b89c058830c8428d12ce","unresolved":true,"context_lines":[{"line_number":703,"context_line":"    def create_cloned_volume(self, volume, src_vref):"},{"line_number":704,"context_line":"        \"\"\"Create a clone from source volume."},{"line_number":705,"context_line":""},{"line_number":706,"context_line":"        InfiniBox does not yet support detached clone so use dd to copy data."},{"line_number":707,"context_line":"        This could be a lengthy operation."},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"        * create a snapshot"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a8f671f_7e923f8e","line":706,"range":{"start_line":706,"start_character":54,"end_line":706,"end_character":76},"updated":"2022-08-01 21:43:49.000000000","message":"This comment seems to no longer be accurate?","commit_id":"aaf8990f8e0f78095af8abb1874ad2752a6ce3ec"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"d3012fb44b5e221ebc46f1191e8530b959408cd2","unresolved":false,"context_lines":[{"line_number":703,"context_line":"    def create_cloned_volume(self, volume, src_vref):"},{"line_number":704,"context_line":"        \"\"\"Create a clone from source volume."},{"line_number":705,"context_line":""},{"line_number":706,"context_line":"        InfiniBox does not yet support detached clone so use dd to copy data."},{"line_number":707,"context_line":"        This could be a lengthy operation."},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"        * create a snapshot"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c0842ba_a0f5135f","line":706,"range":{"start_line":706,"start_character":54,"end_line":706,"end_character":76},"in_reply_to":"1a8f671f_7e923f8e","updated":"2022-08-01 21:52:21.000000000","message":"Hello Eric,\n\nThank you very much for the review!\n\nAt the moment, Infinidat API does not support creating independent clones.\nTherefore, in order to be able to delete the parent volume, we steel need to create a  clone as a full copy using dd. So the comment is still valid.\n\nThank you!","commit_id":"aaf8990f8e0f78095af8abb1874ad2752a6ce3ec"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ae8cba0ec09288ac9342112186162cfd32ebb4ae","unresolved":true,"context_lines":[{"line_number":675,"context_line":"        - unmap volume and clone and delete the clone"},{"line_number":676,"context_line":"        \"\"\""},{"line_number":677,"context_line":"        infinidat_snapshot \u003d self._get_infinidat_snapshot(snapshot)"},{"line_number":678,"context_line":"        name_id \u003d str(uuid.uuid4())"},{"line_number":679,"context_line":"        attributes \u003d [\u0027name_id\u0027, \u0027multiattach\u0027, \u0027volume_attachment\u0027]"},{"line_number":680,"context_line":"        instance \u003d collections.namedtuple(\u0027Volume\u0027, attributes)"},{"line_number":681,"context_line":"        clone \u003d instance(name_id\u003dname_id, multiattach\u003dFalse,"},{"line_number":682,"context_line":"                         volume_attachment\u003dNone)"},{"line_number":683,"context_line":"        name \u003d self._make_volume_name(clone)"},{"line_number":684,"context_line":"        infinidat_clone \u003d infinidat_snapshot.create_snapshot(name\u003dname)"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3c4876d9_dc880118","line":683,"range":{"start_line":678,"start_character":0,"end_line":683,"end_character":44},"updated":"2022-09-07 08:40:06.000000000","message":"-1: You can use a Volume object (as long as you don\u0027t save it there\u0027s no problem). \n I think something like this would work:\n\n  from cinder import objects\n  \n  objects.Volume(None, \n                 id\u003dstr(uuid.uuid4()),\n                 multiattach\u003dFalse,\n                 volume_attachment\u003dobjects.VolumeAttachmentList(None)))\n\nDepending on your code that uses volume_attachment maybe even this would work:\n\n  objects.Volume(None, \n                 id\u003dstr(uuid.uuid4()),\n                 multiattach\u003dFalse,\n                 volume_attachment\u003dNone)\n                                \nAm I correct to assume that Infinidat doesn\u0027t have an efficient create volume from snapshot or clone volume operation that can be done by the storage system itself?\n\nnit: In general I would ask that this change (removing the mock) be done in a separate patch, since this patch ends up doing 2 different things, but I won\u0027t downvote in this case.","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[{"line_number":675,"context_line":"        - unmap volume and clone and delete the clone"},{"line_number":676,"context_line":"        \"\"\""},{"line_number":677,"context_line":"        infinidat_snapshot \u003d self._get_infinidat_snapshot(snapshot)"},{"line_number":678,"context_line":"        name_id \u003d str(uuid.uuid4())"},{"line_number":679,"context_line":"        attributes \u003d [\u0027name_id\u0027, \u0027multiattach\u0027, \u0027volume_attachment\u0027]"},{"line_number":680,"context_line":"        instance \u003d collections.namedtuple(\u0027Volume\u0027, attributes)"},{"line_number":681,"context_line":"        clone \u003d instance(name_id\u003dname_id, multiattach\u003dFalse,"},{"line_number":682,"context_line":"                         volume_attachment\u003dNone)"},{"line_number":683,"context_line":"        name \u003d self._make_volume_name(clone)"},{"line_number":684,"context_line":"        infinidat_clone \u003d infinidat_snapshot.create_snapshot(name\u003dname)"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"}],"source_content_type":"text/x-python","patch_set":5,"id":"802ecb79_ae85c351","line":683,"range":{"start_line":678,"start_character":0,"end_line":683,"end_character":44},"in_reply_to":"3c4876d9_dc880118","updated":"2022-11-25 12:38:29.000000000","message":"Hello Gorka,\n\nThank you very much for the review!\n\n\u003e\u003e Depending on your code that uses volume_attachment maybe even this would work:\n\nThank you very much for the detailed explanation!\n\nI removed temporary volume creation and use a snapshot as a source for backup.\n\n\u003e\u003e Am I correct to assume that Infinidat doesn\u0027t have an efficient create volume from snapshot or clone volume operation that can be done by the storage system itself?\n\nYes - please review previous comments.\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ae8cba0ec09288ac9342112186162cfd32ebb4ae","unresolved":true,"context_lines":[{"line_number":681,"context_line":"        clone \u003d instance(name_id\u003dname_id, multiattach\u003dFalse,"},{"line_number":682,"context_line":"                         volume_attachment\u003dNone)"},{"line_number":683,"context_line":"        name \u003d self._make_volume_name(clone)"},{"line_number":684,"context_line":"        infinidat_clone \u003d infinidat_snapshot.create_snapshot(name\u003dname)"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"},{"line_number":687,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"4a14bd6e_28d84536","line":684,"updated":"2022-09-07 08:40:06.000000000","message":"?: Why do we need to create a snapshot if the source is already a snapshot?  This seems to indicate that Infinidat allows attaching snapshots...","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[{"line_number":681,"context_line":"        clone \u003d instance(name_id\u003dname_id, multiattach\u003dFalse,"},{"line_number":682,"context_line":"                         volume_attachment\u003dNone)"},{"line_number":683,"context_line":"        name \u003d self._make_volume_name(clone)"},{"line_number":684,"context_line":"        infinidat_clone \u003d infinidat_snapshot.create_snapshot(name\u003dname)"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"},{"line_number":687,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"68974f5e_53122352","line":684,"in_reply_to":"4a14bd6e_28d84536","updated":"2022-11-25 12:38:29.000000000","message":"Hello Gorka,\n\nThank you very much for the review!\n\n\u003e\u003eWhy do we need to create a snapshot if the source is already a snapshot?  This seems to indicate that Infinidat allows attaching snapshots...\n\nYes. you are right. I fixed this in patch set #2:\n\nAdded initialize_connection_snapshot,  terminate_connection_snapshot and backup_use_temp_snapshot to make it possible to create a backup from a temporary snapshot.\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0339094acc44d4154d4a5706a74939ac4b153b9f","unresolved":true,"context_lines":[{"line_number":724,"context_line":"        * unmap both volumes"},{"line_number":725,"context_line":"        * delete snapshot"},{"line_number":726,"context_line":"        \"\"\""},{"line_number":727,"context_line":"        snapshot_id \u003d str(uuid.uuid4())"},{"line_number":728,"context_line":"        snapshot_name \u003d CONF.snapshot_name_template % snapshot_id"},{"line_number":729,"context_line":"        attributes \u003d [\u0027id\u0027, \u0027name\u0027, \u0027volume\u0027]"},{"line_number":730,"context_line":"        Snapshot \u003d collections.namedtuple(\u0027Snapshot\u0027, attributes)"},{"line_number":731,"context_line":"        snapshot \u003d Snapshot(id\u003dsnapshot_id, name\u003dsnapshot_name,"},{"line_number":732,"context_line":"                            volume\u003dsrc_vref)"},{"line_number":733,"context_line":"        self.create_snapshot(snapshot)"},{"line_number":734,"context_line":"        self.create_volume_from_snapshot(volume, snapshot)"},{"line_number":735,"context_line":"        self.delete_snapshot(snapshot)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    def _build_initiator_target_map(self, connector, all_target_wwns):"},{"line_number":738,"context_line":"        \"\"\"Build the target_wwns and the initiator target map.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"4504aaae_0784d81a","line":735,"range":{"start_line":727,"start_character":0,"end_line":735,"end_character":38},"updated":"2022-09-06 20:09:02.000000000","message":"this method is implemented to perform efficient clone of a volume by driver and here we are just creating a volume from snapshot (which is a different operation) and doesn\u0027t look right at all.","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ae8cba0ec09288ac9342112186162cfd32ebb4ae","unresolved":true,"context_lines":[{"line_number":724,"context_line":"        * unmap both volumes"},{"line_number":725,"context_line":"        * delete snapshot"},{"line_number":726,"context_line":"        \"\"\""},{"line_number":727,"context_line":"        snapshot_id \u003d str(uuid.uuid4())"},{"line_number":728,"context_line":"        snapshot_name \u003d CONF.snapshot_name_template % snapshot_id"},{"line_number":729,"context_line":"        attributes \u003d [\u0027id\u0027, \u0027name\u0027, \u0027volume\u0027]"},{"line_number":730,"context_line":"        Snapshot \u003d collections.namedtuple(\u0027Snapshot\u0027, attributes)"},{"line_number":731,"context_line":"        snapshot \u003d Snapshot(id\u003dsnapshot_id, name\u003dsnapshot_name,"},{"line_number":732,"context_line":"                            volume\u003dsrc_vref)"},{"line_number":733,"context_line":"        self.create_snapshot(snapshot)"},{"line_number":734,"context_line":"        self.create_volume_from_snapshot(volume, snapshot)"},{"line_number":735,"context_line":"        self.delete_snapshot(snapshot)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    def _build_initiator_target_map(self, connector, all_target_wwns):"},{"line_number":738,"context_line":"        \"\"\"Build the target_wwns and the initiator target map.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"d3a07212_34d4e68a","line":735,"range":{"start_line":727,"start_character":0,"end_line":735,"end_character":38},"in_reply_to":"4504aaae_0784d81a","updated":"2022-09-07 08:40:06.000000000","message":"I believe this is related to some storage system limitations.  The original create_cloned_volume only worked for \"available\" volumes because it was actually attaching the source volume and then doing dd to copy the data to the destination, so it wasn\u0027t \"efficient\", but it worked.\n\nAfaik there are other drivers like qnap that do exactly this, create snapshot and then use it to create a volume and then attach this temp volume and copy it to the destination.\n\n@Alexander: Could you confirm that Ifinidat doesn\u0027t have efficient backend cloning on the storage system?  If it doesn\u0027t, is it able to export/map snapshots so Cinder can attach them and skip the creating volume from snapshot part?\n\nMaybe the code could skip the snapshot creation if the volume is available and only go through that path when it\u0027s attached.","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"b4ce0358de05772452754a72433dba8467d7c35e","unresolved":false,"context_lines":[{"line_number":724,"context_line":"        * unmap both volumes"},{"line_number":725,"context_line":"        * delete snapshot"},{"line_number":726,"context_line":"        \"\"\""},{"line_number":727,"context_line":"        snapshot_id \u003d str(uuid.uuid4())"},{"line_number":728,"context_line":"        snapshot_name \u003d CONF.snapshot_name_template % snapshot_id"},{"line_number":729,"context_line":"        attributes \u003d [\u0027id\u0027, \u0027name\u0027, \u0027volume\u0027]"},{"line_number":730,"context_line":"        Snapshot \u003d collections.namedtuple(\u0027Snapshot\u0027, attributes)"},{"line_number":731,"context_line":"        snapshot \u003d Snapshot(id\u003dsnapshot_id, name\u003dsnapshot_name,"},{"line_number":732,"context_line":"                            volume\u003dsrc_vref)"},{"line_number":733,"context_line":"        self.create_snapshot(snapshot)"},{"line_number":734,"context_line":"        self.create_volume_from_snapshot(volume, snapshot)"},{"line_number":735,"context_line":"        self.delete_snapshot(snapshot)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"    def _build_initiator_target_map(self, connector, all_target_wwns):"},{"line_number":738,"context_line":"        \"\"\"Build the target_wwns and the initiator target map.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"13754d1c_aedb9901","line":735,"range":{"start_line":727,"start_character":0,"end_line":735,"end_character":38},"in_reply_to":"d3a07212_34d4e68a","updated":"2022-11-25 12:38:29.000000000","message":"Hello Gorka,\n\nThank you very much for the review!\n\n\u003e\u003e Could you confirm that Ifinidat doesn\u0027t have efficient backend cloning on the storage system? \n\nYes, I can confirm with some details: Infinidat Storage API can create an efficient clone on the storage system side, but some reason InfiniBox does not yet support detached clones. And if we create a clone from some parent volume, then we will not be able to delete the parent volume while it has at least one clone. And since the InfiniBox API does not support other ways to create a clone or copy of the volume, we are forced to create a full copy of the volume on the client side (attach source and destination volumes and run dd command). For example:\n\n* create a volume (volume1)\n* create a clone from the volume1 (volume2)\n* try to delete volume1 (Cinder architecture allows us to do this)\n\nResult:\nadmin@localhost\u003e vol.create name\u003dvolume1 size\u003d1G pool\u003ddevstack-iscsi1\nVolume \"volume1\" created\n\nadmin@localhost\u003e vol.clone.create vol\u003dvolume1 name\u003dvolume2 \nVolume clone \"volume2\" created\n\nadmin@localhost\u003e vol.delete vol\u003dvolume1\nDeleting the volume \u0027volume1\u0027 will also delete all of its clones: volume2\n\nSo we can\u0027t use this feature and forced to create a full copy on the client side.\n\n\u003e\u003e If it doesn\u0027t, is it able to export/map snapshots so Cinder can attach them and skip the creating volume from snapshot part?\n\nYes, thank you very much for this suggestion! Fixed in patch set #2:\nAdded initialize_connection_snapshot and terminate_connection_snapshot to make it possible to create a backup from a temporary snapshot. Thus, for each backup, a separate unique temporary snapshot will be created and there will be no race effect.\n\n\u003e\u003e Maybe the code could skip the snapshot creation if the volume is available and only go through that path when it\u0027s attached.\n\nI think it\u0027s safer to use a temporary snapshot in terms of data consistency.\nAnd this approach will allow to have a common flow for creating a clone and for performing a backup.\n\nThank you!","commit_id":"c392f4c8ceab4ca02ccfacfa2e6c7de25a122564"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c5f057dc4b8c9c469c9f625c5176f3847888deee","unresolved":true,"context_lines":[{"line_number":718,"context_line":"        # we need a cinder-volume-like object to map the clone by name"},{"line_number":719,"context_line":"        # (which is derived from the cinder id) but the clone is internal"},{"line_number":720,"context_line":"        # so there is no such object. mock one"},{"line_number":721,"context_line":"        clone \u003d mock.Mock(name_id\u003dstr(volume.name_id) + \u0027-internal\u0027,"},{"line_number":722,"context_line":"                          multiattach\u003dFalse, volume_attachment\u003d[])"},{"line_number":723,"context_line":"        try:"},{"line_number":724,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"}],"source_content_type":"text/x-python","patch_set":7,"id":"cecc6ce4_fb8b5db7","side":"PARENT","line":721,"range":{"start_line":721,"start_character":16,"end_line":721,"end_character":25},"updated":"2023-01-20 14:48:45.000000000","message":"Nice to see this is removed.","commit_id":"7d442e5b5bf990d54385dfb43fe6de5ac5625b7f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"86bfd296a8ee15392bf224e1f4113457b6235051","unresolved":false,"context_lines":[{"line_number":718,"context_line":"        # we need a cinder-volume-like object to map the clone by name"},{"line_number":719,"context_line":"        # (which is derived from the cinder id) but the clone is internal"},{"line_number":720,"context_line":"        # so there is no such object. mock one"},{"line_number":721,"context_line":"        clone \u003d mock.Mock(name_id\u003dstr(volume.name_id) + \u0027-internal\u0027,"},{"line_number":722,"context_line":"                          multiattach\u003dFalse, volume_attachment\u003d[])"},{"line_number":723,"context_line":"        try:"},{"line_number":724,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"}],"source_content_type":"text/x-python","patch_set":7,"id":"795df339_578f176e","side":"PARENT","line":721,"range":{"start_line":721,"start_character":16,"end_line":721,"end_character":25},"in_reply_to":"cecc6ce4_fb8b5db7","updated":"2023-01-24 13:44:46.000000000","message":"Hello Sofia,\n\nThank you very much for the review!","commit_id":"7d442e5b5bf990d54385dfb43fe6de5ac5625b7f"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c5f057dc4b8c9c469c9f625c5176f3847888deee","unresolved":true,"context_lines":[{"line_number":754,"context_line":"                                         dst_dev[\u0027device\u0027][\u0027path\u0027],"},{"line_number":755,"context_line":"                                         snapshot.volume.size * units.Ki,"},{"line_number":756,"context_line":"                                         dd_block_size, sparse\u003dTrue)"},{"line_number":757,"context_line":"        except Exception:"},{"line_number":758,"context_line":"            infinidat_volume.delete()"},{"line_number":759,"context_line":"            raise"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"8eaf17b3_d772d422","line":757,"range":{"start_line":757,"start_character":15,"end_line":757,"end_character":24},"updated":"2023-01-20 14:48:45.000000000","message":":-1: maybe for a follow-up patch, to create a new exception to handle the errors and be more useful to the user.","commit_id":"6bec1773519be99ff7a7e4a483a25af6eb5659ff"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"86bfd296a8ee15392bf224e1f4113457b6235051","unresolved":false,"context_lines":[{"line_number":754,"context_line":"                                         dst_dev[\u0027device\u0027][\u0027path\u0027],"},{"line_number":755,"context_line":"                                         snapshot.volume.size * units.Ki,"},{"line_number":756,"context_line":"                                         dd_block_size, sparse\u003dTrue)"},{"line_number":757,"context_line":"        except Exception:"},{"line_number":758,"context_line":"            infinidat_volume.delete()"},{"line_number":759,"context_line":"            raise"},{"line_number":760,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"04324ba2_49c89163","line":757,"range":{"start_line":757,"start_character":15,"end_line":757,"end_character":24},"in_reply_to":"8eaf17b3_d772d422","updated":"2023-01-24 13:44:46.000000000","message":"Hello Sofia,\n\nThank you very much for the review!\n\nOf course - I\u0027ll investigate and add it in the next pull requests!","commit_id":"6bec1773519be99ff7a7e4a483a25af6eb5659ff"},{"author":{"_account_id":34815,"name":"Aneesh Pachilangottil","email":"aneesh.p@fungible.com","username":"aneeeshp1"},"change_message_id":"7f4fc27ed084f41deb3ea905df8e10f14059c739","unresolved":true,"context_lines":[{"line_number":789,"context_line":"                            volume\u003dsrc_vref)"},{"line_number":790,"context_line":"        self.create_snapshot(snapshot)"},{"line_number":791,"context_line":"        self.create_volume_from_snapshot(volume, snapshot)"},{"line_number":792,"context_line":"        self.delete_snapshot(snapshot)"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"    def _build_initiator_target_map(self, connector, all_target_wwns):"},{"line_number":795,"context_line":"        \"\"\"Build the target_wwns and the initiator target map.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"45267131_405bcfc9","line":792,"updated":"2023-01-25 02:10:46.000000000","message":"create_volume_from_snapshot will raise exception in case of error and that will leave the snapshot undeleted. I think you should add try - except here and call delete_snapshot in the except as well.","commit_id":"10a159c2ff9ac3e58aeca36ebce08e130e12eb78"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"2fb23e8b4c203aec586bdf8b7875b7f3ff7da885","unresolved":false,"context_lines":[{"line_number":789,"context_line":"                            volume\u003dsrc_vref)"},{"line_number":790,"context_line":"        self.create_snapshot(snapshot)"},{"line_number":791,"context_line":"        self.create_volume_from_snapshot(volume, snapshot)"},{"line_number":792,"context_line":"        self.delete_snapshot(snapshot)"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"    def _build_initiator_target_map(self, connector, all_target_wwns):"},{"line_number":795,"context_line":"        \"\"\"Build the target_wwns and the initiator target map.\"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"59001b52_97372527","line":792,"in_reply_to":"45267131_405bcfc9","updated":"2023-01-31 12:42:58.000000000","message":"Hello Aneesh,\n\nThank you very much for the review!\n\nFixed in patch set #10.\n\nThank you!","commit_id":"10a159c2ff9ac3e58aeca36ebce08e130e12eb78"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":548,"context_line":"    def initialize_connection_snapshot(self, snapshot, connector, **kwargs):"},{"line_number":549,"context_line":"        \"\"\"Map an InfiniBox snapshot to the host\"\"\""},{"line_number":550,"context_line":"        infinidat_snapshot \u003d self._get_infinidat_snapshot(snapshot)"},{"line_number":551,"context_line":"        return self._initialize_connection(infinidat_snapshot, connector)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    @coordination.synchronized(\u0027infinidat-{self.management_address}-lock\u0027)"},{"line_number":554,"context_line":"    def _terminate_connection(self, infinidat_volume, connector):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e825df74_5d8a9ef1","line":551,"range":{"start_line":551,"start_character":20,"end_line":551,"end_character":42},"updated":"2023-01-30 17:55:30.000000000","message":"so in infinidat driver, volumes and snapshots attach in same way?","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":548,"context_line":"    def initialize_connection_snapshot(self, snapshot, connector, **kwargs):"},{"line_number":549,"context_line":"        \"\"\"Map an InfiniBox snapshot to the host\"\"\""},{"line_number":550,"context_line":"        infinidat_snapshot \u003d self._get_infinidat_snapshot(snapshot)"},{"line_number":551,"context_line":"        return self._initialize_connection(infinidat_snapshot, connector)"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"    @coordination.synchronized(\u0027infinidat-{self.management_address}-lock\u0027)"},{"line_number":554,"context_line":"    def _terminate_connection(self, infinidat_volume, connector):"}],"source_content_type":"text/x-python","patch_set":9,"id":"f607a288_13da33fd","line":551,"range":{"start_line":551,"start_character":20,"end_line":551,"end_character":42},"in_reply_to":"e825df74_5d8a9ef1","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e so in infinidat driver, volumes and snapshots attach in same way?\n\nYes.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":741,"context_line":"        - create destination volume"},{"line_number":742,"context_line":"        - map source snapshot and destination volume"},{"line_number":743,"context_line":"        - copy data from snapshot to volume"},{"line_number":744,"context_line":"        - unmap volume and clone"},{"line_number":745,"context_line":"        \"\"\""},{"line_number":746,"context_line":"        infinidat_snapshot \u003d self._get_infinidat_snapshot(snapshot)"},{"line_number":747,"context_line":"        infinidat_volume \u003d self._create_volume(volume)"}],"source_content_type":"text/x-python","patch_set":9,"id":"a2a7fdec_c9415708","line":744,"range":{"start_line":744,"start_character":27,"end_line":744,"end_character":32},"updated":"2023-01-30 17:55:30.000000000","message":"what is a clone here? I think this should be snapshot","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"0d125eaed5684e1068f736ad3dbc53224c5b17a1","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        - create destination volume"},{"line_number":742,"context_line":"        - map source snapshot and destination volume"},{"line_number":743,"context_line":"        - copy data from snapshot to volume"},{"line_number":744,"context_line":"        - unmap volume and clone"},{"line_number":745,"context_line":"        \"\"\""},{"line_number":746,"context_line":"        infinidat_snapshot \u003d self._get_infinidat_snapshot(snapshot)"},{"line_number":747,"context_line":"        infinidat_volume \u003d self._create_volume(volume)"}],"source_content_type":"text/x-python","patch_set":9,"id":"946599e0_1e8774d8","line":744,"range":{"start_line":744,"start_character":27,"end_line":744,"end_character":32},"in_reply_to":"a2a7fdec_c9415708","updated":"2023-01-31 12:40:57.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e what is a clone here? I think this should be snapshot\n\nAh yes - it is my mistake!\n\nFixed in patch set #10.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":false,"context_lines":[{"line_number":775,"context_line":"        This could be a lengthy operation."},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        * create temporary snapshot from source volume"},{"line_number":778,"context_line":"        * map temporary snapshot"},{"line_number":779,"context_line":"        * create and map new volume"},{"line_number":780,"context_line":"        * copy data from temporary snapshot to new volume"},{"line_number":781,"context_line":"        * unmap volume and temporary snapshot"},{"line_number":782,"context_line":"        * delete temporary snapshot"},{"line_number":783,"context_line":"        \"\"\""},{"line_number":784,"context_line":"        attributes \u003d (\u0027id\u0027, \u0027name\u0027, \u0027volume\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7cbbcf46_0d4b397c","line":781,"range":{"start_line":778,"start_character":0,"end_line":781,"end_character":45},"updated":"2023-01-30 17:55:30.000000000","message":"these steps are already mentioned in create_volume_from_snapshot method\nI don\u0027t mind keeping it here but good to keep abstraction in methods i.e. we call create_volume_from_snapshot to copy snapshot data to volume. we don\u0027t know how it does it but we know what it does. Also if we do any change in the called method flow (i.e. create_volume_from_snapshot) then we need to repeat the same.","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"}],"releasenotes/notes/bug-1983287-infinidat-fix-backup-attached-volume-b28e5dd5c25a24ec.yaml":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f8dc01f49e0bbe6e639237e47a0f526705653eec","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Infinidat Driver `bug #1983287"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/1983287\u003e`_:"},{"line_number":6,"context_line":"    Fixed Infinidat driver to allow backup the attached volume."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"5671d4ff_89e40d7c","line":6,"range":{"start_line":6,"start_character":43,"end_line":6,"end_character":46},"updated":"2023-01-30 17:55:30.000000000","message":"nit: of an","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"2fb23e8b4c203aec586bdf8b7875b7f3ff7da885","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Infinidat Driver `bug #1983287"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/1983287\u003e`_:"},{"line_number":6,"context_line":"    Fixed Infinidat driver to allow backup the attached volume."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"11f494b0_ccff7857","line":6,"range":{"start_line":6,"start_character":43,"end_line":6,"end_character":46},"in_reply_to":"5671d4ff_89e40d7c","updated":"2023-01-31 12:42:58.000000000","message":"Hello Rajat!\n\nThank you very much for the review!\n\nFixed in patch set #11.\n\nThank you!","commit_id":"d8ca05b8a3843755793037df9e4dc2d071063fad"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"79380dd83f4737169086c0e3cd47510badf7dd2f","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Infinidat Driver `bug #1983287"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/1983287\u003e`_:"},{"line_number":6,"context_line":"    Fixed Infinidat driver to allow backup an attached volume."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"a67b6c37_12de1063","line":6,"range":{"start_line":6,"start_character":43,"end_line":6,"end_character":45},"updated":"2023-01-31 15:33:04.000000000","message":"allow backup \"of an\" attached volume","commit_id":"aee9aafe4ad7a6075e361ddf803e80b6245df7ee"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"7c6b0c433dbf2ab3c5aff30b6beca36888787170","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Infinidat Driver `bug #1983287"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/1983287\u003e`_:"},{"line_number":6,"context_line":"    Fixed Infinidat driver to allow backup an attached volume."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"0730ef7d_707ba24d","line":6,"range":{"start_line":6,"start_character":43,"end_line":6,"end_character":45},"in_reply_to":"a67b6c37_12de1063","updated":"2023-01-31 15:41:25.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\nFixed in patch set #12.\n\nThank you!","commit_id":"aee9aafe4ad7a6075e361ddf803e80b6245df7ee"}]}