)]}'
{"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":21428,"name":"JiWei","email":"17366060309@163.com","username":"WeiJi"},"change_message_id":"785543d15faf01285a07705a07b61c86b0291344","unresolved":false,"context_lines":[{"line_number":288,"context_line":"    @mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027)"},{"line_number":289,"context_line":"    def test_manage_existing_snapshot_get_size(self, volume_get_by_id):"},{"line_number":290,"context_line":"        # create a cinder volume"},{"line_number":291,"context_line":"        volume_get_by_id.return_value \u003d self.volume_a"},{"line_number":292,"context_line":"        self.driver.create_volume(self.volume_a)"},{"line_number":293,"context_line":"        # create a rbd snap"},{"line_number":294,"context_line":"        snapshot \u003d self.mock_rbd.ImageSnapshot()"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_485a1ff3","line":291,"range":{"start_line":291,"start_character":45,"end_line":291,"end_character":53},"updated":"2016-08-24 00:51:52.000000000","message":"Maybe we need to change the name of the variable to make it easier to understand.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":21428,"name":"JiWei","email":"17366060309@163.com","username":"WeiJi"},"change_message_id":"785543d15faf01285a07705a07b61c86b0291344","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            exist_snap \u003d \u0027snap-exist\u0027"},{"line_number":317,"context_line":"            existing_ref \u003d {\u0027source-name\u0027: exist_snap}"},{"line_number":318,"context_line":"            mock_rbd_snap_rename.return_value \u003d 0"},{"line_number":319,"context_line":"            self.driver.manage_existing_snapshot(snapshot, existing_ref)"},{"line_number":320,"context_line":"            mock_rbd_snap_rename.assert_called_with("},{"line_number":321,"context_line":"                client.ioctx,"},{"line_number":322,"context_line":"                exist_snap,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_c8144fea","line":319,"range":{"start_line":319,"start_character":49,"end_line":319,"end_character":57},"updated":"2016-08-24 00:51:52.000000000","message":"snapshot is not defined","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"a73fbf6c9470e19de5723719549b64d002e47ec1","unresolved":false,"context_lines":[{"line_number":297,"context_line":"        self.assertEqual([self.mock_rbd.ImageNotFound],"},{"line_number":298,"context_line":"                         RAISED_EXCEPTIONS)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"    def test_manage_existing_snapshot_get_size(self):"},{"line_number":301,"context_line":"        with mock.patch.object(self.driver.rbd.Image(self.snapshot), \u0027size\u0027) as \\"},{"line_number":302,"context_line":"                mock_rbd_snapshot_size:"},{"line_number":303,"context_line":"            with mock.patch.object(self.driver.rbd.Image(), \u0027close\u0027) \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a95cdbc_fe8aeb62","line":300,"updated":"2016-10-10 02:00:16.000000000","message":"Better to have a negative case that \u0027source-name\u0027 is not in existing_ref.","commit_id":"9609f746e2171089b7f3c4f4b4dacf638f409501"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1080,"context_line":"        \"\"\"Manages an existing rbd snapshot."},{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        Renames the snap name to match the expected name for the cinder snap."},{"line_number":1083,"context_line":"        Error checking done by manage_existing_snapshot_get_size is not repeated"},{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"        :param snapshot:"},{"line_number":1086,"context_line":"            snapshot ref info to be set"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_edef60ae","line":1083,"updated":"2016-08-26 09:07:21.000000000","message":"nit: Missing period at EOL","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1083,"context_line":"        Error checking done by manage_existing_snapshot_get_size is not repeated"},{"line_number":1084,"context_line":""},{"line_number":1085,"context_line":"        :param snapshot:"},{"line_number":1086,"context_line":"            snapshot ref info to be set"},{"line_number":1087,"context_line":"        :param existing_ref:"},{"line_number":1088,"context_line":"            existing_ref is a dictionary of the form:"},{"line_number":1089,"context_line":"            {\u0027source-name\u0027: \u003cname of rbd snapshot\u003e}"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_2d9a2815","line":1086,"updated":"2016-08-26 09:07:21.000000000","message":"nit: For consistency with the rest of Cinder code, please include the parameter description on the same line as you define the param.\n\n :param snapshot: snapshot ref info to be set\n\nSame should be done for the other cases.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"        :param snapshot:"},{"line_number":1086,"context_line":"            snapshot ref info to be set"},{"line_number":1087,"context_line":"        :param existing_ref:"},{"line_number":1088,"context_line":"            existing_ref is a dictionary of the form:"},{"line_number":1089,"context_line":"            {\u0027source-name\u0027: \u003cname of rbd snapshot\u003e}"},{"line_number":1090,"context_line":"        \"\"\""},{"line_number":1091,"context_line":"        # Raise an exception if we didn\u0027t find a suitable rbd snap."}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_adad5833","line":1088,"updated":"2016-08-26 09:07:21.000000000","message":"I believe you also get \u0027source-id\u0027","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"922019fb8dd7e1d4fd8c42e8dfc7cbd7bf5d9840","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"        :param snapshot:"},{"line_number":1086,"context_line":"            snapshot ref info to be set"},{"line_number":1087,"context_line":"        :param existing_ref:"},{"line_number":1088,"context_line":"            existing_ref is a dictionary of the form:"},{"line_number":1089,"context_line":"            {\u0027source-name\u0027: \u003cname of rbd snapshot\u003e}"},{"line_number":1090,"context_line":"        \"\"\""},{"line_number":1091,"context_line":"        # Raise an exception if we didn\u0027t find a suitable rbd snap."}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_c72ec641","line":1088,"in_reply_to":"7a8ec9b2_bf484fcd","updated":"2016-09-20 15:15:53.000000000","message":"You are right, I was thinking about volume managing.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"a2bc87667db231da495d26cfb11d2a15c6258078","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"        :param snapshot:"},{"line_number":1086,"context_line":"            snapshot ref info to be set"},{"line_number":1087,"context_line":"        :param existing_ref:"},{"line_number":1088,"context_line":"            existing_ref is a dictionary of the form:"},{"line_number":1089,"context_line":"            {\u0027source-name\u0027: \u003cname of rbd snapshot\u003e}"},{"line_number":1090,"context_line":"        \"\"\""},{"line_number":1091,"context_line":"        # Raise an exception if we didn\u0027t find a suitable rbd snap."}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_bf484fcd","line":1088,"in_reply_to":"fa7ab95a_adad5833","updated":"2016-09-19 01:37:21.000000000","message":"I don\u0027t get source-id","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"15c430db5eb27796947f869be6e9993b51c39667","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":1095,"context_line":"                                   utils.convert_str(rbd_snap_name),"},{"line_number":1096,"context_line":"                                   utils.convert_str(snapshot.name))"},{"line_number":1097,"context_line":"            # error when i try to rename"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":"    def manage_existing_snapshot_get_size(self, snapshot, existing_ref):"},{"line_number":1100,"context_line":"        \"\"\"Return size of an existing rbd snap for manage_existing_snapshot."}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_0e73bd81","line":1097,"range":{"start_line":1097,"start_character":12,"end_line":1097,"end_character":40},"updated":"2016-08-20 09:48:04.000000000","message":"comments should be removed.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":1095,"context_line":"                                   utils.convert_str(rbd_snap_name),"},{"line_number":1096,"context_line":"                                   utils.convert_str(snapshot.name))"},{"line_number":1097,"context_line":"            # error when i try to rename"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":"    def manage_existing_snapshot_get_size(self, snapshot, existing_ref):"},{"line_number":1100,"context_line":"        \"\"\"Return size of an existing rbd snap for manage_existing_snapshot."}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_50e8e37c","line":1097,"range":{"start_line":1097,"start_character":12,"end_line":1097,"end_character":40},"in_reply_to":"1ac06dbe_0e73bd81","updated":"2016-08-26 09:07:21.000000000","message":"-1: You need to handle the case where you cannot rename the snapshot, or isn\u0027t that a possibility?  In my opinion RBD driver could leverage the provider_id field for cases such as this.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"a2bc87667db231da495d26cfb11d2a15c6258078","unresolved":false,"context_lines":[{"line_number":1094,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":1095,"context_line":"                                   utils.convert_str(rbd_snap_name),"},{"line_number":1096,"context_line":"                                   utils.convert_str(snapshot.name))"},{"line_number":1097,"context_line":"            # error when i try to rename"},{"line_number":1098,"context_line":""},{"line_number":1099,"context_line":"    def manage_existing_snapshot_get_size(self, snapshot, existing_ref):"},{"line_number":1100,"context_line":"        \"\"\"Return size of an existing rbd snap for manage_existing_snapshot."}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_9f6b931d","line":1097,"range":{"start_line":1097,"start_character":12,"end_line":1097,"end_character":40},"in_reply_to":"fa7ab95a_50e8e37c","updated":"2016-09-19 01:37:21.000000000","message":"My ceph version (0.94.6) doesn\u0027t allow rename. I need to handle the case.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1113,"context_line":"                existing_ref\u003dexisting_ref, reason\u003dreason)"},{"line_number":1114,"context_line":""},{"line_number":1115,"context_line":"        rbd_snap_name \u003d utils.convert_str(existing_ref[\u0027source-name\u0027])"},{"line_number":1116,"context_line":"        snap_size \u003d snapshot.volume_size"},{"line_number":1117,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1118,"context_line":"            # Raise an exception if we didn\u0027t find a suitable rbd snap."},{"line_number":1119,"context_line":"            # try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_ebc10830","line":1116,"updated":"2016-08-26 09:07:21.000000000","message":"-1: This method should be checking the size reported by the backend, not the size of the snapshot OVO instance.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"15c430db5eb27796947f869be6e9993b51c39667","unresolved":false,"context_lines":[{"line_number":1115,"context_line":"        rbd_snap_name \u003d utils.convert_str(existing_ref[\u0027source-name\u0027])"},{"line_number":1116,"context_line":"        snap_size \u003d snapshot.volume_size"},{"line_number":1117,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1118,"context_line":"            # Raise an exception if we didn\u0027t find a suitable rbd snap."},{"line_number":1119,"context_line":"            # try:"},{"line_number":1120,"context_line":"                # rbd_image isn\u0027t working"},{"line_number":1121,"context_line":"                # rbd_image \u003d self.rbd.Image(client.ioctx, snapshot[\u0027volume_id\u0027])"},{"line_number":1122,"context_line":"                # rbd_image.close()"},{"line_number":1123,"context_line":"            # except self.rbd.ImageNotFound:"},{"line_number":1124,"context_line":"            #     kwargs \u003d {\u0027existing_ref\u0027: rbd_snap_name,"},{"line_number":1125,"context_line":"            #               \u0027reason\u0027: \u0027Specified rbd snap does not exist.\u0027}"},{"line_number":1126,"context_line":"            #     raise exception.ManageExistingInvalidReference(**kwargs)"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"            # RBD snap size is returned in bytes."},{"line_number":1129,"context_line":"            # Attempt to parse size as a float and round up to the next integer."}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_ce5885fd","line":1126,"range":{"start_line":1118,"start_character":12,"end_line":1126,"end_character":74},"updated":"2016-08-20 09:48:04.000000000","message":"comments codes?","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1115,"context_line":"        rbd_snap_name \u003d utils.convert_str(existing_ref[\u0027source-name\u0027])"},{"line_number":1116,"context_line":"        snap_size \u003d snapshot.volume_size"},{"line_number":1117,"context_line":"        with RADOSClient(self) as client:"},{"line_number":1118,"context_line":"            # Raise an exception if we didn\u0027t find a suitable rbd snap."},{"line_number":1119,"context_line":"            # try:"},{"line_number":1120,"context_line":"                # rbd_image isn\u0027t working"},{"line_number":1121,"context_line":"                # rbd_image \u003d self.rbd.Image(client.ioctx, snapshot[\u0027volume_id\u0027])"},{"line_number":1122,"context_line":"                # rbd_image.close()"},{"line_number":1123,"context_line":"            # except self.rbd.ImageNotFound:"},{"line_number":1124,"context_line":"            #     kwargs \u003d {\u0027existing_ref\u0027: rbd_snap_name,"},{"line_number":1125,"context_line":"            #               \u0027reason\u0027: \u0027Specified rbd snap does not exist.\u0027}"},{"line_number":1126,"context_line":"            #     raise exception.ManageExistingInvalidReference(**kwargs)"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"            # RBD snap size is returned in bytes."},{"line_number":1129,"context_line":"            # Attempt to parse size as a float and round up to the next integer."}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_70a40757","line":1126,"range":{"start_line":1118,"start_character":12,"end_line":1126,"end_character":74},"in_reply_to":"1ac06dbe_ce5885fd","updated":"2016-08-26 09:07:21.000000000","message":"I think this is the part where we should be getting snap_size from the backend.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":16883,"name":"huyang","email":"huyang1@huawei.com","username":"huyang"},"change_message_id":"15c430db5eb27796947f869be6e9993b51c39667","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                    data\u003dexception_message)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    def unmanage_snapshot(self, snapshot):"},{"line_number":1144,"context_line":"        pass"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume,"},{"line_number":1147,"context_line":"                               original_volume_status):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_8e5e8de7","line":1144,"range":{"start_line":1144,"start_character":8,"end_line":1144,"end_character":12},"updated":"2016-08-20 09:48:04.000000000","message":"Better to add LOG unmanage which snapshot.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e0c375614f57575bec8993e2a3aed4e84f0184b3","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                    data\u003dexception_message)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    def unmanage_snapshot(self, snapshot):"},{"line_number":1144,"context_line":"        pass"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume,"},{"line_number":1147,"context_line":"                               original_volume_status):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_ada0f891","line":1144,"range":{"start_line":1144,"start_character":8,"end_line":1144,"end_character":12},"in_reply_to":"1ac06dbe_8e5e8de7","updated":"2016-08-26 09:07:21.000000000","message":"I think this method should be removed, it is already defined in the base class.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"a2bc87667db231da495d26cfb11d2a15c6258078","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                    data\u003dexception_message)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    def unmanage_snapshot(self, snapshot):"},{"line_number":1144,"context_line":"        pass"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume,"},{"line_number":1147,"context_line":"                               original_volume_status):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_ff7bb7f2","line":1144,"range":{"start_line":1144,"start_character":8,"end_line":1144,"end_character":12},"in_reply_to":"1ac06dbe_8e5e8de7","updated":"2016-09-19 01:37:21.000000000","message":"LOG is already added. I think, there isn\u0027t need to add LOG here.","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"922019fb8dd7e1d4fd8c42e8dfc7cbd7bf5d9840","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                    data\u003dexception_message)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    def unmanage_snapshot(self, snapshot):"},{"line_number":1144,"context_line":"        pass"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume,"},{"line_number":1147,"context_line":"                               original_volume_status):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_e798226d","line":1144,"range":{"start_line":1144,"start_character":8,"end_line":1144,"end_character":12},"in_reply_to":"7a8ec9b2_7f4247af","updated":"2016-09-20 15:15:53.000000000","message":"That is because you forgot to add ManageableSnapshotsVD to the list of base classes of the RBDDriver, which is something I missed in previous reviews.  ;-)","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"a2bc87667db231da495d26cfb11d2a15c6258078","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                    data\u003dexception_message)"},{"line_number":1142,"context_line":""},{"line_number":1143,"context_line":"    def unmanage_snapshot(self, snapshot):"},{"line_number":1144,"context_line":"        pass"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"    def update_migrated_volume(self, ctxt, volume, new_volume,"},{"line_number":1147,"context_line":"                               original_volume_status):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_7f4247af","line":1144,"range":{"start_line":1144,"start_character":8,"end_line":1144,"end_character":12},"in_reply_to":"fa7ab95a_ada0f891","updated":"2016-09-19 01:37:21.000000000","message":"This method is necessary for the succeed of the snapshot-unmanage operation.  If the method isn\u0027t present you get a: \u0027RBDDriver\u0027 object has no attribute \u0027unmanage_snapshot\u0027","commit_id":"c4809a7fe908816fdf4f262684911b5873775c42"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"9aa6727e88af7f7abce73ae1cfeed95b2024382a","unresolved":false,"context_lines":[{"line_number":1084,"context_line":"    def manage_existing_snapshot(self, snapshot, existing_ref):"},{"line_number":1085,"context_line":"        \"\"\"Manages an existing rbd snapshot."},{"line_number":1086,"context_line":""},{"line_number":1087,"context_line":"        Renames the snap name to match the expected name for the cinder snap."},{"line_number":1088,"context_line":"        Error checking done by manage_existing_snapshot_get_size is not"},{"line_number":1089,"context_line":"        repeated."},{"line_number":1090,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_bac9e07b","line":1087,"updated":"2016-11-30 21:44:44.000000000","message":"I don\u0027t think a rename will be done in the manager, so issuing \u0027pass\u0027 here will leave it as is - so we should update this comment as such.","commit_id":"9609f746e2171089b7f3c4f4b4dacf638f409501"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"54041e5b9b2d3ed2fd7e55abbd4da6dfe0018a41","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"        with RBDVolumeProxy(self, snapshot.volume_name) as volume:"},{"line_number":1323,"context_line":"            rbd_name \u003d existing_ref[\u0027source-name\u0027]"},{"line_number":1324,"context_line":"            snap \u003d utils.convert_str(snapshot.name)"},{"line_number":1325,"context_line":"            volume.rename_snap(rbd_name, snap)"},{"line_number":1326,"context_line":""},{"line_number":1327,"context_line":"    def manage_existing_snapshot_get_size(self, snapshot, existing_ref):"},{"line_number":1328,"context_line":"        \"\"\"Return size of an existing rbd snap for manage_existing_snapshot."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a461143_6c37174d","line":1325,"updated":"2017-01-31 19:42:58.000000000","message":"I think three things are needed here:\n\n1. if the backing ceph cluster does not support the rename operation, I think we should raise an appropriate exception that indicates what happened and for what reason.  As older versions fall out of maintainence, we could remove this at some point in the future.\n\n2. there should be some guard against re-managing an already managed snapshot.  Since all managed snapshots will conform to a naming convention, perhaps rejecting any already-conforming RBD image will be sufficient.\n\n3. I think we should include an implementation for \u0027get_manageable_snapshots\u0027 to tie everything together.\n\nThe code posted works, just a bit more and it will be in good shape for merge.","commit_id":"05c0b2427f2c911e6c8f24657a11c83f6e0444e3"}]}