)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"4222dda039618ed0ab5dc9d83c60bd35935aee3c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"non-attachable volumes. Previously in generic volume migration, each"},{"line_number":11,"context_line":"volume was attached locally and dd was executed on the block device"},{"line_number":12,"context_line":"paths to move volume contents from source to target. This patch adds an"},{"line_number":13,"context_line":"\u0027is_attachable\u0027 method that volume drivers can use to express whether or"},{"line_number":14,"context_line":"not they support local attachment. If one or both of the volumes"},{"line_number":15,"context_line":"involved in a migration operation cannot be attached locally, the"},{"line_number":16,"context_line":"manager will now attempt to open a file handle for both volumes and move"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa32b979_551f1c09","line":13,"updated":"2015-06-26 18:17:27.000000000","message":"Where is this function?? In the code I see that you are using isinstance() but that does not make sense to select whether the volume is attachable or not.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3502f523f22f9c5ef02d7c288e2d2e09fd081deb","unresolved":false,"context_lines":[{"line_number":10,"context_line":"non-attachable volumes. Previously in generic volume migration, each"},{"line_number":11,"context_line":"volume was attached locally and dd was executed on the block device"},{"line_number":12,"context_line":"paths to move volume contents from source to target. This patch adds an"},{"line_number":13,"context_line":"\u0027is_attachable\u0027 method that volume drivers can use to express whether or"},{"line_number":14,"context_line":"not they support local attachment. If one or both of the volumes"},{"line_number":15,"context_line":"involved in a migration operation cannot be attached locally, the"},{"line_number":16,"context_line":"manager will now attempt to open a file handle for both volumes and move"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_bd5f3fe4","line":13,"in_reply_to":"fa32b979_551f1c09","updated":"2015-06-30 20:42:35.000000000","message":"I forgot to change the commit message from the previous implementation attempt, I\u0027ll include updated text in the next upload.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"4222dda039618ed0ab5dc9d83c60bd35935aee3c","unresolved":false,"context_lines":[{"line_number":16,"context_line":"manager will now attempt to open a file handle for both volumes and move"},{"line_number":17,"context_line":"data from source to target using file I/O. This allows drivers like RBD"},{"line_number":18,"context_line":"to participate in volume migration using file I/O when it would"},{"line_number":19,"context_line":"otherwise be impossible as attachment is not supported."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Because an instance of both source and target volumes is necessary to"},{"line_number":22,"context_line":"open the needed file handles, the migration logic is moved from the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa32b979_b59c685e","line":19,"updated":"2015-06-26 18:17:27.000000000","message":"What kind of file is the RDB files? How do they shown in the OS? HNAS NFS is not attachable and I still can migrate using the \u0027dd\u0027 copy. Also, if you can open the file with open, why is not possible to dd to it? in the end both are the same operation.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"4222dda039618ed0ab5dc9d83c60bd35935aee3c","unresolved":false,"context_lines":[{"line_number":22,"context_line":"open the needed file handles, the migration logic is moved from the"},{"line_number":23,"context_line":"volume driver into the manager. A previous approach attempted to leave"},{"line_number":24,"context_line":"the logic in the volume driver, but instantiating a new driver at that"},{"line_number":25,"context_line":"layer became too messy."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Support for RBD is included as a reference for other drivers that wish"},{"line_number":28,"context_line":"to take advantage of this alternative approach."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa32b979_1592f4f6","line":25,"updated":"2015-06-26 18:17:27.000000000","message":"What do you mean with this 2 instances? Can you better expain this? If the \u0027dd\u0027 code can access both src and dest from driver code, why using open() is not possible?","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":16,"context_line":"This patch improves the generic migration logic to determine whether a"},{"line_number":17,"context_line":"migration operation can proceed with dd using block device paths or file"},{"line_number":18,"context_line":"operations on handles returned from the os-brick connectors."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Blueprint: generic-volume-migration"},{"line_number":21,"context_line":"Change-Id: Iece2776fa751152f97b389ddab426e50c6f79bea"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"fa1b9901_9506cccc","line":19,"updated":"2015-08-19 13:00:02.000000000","message":"Ceph changes should be mentioned","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0538f449c510c780df084e21ce4929aa31f8360a","unresolved":false,"context_lines":[{"line_number":20,"context_line":"Changes to the RBD driver are included to correctly rename the target"},{"line_number":21,"context_line":"volume during the completion phase of a successful migration."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"It appears there is still some work to be done for attached in-use"},{"line_number":24,"context_line":"volume migration in general.  This patch should not introduce additional"},{"line_number":25,"context_line":"bugs to that use case and a follow-up patch will address the outstanding"},{"line_number":26,"context_line":"issues."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"da20952f_b8c3b556","line":23,"updated":"2015-08-28 19:05:49.000000000","message":"-1: Please update this paragraph, because this actually works for in-use migrations from Ceph to LVM and NFS.\n\nIt just doesn\u0027t work for LVM, NFS to Ceph, but that is a Nova thing, nothing to do with this patch.","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"}],"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4222373da32b43758f5c0c36c69b6b0ef6330586","unresolved":false,"context_lines":[{"line_number":4135,"context_line":"        self.volume._migrate_volume_generic(self.context, volume,"},{"line_number":4136,"context_line":"                                            host_obj, None)"},{"line_number":4137,"context_line":"        self.assertFalse(migrate_volume_completion.called)"},{"line_number":4138,"context_line":"        with mock.patch.object(self.volume.driver, \u0027copy_volume_data\u0027) as \\"},{"line_number":4139,"context_line":"                mock_copy_volume:"},{"line_number":4140,"context_line":"            self.volume._migrate_volume_generic(self.context, volume,"},{"line_number":4141,"context_line":"                                                host_obj, None)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_16d42645","line":4138,"updated":"2015-08-18 19:50:43.000000000","message":"I think we don\u0027t need this mock because copy_volume_data is not called attached_volume path.","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4222373da32b43758f5c0c36c69b6b0ef6330586","unresolved":false,"context_lines":[{"line_number":4138,"context_line":"        with mock.patch.object(self.volume.driver, \u0027copy_volume_data\u0027) as \\"},{"line_number":4139,"context_line":"                mock_copy_volume:"},{"line_number":4140,"context_line":"            self.volume._migrate_volume_generic(self.context, volume,"},{"line_number":4141,"context_line":"                                                host_obj, None)"},{"line_number":4142,"context_line":"            self.assertFalse(mock_copy_volume.called)"},{"line_number":4143,"context_line":"            self.assertFalse(migrate_volume_completion.called)"},{"line_number":4144,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_96c0367e","line":4141,"updated":"2015-08-18 19:50:43.000000000","message":"This assertFalse is unnecessary if above mock will be removed.","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4222373da32b43758f5c0c36c69b6b0ef6330586","unresolved":false,"context_lines":[{"line_number":4161,"context_line":"                mock_migrate_volume,\\"},{"line_number":4162,"context_line":"                mock.patch.object(self.volume, \u0027_copy_volume_data\u0027), \\"},{"line_number":4163,"context_line":"                mock.patch.object(self.volume, \u0027_attach_volume\u0027), \\"},{"line_number":4164,"context_line":"                mock.patch.object(self.volume, \u0027_detach_volume\u0027):"},{"line_number":4165,"context_line":"            create_volume.side_effect \u003d fake_create_volume"},{"line_number":4166,"context_line":"            self.volume.migrate_volume(self.context, fake_volume[\u0027id\u0027],"},{"line_number":4167,"context_line":"                                       host_obj, True)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_993629e6","line":4164,"updated":"2015-08-18 19:50:43.000000000","message":"nit: Do we need to mock out _attach_volume and _detach_volume even if they are only used at _copy_volume_data()?","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4222373da32b43758f5c0c36c69b6b0ef6330586","unresolved":false,"context_lines":[{"line_number":4188,"context_line":"                mock.patch.object(self.volume, \u0027migrate_volume_completion\u0027),\\"},{"line_number":4189,"context_line":"                mock.patch.object(self.volume.driver, \u0027create_export\u0027), \\"},{"line_number":4190,"context_line":"                mock.patch.object(self.volume, \u0027_attach_volume\u0027), \\"},{"line_number":4191,"context_line":"                mock.patch.object(self.volume, \u0027_detach_volume\u0027):"},{"line_number":4192,"context_line":""},{"line_number":4193,"context_line":"            # Exception case at migrate_volume_generic"},{"line_number":4194,"context_line":"            # source_volume[\u0027migration_status\u0027] is \u0027migrating\u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_593091e8","line":4191,"updated":"2015-08-18 19:50:43.000000000","message":"nit: Do we need to mock out _attach_volume and _detach_volume even if they are only used at _copy_volume_data()?","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4222373da32b43758f5c0c36c69b6b0ef6330586","unresolved":false,"context_lines":[{"line_number":4326,"context_line":"                mock.patch.object(self.volume.driver, \u0027create_export\u0027) as \\"},{"line_number":4327,"context_line":"                mock_create_export, \\"},{"line_number":4328,"context_line":"                mock.patch.object(self.volume, \u0027_attach_volume\u0027), \\"},{"line_number":4329,"context_line":"                mock.patch.object(self.volume, \u0027_detach_volume\u0027):"},{"line_number":4330,"context_line":""},{"line_number":4331,"context_line":"            # Exception case at create_export"},{"line_number":4332,"context_line":"            mock_create_volume.side_effect \u003d fake_create_volume"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_b9a52d05","line":4329,"updated":"2015-08-18 19:50:43.000000000","message":"nit: Do we need to mock out _attach_volume and _detach_volume even if they are only used at _copy_volume_data()?","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"4222373da32b43758f5c0c36c69b6b0ef6330586","unresolved":false,"context_lines":[{"line_number":4360,"context_line":"        with mock.patch.object(self.volume.driver, \u0027migrate_volume\u0027),\\"},{"line_number":4361,"context_line":"                mock.patch.object(volume_rpcapi.VolumeAPI, \u0027create_volume\u0027)\\"},{"line_number":4362,"context_line":"                as mock_create_volume,\\"},{"line_number":4363,"context_line":"                mock.patch.object(self.volume.driver, \u0027copy_volume_data\u0027),\\"},{"line_number":4364,"context_line":"                mock.patch.object(volume_rpcapi.VolumeAPI, \u0027delete_volume\u0027),\\"},{"line_number":4365,"context_line":"                mock.patch.object(self.volume, \u0027migrate_volume_completion\u0027)\\"},{"line_number":4366,"context_line":"                as mock_migrate_compl,\\"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_160986dd","line":4363,"updated":"2015-08-18 19:50:43.000000000","message":"I think this mock for copy_volume_data() is unnecessary. Or we can mock out _copy_volume_data() here.","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73b938bff48e3e0fc6031fe25e44c8e0c3b1dbab","unresolved":false,"context_lines":[{"line_number":4139,"context_line":""},{"line_number":4140,"context_line":"        self.volume._migrate_volume_generic(self.context, volume,"},{"line_number":4141,"context_line":"                                            host_obj, None)"},{"line_number":4142,"context_line":"        self.assertFalse(migrate_volume_completion.called)"},{"line_number":4143,"context_line":""},{"line_number":4144,"context_line":"    @mock.patch.object(volume_rpcapi.VolumeAPI, \u0027update_migrated_volume\u0027)"},{"line_number":4145,"context_line":"    @mock.patch.object(volume_rpcapi.VolumeAPI, \u0027delete_volume\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_ab92f69d","line":4142,"updated":"2015-08-21 21:19:59.000000000","message":"-1: Lines 4140-4142 are the same as 4136-4138.  You can delete one of those.\n\nAlso it should be asserted that update_server_volume is called with the right arguments while we are at it.","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"}],"cinder/tests/unit/test_volume_rpcapi.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"c6e97cadad35318b73b49999e8310f8ba4509f9b","unresolved":false,"context_lines":[{"line_number":407,"context_line":"        self._test_volume_api(\u0027remove_export\u0027,"},{"line_number":408,"context_line":"                              rpc_method\u003d\u0027cast\u0027,"},{"line_number":409,"context_line":"                              volume\u003dself.fake_volume,"},{"line_number":410,"context_line":"                              version\u003d\u00271.29\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"da20952f_2ff30fa0","line":410,"updated":"2015-08-27 21:43:24.000000000","message":"There is already another patch out there trying to use this version.  Going to be a race condition as to which merges first.","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0538f449c510c780df084e21ce4929aa31f8360a","unresolved":false,"context_lines":[{"line_number":407,"context_line":"        self._test_volume_api(\u0027remove_export\u0027,"},{"line_number":408,"context_line":"                              rpc_method\u003d\u0027cast\u0027,"},{"line_number":409,"context_line":"                              volume\u003dself.fake_volume,"},{"line_number":410,"context_line":"                              version\u003d\u00271.29\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"da20952f_382325a9","line":410,"in_reply_to":"da20952f_2ff30fa0","updated":"2015-08-28 19:05:49.000000000","message":"Actually I think there are another 2.   XD","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"0d4a286e4fa4edd3216c8ffe80ebbde079398704","unresolved":false,"context_lines":[{"line_number":1031,"context_line":"                raise exception.VolumeBackendAPIException("},{"line_number":1032,"context_line":"                    data\u003dexception_message)"},{"line_number":1033,"context_line":""},{"line_number":1034,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume):"},{"line_number":1035,"context_line":"        \"\"\"Return model update for migrated volume."},{"line_number":1036,"context_line":""},{"line_number":1037,"context_line":"        :param volume: The original volume that was migrated to this backend"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_f9b25b48","line":1034,"updated":"2015-06-24 16:28:11.000000000","message":"I suggest you take a look at patch https://review.openstack.org/#/c/180873/.\nIf it is necessary, you may implement this methhod for rbd.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"4222dda039618ed0ab5dc9d83c60bd35935aee3c","unresolved":false,"context_lines":[{"line_number":1031,"context_line":"                raise exception.VolumeBackendAPIException("},{"line_number":1032,"context_line":"                    data\u003dexception_message)"},{"line_number":1033,"context_line":""},{"line_number":1034,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume):"},{"line_number":1035,"context_line":"        \"\"\"Return model update for migrated volume."},{"line_number":1036,"context_line":""},{"line_number":1037,"context_line":"        :param volume: The original volume that was migrated to this backend"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_48a57345","line":1034,"in_reply_to":"fa32b979_f9b25b48","updated":"2015-06-26 18:17:27.000000000","message":"Link Not Found!","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3502f523f22f9c5ef02d7c288e2d2e09fd081deb","unresolved":false,"context_lines":[{"line_number":1031,"context_line":"                raise exception.VolumeBackendAPIException("},{"line_number":1032,"context_line":"                    data\u003dexception_message)"},{"line_number":1033,"context_line":""},{"line_number":1034,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume):"},{"line_number":1035,"context_line":"        \"\"\"Return model update for migrated volume."},{"line_number":1036,"context_line":""},{"line_number":1037,"context_line":"        :param volume: The original volume that was migrated to this backend"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_9df7dbf4","line":1034,"in_reply_to":"fa32b979_f9b25b48","updated":"2015-06-30 20:42:35.000000000","message":"Will do, thanks for mentioning it.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"c5fd9d17b278a6d6b52595ceaf468bac160b1deb","unresolved":false,"context_lines":[{"line_number":1080,"context_line":"                                           utils.convert_str(wanted_name))"},{"line_number":1081,"context_line":"                except self.rbd.ImageNotFound:"},{"line_number":1082,"context_line":"                    LOG.error(_LE(\u0027Unable to rename the logical volume \u0027"},{"line_number":1083,"context_line":"                                  \u0027for volume: %s\u0027), volume[\u0027name\u0027])"},{"line_number":1084,"context_line":"                    # If the rename fails, _name_id should be set to the new"},{"line_number":1085,"context_line":"                    # volume id and provider_location should be set to the"},{"line_number":1086,"context_line":"                    # one from the new volume as well."}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_81026c0a","line":1083,"updated":"2015-08-08 09:27:50.000000000","message":"Missing . after \"%s\".","commit_id":"309cd3c1254b698f23e902af0a0dbdfaf757d4e0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"73b938bff48e3e0fc6031fe25e44c8e0c3b1dbab","unresolved":false,"context_lines":[{"line_number":1070,"context_line":"        \"\"\""},{"line_number":1071,"context_line":"        name_id \u003d None"},{"line_number":1072,"context_line":"        provider_location \u003d None"},{"line_number":1073,"context_line":"        if original_volume_status \u003d\u003d \u0027available\u0027:"},{"line_number":1074,"context_line":"            existing_name \u003d CONF.volume_name_template % new_volume[\u0027id\u0027]"},{"line_number":1075,"context_line":"            wanted_name \u003d CONF.volume_name_template % volume[\u0027id\u0027]"},{"line_number":1076,"context_line":"            with RADOSClient(self) as client:"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_1036b1b4","line":1073,"updated":"2015-08-21 21:19:59.000000000","message":"-1: Why only rename if it was originally available?  By the time we reach this point it should have been detached already.","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"},{"author":{"_account_id":16793,"name":"Liucheng Jiang","email":"jiangliucheng@huawei.com","username":"Jiangliucheng"},"change_message_id":"004a463c2a7aca42b71f9dda366bc10c3c39ff27","unresolved":false,"context_lines":[{"line_number":1071,"context_line":"        name_id \u003d None"},{"line_number":1072,"context_line":"        provider_location \u003d None"},{"line_number":1073,"context_line":""},{"line_number":1074,"context_line":"        existing_name \u003d CONF.volume_name_template % new_volume[\u0027id\u0027]"},{"line_number":1075,"context_line":"        wanted_name \u003d CONF.volume_name_template % volume[\u0027id\u0027]"},{"line_number":1076,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1077,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa1b9901_f40391b6","line":1074,"updated":"2015-08-25 07:23:25.000000000","message":"CONF -\u003e self.configuration","commit_id":"545ebba293be690636806abfed1b91e3255009d3"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"197ef9f644004b6680ad9b6264e58ae71ec75908","unresolved":false,"context_lines":[{"line_number":1071,"context_line":"        name_id \u003d None"},{"line_number":1072,"context_line":"        provider_location \u003d None"},{"line_number":1073,"context_line":""},{"line_number":1074,"context_line":"        existing_name \u003d CONF.volume_name_template % new_volume[\u0027id\u0027]"},{"line_number":1075,"context_line":"        wanted_name \u003d CONF.volume_name_template % volume[\u0027id\u0027]"},{"line_number":1076,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1077,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":19,"id":"fa1b9901_e2888573","line":1074,"in_reply_to":"fa1b9901_f40391b6","updated":"2015-08-25 21:45:36.000000000","message":"I don\u0027t believe that\u0027s true for volume_name_template","commit_id":"545ebba293be690636806abfed1b91e3255009d3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"64a22083ec1668c2176c384b5832e1647ce515a5","unresolved":false,"context_lines":[{"line_number":1080,"context_line":"                                       utils.convert_str(wanted_name))"},{"line_number":1081,"context_line":"            except self.rbd.ImageNotFound:"},{"line_number":1082,"context_line":"                LOG.error(_LE(\u0027Unable to rename the logical volume \u0027"},{"line_number":1083,"context_line":"                              \u0027for volume %s.\u0027), volume[\u0027name\u0027])"},{"line_number":1084,"context_line":"                # If the rename fails, _name_id should be set to the new"},{"line_number":1085,"context_line":"                # volume id and provider_location should be set to the"},{"line_number":1086,"context_line":"                # one from the new volume as well."}],"source_content_type":"text/x-python","patch_set":19,"id":"fa1b9901_0cb29594","line":1083,"updated":"2015-08-25 20:08:34.000000000","message":"Please, add volume ID to logs too","commit_id":"545ebba293be690636806abfed1b91e3255009d3"}],"cinder/volume/manager.py":[{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"78e2921421b0836c873b0500d9a795d270631d73","unresolved":false,"context_lines":[{"line_number":1286,"context_line":""},{"line_number":1287,"context_line":"        properties \u003d utils.brick_get_connector_properties()"},{"line_number":1288,"context_line":""},{"line_number":1289,"context_line":"        dest_remote \u003d True if remote in [\u0027dest\u0027, \u0027both\u0027] else False"},{"line_number":1290,"context_line":"        dest_attach_info \u003d self._attach_volume(ctxt, dest_vol, properties,"},{"line_number":1291,"context_line":"                                               remote\u003ddest_remote)"},{"line_number":1292,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_0f74e365","line":1289,"updated":"2015-07-01 05:49:34.000000000","message":"here and three lines below, the ternary condition can be removed, e.g. dest_remote \u003d remote in [\u0027dest\u0027, \u0027both\u0027]","commit_id":"911d3bd546a496dd6e512971938f41e3582165cf"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"78e2921421b0836c873b0500d9a795d270631d73","unresolved":false,"context_lines":[{"line_number":1307,"context_line":"                msg \u003d _(\"Failed to copy volume %(src)s to %(dest)s.\")"},{"line_number":1308,"context_line":"                LOG.error(msg % {\u0027src\u0027: src_vol[\u0027id\u0027], \u0027dest\u0027: dest_vol[\u0027id\u0027]})"},{"line_number":1309,"context_line":"        finally:"},{"line_number":1310,"context_line":"            self._detach_volume(ctxt, dest_attach_info, dest_vol,"},{"line_number":1311,"context_line":"                                properties, force\u003dcopy_error,"},{"line_number":1312,"context_line":"                                remote\u003ddest_remote)"},{"line_number":1313,"context_line":"            self._detach_volume(ctxt, src_attach_info, src_vol,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_6f86cf14","line":1310,"updated":"2015-07-01 05:49:34.000000000","message":"wrap this in try/finally so src_vol is always detached too","commit_id":"911d3bd546a496dd6e512971938f41e3582165cf"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"834ff341c9bf9dab65ec07a94f7b86ce6dadd976","unresolved":false,"context_lines":[{"line_number":1174,"context_line":"            err_msg \u003d (_(\u0027Remove volume export failed: %(err)s\u0027)"},{"line_number":1175,"context_line":"                       % {\u0027err\u0027: err})"},{"line_number":1176,"context_line":"            LOG.error(err_msg, resource\u003dvolume_ref)"},{"line_number":1177,"context_line":"            raise exception.VolumeBackendAPIException(data\u003derr_msg)"},{"line_number":1178,"context_line":""},{"line_number":1179,"context_line":"        LOG.info(_LI(\"Terminate volume connection completed successfully.\"),"},{"line_number":1180,"context_line":"                 resource\u003dvolume_ref)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_cb0c8fef","line":1177,"updated":"2015-07-01 16:06:54.000000000","message":"Nova\u0027s _post_live_migration() in compute/manager.py calls terminate_connection() during instance migration with volume.\n\nIf user uses LVM backend for the volume, iSCSI target will be forcibly removed by this remove_export call and the volume will become inaccessible status after live migration.\n\nI suppose I\u0027m trying to solve a similar problem which you want to resolve.\nPlease refer following review.\n\nhttps://review.openstack.org/#/c/194223/3","commit_id":"763a65039182005c2b01da013039413b7df326f3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"3b1a2b65fda59c1ea0b760440b909eaaafbc9ffb","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":1179,"context_line":"        try:"},{"line_number":1180,"context_line":"            self.driver.remove_export(context, volume_ref)"},{"line_number":1181,"context_line":"        except Exception as err:"},{"line_number":1182,"context_line":"            err_msg \u003d (_(\u0027Remove volume export failed: %(err)s\u0027)"},{"line_number":1183,"context_line":"                       % {\u0027err\u0027: err})"},{"line_number":1184,"context_line":"            LOG.error(err_msg, resource\u003dvolume_ref)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_1be8b0e1","line":1181,"updated":"2015-07-07 14:55:38.000000000","message":"Could we handle more specific exception here?","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"3b1a2b65fda59c1ea0b760440b909eaaafbc9ffb","unresolved":false,"context_lines":[{"line_number":1181,"context_line":"        except Exception as err:"},{"line_number":1182,"context_line":"            err_msg \u003d (_(\u0027Remove volume export failed: %(err)s\u0027)"},{"line_number":1183,"context_line":"                       % {\u0027err\u0027: err})"},{"line_number":1184,"context_line":"            LOG.error(err_msg, resource\u003dvolume_ref)"},{"line_number":1185,"context_line":"            raise exception.VolumeBackendAPIException(data\u003derr_msg)"},{"line_number":1186,"context_line":""},{"line_number":1187,"context_line":"        LOG.info(_LI(\"Remove volume export completed successfully.\"),"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_3be33407","line":1184,"updated":"2015-07-07 14:55:38.000000000","message":"LOG.exception","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"84216abea4da2b062ef84d6eb501ea3644a39d86","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"            except Exception:"},{"line_number":1263,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1264,"context_line":"                    msg \u003d _(\"Failed to attach volume %(vol)s\")"},{"line_number":1265,"context_line":"                    LOG.error(msg % {\u0027vol\u0027: volume[\u0027id\u0027]})"},{"line_number":1266,"context_line":"                    self.db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":1267,"context_line":"                                          {\u0027status\u0027: status})"},{"line_number":1268,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_7eae8a99","line":1265,"updated":"2015-07-07 15:15:28.000000000","message":"According to logging guidelines, please change % \u003d\u003e ,\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"84216abea4da2b062ef84d6eb501ea3644a39d86","unresolved":false,"context_lines":[{"line_number":1295,"context_line":"        \"\"\"Copy data from src_vol to dest_vol.\"\"\""},{"line_number":1296,"context_line":""},{"line_number":1297,"context_line":"        LOG.debug((\u0027copy_data_between_volumes %(src)s -\u003e %(dest)s.\u0027)"},{"line_number":1298,"context_line":"                  % {\u0027src\u0027: src_vol[\u0027name\u0027], \u0027dest\u0027: dest_vol[\u0027name\u0027]})"},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":"        properties \u003d utils.brick_get_connector_properties()"},{"line_number":1301,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_1ecbbe54","line":1298,"updated":"2015-07-07 15:15:28.000000000","message":"the same, please change % \u003d\u003e ,","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"84216abea4da2b062ef84d6eb501ea3644a39d86","unresolved":false,"context_lines":[{"line_number":1318,"context_line":"        except Exception:"},{"line_number":1319,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1320,"context_line":"                msg \u003d _(\"Failed to copy volume %(src)s to %(dest)s.\")"},{"line_number":1321,"context_line":"                LOG.error(msg % {\u0027src\u0027: src_vol[\u0027id\u0027], \u0027dest\u0027: dest_vol[\u0027id\u0027]})"},{"line_number":1322,"context_line":"        finally:"},{"line_number":1323,"context_line":"            try:"},{"line_number":1324,"context_line":"                self._detach_volume(ctxt, dest_attach_info, dest_vol,"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_febffaa7","line":1321,"updated":"2015-07-07 15:15:28.000000000","message":"the same, please change % \u003d\u003e ,","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"5f5a241a508cf031e20cca103c8aeaea60c1eb20","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"                conn \u003d rpcapi.initialize_connection(ctxt, volume, properties)"},{"line_number":1262,"context_line":"            except Exception:"},{"line_number":1263,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1264,"context_line":"                    msg \u003d _(\"Failed to attach volume %(vol)s\")"},{"line_number":1265,"context_line":"                    LOG.error(msg, {\u0027vol\u0027: volume[\u0027id\u0027]})"},{"line_number":1266,"context_line":"                    self.db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":1267,"context_line":"                                          {\u0027status\u0027: status})"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_a61dd967","line":1264,"updated":"2015-07-07 21:14:45.000000000","message":"\u0027.\u0027 at EOL","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"2937eae43bc197e232a8ecd54b9358b3ad2084b1","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"        else:"},{"line_number":1285,"context_line":"            try:"},{"line_number":1286,"context_line":"                self.terminate_connection(ctxt, volume[\u0027id\u0027], connector,"},{"line_number":1287,"context_line":"                                          force\u003dforce)"},{"line_number":1288,"context_line":"            except Exception as err:"},{"line_number":1289,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1290,"context_line":"                    err_msg \u003d (_(\u0027Unable to terminate volume connection: \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_5a280b86","line":1287,"updated":"2015-07-09 08:17:20.000000000","message":"Is remove_export also needed here?","commit_id":"8b28c85adae88e42142697a2917386d736ade580"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"f5943514f38549b78591a845d0de23b1f0a4f8a9","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"        else:"},{"line_number":1285,"context_line":"            try:"},{"line_number":1286,"context_line":"                self.terminate_connection(ctxt, volume[\u0027id\u0027], connector,"},{"line_number":1287,"context_line":"                                          force\u003dforce)"},{"line_number":1288,"context_line":"            except Exception as err:"},{"line_number":1289,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1290,"context_line":"                    err_msg \u003d (_(\u0027Unable to terminate volume connection: \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_b8e90de8","line":1287,"in_reply_to":"ba3cc151_5a280b86","updated":"2015-07-09 14:17:41.000000000","message":"That was my initial approach, but Mitsuhiro indicated that it would break live migration, so I decided to separate these two for that reason.\n\nAhh, now I see your point.  So, terminate_connection() behaves differently when called over rpc.  When it\u0027s called locally, the export remains, and over rpc the export is removed.  If I \"fix\" that I worry I\u0027ll break many other things.  What is your take?","commit_id":"8b28c85adae88e42142697a2917386d736ade580"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"7877ad644949a3b92894adf2fe92e09724f923bc","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"        else:"},{"line_number":1285,"context_line":"            try:"},{"line_number":1286,"context_line":"                self.terminate_connection(ctxt, volume[\u0027id\u0027], connector,"},{"line_number":1287,"context_line":"                                          force\u003dforce)"},{"line_number":1288,"context_line":"            except Exception as err:"},{"line_number":1289,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1290,"context_line":"                    err_msg \u003d (_(\u0027Unable to terminate volume connection: \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_e084970c","line":1287,"in_reply_to":"ba3cc151_b37e0e17","updated":"2015-07-10 06:38:10.000000000","message":"Why does the _detach_volume local missing remove_export cause no issue so far? Because the _detach_volume local is for the original volume. After a complete migration, the original volume is deleted. During volume_delete, the export is removed.\n\nThe _detach_volume remote is different. If the export is not removed, there is going to be name consistency issue after update_migrated_volume is implemented.\n\nHowever, from my perspective, _detach_volume local should have a remove_export to make sure the action is complete after the migration is done.","commit_id":"8b28c85adae88e42142697a2917386d736ade580"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"d7e54d10714d7f7c05e537ca27c9b369330ec8fa","unresolved":false,"context_lines":[{"line_number":1284,"context_line":"        else:"},{"line_number":1285,"context_line":"            try:"},{"line_number":1286,"context_line":"                self.terminate_connection(ctxt, volume[\u0027id\u0027], connector,"},{"line_number":1287,"context_line":"                                          force\u003dforce)"},{"line_number":1288,"context_line":"            except Exception as err:"},{"line_number":1289,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1290,"context_line":"                    err_msg \u003d (_(\u0027Unable to terminate volume connection: \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_b37e0e17","line":1287,"in_reply_to":"ba3cc151_b8e90de8","updated":"2015-07-09 18:26:47.000000000","message":"I think we need to call remove_export() after terminate_connection() for local path because original _detach_volume() do same thing.\n\nMy comment for patch v4, since you included remove_export() inside terminate_connection() I indicated that broke live migration code of Nova(_post_live_migration()).\n\nBut I think call terminate_connection and remove_export for both remote and local path separately here, it\u0027s doesn\u0027t cause problem.\n\nBy the way, I\u0027m proposing similar fix but I\u0027m using rpc.attach_volume, rpc.detach_volume instead of adding remove_export for RPC call.\n\nWhich way is better??\nhttps://review.openstack.org/#/c/194223/","commit_id":"8b28c85adae88e42142697a2917386d736ade580"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"2937eae43bc197e232a8ecd54b9358b3ad2084b1","unresolved":false,"context_lines":[{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        copy_error \u003d True"},{"line_number":1311,"context_line":"        try:"},{"line_number":1312,"context_line":"            size_in_mb \u003d int(src_vol[\u0027size\u0027]) * 1024    # vol size is in GB"},{"line_number":1313,"context_line":"            vol_utils.copy_volume(src_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1314,"context_line":"                                  dest_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1315,"context_line":"                                  size_in_mb,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_7a2b8f82","line":1312,"updated":"2015-07-09 08:17:20.000000000","message":"replace 1024 with constant in oslo_utils.unit","commit_id":"8b28c85adae88e42142697a2917386d736ade580"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"f5943514f38549b78591a845d0de23b1f0a4f8a9","unresolved":false,"context_lines":[{"line_number":1309,"context_line":""},{"line_number":1310,"context_line":"        copy_error \u003d True"},{"line_number":1311,"context_line":"        try:"},{"line_number":1312,"context_line":"            size_in_mb \u003d int(src_vol[\u0027size\u0027]) * 1024    # vol size is in GB"},{"line_number":1313,"context_line":"            vol_utils.copy_volume(src_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1314,"context_line":"                                  dest_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1315,"context_line":"                                  size_in_mb,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_f8e475e0","line":1312,"in_reply_to":"ba3cc151_7a2b8f82","updated":"2015-07-09 14:17:41.000000000","message":"Will do.","commit_id":"8b28c85adae88e42142697a2917386d736ade580"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"e1d23d370d9eec6a019fc4fdb390917ab83f083d","unresolved":false,"context_lines":[{"line_number":1257,"context_line":""},{"line_number":1258,"context_line":"        return {\u0027conn\u0027: conn, \u0027device\u0027: vol_handle, \u0027connector\u0027: connector}"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"    def _attach_volume(self, ctxt, volume, properties, remote\u003dFalse):"},{"line_number":1261,"context_line":""},{"line_number":1262,"context_line":"        status \u003d volume[\u0027status\u0027]"},{"line_number":1263,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_f5a5d346","line":1260,"updated":"2015-07-27 12:15:40.000000000","message":"Xing, that was a legacy of poorly designed, tightly-coupled-to-volume-service backup service.  We can\u0027t simply remove _attach_volume(), _detach_volume() from driver.py due to the exact reason you mentioned. Actually Jon\u0027s change doesn\u0027t remove it from driver.py, like I mentioned before, we will remove it when other parts get fixed.  But having these function as part of volume manager is one baby step towards a cleaner interface.","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"5a3d690082a06a456c0ce4e888b88a173a54bae9","unresolved":false,"context_lines":[{"line_number":1257,"context_line":""},{"line_number":1258,"context_line":"        return {\u0027conn\u0027: conn, \u0027device\u0027: vol_handle, \u0027connector\u0027: connector}"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"    def _attach_volume(self, ctxt, volume, properties, remote\u003dFalse):"},{"line_number":1261,"context_line":""},{"line_number":1262,"context_line":"        status \u003d volume[\u0027status\u0027]"},{"line_number":1263,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_615cf6ed","line":1260,"updated":"2015-07-17 04:31:00.000000000","message":"_attach_volume and _detach_volume and copy_volume_data are currently implemented in driver.py. Now they are moved to manager.py in this patch. Is the plan to remove those functions in driver.py in the future?\n\n_attach_volume and _detach_volume are called by backup_volume in volume driver.py.  backup_volume is called by create_backup in the backup manager.  If those methods are only in volume manager in the future, how does the volume driver call them?","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"e7ed29aef1654af61707c6ab8f367fc70cb490b7","unresolved":false,"context_lines":[{"line_number":1257,"context_line":""},{"line_number":1258,"context_line":"        return {\u0027conn\u0027: conn, \u0027device\u0027: vol_handle, \u0027connector\u0027: connector}"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"    def _attach_volume(self, ctxt, volume, properties, remote\u003dFalse):"},{"line_number":1261,"context_line":""},{"line_number":1262,"context_line":"        status \u003d volume[\u0027status\u0027]"},{"line_number":1263,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_55190e87","line":1260,"in_reply_to":"3a50d1a3_f5a5d346","updated":"2015-07-27 20:10:13.000000000","message":"Hi Winston,\n\nI was asking what is the future plan for the code in driver and how to handle backup if the code in driver is removed.  I didn\u0027t say it has to be moved back to the driver. I was brought to attention to this patch by some comments in a different patch.  Just want to make sure there is a plan there. So you have answered my question. I also saw another patch that is moving this code to backup manager to decouple the backup manager from the driver after I reviewed this one. I\u0027m okay with the approach. Thanks for the explanation.","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2705f01b94dabde0e625814c660e47b74e238592","unresolved":false,"context_lines":[{"line_number":1257,"context_line":""},{"line_number":1258,"context_line":"        return {\u0027conn\u0027: conn, \u0027device\u0027: vol_handle, \u0027connector\u0027: connector}"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"    def _attach_volume(self, ctxt, volume, properties, remote\u003dFalse):"},{"line_number":1261,"context_line":""},{"line_number":1262,"context_line":"        status \u003d volume[\u0027status\u0027]"},{"line_number":1263,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a50d1a3_23d35a3a","line":1260,"in_reply_to":"9a41bdd9_615cf6ed","updated":"2015-07-20 08:34:27.000000000","message":"I prefer to keep these methods in driver and use the ones in driver.\nSince it has been long time that there is no response from Jon, I help him to change this patch.\nIf there is future concern, please comment on the new patch.","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"c5fd9d17b278a6d6b52595ceaf468bac160b1deb","unresolved":false,"context_lines":[{"line_number":1300,"context_line":"            except Exception as err:"},{"line_number":1301,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1302,"context_line":"                    err_msg \u003d (_(\u0027Unable to terminate volume connection: \u0027"},{"line_number":1303,"context_line":"                                 \u0027%(err)s\u0027) % {\u0027err\u0027: err})"},{"line_number":1304,"context_line":"                    LOG.error(err_msg)"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"    def _copy_volume_data(self, ctxt, src_vol, dest_vol, remote\u003dNone):"}],"source_content_type":"text/x-python","patch_set":14,"id":"1a4dcd0f_9c95ef89","line":1303,"updated":"2015-08-08 09:27:50.000000000","message":"Missing . after \"%(err)s\".","commit_id":"309cd3c1254b698f23e902af0a0dbdfaf757d4e0"},{"author":{"_account_id":170,"name":"Mike Perez","email":"thingee@gmail.com","username":"thingee"},"change_message_id":"39ac3c4f35ac5f6fc6533d00db6fe9b1f2fa098f","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":1190,"context_line":"        try:"},{"line_number":1191,"context_line":"            self.driver.remove_export(context, volume_ref)"},{"line_number":1192,"context_line":"        except (exception.ISCSITargetRemoveFailed,"},{"line_number":1193,"context_line":"                exception.ISCSITargetHelperCommandFailed):"},{"line_number":1194,"context_line":"            msg \u003d _(\"Remove volume export failed.\")"},{"line_number":1195,"context_line":"            LOG.exception(msg, resource\u003dvolume_ref)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a4dcd0f_aff74a58","line":1192,"updated":"2015-08-14 04:29:39.000000000","message":"Why are these ISCSI exceptions leaking into the manager which is agnostic of these things? Catch exception.TargetRemovedFailed which ISCSITargetRemoveFailed could inherit from, but we should not have this fabric specific information here.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"445b6d5dc491e8b7f8007ed2e87ddcc58ddebf8b","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":1190,"context_line":"        try:"},{"line_number":1191,"context_line":"            self.driver.remove_export(context, volume_ref)"},{"line_number":1192,"context_line":"        except (exception.ISCSITargetRemoveFailed,"},{"line_number":1193,"context_line":"                exception.ISCSITargetHelperCommandFailed):"},{"line_number":1194,"context_line":"            msg \u003d _(\"Remove volume export failed.\")"},{"line_number":1195,"context_line":"            LOG.exception(msg, resource\u003dvolume_ref)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa1b9901_4bad5a87","line":1192,"in_reply_to":"1a4dcd0f_aff74a58","updated":"2015-08-17 15:17:17.000000000","message":"Yep, I missed that. Thanks.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c420a24d1966b672847a13d440fea6d5c0afbd22","unresolved":false,"context_lines":[{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"        src_remote \u003d remote in [\u0027src\u0027, \u0027both\u0027]"},{"line_number":1319,"context_line":"        src_attach_info \u003d self._attach_volume(ctxt, src_vol, properties,"},{"line_number":1320,"context_line":"                                              remote\u003dsrc_remote)"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"        copy_error \u003d True"},{"line_number":1323,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a4dcd0f_d9e6d243","line":1320,"updated":"2015-08-17 08:27:59.000000000","message":"If it succeeds in attaching the remote volume but fails when attaching the src volume, shall we detach the remote volume for this case?\nWith the current code, it will end up with the remote/dest volume still attached for the above case.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"445b6d5dc491e8b7f8007ed2e87ddcc58ddebf8b","unresolved":false,"context_lines":[{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"        src_remote \u003d remote in [\u0027src\u0027, \u0027both\u0027]"},{"line_number":1319,"context_line":"        src_attach_info \u003d self._attach_volume(ctxt, src_vol, properties,"},{"line_number":1320,"context_line":"                                              remote\u003dsrc_remote)"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"        copy_error \u003d True"},{"line_number":1323,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa1b9901_4bc47a3f","line":1320,"in_reply_to":"1a4dcd0f_d9e6d243","updated":"2015-08-17 15:17:17.000000000","message":"Good point, I\u0027ll fix that in the next update.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"c420a24d1966b672847a13d440fea6d5c0afbd22","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"        copy_error \u003d True"},{"line_number":1323,"context_line":"        try:"},{"line_number":1324,"context_line":"            size_in_mb \u003d int(src_vol[\u0027size\u0027]) * units.Ki    # vol size is in GB"},{"line_number":1325,"context_line":"            vol_utils.copy_volume(src_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1326,"context_line":"                                  dest_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1327,"context_line":"                                  size_in_mb,"},{"line_number":1328,"context_line":"                                  self.configuration.volume_dd_blocksize)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a4dcd0f_b90746c8","line":1325,"updated":"2015-08-17 08:27:59.000000000","message":"With the current code, whether it is the file I/O based copy or the host assisted copy is determined in copy_volume.\n\nThe host assisted copy uses \u0027dd\u0027 and depends on the volume is attached to the host, does file I/O based copy also depend on the volume attach?\n\nI assume not. If my assumption is correct, we can check if it is file I/O based copy here. If it is, skip the attach and detach code.\n\nIf my assumption is wrong. Disregard this comment.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"445b6d5dc491e8b7f8007ed2e87ddcc58ddebf8b","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"        copy_error \u003d True"},{"line_number":1323,"context_line":"        try:"},{"line_number":1324,"context_line":"            size_in_mb \u003d int(src_vol[\u0027size\u0027]) * units.Ki    # vol size is in GB"},{"line_number":1325,"context_line":"            vol_utils.copy_volume(src_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1326,"context_line":"                                  dest_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1327,"context_line":"                                  size_in_mb,"},{"line_number":1328,"context_line":"                                  self.configuration.volume_dd_blocksize)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa1b9901_cbe72aab","line":1325,"in_reply_to":"1a4dcd0f_b90746c8","updated":"2015-08-17 15:17:17.000000000","message":"file I/O only depends on volume attach for volumes that can be attached in order to open(2) them to obtain a file handle.\n\nI think the piece you\u0027re looking for is that \u0027_attach_volume\u0027 in the manager will call initalize_connection, which will call into os_brick and either attach and return a file path, or return a file handle for file I/O.  We then examine the two pieces in the copy code to determine how to proceed.  So in the manager, attach/detach is the abstraction used to \"give me a handle to volume\".  And then we use the nature of those handles to determine which copy method to use later on.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"10bd1a75ba50922f24710897874e46ccb0a6b562","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"        copy_error \u003d True"},{"line_number":1323,"context_line":"        try:"},{"line_number":1324,"context_line":"            size_in_mb \u003d int(src_vol[\u0027size\u0027]) * units.Ki    # vol size is in GB"},{"line_number":1325,"context_line":"            vol_utils.copy_volume(src_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1326,"context_line":"                                  dest_attach_info[\u0027device\u0027][\u0027path\u0027],"},{"line_number":1327,"context_line":"                                  size_in_mb,"},{"line_number":1328,"context_line":"                                  self.configuration.volume_dd_blocksize)"}],"source_content_type":"text/x-python","patch_set":15,"id":"fa1b9901_cb815191","line":1325,"in_reply_to":"fa1b9901_cbe72aab","updated":"2015-08-18 02:18:14.000000000","message":"Sweet. Thank you for the explanations.","commit_id":"d7d21c6a0f02f5f72c381c4dd343fa8dfbab8e46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":1197,"context_line":"        try:"},{"line_number":1198,"context_line":"            self.driver.remove_export(context, volume_ref)"},{"line_number":1199,"context_line":"        except exception.CinderException:"},{"line_number":1200,"context_line":"            msg \u003d _(\"Remove volume export failed.\")"},{"line_number":1201,"context_line":"            LOG.exception(msg, resource\u003dvolume_ref)"},{"line_number":1202,"context_line":"            raise exception.VolumeBackendAPIException(data\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_1bac2dec","line":1199,"updated":"2015-08-19 13:00:02.000000000","message":"Why CinderException instead of Exception?","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":1254,"context_line":"        root_access \u003d True"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":"        if not connector.check_valid_device(vol_handle[\u0027path\u0027], root_access):"},{"line_number":1257,"context_line":"            if vol_handle.get(\u0027path\u0027):"},{"line_number":1258,"context_line":"                raise exception.DeviceUnavailable("},{"line_number":1259,"context_line":"                    path\u003dvol_handle[\u0027path\u0027],"},{"line_number":1260,"context_line":"                    reason\u003d(_(\"Unable to access the backend storage via the \""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_466f28c5","line":1257,"updated":"2015-08-19 13:00:02.000000000","message":"Why use dict access above, .get here and 2 lines below dict access again?\n\nAnd should this be\n\n if isinstance(vol_handle[\u0027path\u0027], six.string_types):","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":1296,"context_line":""},{"line_number":1297,"context_line":"        if remote:"},{"line_number":1298,"context_line":"            rpcapi \u003d volume_rpcapi.VolumeAPI()"},{"line_number":1299,"context_line":"            rpcapi.terminate_connection(ctxt, volume, properties, force\u003dforce)"},{"line_number":1300,"context_line":"            rpcapi.remove_export(ctxt, volume)"},{"line_number":1301,"context_line":"        else:"},{"line_number":1302,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_067e1019","line":1299,"range":{"start_line":1299,"start_character":54,"end_line":1299,"end_character":64},"updated":"2015-08-19 13:00:02.000000000","message":"s/properties/connector","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":1312,"context_line":"    def _copy_volume_data(self, ctxt, src_vol, dest_vol, remote\u003dNone):"},{"line_number":1313,"context_line":"        \"\"\"Copy data from src_vol to dest_vol.\"\"\""},{"line_number":1314,"context_line":""},{"line_number":1315,"context_line":"        LOG.debug((\u0027copy_data_between_volumes %(src)s -\u003e %(dest)s.\u0027),"},{"line_number":1316,"context_line":"                  {\u0027src\u0027: src_vol[\u0027name\u0027], \u0027dest\u0027: dest_vol[\u0027name\u0027]})"},{"line_number":1317,"context_line":""},{"line_number":1318,"context_line":"        properties \u003d utils.brick_get_connector_properties()"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_fb21394e","line":1315,"range":{"start_line":1315,"start_character":17,"end_line":1315,"end_character":20},"updated":"2015-08-19 13:00:02.000000000","message":"Why this extra parenthesis surrounding the string?","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":1327,"context_line":"                                                  remote\u003dsrc_remote)"},{"line_number":1328,"context_line":"        except Exception:"},{"line_number":1329,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1330,"context_line":"                LOG.error(_LE(\"Failed to access source volume for copy\"))"},{"line_number":1331,"context_line":"                self._detach_volume(ctxt, dest_attach_info, dest_vol,"},{"line_number":1332,"context_line":"                                    properties, remote\u003ddest_remote)"},{"line_number":1333,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_9b935d4f","line":1330,"range":{"start_line":1330,"start_character":41,"end_line":1330,"end_character":47},"updated":"2015-08-19 13:00:02.000000000","message":"s/access/attach","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":1341,"context_line":"            copy_error \u003d False"},{"line_number":1342,"context_line":"        except Exception:"},{"line_number":1343,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1344,"context_line":"                msg \u003d _(\"Failed to copy volume %(src)s to %(dest)s.\")"},{"line_number":1345,"context_line":"                LOG.error(msg, {\u0027src\u0027: src_vol[\u0027id\u0027], \u0027dest\u0027: dest_vol[\u0027id\u0027]})"},{"line_number":1346,"context_line":"        finally:"},{"line_number":1347,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_2648f42c","line":1344,"updated":"2015-08-19 13:00:02.000000000","message":"s/_/_LE","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"b20e07b70d056299a86c8ba380575af016deeeeb","unresolved":false,"context_lines":[{"line_number":1241,"context_line":"        return model_update"},{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"    def _connect_device(self, conn):"},{"line_number":1244,"context_line":""},{"line_number":1245,"context_line":"        use_multipath \u003d self.configuration.use_multipath_for_image_xfer"},{"line_number":1246,"context_line":"        device_scan_attempts \u003d self.configuration.num_volume_device_scan_tries"},{"line_number":1247,"context_line":"        protocol \u003d conn[\u0027driver_volume_type\u0027]"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_ecc30889","line":1244,"updated":"2015-08-24 01:02:37.000000000","message":"Extra blank line.","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"b20e07b70d056299a86c8ba380575af016deeeeb","unresolved":false,"context_lines":[{"line_number":1270,"context_line":"        return {\u0027conn\u0027: conn, \u0027device\u0027: vol_handle, \u0027connector\u0027: connector}"},{"line_number":1271,"context_line":""},{"line_number":1272,"context_line":"    def _attach_volume(self, ctxt, volume, properties, remote\u003dFalse):"},{"line_number":1273,"context_line":""},{"line_number":1274,"context_line":"        status \u003d volume[\u0027status\u0027]"},{"line_number":1275,"context_line":""},{"line_number":1276,"context_line":"        if remote:"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_0cc19c80","line":1273,"updated":"2015-08-24 01:02:37.000000000","message":"Extra blank line.","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"b20e07b70d056299a86c8ba380575af016deeeeb","unresolved":false,"context_lines":[{"line_number":1290,"context_line":""},{"line_number":1291,"context_line":"    def _detach_volume(self, ctxt, attach_info, volume, properties,"},{"line_number":1292,"context_line":"                       force\u003dFalse, remote\u003dFalse):"},{"line_number":1293,"context_line":""},{"line_number":1294,"context_line":"        connector \u003d attach_info[\u0027connector\u0027]"},{"line_number":1295,"context_line":"        connector.disconnect_volume(attach_info[\u0027conn\u0027][\u0027data\u0027],"},{"line_number":1296,"context_line":"                                    attach_info[\u0027device\u0027])"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_acbd1004","line":1293,"updated":"2015-08-24 01:02:37.000000000","message":"Extra blank line.","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"b20e07b70d056299a86c8ba380575af016deeeeb","unresolved":false,"context_lines":[{"line_number":1328,"context_line":"                                                  remote\u003dsrc_remote)"},{"line_number":1329,"context_line":"        except Exception:"},{"line_number":1330,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1331,"context_line":"                LOG.error(_LE(\"Failed to attach source volume for copy\"))"},{"line_number":1332,"context_line":"                self._detach_volume(ctxt, dest_attach_info, dest_vol,"},{"line_number":1333,"context_line":"                                    properties, remote\u003ddest_remote)"},{"line_number":1334,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_2c57c0cb","line":1331,"updated":"2015-08-24 01:02:37.000000000","message":"Missing . after \"copy\".","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"c6e97cadad35318b73b49999e8310f8ba4509f9b","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            except Exception:"},{"line_number":1418,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1419,"context_line":"                    msg \u003d _(\"Failed to attach volume %(vol)s.\")"},{"line_number":1420,"context_line":"                    LOG.error(msg, {\u0027vol\u0027: volume[\u0027id\u0027]})"},{"line_number":1421,"context_line":"                    self.db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":1422,"context_line":"                                          {\u0027status\u0027: status})"},{"line_number":1423,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":21,"id":"da20952f_af5abf81","line":1420,"updated":"2015-08-27 21:43:24.000000000","message":"This really should be collapsed down into just LOG.error(_LE())","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"c6e97cadad35318b73b49999e8310f8ba4509f9b","unresolved":false,"context_lines":[{"line_number":1444,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1445,"context_line":"                    err_msg \u003d (_(\u0027Unable to terminate volume connection: \u0027"},{"line_number":1446,"context_line":"                                 \u0027%(err)s.\u0027) % {\u0027err\u0027: err})"},{"line_number":1447,"context_line":"                    LOG.error(err_msg)"},{"line_number":1448,"context_line":""},{"line_number":1449,"context_line":"    def _copy_volume_data(self, ctxt, src_vol, dest_vol, remote\u003dNone):"},{"line_number":1450,"context_line":"        \"\"\"Copy data from src_vol to dest_vol.\"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"da20952f_0fcbab1f","line":1447,"updated":"2015-08-27 21:43:24.000000000","message":"Same here, just put everything into LOG.error please.","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"}],"cinder/volume/rpcapi.py":[{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"9583b535cd63d283dc224bf30f9a0668f2840f0d","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027terminate_connection\u0027, volume_id\u003dvolume[\u0027id\u0027],"},{"line_number":199,"context_line":"                          connector\u003dconnector, force\u003dforce)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def remove_export(self, ctxt, volume):"},{"line_number":202,"context_line":"        new_host \u003d utils.extract_host(volume[\u0027host\u0027])"},{"line_number":203,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003d\u00271.24\u0027)"},{"line_number":204,"context_line":"        return cctxt.call(ctxt, \u0027remove_export\u0027, volume_id\u003dvolume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_ca2ed78b","line":201,"updated":"2015-07-08 03:15:35.000000000","message":"I am ok with adding this method, but I am wondering if it should be put in another patch, because I have not found anywhere this patch is using this method. If I makes a mistake, please help me with the explanation.\n\nBTW, why do we use cctx.call here instead of cctx.cast?","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"1c8ab6223daab9a3d23434bfd26353388a306db5","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027terminate_connection\u0027, volume_id\u003dvolume[\u0027id\u0027],"},{"line_number":199,"context_line":"                          connector\u003dconnector, force\u003dforce)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def remove_export(self, ctxt, volume):"},{"line_number":202,"context_line":"        new_host \u003d utils.extract_host(volume[\u0027host\u0027])"},{"line_number":203,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003d\u00271.24\u0027)"},{"line_number":204,"context_line":"        return cctxt.call(ctxt, \u0027remove_export\u0027, volume_id\u003dvolume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_4f45dd40","line":201,"in_reply_to":"ba3cc151_ca2ed78b","updated":"2015-07-08 14:15:23.000000000","message":"Hmm, yeah, cast is more appropriate. Nice catch.","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"6acf47f87154082d088ebba07390cd1edce49c1f","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027terminate_connection\u0027, volume_id\u003dvolume[\u0027id\u0027],"},{"line_number":199,"context_line":"                          connector\u003dconnector, force\u003dforce)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def remove_export(self, ctxt, volume):"},{"line_number":202,"context_line":"        new_host \u003d utils.extract_host(volume[\u0027host\u0027])"},{"line_number":203,"context_line":"        cctxt \u003d self.client.prepare(server\u003dnew_host, version\u003d\u00271.24\u0027)"},{"line_number":204,"context_line":"        return cctxt.call(ctxt, \u0027remove_export\u0027, volume_id\u003dvolume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_e53bcccf","line":201,"in_reply_to":"ba3cc151_ca2ed78b","updated":"2015-07-08 03:52:44.000000000","message":"It seems this RPC API is called at _detach_volume() line 1283 in manager.py.","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"}],"cinder/volume/utils.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"4222dda039618ed0ab5dc9d83c60bd35935aee3c","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                           sync\u003dFalse, execute\u003dutils.execute, ionice\u003dNone,"},{"line_number":398,"context_line":"                           sparse\u003dFalse):"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    src_handle \u003d srcstr"},{"line_number":401,"context_line":"    if isinstance(srcstr, six.string_types):"},{"line_number":402,"context_line":"        src_handle \u003d _open_volume_with_path(srcstr, \u0027r\u0027)"},{"line_number":403,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_1248cacb","line":400,"updated":"2015-06-26 18:17:27.000000000","message":"src_handle \u003d None?","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3502f523f22f9c5ef02d7c288e2d2e09fd081deb","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                           sync\u003dFalse, execute\u003dutils.execute, ionice\u003dNone,"},{"line_number":398,"context_line":"                           sparse\u003dFalse):"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    src_handle \u003d srcstr"},{"line_number":401,"context_line":"    if isinstance(srcstr, six.string_types):"},{"line_number":402,"context_line":"        src_handle \u003d _open_volume_with_path(srcstr, \u0027r\u0027)"},{"line_number":403,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_fafdcd3a","line":400,"in_reply_to":"fa32b979_1248cacb","updated":"2015-06-30 20:42:35.000000000","message":"Only if you want it to fail ;)  For this comment and the one below, if you follow the code path from the connector, we reach this routine iff one or both of the volumes cannot be attached locally.  So srcstr will either be of type string if it\u0027s a path to a block device, or it will be a connector object that behaves like a file handle.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"600cb60d72448e19e0625b5e67c00a018436cf47","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        raise exception.DeviceUnavailable(\"Either source or destination are \""},{"line_number":410,"context_line":"                                          \"not available\")"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    _transfer_data(src_handle, dest_handle, size_in_m * units.Mi, units.Mi * 4)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"def copy_volume(srcstr, deststr, size_in_m, blocksize, sync\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_b3bd2bd9","line":412,"updated":"2015-06-23 02:32:29.000000000","message":"I think we should put this time consuming/resource eating job into green thread, or a python thread, ideally another process.  We can do similar things like 9f83602a82153e0be8c127c3d936b512327336f7.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"5cdc84e1a59ab528fa6dc982855f6893580b5938","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        raise exception.DeviceUnavailable(\"Either source or destination are \""},{"line_number":410,"context_line":"                                          \"not available\")"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    _transfer_data(src_handle, dest_handle, size_in_m * units.Mi, units.Mi * 4)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"def copy_volume(srcstr, deststr, size_in_m, blocksize, sync\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_d2e95ee7","line":412,"in_reply_to":"fa32b979_57e239a1","updated":"2015-06-24 23:59:08.000000000","message":"Yeah, they are different, but Jon has found this change causing periodic job failing to finish within designated interval, which in turn causes c-vol service failing to update its status to scheduler.\n\nMaybe it\u0027s more fair to compare _copy_volume_with_path() with _copy_volume_with_file().  The former does the copy job in a separate process, but the latter is stuck with the rest c-vol greenthreads.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"303636a5360017d4fd621249165df6cfd7384304","unresolved":false,"context_lines":[{"line_number":409,"context_line":"        raise exception.DeviceUnavailable(\"Either source or destination are \""},{"line_number":410,"context_line":"                                          \"not available\")"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    _transfer_data(src_handle, dest_handle, size_in_m * units.Mi, units.Mi * 4)"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"def copy_volume(srcstr, deststr, size_in_m, blocksize, sync\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_57e239a1","line":412,"in_reply_to":"fa32b979_b3bd2bd9","updated":"2015-06-24 14:17:37.000000000","message":"IIUC, this is a little different since it will allow other threads to run after each 4MB is moved, rather than blocking them for the duration of moving the entire volume\u0027s data which is what happened in the patch you reference.\n\nSo I\u0027m not sure this is really a problem here.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"4222dda039618ed0ab5dc9d83c60bd35935aee3c","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        throttle \u003d throttling.Throttle.get_default()"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    if (isinstance(srcstr, six.string_types) and"},{"line_number":422,"context_line":"            isinstance(deststr, six.string_types)):"},{"line_number":423,"context_line":"        with throttle.subcommand(srcstr, deststr) as throttle_cmd:"},{"line_number":424,"context_line":"            _copy_volume_with_path(throttle_cmd[\u0027prefix\u0027], srcstr, deststr,"},{"line_number":425,"context_line":"                                   size_in_m, blocksize, sync\u003dsync,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_b20c7e3c","line":422,"updated":"2015-06-26 18:17:27.000000000","message":"What are you trying to check here? src and dest are valid string types? Why you different copy methods (path or file) based on this? How does defines whether the volume is iSCSI base or not?","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3502f523f22f9c5ef02d7c288e2d2e09fd081deb","unresolved":false,"context_lines":[{"line_number":419,"context_line":"        throttle \u003d throttling.Throttle.get_default()"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    if (isinstance(srcstr, six.string_types) and"},{"line_number":422,"context_line":"            isinstance(deststr, six.string_types)):"},{"line_number":423,"context_line":"        with throttle.subcommand(srcstr, deststr) as throttle_cmd:"},{"line_number":424,"context_line":"            _copy_volume_with_path(throttle_cmd[\u0027prefix\u0027], srcstr, deststr,"},{"line_number":425,"context_line":"                                   size_in_m, blocksize, sync\u003dsync,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_bd83bf55","line":422,"in_reply_to":"fa32b979_b20c7e3c","updated":"2015-06-30 20:42:35.000000000","message":"This is the entire point of the changeset, some drivers (RBD) cannot be attached locally, and so there is no path to a block device upon which to call open().  Access to an RBD volume is made available through a file handle wrapper that proxies read() and write() calls to the ceph cluster.\n\nI think you might be saying \"it\u0027s unclear from this context why you\u0027re calling isinstance() and so a properly named function or comment would go a long way to help the reader\".  I can certainly make that change if think it\u0027s needed.","commit_id":"dfe13f7d9f279dfad6f077faafb63f9697b39ec8"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"78e2921421b0836c873b0500d9a795d270631d73","unresolved":false,"context_lines":[{"line_number":374,"context_line":"            return"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":377,"context_line":"        tpool.execute(dest.flush)"},{"line_number":378,"context_line":"        delta \u003d (time.time() - before)"},{"line_number":379,"context_line":"        rate \u003d (chunk_size / delta) / 1024"},{"line_number":380,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s)\" %"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_096f032c","line":377,"updated":"2015-07-01 05:49:34.000000000","message":"why flush after every write instead of once at the end?","commit_id":"911d3bd546a496dd6e512971938f41e3582165cf"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"78e2921421b0836c873b0500d9a795d270631d73","unresolved":false,"context_lines":[{"line_number":391,"context_line":"            tpool.execute(dest.write, data)"},{"line_number":392,"context_line":"            tpool.execute(dest.flush)"},{"line_number":393,"context_line":"            # yield to any other pending backups"},{"line_number":394,"context_line":"            eventlet.sleep(0)"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"def _copy_volume_with_file(prefix, srcstr, deststr, size_in_m, blocksize,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_c9f35b9d","line":394,"updated":"2015-07-01 05:49:34.000000000","message":"no need for this final sleep outside of the loop","commit_id":"911d3bd546a496dd6e512971938f41e3582165cf"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"84216abea4da2b062ef84d6eb501ea3644a39d86","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            return handle"},{"line_number":355,"context_line":"    except Exception:"},{"line_number":356,"context_line":"        LOG.error(_LE(\"Failed to open volume from %(path)s.\") %"},{"line_number":357,"context_line":"                  {\u0027path\u0027: path})"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"def _transfer_data(src, dest, length, chunk_size):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_de779600","line":357,"updated":"2015-07-07 15:15:28.000000000","message":"Please change % \u003d\u003e ,","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"84216abea4da2b062ef84d6eb501ea3644a39d86","unresolved":false,"context_lines":[{"line_number":363,"context_line":"    chunks \u003d int(length / chunk_size)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    LOG.debug(\"%(chunks)s chunks of %(bytes)s bytes to be transferred\" %"},{"line_number":366,"context_line":"              {\u0027chunks\u0027: chunks, \u0027bytes\u0027: chunk_size})"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    for chunk in xrange(0, chunks):"},{"line_number":369,"context_line":"        before \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_fe1b7ac8","line":366,"updated":"2015-07-07 15:15:28.000000000","message":"Please change % \u003d\u003e ,","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"3b1a2b65fda59c1ea0b760440b909eaaafbc9ffb","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    for chunk in xrange(0, chunks):"},{"line_number":369,"context_line":"        before \u003d time.time()"},{"line_number":370,"context_line":"        data \u003d tpool.execute(src.read, chunk_size)"},{"line_number":371,"context_line":"        # If we have reach end of source, discard any extraneous bytes from"},{"line_number":372,"context_line":"        # destination volume if trim is enabled and stop writing."},{"line_number":373,"context_line":"        if data \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_9b884024","line":370,"updated":"2015-07-07 14:55:38.000000000","message":"Why do we need it here? Will greenlet work in this case? The same comment for the code below","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"06b2c0b6eabdd01d8661f130395514aa11b2b6e0","unresolved":false,"context_lines":[{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    for chunk in xrange(0, chunks):"},{"line_number":369,"context_line":"        before \u003d time.time()"},{"line_number":370,"context_line":"        data \u003d tpool.execute(src.read, chunk_size)"},{"line_number":371,"context_line":"        # If we have reach end of source, discard any extraneous bytes from"},{"line_number":372,"context_line":"        # destination volume if trim is enabled and stop writing."},{"line_number":373,"context_line":"        if data \u003d\u003d \u0027\u0027:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_7bb68ddd","line":370,"in_reply_to":"ba3cc151_9b884024","updated":"2015-07-07 19:55:45.000000000","message":"I found greenlet not to work,  without tpool wrapper, these calls block the periodic status update tasks from completing within their time limit, and you see many loopingcall warnings.","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"3b1a2b65fda59c1ea0b760440b909eaaafbc9ffb","unresolved":false,"context_lines":[{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":377,"context_line":"        delta \u003d (time.time() - before)"},{"line_number":378,"context_line":"        rate \u003d (chunk_size / delta) / 1024"},{"line_number":379,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s)\" %"},{"line_number":380,"context_line":"                  {\u0027chunk\u0027: chunk + 1, \u0027chunks\u0027: chunks, \u0027rate\u0027: rate})"},{"line_number":381,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_3bc6d449","line":378,"updated":"2015-07-07 14:55:38.000000000","message":"Please, use oslo_utils.units here","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":14305,"name":"Yuriy Nesenenko","email":"ynesenenko@mirantis.com","username":"yuriy_n"},"change_message_id":"84216abea4da2b062ef84d6eb501ea3644a39d86","unresolved":false,"context_lines":[{"line_number":377,"context_line":"        delta \u003d (time.time() - before)"},{"line_number":378,"context_line":"        rate \u003d (chunk_size / delta) / 1024"},{"line_number":379,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s)\" %"},{"line_number":380,"context_line":"                  {\u0027chunk\u0027: chunk + 1, \u0027chunks\u0027: chunks, \u0027rate\u0027: rate})"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # yield to any other pending backups"},{"line_number":383,"context_line":"        eventlet.sleep(0)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_de49d6d1","line":380,"updated":"2015-07-07 15:15:28.000000000","message":"Please change % \u003d\u003e ,","commit_id":"864e6e4f616795cad2d6d6d9fd6bad21972a0146"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"9583b535cd63d283dc224bf30f9a0668f2840f0d","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from six.moves import range"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"import eventlet"},{"line_number":31,"context_line":"from eventlet import tpool"},{"line_number":32,"context_line":"import six"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from cinder.brick.local_dev import lvm as brick_lvm"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_bff6335b","line":31,"updated":"2015-07-08 03:15:35.000000000","message":"Line 30 and 31 should be put between Line 21 and 22.","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"9583b535cd63d283dc224bf30f9a0668f2840f0d","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"import eventlet"},{"line_number":31,"context_line":"from eventlet import tpool"},{"line_number":32,"context_line":"import six"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from cinder.brick.local_dev import lvm as brick_lvm"},{"line_number":35,"context_line":"from cinder import db"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_9ff9ef50","line":32,"updated":"2015-07-08 03:15:35.000000000","message":"Line 32 should be put above Line 28.","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"9583b535cd63d283dc224bf30f9a0668f2840f0d","unresolved":false,"context_lines":[{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    chunks \u003d int(length / chunk_size)"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    LOG.debug(\"%(chunks)s chunks of %(bytes)s bytes to be transferred\","},{"line_number":365,"context_line":"              {\u0027chunks\u0027: chunks, \u0027bytes\u0027: chunk_size})"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"    for chunk in xrange(0, chunks):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_7fc2ebea","line":364,"updated":"2015-07-08 03:15:35.000000000","message":"\u0027.\u0027 at EOL","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"5f5a241a508cf031e20cca103c8aeaea60c1eb20","unresolved":false,"context_lines":[{"line_number":375,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":376,"context_line":"        delta \u003d (time.time() - before)"},{"line_number":377,"context_line":"        rate \u003d (chunk_size / delta) / units.Ki"},{"line_number":378,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s)\","},{"line_number":379,"context_line":"                  {\u0027chunk\u0027: chunk + 1, \u0027chunks\u0027: chunks, \u0027rate\u0027: rate})"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        # yield to any other pending backups"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_8650d50b","line":378,"updated":"2015-07-07 21:14:45.000000000","message":"\u0027.\u0027 at EOL","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"5f5a241a508cf031e20cca103c8aeaea60c1eb20","unresolved":false,"context_lines":[{"line_number":383,"context_line":""},{"line_number":384,"context_line":"    rem \u003d int(length % chunk_size)"},{"line_number":385,"context_line":"    if rem:"},{"line_number":386,"context_line":"        LOG.debug(\"Transferring remaining %s bytes\", rem)"},{"line_number":387,"context_line":"        data \u003d tpool.execute(src.read, rem)"},{"line_number":388,"context_line":"        if data !\u003d \u0027\u0027:"},{"line_number":389,"context_line":"            tpool.execute(dest.write, data)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_063da5ba","line":386,"updated":"2015-07-07 21:14:45.000000000","message":"\u0027.\u0027 at EOL","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"5f5a241a508cf031e20cca103c8aeaea60c1eb20","unresolved":false,"context_lines":[{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    if not src_handle or not dest_handle:"},{"line_number":407,"context_line":"        raise exception.DeviceUnavailable(\"Either source or destination are \""},{"line_number":408,"context_line":"                                          \"not available\")"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    start_time \u003d timeutils.utcnow()"},{"line_number":411,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_c646dd4e","line":408,"updated":"2015-07-07 21:14:45.000000000","message":"\u0027.\u0027 at EOL","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"9583b535cd63d283dc224bf30f9a0668f2840f0d","unresolved":false,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"    duration \u003d max(1, timeutils.delta_seconds(start_time, timeutils.utcnow()))"},{"line_number":415,"context_line":"    mbps \u003d (size_in_m / duration)"},{"line_number":416,"context_line":"    LOG.info(_LI(\"Volume copy %(size_in_m).2f MB at %(mbps).2f MB/s\"),"},{"line_number":417,"context_line":"             {\u0027size_in_m\u0027: size_in_m, \u0027mbps\u0027: mbps})"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_9fa7af4f","line":416,"updated":"2015-07-08 03:15:35.000000000","message":"\u0027.\u0027 at EOL","commit_id":"f743fb725091b821aa7e7c49b289c006baef3b5a"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"3f56bf399a50e4d102055975a9e805ca866f3a4a","unresolved":false,"context_lines":[{"line_number":410,"context_line":""},{"line_number":411,"context_line":"    _transfer_data(src_handle, dest_handle, size_in_m * units.Mi, units.Mi * 4)"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"    duration \u003d max(1, timeutils.delta_seconds(start_time, timeutils.utcnow()))"},{"line_number":414,"context_line":"    mbps \u003d (size_in_m / duration)"},{"line_number":415,"context_line":"    LOG.info(_LI(\"Volume copy %(size_in_m).2f MB at %(mbps).2f MB/s.\"),"},{"line_number":416,"context_line":"             {\u0027size_in_m\u0027: size_in_m, \u0027mbps\u0027: mbps})"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_fd65fee0","line":413,"updated":"2015-07-13 03:55:42.000000000","message":"I think we need to close the handles for both src_handle and dest_handle to free up the system resources after the transfer is done.","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"a167a2f21f635e0f608bf8f21f0b45473b7c15fd","unresolved":false,"context_lines":[{"line_number":410,"context_line":""},{"line_number":411,"context_line":"    _transfer_data(src_handle, dest_handle, size_in_m * units.Mi, units.Mi * 4)"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"    duration \u003d max(1, timeutils.delta_seconds(start_time, timeutils.utcnow()))"},{"line_number":414,"context_line":"    mbps \u003d (size_in_m / duration)"},{"line_number":415,"context_line":"    LOG.info(_LI(\"Volume copy %(size_in_m).2f MB at %(mbps).2f MB/s.\"),"},{"line_number":416,"context_line":"             {\u0027size_in_m\u0027: size_in_m, \u0027mbps\u0027: mbps})"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_7826dcbb","line":413,"in_reply_to":"9a41bdd9_fd65fee0","updated":"2015-07-13 05:01:13.000000000","message":"For RBD handle, no need to close, but for path type volume, close handle maybe yes.  disconnect_volume for path type volume doesn\u0027t take care of handle because those handles are created within this function.","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"3f56bf399a50e4d102055975a9e805ca866f3a4a","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                                   size_in_m, blocksize, sync\u003dsync,"},{"line_number":430,"context_line":"                                   execute\u003dexecute, ionice\u003dionice,"},{"line_number":431,"context_line":"                                   sparse\u003dsparse)"},{"line_number":432,"context_line":"    else:"},{"line_number":433,"context_line":"        _copy_volume_with_file(throttle, srcstr, deststr, size_in_m, blocksize,"},{"line_number":434,"context_line":"                               sync\u003dsync, execute\u003dexecute, ionice\u003dionice,"},{"line_number":435,"context_line":"                               sparse\u003dsparse)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_7dd56e41","line":432,"updated":"2015-07-13 03:55:42.000000000","message":"When both srcstr and deststr are in string_types, we use _copy_volume_with_path. \nWhat is the type for srcstr/deststr if we are using rbd?\nAre we 100% sure that _copy_volume_with_file can well handle all the other types for srcstr and deststr?","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"a167a2f21f635e0f608bf8f21f0b45473b7c15fd","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                                   size_in_m, blocksize, sync\u003dsync,"},{"line_number":430,"context_line":"                                   execute\u003dexecute, ionice\u003dionice,"},{"line_number":431,"context_line":"                                   sparse\u003dsparse)"},{"line_number":432,"context_line":"    else:"},{"line_number":433,"context_line":"        _copy_volume_with_file(throttle, srcstr, deststr, size_in_m, blocksize,"},{"line_number":434,"context_line":"                               sync\u003dsync, execute\u003dexecute, ionice\u003dionice,"},{"line_number":435,"context_line":"                               sparse\u003dsparse)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a41bdd9_78d01c54","line":432,"in_reply_to":"9a41bdd9_7dd56e41","updated":"2015-07-13 05:01:13.000000000","message":"srcstr/deststr for RBD is an instance of io.RawIOBase.  And I did a quick scan of all connector (even for those still in review), it seems all other connector would return a string_type.  But it is a possible in future so other backend may use other type of interface that might not work like file?  At least, some additional comment is helpful to explain why we are doing it this way here.","commit_id":"2c03e733f14394a63eb32e288c7a0546b4704a08"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"5cb174ea8b9212f9289b12dbcd81c878db0266ff","unresolved":false,"context_lines":[{"line_number":374,"context_line":"        # If we have reach end of source, discard any extraneous bytes from"},{"line_number":375,"context_line":"        # destination volume if trim is enabled and stop writing."},{"line_number":376,"context_line":"        if data \u003d\u003d \u0027\u0027:"},{"line_number":377,"context_line":"            return"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":380,"context_line":"        delta \u003d (time.time() - before)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_615482fd","line":377,"updated":"2015-07-28 04:49:07.000000000","message":"should this break instead of return? don\u0027t we want to do the flush?","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"68c90be2149edfe6e18356a7020144919345e71e","unresolved":false,"context_lines":[{"line_number":374,"context_line":"        # If we have reach end of source, discard any extraneous bytes from"},{"line_number":375,"context_line":"        # destination volume if trim is enabled and stop writing."},{"line_number":376,"context_line":"        if data \u003d\u003d \u0027\u0027:"},{"line_number":377,"context_line":"            return"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":380,"context_line":"        delta \u003d (time.time() - before)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_fa36e201","line":377,"in_reply_to":"3a50d1a3_615482fd","updated":"2015-07-28 14:03:50.000000000","message":"Yeah, this is a good point.  For RBD, an empty read when data is expected would indicate a connection issue which is not recoverable in this context.  But I supposed it depends on the connector\u0027s implementation and may not hold for others.","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"5cb174ea8b9212f9289b12dbcd81c878db0266ff","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s).\","},{"line_number":383,"context_line":"                  {\u0027chunk\u0027: chunk + 1, \u0027chunks\u0027: chunks, \u0027rate\u0027: rate})"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"        # yield to any other pending backups"},{"line_number":386,"context_line":"        eventlet.sleep(0)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    rem \u003d int(length % chunk_size)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_213c3ac7","line":385,"updated":"2015-07-28 04:49:07.000000000","message":"shouldn\u0027t this just be \"any other pending operations\"? This code doesn\u0027t appear to be backup specific","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"68c90be2149edfe6e18356a7020144919345e71e","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s).\","},{"line_number":383,"context_line":"                  {\u0027chunk\u0027: chunk + 1, \u0027chunks\u0027: chunks, \u0027rate\u0027: rate})"},{"line_number":384,"context_line":""},{"line_number":385,"context_line":"        # yield to any other pending backups"},{"line_number":386,"context_line":"        eventlet.sleep(0)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"    rem \u003d int(length % chunk_size)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_ba3cdae1","line":385,"in_reply_to":"3a50d1a3_213c3ac7","updated":"2015-07-28 14:03:50.000000000","message":"Yes it should, thanks.","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"5cb174ea8b9212f9289b12dbcd81c878db0266ff","unresolved":false,"context_lines":[{"line_number":395,"context_line":"    tpool.execute(dest.flush)"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"def _copy_volume_with_file(prefix, srcstr, deststr, size_in_m, blocksize,"},{"line_number":399,"context_line":"                           sync\u003dFalse, execute\u003dutils.execute, ionice\u003dNone,"},{"line_number":400,"context_line":"                           sparse\u003dFalse):"},{"line_number":401,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_e15f320d","line":398,"updated":"2015-07-28 04:49:07.000000000","message":"Why not have srcstr and deststr just be src and dest if they are not guarenteed to be str\u0027s? Seems confusing to call something srcstr but then be allowed to let it be something else...","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"68c90be2149edfe6e18356a7020144919345e71e","unresolved":false,"context_lines":[{"line_number":395,"context_line":"    tpool.execute(dest.flush)"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"def _copy_volume_with_file(prefix, srcstr, deststr, size_in_m, blocksize,"},{"line_number":399,"context_line":"                           sync\u003dFalse, execute\u003dutils.execute, ionice\u003dNone,"},{"line_number":400,"context_line":"                           sparse\u003dFalse):"},{"line_number":401,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_ed794e94","line":398,"in_reply_to":"3a50d1a3_e15f320d","updated":"2015-07-28 14:03:50.000000000","message":"Yeah, good point.","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"5cb174ea8b9212f9289b12dbcd81c878db0266ff","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    optionally return a volume handle of type RawIOBase for volumes that are"},{"line_number":438,"context_line":"    not available on the local filesystem for open/close operations."},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    If either \u0027srcstr\u0027 or \u0027deststr\u0027 are not of type str, then they are assumed"},{"line_number":441,"context_line":"    to be of type RawIOBase or any derivative that supports file operations"},{"line_number":442,"context_line":"    such as read and write.  In this case, the handles are treated as file"},{"line_number":443,"context_line":"    handles instead of file paths."}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_015d7e14","line":440,"updated":"2015-07-28 04:49:07.000000000","message":"Same thing for this method about renaming them to src and dest instead of srcstr and deststr","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"68c90be2149edfe6e18356a7020144919345e71e","unresolved":false,"context_lines":[{"line_number":437,"context_line":"    optionally return a volume handle of type RawIOBase for volumes that are"},{"line_number":438,"context_line":"    not available on the local filesystem for open/close operations."},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    If either \u0027srcstr\u0027 or \u0027deststr\u0027 are not of type str, then they are assumed"},{"line_number":441,"context_line":"    to be of type RawIOBase or any derivative that supports file operations"},{"line_number":442,"context_line":"    such as read and write.  In this case, the handles are treated as file"},{"line_number":443,"context_line":"    handles instead of file paths."}],"source_content_type":"text/x-python","patch_set":11,"id":"3a50d1a3_2de1762a","line":440,"in_reply_to":"3a50d1a3_015d7e14","updated":"2015-07-28 14:03:50.000000000","message":"Will be included in the next upload.","commit_id":"6d6abd6242e4c0275e9d10a5237345ebb09f9855"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"0824bf401562e41f1a26596886ca2bed86b0b8af","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        if data \u003d\u003d \u0027\u0027:"},{"line_number":377,"context_line":"            break"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":380,"context_line":"        delta \u003d (time.time() - before)"},{"line_number":381,"context_line":"        rate \u003d (chunk_size / delta) / units.Ki"},{"line_number":382,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s).\","}],"source_content_type":"text/x-python","patch_set":12,"id":"3a50d1a3_3bc0452a","line":379,"updated":"2015-07-30 20:45:14.000000000","message":"Do we really need tpool? What could block green thread?","commit_id":"abb1765db9381029f16c691f994646cfe9e74ea6"},{"author":{"_account_id":1107,"name":"Josh Durgin","email":"jdurgin@redhat.com","username":"jdurgin"},"change_message_id":"3848eae45b756c3bb3bb5d3b40704ef0d7a1895f","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        if data \u003d\u003d \u0027\u0027:"},{"line_number":377,"context_line":"            break"},{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        tpool.execute(dest.write, data)"},{"line_number":380,"context_line":"        delta \u003d (time.time() - before)"},{"line_number":381,"context_line":"        rate \u003d (chunk_size / delta) / units.Ki"},{"line_number":382,"context_line":"        LOG.debug(\"Transferred chunk %(chunk)s of %(chunks)s (%(rate)dK/s).\","}],"source_content_type":"text/x-python","patch_set":12,"id":"3a50d1a3_866ca0d1","line":379,"in_reply_to":"3a50d1a3_3bc0452a","updated":"2015-07-30 20:56:24.000000000","message":"currently librbd, in the future potentially other file-like objects that are not handled by eventlet directly, and would block the greenlet.","commit_id":"abb1765db9381029f16c691f994646cfe9e74ea6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":366,"context_line":"def _transfer_data(src, dest, length, chunk_size):"},{"line_number":367,"context_line":"    \"\"\"Transfer data between files (Python IO objects).\"\"\""},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    chunks \u003d int(length / chunk_size)"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"    LOG.debug(\"%(chunks)s chunks of %(bytes)s bytes to be transferred.\","},{"line_number":372,"context_line":"              {\u0027chunks\u0027: chunks, \u0027bytes\u0027: chunk_size})"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_f36e3d2e","line":369,"updated":"2015-08-19 13:00:02.000000000","message":"This is just my opinion, so feel free to ignore it.\n\nIt would look better if last chunk wasn\u0027t treated differently.\n\nYou could calculate chunks to include last chunk (with math.ceil or adding chunk_size - 1 to length in the division. And use a variable to track saved data\n\n remaining_length \u003d length\n\nThen you just loop until remaining_length is 0:\n\n data \u003d tpool.execute(src.read, min(chunk_size, remaining_length))\n remaining_length -\u003d len(data)\n\nThat way you don\u0027t have to treat the last chunk any different and you won\u0027t try to read again if you did a break in line 380.","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":374,"context_line":"    for chunk in xrange(0, chunks):"},{"line_number":375,"context_line":"        before \u003d time.time()"},{"line_number":376,"context_line":"        data \u003d tpool.execute(src.read, chunk_size)"},{"line_number":377,"context_line":"        # If we have reach end of source, discard any extraneous bytes from"},{"line_number":378,"context_line":"        # destination volume if trim is enabled and stop writing."},{"line_number":379,"context_line":"        if data \u003d\u003d \u0027\u0027:"},{"line_number":380,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_f3fabdbc","line":377,"range":{"start_line":377,"start_character":21,"end_line":377,"end_character":26},"updated":"2015-08-19 13:00:02.000000000","message":"s/reach/reached","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":402,"context_line":""},{"line_number":403,"context_line":"    src_handle \u003d src"},{"line_number":404,"context_line":"    if isinstance(src, six.string_types):"},{"line_number":405,"context_line":"        src_handle \u003d _open_volume_with_path(src, \u0027r\u0027)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    dest_handle \u003d dest"},{"line_number":408,"context_line":"    if isinstance(dest, six.string_types):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_c1dc8214","line":405,"range":{"start_line":405,"start_character":44,"end_line":405,"end_character":47},"updated":"2015-08-19 13:00:02.000000000","message":"Use \u0027rb\u0027?","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":424,"context_line":"        dest_handle.close()"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"    mbps \u003d (size_in_m / duration)"},{"line_number":427,"context_line":"    LOG.info(_LI(\"Volume copy %(size_in_m).2f MB at %(mbps).2f MB/s.\"),"},{"line_number":428,"context_line":"             {\u0027size_in_m\u0027: size_in_m, \u0027mbps\u0027: mbps})"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_a1f2ae6d","line":427,"updated":"2015-08-19 13:00:02.000000000","message":"It doesn\u0027t mention that copy has completed/finished","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":441,"context_line":"    If either \u0027src\u0027 or \u0027dest\u0027 are not of type str, then they are assumed to be"},{"line_number":442,"context_line":"    of type RawIOBase or any derivative that supports file operations such as"},{"line_number":443,"context_line":"    read and write.  In this case, the handles are treated as file handles"},{"line_number":444,"context_line":"    instead of file paths."},{"line_number":445,"context_line":"    \"\"\""},{"line_number":446,"context_line":"    if not throttle:"},{"line_number":447,"context_line":"        throttle \u003d throttling.Throttle.get_default()"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_16107f4d","line":444,"updated":"2015-08-19 13:00:02.000000000","message":"It should mention that throttle will not always be used","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"502788fbb47fec0a4f69494a910a3aaadbd00d63","unresolved":false,"context_lines":[{"line_number":443,"context_line":"    read and write.  In this case, the handles are treated as file handles"},{"line_number":444,"context_line":"    instead of file paths."},{"line_number":445,"context_line":"    \"\"\""},{"line_number":446,"context_line":"    if not throttle:"},{"line_number":447,"context_line":"        throttle \u003d throttling.Throttle.get_default()"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    if (isinstance(src, six.string_types) and"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa1b9901_962a4f7d","line":446,"range":{"start_line":446,"start_character":11,"end_line":446,"end_character":19},"updated":"2015-08-19 13:00:02.000000000","message":"Since we only use throttle in one case we should move this to line 451 inside the if","commit_id":"7b5039f3925614ad4b31139e0c51201eab7b2070"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"b20e07b70d056299a86c8ba380575af016deeeeb","unresolved":false,"context_lines":[{"line_number":395,"context_line":""},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"def _copy_volume_with_file(src, dest, size_in_m):"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"    src_handle \u003d src"},{"line_number":400,"context_line":"    if isinstance(src, six.string_types):"},{"line_number":401,"context_line":"        src_handle \u003d _open_volume_with_path(src, \u0027rb\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa1b9901_0c6f5c93","line":398,"updated":"2015-08-24 01:02:37.000000000","message":"Extra blank line","commit_id":"444c244c9e71473d526f700ce3d39e40814904bc"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"c6e97cadad35318b73b49999e8310f8ba4509f9b","unresolved":false,"context_lines":[{"line_number":405,"context_line":"        dest_handle \u003d _open_volume_with_path(dest, \u0027wb\u0027)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    if not src_handle or not dest_handle:"},{"line_number":408,"context_line":"        raise exception.DeviceUnavailable(\"Either source or destination are \""},{"line_number":409,"context_line":"                                          \"not available.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":"    start_time \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":21,"id":"da20952f_4fa68354","line":408,"updated":"2015-08-27 21:43:24.000000000","message":"Wouldn\u0027t it be more helpful to figure out which one was unavailable here?","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0538f449c510c780df084e21ce4929aa31f8360a","unresolved":false,"context_lines":[{"line_number":405,"context_line":"        dest_handle \u003d _open_volume_with_path(dest, \u0027wb\u0027)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    if not src_handle or not dest_handle:"},{"line_number":408,"context_line":"        raise exception.DeviceUnavailable(\"Either source or destination are \""},{"line_number":409,"context_line":"                                          \"not available.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":"    start_time \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":21,"id":"da20952f_986a7180","line":408,"in_reply_to":"da20952f_4fa68354","updated":"2015-08-28 19:05:49.000000000","message":"-1: missing _() for the text.\n\nnit: I wouldn\u0027t mind seeing this give a better explanation like Jay suggested","commit_id":"f1749944f779a9fd7baf81a97712db72a8f79630"}]}