)]}'
{"cinder/tests/unit/test_rbd.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1022,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rados\u0027)"},{"line_number":1023,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.RBDClient\u0027)"},{"line_number":1024,"context_line":"    def test_migrate_volume(self, mock_client, mock_rados, mock_rbd):"},{"line_number":1025,"context_line":"        host \u003d dict(capabilities \u003d dict("},{"line_number":1026,"context_line":"            vendor_name\u003d\u0027Open Source\u0027,"},{"line_number":1027,"context_line":"            storage_protocol\u003d\u0027ceph\u0027,"},{"line_number":1028,"context_line":"            location_info\u003d\u0027nondefault:None:None:rbd\u0027))"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_14a289ae","line":1025,"updated":"2016-05-19 09:33:23.000000000","message":"nit: This is not very readable, I think it would be best with:\n\n host \u003d {\n     \u0027capabilities\u0027: {\n         \u0027vendor_name\u0027: \u0027Open Source\u0027,\n         ...\n     }\n }","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1032,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1033,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1034,"context_line":"            with mock.patch.object(self.driver, \u0027delete_volume\u0027) as \\"},{"line_number":1035,"context_line":"                    mock_delete:"},{"line_number":1036,"context_line":"                proxy \u003d self.mock_proxy.return_value"},{"line_number":1037,"context_line":"                proxy.__enter__.return_value \u003d proxy"},{"line_number":1038,"context_line":"                ret \u003d self.driver.migrate_volume(context, self.volume_a,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_b4f8dd4a","line":1035,"updated":"2016-05-19 09:33:23.000000000","message":"nit: I think this should be grouped with the other context manager instead of nesting them like this...","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"}],"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1079,"context_line":"            \u0027{\"name\":\"volumes\",\"id\":3,\"stats\":{\"kb_used\":0,\"bytes_used\":0,\u0027"},{"line_number":1080,"context_line":"            \u0027\"max_avail\":28987613184,\"objects\":0}}]}\\n\u0027, \u0027\u0027)"},{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        expected_location_info \u003d \\"},{"line_number":1083,"context_line":"            \u0027nondefault:%s:abc:%s:rbd\u0027 % (self.cfg.rbd_ceph_conf,"},{"line_number":1084,"context_line":"                                          self.cfg.rbd_user)"},{"line_number":1085,"context_line":"        expected \u003d dict("}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_6a904c4f","line":1082,"range":{"start_line":1082,"start_character":33,"end_line":1082,"end_character":34},"updated":"2017-06-01 16:07:08.000000000","message":"-1: No using \\ for multiple lines, we should use parenthesis according to the guidelines:\n\n    expected_location_info \u003d (\u0027nondefault:%s:abc:%s:rbd\u0027 %\n                              (self.cfg.rbd_ceph_conf, self.cfg.rbd_user))","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1110,"context_line":"                         mock_driver_configuration)"},{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1113,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1114,"context_line":"            actual \u003d self.driver.get_volume_stats(True)"},{"line_number":1115,"context_line":""},{"line_number":1116,"context_line":"        client.cluster.mon_command.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_6ac72c47","line":1113,"range":{"start_line":1113,"start_character":41,"end_line":1113,"end_character":46},"updated":"2017-06-01 16:07:08.000000000","message":"nit: Using a magic string here seems confusing, I think it would me clearer if we defined a fsid variable and use it as part of the formatting in L1082 like you do with the self.cfg.rbd_* variables since you are also using it in there as a magic string","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1129,"context_line":"        self.mock_object(self.driver.configuration, \u0027safe_get\u0027,"},{"line_number":1130,"context_line":"                         mock_driver_configuration)"},{"line_number":1131,"context_line":""},{"line_number":1132,"context_line":"        expected_location_info \u003d \\"},{"line_number":1133,"context_line":"            \u0027nondefault:%s:abc:%s:rbd\u0027 % (self.cfg.rbd_ceph_conf,"},{"line_number":1134,"context_line":"                                          self.cfg.rbd_user)"},{"line_number":1135,"context_line":"        expected \u003d dict(volume_backend_name\u003d\u0027RBD\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_4adb481a","line":1132,"range":{"start_line":1132,"start_character":33,"end_line":1132,"end_character":34},"updated":"2017-06-01 16:07:08.000000000","message":"-1: Parenthesis instead of \\","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1147,"context_line":"                        location_info\u003dexpected_location_info)"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1150,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1151,"context_line":"            actual \u003d self.driver.get_volume_stats(True)"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        client.cluster.mon_command.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_4af46886","line":1150,"range":{"start_line":1150,"start_character":41,"end_line":1150,"end_character":46},"updated":"2017-06-01 16:07:08.000000000","message":"nit: magic string","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1623,"context_line":"                                          \u0027mirror_image_promote\u0027, False)"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"    def test_migrate_volume_bad_volume_status(self):"},{"line_number":1626,"context_line":"        self.volume_a.status \u003d \u0027invalid status\u0027"},{"line_number":1627,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, None)"},{"line_number":1628,"context_line":"        self.assertEqual(ret, (False, None))"},{"line_number":1629,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_28e25c30","line":1626,"range":{"start_line":1626,"start_character":31,"end_line":1626,"end_character":47},"updated":"2017-06-01 16:07:08.000000000","message":"nit: We should use a real invalid value in case we change the OVO later to use an enumerate field instead of a free field","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"    def test_migrate_volume_bad_volume_status(self):"},{"line_number":1626,"context_line":"        self.volume_a.status \u003d \u0027invalid status\u0027"},{"line_number":1627,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, None)"},{"line_number":1628,"context_line":"        self.assertEqual(ret, (False, None))"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"    def test_migrate_volume_bad_host(self):"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_485590c5","line":1627,"range":{"start_line":1627,"start_character":65,"end_line":1627,"end_character":69},"updated":"2017-06-01 16:07:08.000000000","message":"nice!! this way we confirm that it hasn\u0027t advance to the point of checking the host capabilities.  :-)","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"    def test_migrate_volume_bad_volume_status(self):"},{"line_number":1626,"context_line":"        self.volume_a.status \u003d \u0027invalid status\u0027"},{"line_number":1627,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, None)"},{"line_number":1628,"context_line":"        self.assertEqual(ret, (False, None))"},{"line_number":1629,"context_line":""},{"line_number":1630,"context_line":"    def test_migrate_volume_bad_host(self):"},{"line_number":1631,"context_line":"        host \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_a812ec07","line":1628,"updated":"2017-06-01 16:07:08.000000000","message":"-1: The order of the assert should be expected, actual:\n\n self.assertEqual((False, None), ret)","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1632,"context_line":"            \u0027capabilities\u0027: {"},{"line_number":1633,"context_line":"                \u0027storage_protocol\u0027: \u0027not-ceph\u0027}}"},{"line_number":1634,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1635,"context_line":"        self.assertEqual(ret, (False, None))"},{"line_number":1636,"context_line":""},{"line_number":1637,"context_line":"    def test_migrate_volume_missing_location_info(self):"},{"line_number":1638,"context_line":"        host \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_88f968c9","line":1635,"updated":"2017-06-01 16:07:08.000000000","message":"-1: ditto","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1639,"context_line":"            \u0027capabilities\u0027: {"},{"line_number":1640,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027}}"},{"line_number":1641,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1642,"context_line":"        self.assertEqual(ret, (False, None))"},{"line_number":1643,"context_line":""},{"line_number":1644,"context_line":"    def test_migrate_volume_invalid_location_info(self):"},{"line_number":1645,"context_line":"        host \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_082f983f","line":1642,"updated":"2017-06-01 16:07:08.000000000","message":"-1: ditto","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1647,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027,"},{"line_number":1648,"context_line":"                \u0027location_info\u0027: \u0027foo:bar:baz\u0027}}"},{"line_number":1649,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1650,"context_line":"        self.assertEqual(ret, (False, None))"},{"line_number":1651,"context_line":""},{"line_number":1652,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rbd\u0027)"},{"line_number":1653,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rados\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_08d4381f","line":1650,"updated":"2017-06-01 16:07:08.000000000","message":"-1: ditto","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1652,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rbd\u0027)"},{"line_number":1653,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rados\u0027)"},{"line_number":1654,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.RBDClient\u0027)"},{"line_number":1655,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.rbd.RBDVolumeProxy\u0027)"},{"line_number":1656,"context_line":"    def test_migrate_volume_mismatch_fsid(self, mock_proxy, mock_client,"},{"line_number":1657,"context_line":"                                          mock_rados, mock_rbd):"},{"line_number":1658,"context_line":"        host \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_e8658446","line":1655,"updated":"2017-06-01 16:07:08.000000000","message":"nit: Since we are not going to use the mock_proxy or mock_rados we can just do:\n\n @mock.patch(\u0027cinder.volume.drivers.rbd.RBDVolumeProxy\u0027, mock.Mock())\n @mock.patch(\u0027os_brick.initiator.linuxrbd.rados\u0027, mock.Mock())\n\nAnd then we don\u0027t need to add mock_proxy and mock_rados as parameters to the method","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1661,"context_line":"                \u0027location_info\u0027: \u0027nondefault:None:abc:None:rbd\u0027}}"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        mock_client.return_value.__enter__.return_value.client.\\"},{"line_number":1664,"context_line":"            get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1665,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1666,"context_line":"            mock_get_fsid.return_value \u003d \u0027not-abc\u0027"},{"line_number":1667,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_08079884","line":1664,"updated":"2017-06-01 16:07:08.000000000","message":"nit: if you are tight for space and you are not going to check the calls you can do:\n\n mock_client().__enter__().client.get_fsid.return_value \u003d \u0027abc\u0027","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1665,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1666,"context_line":"            mock_get_fsid.return_value \u003d \u0027not-abc\u0027"},{"line_number":1667,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1668,"context_line":"            self.assertEqual(ret, (False, None))"},{"line_number":1669,"context_line":""},{"line_number":1670,"context_line":"        mock_client.return_value.__enter__.return_value.client.\\"},{"line_number":1671,"context_line":"            get_fsid.return_value \u003d \u0027not-abc\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_08b9d8ca","line":1668,"updated":"2017-06-01 16:07:08.000000000","message":"-1: ditto","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1668,"context_line":"            self.assertEqual(ret, (False, None))"},{"line_number":1669,"context_line":""},{"line_number":1670,"context_line":"        mock_client.return_value.__enter__.return_value.client.\\"},{"line_number":1671,"context_line":"            get_fsid.return_value \u003d \u0027not-abc\u0027"},{"line_number":1672,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1673,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_48223002","line":1670,"updated":"2017-06-01 16:07:08.000000000","message":"I\u0027m not too happy about having multiple tests inside the same method (even if they are all related), ideally we would have either multiple methods or use ddt","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1668,"context_line":"            self.assertEqual(ret, (False, None))"},{"line_number":1669,"context_line":""},{"line_number":1670,"context_line":"        mock_client.return_value.__enter__.return_value.client.\\"},{"line_number":1671,"context_line":"            get_fsid.return_value \u003d \u0027not-abc\u0027"},{"line_number":1672,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1673,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1674,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_c800806a","line":1671,"updated":"2017-06-01 16:07:08.000000000","message":"nit: if you are tight for space and you are not going to check the calls you can do:\n\n mock_client().__enter__().client.get_fsid.return_value \u003d \u0027non_abc\u0027","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1672,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1673,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1674,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1675,"context_line":"            self.assertEqual(ret, (False, None))"},{"line_number":1676,"context_line":""},{"line_number":1677,"context_line":"        host \u003d {"},{"line_number":1678,"context_line":"            \u0027capabilities\u0027: {"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_88ccc86a","line":1675,"updated":"2017-06-01 16:07:08.000000000","message":"-1: order","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1680,"context_line":"                \u0027location_info\u0027: \u0027nondefault:None:not-abc:None:rbd\u0027}}"},{"line_number":1681,"context_line":""},{"line_number":1682,"context_line":"        mock_client.return_value.__enter__.return_value.client.\\"},{"line_number":1683,"context_line":"            get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1684,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1685,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1686,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_c8352048","line":1683,"updated":"2017-06-01 16:07:08.000000000","message":"nit: if you are tight for space and you are not going to check the calls you can do:\n\n mock_client().__enter__().client.get_fsid.return_value \u003d \u0027abc\u0027","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1684,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid:"},{"line_number":1685,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1686,"context_line":"            ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1687,"context_line":"            self.assertEqual(ret, (False, None))"},{"line_number":1688,"context_line":""},{"line_number":1689,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rbd\u0027)"},{"line_number":1690,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rados\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_48c6b04a","line":1687,"updated":"2017-06-01 16:07:08.000000000","message":"-1: order","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1691,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.RBDClient\u0027)"},{"line_number":1692,"context_line":"    @mock.patch(\u0027cinder.volume.drivers.rbd.RBDVolumeProxy\u0027)"},{"line_number":1693,"context_line":"    def test_migrate_volume(self, mock_proxy, mock_client, mock_rados,"},{"line_number":1694,"context_line":"                            mock_rbd):"},{"line_number":1695,"context_line":"        host \u003d {"},{"line_number":1696,"context_line":"            \u0027capabilities\u0027: {"},{"line_number":1697,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_8804c84b","line":1694,"updated":"2017-06-01 16:07:08.000000000","message":"nit: Since we are not going to use the mock_rados we can just do:\n\n @mock.patch(\u0027os_brick.initiator.linuxrbd.rados\u0027, mock.Mock())\n\nAnd then we don\u0027t need to add mock_rados as a parameter to the method","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1698,"context_line":"                \u0027location_info\u0027: \u0027nondefault:None:abc:None:rbd\u0027}}"},{"line_number":1699,"context_line":""},{"line_number":1700,"context_line":"        mock_client.return_value.__enter__.return_value.client.\\"},{"line_number":1701,"context_line":"            get_fsid.return_value \u003d \u0027abc\u0027"},{"line_number":1702,"context_line":"        with mock.patch.object(self.driver, \u0027_get_fsid\u0027) as mock_get_fsid, \\"},{"line_number":1703,"context_line":"                mock.patch.object(self.driver, \u0027delete_volume\u0027) as mock_delete:"},{"line_number":1704,"context_line":"            mock_get_fsid.return_value \u003d \u0027abc\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_a8a2ec49","line":1701,"updated":"2017-06-01 16:07:08.000000000","message":"nit: if you are tight for space and you are not going to check the calls you can do:\n\n mock_client().__enter__().client.get_fsid.return_value \u003d \u0027abc\u0027","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1710,"context_line":"                mock_client.return_value.__enter__.return_value.ioctx,"},{"line_number":1711,"context_line":"                self.volume_a.name)"},{"line_number":1712,"context_line":"            mock_delete.assert_called_once_with(self.volume_a)"},{"line_number":1713,"context_line":"            self.assertEqual(ret, (True, None))"},{"line_number":1714,"context_line":""},{"line_number":1715,"context_line":""},{"line_number":1716,"context_line":"class ManagedRBDTestCase(test_driver.BaseDriverTestCase):"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_682054b1","line":1713,"updated":"2017-06-01 16:07:08.000000000","message":"-1: Order","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"c178632a7eb4a29968759d3efa4b0fc2baf8bebc","unresolved":false,"context_lines":[{"line_number":1778,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, None)"},{"line_number":1779,"context_line":"        self.assertEqual((False, None), ret)"},{"line_number":1780,"context_line":""},{"line_number":1781,"context_line":"    def test_migrate_volume_bad_host(self):"},{"line_number":1782,"context_line":"        host \u003d {"},{"line_number":1783,"context_line":"            \u0027capabilities\u0027: {"},{"line_number":1784,"context_line":"                \u0027storage_protocol\u0027: \u0027not-ceph\u0027}}"},{"line_number":1785,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1786,"context_line":"        self.assertEqual((False, None), ret)"},{"line_number":1787,"context_line":""},{"line_number":1788,"context_line":"    def test_migrate_volume_missing_location_info(self):"},{"line_number":1789,"context_line":"        host \u003d {"},{"line_number":1790,"context_line":"            \u0027capabilities\u0027: {"},{"line_number":1791,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027}}"},{"line_number":1792,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1793,"context_line":"        self.assertEqual((False, None), ret)"},{"line_number":1794,"context_line":""},{"line_number":1795,"context_line":"    def test_migrate_volume_invalid_location_info(self):"},{"line_number":1796,"context_line":"        host \u003d {"},{"line_number":1797,"context_line":"            \u0027capabilities\u0027: {"},{"line_number":1798,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027,"},{"line_number":1799,"context_line":"                \u0027location_info\u0027: \u0027foo:bar:baz\u0027}}"},{"line_number":1800,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1801,"context_line":"        self.assertEqual((False, None), ret)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rbd\u0027)"},{"line_number":1804,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.RBDClient\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ff346bd7_37ffbeab","line":1801,"range":{"start_line":1781,"start_character":0,"end_line":1801,"end_character":44},"updated":"2017-07-24 08:54:53.000000000","message":"Just think those tests could be more simple and merged as one test like this:\n\ndef test_mirgrate_volume_with_invalid_arguments(self):\n   invalid_hosts \u003d [{}, {}, {}]\n   for invalid_host in invalid_hosts:\n       ret \u003d self.driver.migrate_volume(context, self.volume_a, invalid_host)\n        self.assertEqual((False, None), ret)","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b40798b3cd7d4259c6c51c879a7a956a841a6639","unresolved":false,"context_lines":[{"line_number":1798,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027,"},{"line_number":1799,"context_line":"                \u0027location_info\u0027: \u0027foo:bar:baz\u0027}}"},{"line_number":1800,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1801,"context_line":"        self.assertEqual((False, None), ret)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rbd\u0027)"},{"line_number":1804,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.RBDClient\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7f287b81_1afa1042","line":1801,"in_reply_to":"7f287b81_5d659944","updated":"2017-09-08 11:36:43.000000000","message":"I stand by my comment: \"I\u0027m not too happy about having multiple tests inside the same method (even if they are all related), ideally we would have either multiple methods or use ddt\", because the original code didn\u0027t use a loop, it did the tests sequentially. \n\nwanghao\u0027s approach is a variant of the ddt approach.\n\nYou decided to go with the multiple-methods approach instead of the ddt, and while it is more code I think it also has it\u0027s merits, because it\u0027s very easy to know what is being tested on each case with the good method naming you have used, whereas the ddt and loop approaches sacrifice clarity of what\u0027s being tested in the name of code brevity.\n\nIn this case, where the code duplication between both cases is not great, I consider this a personal choice, that\u0027s why I proposed both.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"51f5e69618895db605f8f7adae145618c3dd17ff","unresolved":false,"context_lines":[{"line_number":1798,"context_line":"                \u0027storage_protocol\u0027: \u0027ceph\u0027,"},{"line_number":1799,"context_line":"                \u0027location_info\u0027: \u0027foo:bar:baz\u0027}}"},{"line_number":1800,"context_line":"        ret \u003d self.driver.migrate_volume(context, self.volume_a, host)"},{"line_number":1801,"context_line":"        self.assertEqual((False, None), ret)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.rbd\u0027)"},{"line_number":1804,"context_line":"    @mock.patch(\u0027os_brick.initiator.linuxrbd.RBDClient\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7f287b81_5d659944","line":1801,"in_reply_to":"ff346bd7_37ffbeab","updated":"2017-08-24 16:27:37.000000000","message":"Gorka, in his last review, suggested that this was not desireable.  Can you two communicate and reach a consensus?","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":12736,"name":"Xuepeng Ji","email":"ji.xuepeng@zte.com.cn","username":"jixuepeng"},"change_message_id":"95fa9bbdcc5a5978cad830eb9ce1eeca4344b5c0","unresolved":false,"context_lines":[{"line_number":382,"context_line":"        return hosts, ports"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"    def _update_volume_stats(self):"},{"line_number":385,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":386,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":387,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":388,"context_line":"                                         self.configuration.rbd_pool)"},{"line_number":389,"context_line":"        stats \u003d {"},{"line_number":390,"context_line":"            \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":391,"context_line":"            \u0027driver_version\u0027: self.VERSION,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a18252c_a2b83c76","line":388,"range":{"start_line":385,"start_character":0,"end_line":388,"end_character":69},"updated":"2016-04-15 03:15:57.000000000","message":"I think that fsid and rbd_pool can be used as location_info. \nDo not need to use rbd_cluster_name,rbd_ceph_conf and rbd_user.","commit_id":"911c698d8c53bfb4489965e6e63a512d5eb9459f"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"e3bfb6091672fe22a722a48e9e77c2b52d138913","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":386,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":387,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":388,"context_line":"                                         self.configuration.rbd_pool)"},{"line_number":389,"context_line":"        stats \u003d {"},{"line_number":390,"context_line":"            \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":391,"context_line":"            \u0027driver_version\u0027: self.VERSION,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_2dcea15a","line":388,"in_reply_to":"5a18252c_a2b83c76","updated":"2016-04-15 11:51:04.000000000","message":"With the user and pool, i can query the fsid (see below).  The cluster name and ceph.conf could be removed technically, but we may want to use the location info for other purposes in the future, and they may come in handy.  I don\u0027t think it hurts to completely describe the backend location here, even if not all of the fields are yet used.","commit_id":"911c698d8c53bfb4489965e6e63a512d5eb9459f"},{"author":{"_account_id":12736,"name":"Xuepeng Ji","email":"ji.xuepeng@zte.com.cn","username":"jixuepeng"},"change_message_id":"a9d6d2701fc74ea386925d3ba4dc97f93081b125","unresolved":false,"context_lines":[{"line_number":1147,"context_line":"                          \u0027Falling back to generic migration.\u0027)"},{"line_number":1148,"context_line":"                return refuse_to_migrate"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"            with RBDVolumeProxy(self, volume.name, read_only\u003dTrue) as source:"},{"line_number":1151,"context_line":"                source.copy(target.ioctx, volume.name)"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        self.delete_volume(volume)"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a18252c_d6fd3d6f","line":1153,"range":{"start_line":1150,"start_character":0,"end_line":1153,"end_character":34},"updated":"2016-04-15 03:38:30.000000000","message":"Volume can be deleted only after the volume copy is successful. So exception should be catched here.","commit_id":"911c698d8c53bfb4489965e6e63a512d5eb9459f"},{"author":{"_account_id":12736,"name":"Xuepeng Ji","email":"ji.xuepeng@zte.com.cn","username":"jixuepeng"},"change_message_id":"79df94d5e06ce818e7c1f576ec94ec301967777a","unresolved":false,"context_lines":[{"line_number":1150,"context_line":"            with RBDVolumeProxy(self, volume.name, read_only\u003dTrue) as source:"},{"line_number":1151,"context_line":"                source.copy(target.ioctx, volume.name)"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        self.delete_volume(volume)"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_998d697e","line":1153,"in_reply_to":"1a122d0e_6d73794e","updated":"2016-04-15 15:55:37.000000000","message":"Yes，there is no problem with the code logic. But if copy fails, the incomplete image may remain in the target storage pool and it may need to be cleaned up. In addition the exception can be saved and raised again, thus you can get the information about the failure. What do you think?","commit_id":"911c698d8c53bfb4489965e6e63a512d5eb9459f"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"bf65d2ae5e71bcf4524825c15df538e69a85d9c2","unresolved":false,"context_lines":[{"line_number":1150,"context_line":"            with RBDVolumeProxy(self, volume.name, read_only\u003dTrue) as source:"},{"line_number":1151,"context_line":"                source.copy(target.ioctx, volume.name)"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        self.delete_volume(volume)"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":5,"id":"dab17558_c380a98d","line":1153,"in_reply_to":"1a122d0e_998d697e","updated":"2016-05-11 20:19:35.000000000","message":"I think you\u0027re absolutely right :)","commit_id":"911c698d8c53bfb4489965e6e63a512d5eb9459f"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"e3bfb6091672fe22a722a48e9e77c2b52d138913","unresolved":false,"context_lines":[{"line_number":1150,"context_line":"            with RBDVolumeProxy(self, volume.name, read_only\u003dTrue) as source:"},{"line_number":1151,"context_line":"                source.copy(target.ioctx, volume.name)"},{"line_number":1152,"context_line":""},{"line_number":1153,"context_line":"        self.delete_volume(volume)"},{"line_number":1154,"context_line":""},{"line_number":1155,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_6d73794e","line":1153,"in_reply_to":"5a18252c_d6fd3d6f","updated":"2016-04-15 11:51:04.000000000","message":"If copy() raises, the function will return without executing delete(), so the behaviour you expect is preserved.  The intent of catching the librbd exception would be to wrap it in something else - but personally I like/need to see the librbd exception, it gives me the information I need to understand the failure.  This is consistent with the rest of the driver code as well.","commit_id":"911c698d8c53bfb4489965e6e63a512d5eb9459f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1112,"context_line":"        LOG.info(_LI(\u0027Attempting RBD enabled volume migration. \u0027"},{"line_number":1113,"context_line":"                     \u0027volume: %(id)s, host: %(host)s, \u0027"},{"line_number":1114,"context_line":"                     \u0027status\u003d%(status)s.\u0027),"},{"line_number":1115,"context_line":"                 {\u0027id\u0027: volume[\u0027id\u0027],"},{"line_number":1116,"context_line":"                  \u0027host\u0027: host,"},{"line_number":1117,"context_line":"                  \u0027status\u0027: volume[\u0027status\u0027]})"},{"line_number":1118,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_343fcdd7","line":1115,"updated":"2016-05-19 09:33:23.000000000","message":"-1: Volume is an OVO, so we should be using attribute notation for new code instead of accessing it as a dictionary.  Please change it in the method.\n\n volume.id\n volume.status\n ...","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1118,"context_line":""},{"line_number":1119,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"        if volume[\u0027status\u0027] not in (\u0027available\u0027, \u0027retyping\u0027):"},{"line_number":1122,"context_line":"            LOG.debug(\u0027Only available volumes can be migrated using backend \u0027"},{"line_number":1123,"context_line":"                      \u0027assisted migration. Falling back to generic migration.\u0027)"},{"line_number":1124,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_9aa4e3b9","line":1121,"updated":"2016-05-19 09:33:23.000000000","message":"-1: I\u0027m not sure this should be checked here again, as this should have been prevented at the API level. And if we do it we should also include \u0027maintenance\u0027 status since it\u0027s also a possibility.","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1127,"context_line":"                host[\u0027capabilities\u0027][\u0027storage_protocol\u0027] !\u003d \u0027ceph\u0027):"},{"line_number":1128,"context_line":"            LOG.debug(\u0027Source and destination drivers need to be RBD \u0027"},{"line_number":1129,"context_line":"                      \u0027to use backend assisted migration. Falling back to \u0027"},{"line_number":1130,"context_line":"                      \u0027generic migration.\u0027)"},{"line_number":1131,"context_line":""},{"line_number":1132,"context_line":"        if \u0027location_info\u0027 not in host[\u0027capabilities\u0027]:"},{"line_number":1133,"context_line":"            LOG.debug(\u0027Could not find location_info in capabilities reported \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_9ab983ce","line":1130,"updated":"2016-05-19 09:33:23.000000000","message":"-1: This is missing:\n\n return refuse_to_migrate","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"                      \u0027migration.\u0027)"},{"line_number":1136,"context_line":"            return refuse_to_migrate"},{"line_number":1137,"context_line":""},{"line_number":1138,"context_line":"        loc_info \u003d host[\u0027capabilities\u0027][\u0027location_info\u0027]"},{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"        try:"},{"line_number":1141,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_user, rbd_pool) \u003d \\"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_fa5a4f82","line":1138,"updated":"2016-05-19 09:33:23.000000000","message":"nit: I think we should move the assignment before L1132 and do it with a get:\n\n loc_info \u003d host[\u0027capabilities\u0027].get(\u0027location_info\u0027)\n\n if not loc_info:\n\nThat way we also do the hash on host and location_info once.","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"                          \u0027Falling back to generic migration.\u0027)"},{"line_number":1155,"context_line":"                return refuse_to_migrate"},{"line_number":1156,"context_line":""},{"line_number":1157,"context_line":"            with RBDVolumeProxy(self, volume.name, read_only\u003dTrue) as source:"},{"line_number":1158,"context_line":"                try:"},{"line_number":1159,"context_line":"                    source.copy(target.ioctx, volume.name)"},{"line_number":1160,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_fa3b6fe9","line":1157,"range":{"start_line":1157,"start_character":38,"end_line":1157,"end_character":49},"updated":"2016-05-19 09:33:23.000000000","message":"Jeje, you are already using attribute notation here  :-)","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1159,"context_line":"                    source.copy(target.ioctx, volume.name)"},{"line_number":1160,"context_line":"                except Exception:"},{"line_number":1161,"context_line":"                    LOG.error(_LE(\u0027Error copying rbd image %s to target.\u0027),"},{"line_number":1162,"context_line":"                              volume.name)"},{"line_number":1163,"context_line":"                    self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1164,"context_line":"                    raise"},{"line_number":1165,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_1d362de7","line":1162,"updated":"2016-05-19 09:33:23.000000000","message":"-1: This should use excutils.save_and_reraise_exception in case the remove also raises an exception.","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1163,"context_line":"                    self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1164,"context_line":"                    raise"},{"line_number":1165,"context_line":""},{"line_number":1166,"context_line":"        self.delete_volume(volume)"},{"line_number":1167,"context_line":""},{"line_number":1168,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_3d8a11c0","line":1166,"updated":"2016-05-19 09:33:23.000000000","message":"?: What happens if delete_volume fails?  Because we will have the volume in both places but the DB will only reference the source, so we lose track of the data at the destination...","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"e70593f517ad12d0ec0309ce33b1ae9aed7073b9","unresolved":false,"context_lines":[{"line_number":1163,"context_line":"                    self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1164,"context_line":"                    raise"},{"line_number":1165,"context_line":""},{"line_number":1166,"context_line":"        self.delete_volume(volume)"},{"line_number":1167,"context_line":""},{"line_number":1168,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_6c309308","line":1166,"in_reply_to":"bab6814e_3d8a11c0","updated":"2016-06-09 20:27:46.000000000","message":"I\u0027m going to log a warning and continue execution.  I could try to delete the target volume, but that could raise as well and we may never exit the loop.  Sound reasonable?","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1165,"context_line":""},{"line_number":1166,"context_line":"        self.delete_volume(volume)"},{"line_number":1167,"context_line":""},{"line_number":1168,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_3d0df13e","line":1168,"updated":"2016-05-19 09:33:23.000000000","message":"?: Should we the \u0027provider_location\u0027 to be updated?  Or the RBD driver isn\u0027t using it?","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"e70593f517ad12d0ec0309ce33b1ae9aed7073b9","unresolved":false,"context_lines":[{"line_number":1165,"context_line":""},{"line_number":1166,"context_line":"        self.delete_volume(volume)"},{"line_number":1167,"context_line":""},{"line_number":1168,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_2caa0b1a","line":1168,"in_reply_to":"bab6814e_3d0df13e","updated":"2016-06-09 20:27:46.000000000","message":"We don\u0027t appear to use it, i *think* we\u0027re fine.","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"9930b0d56bc4bc4fea101eaf06b0e0ddbe5701a7","unresolved":false,"context_lines":[{"line_number":384,"context_line":""},{"line_number":385,"context_line":"    def _update_volume_stats(self):"},{"line_number":386,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":387,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":388,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":389,"context_line":"                                         self.configuration.rbd_pool)"},{"line_number":390,"context_line":"        stats \u003d {"}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_35dddd14","line":387,"updated":"2016-06-12 03:34:21.000000000","message":"rbd_ceph_conf is only useful for those who has access to the configuration file on target volume service, is it necessary to put it into location_info?  Or should mon addr be used instead?","commit_id":"cc7b7c3fd2a5e7b88260f869710e879303aff118"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"8fa76921256062e1735026c15f6f56afa9b62141","unresolved":false,"context_lines":[{"line_number":1138,"context_line":"        loc_info \u003d host[\u0027capabilities\u0027].get(\u0027location_info\u0027)"},{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"        if not loc_info:"},{"line_number":1141,"context_line":"            LOG.debug(\u0027Could not find location_info in capabilities reported \u0027"},{"line_number":1142,"context_line":"                      \u0027by the destination driver. Falling back to generic \u0027"},{"line_number":1143,"context_line":"                      \u0027migration.\u0027)"},{"line_number":1144,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_807ebfe1","line":1141,"range":{"start_line":1141,"start_character":26,"end_line":1141,"end_character":27},"updated":"2016-06-14 10:14:45.000000000","message":"IMO, it should be info, not debug level","commit_id":"cc7b7c3fd2a5e7b88260f869710e879303aff118"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"8fa76921256062e1735026c15f6f56afa9b62141","unresolved":false,"context_lines":[{"line_number":1147,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_user, rbd_pool) \u003d \\"},{"line_number":1148,"context_line":"                loc_info.split(\u0027:\u0027)"},{"line_number":1149,"context_line":"        except ValueError:"},{"line_number":1150,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"},{"line_number":1151,"context_line":"                          \u0027migration not in correct format: %s. Falling back \u0027"},{"line_number":1152,"context_line":"                          \u0027to generic volume migration.\u0027), loc_info)"},{"line_number":1153,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":8,"id":"7aa08908_2070ab0a","line":1150,"updated":"2016-06-14 10:14:45.000000000","message":"The same, as above","commit_id":"cc7b7c3fd2a5e7b88260f869710e879303aff118"},{"author":{"_account_id":23602,"name":"Michael Dovgal","email":"dovgalmichael@gmail.com","username":"mdovgal"},"change_message_id":"49abc51fb5f826ac6ff8165513ae67f41ca3f627","unresolved":false,"context_lines":[{"line_number":313,"context_line":"            \u0027multiattach\u0027: False,"},{"line_number":314,"context_line":"            \u0027thin_provisioning_support\u0027: True,"},{"line_number":315,"context_line":"            \u0027max_over_subscription_ratio\u0027: ("},{"line_number":316,"context_line":"                self.configuration.safe_get(\u0027max_over_subscription_ratio\u0027))"},{"line_number":317,"context_line":"            \u0027location_info\u0027: location_info,"},{"line_number":318,"context_line":"        }"},{"line_number":319,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_3e010606","line":316,"range":{"start_line":316,"start_character":73,"end_line":316,"end_character":75},"updated":"2016-11-01 16:23:57.000000000","message":"You\u0027ve missed \u0027,\u0027 here","commit_id":"c05c071589ff3c3f0254fdc36d634c2e01de43fc"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"39fa07803ddca6f3f48a6adadd33ca61410dd3d8","unresolved":false,"context_lines":[{"line_number":1045,"context_line":"                     \u0027status\u003d%(status)s.\u0027),"},{"line_number":1046,"context_line":"                 {\u0027id\u0027: volume.id,"},{"line_number":1047,"context_line":"                  \u0027host\u0027: host,"},{"line_number":1048,"context_line":"                  \u0027status\u0027: volume.id})"},{"line_number":1049,"context_line":""},{"line_number":1050,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1051,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_d8143ea1","line":1048,"updated":"2016-11-03 17:57:04.000000000","message":"volume.id\u003d\u003evolume.status","commit_id":"c05c071589ff3c3f0254fdc36d634c2e01de43fc"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"39fa07803ddca6f3f48a6adadd33ca61410dd3d8","unresolved":false,"context_lines":[{"line_number":1082,"context_line":"                                utils.convert_str(rbd_pool)) as target:"},{"line_number":1083,"context_line":""},{"line_number":1084,"context_line":"            if self._get_fsid() !\u003d target.client.get_fsid():"},{"line_number":1085,"context_line":"                LOG.debug(\u0027Migration between clusters is not supported. \u0027"},{"line_number":1086,"context_line":"                          \u0027Falling back to generic migration.\u0027)"},{"line_number":1087,"context_line":"                return refuse_to_migrate"},{"line_number":1088,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_3800da65","line":1085,"range":{"start_line":1085,"start_character":37,"end_line":1085,"end_character":38},"updated":"2016-11-03 17:57:04.000000000","message":"IMO, it would be helpful to have this log record in INFO level","commit_id":"c05c071589ff3c3f0254fdc36d634c2e01de43fc"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"39fa07803ddca6f3f48a6adadd33ca61410dd3d8","unresolved":false,"context_lines":[{"line_number":1100,"context_line":"        except Exception:"},{"line_number":1101,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1102,"context_line":"                ctxt.reraise \u003d False"},{"line_number":1103,"context_line":"                LOG.warning(_LW(\u0027Failed to delete migration source volume\u0027))"},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba5da102_1834f647","line":1103,"updated":"2016-11-03 17:57:04.000000000","message":"Please, add volume ID to the log message","commit_id":"c05c071589ff3c3f0254fdc36d634c2e01de43fc"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"5e003ab1aeacb8753c536f252cc4f843e830eb8b","unresolved":false,"context_lines":[{"line_number":297,"context_line":"                        v.diff_iterate(0, v.size(), None, self._iterate_cb)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def _update_volume_stats(self):"},{"line_number":300,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":301,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":302,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":303,"context_line":"                                         self.configuration.rbd_pool)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a629dbe_13abc875","line":300,"range":{"start_line":300,"start_character":25,"end_line":300,"end_character":36},"updated":"2016-11-07 20:18:01.000000000","message":"why not \u0027:\u0027.join(...) ?","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":22998,"name":"XueFengLiu","email":"liu.xuefeng1@zte.com.cn","username":"sgfeng"},"change_message_id":"24cfb866efe446af740bc083348cf71ebd0041cc","unresolved":false,"context_lines":[{"line_number":1054,"context_line":""},{"line_number":1055,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1056,"context_line":""},{"line_number":1057,"context_line":"        if volume[\u0027status\u0027] not in (\u0027available\u0027, \u0027retyping\u0027, \u0027maintenance\u0027):"},{"line_number":1058,"context_line":"            LOG.debug(\u0027Only available volumes can be migrated using backend \u0027"},{"line_number":1059,"context_line":"                      \u0027assisted migration. Falling back to generic migration.\u0027)"},{"line_number":1060,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a629dbe_e3b5b0d0","line":1057,"range":{"start_line":1057,"start_character":36,"end_line":1057,"end_character":74},"updated":"2016-11-07 16:11:40.000000000","message":"Whether these can use const?","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":22998,"name":"XueFengLiu","email":"liu.xuefeng1@zte.com.cn","username":"sgfeng"},"change_message_id":"24cfb866efe446af740bc083348cf71ebd0041cc","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1056,"context_line":""},{"line_number":1057,"context_line":"        if volume[\u0027status\u0027] not in (\u0027available\u0027, \u0027retyping\u0027, \u0027maintenance\u0027):"},{"line_number":1058,"context_line":"            LOG.debug(\u0027Only available volumes can be migrated using backend \u0027"},{"line_number":1059,"context_line":"                      \u0027assisted migration. Falling back to generic migration.\u0027)"},{"line_number":1060,"context_line":"            return refuse_to_migrate"},{"line_number":1061,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"9a629dbe_0330e455","line":1058,"range":{"start_line":1058,"start_character":28,"end_line":1058,"end_character":45},"updated":"2016-11-07 16:11:40.000000000","message":"Give \u0027available\u0027, \u0027retyping\u0027, \u0027maintenance\u0027 a group name?","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"9da241ecc9a2e1efcd1d34ddd020c6d4f21ac182","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1056,"context_line":""},{"line_number":1057,"context_line":"        if volume[\u0027status\u0027] not in (\u0027available\u0027, \u0027retyping\u0027, \u0027maintenance\u0027):"},{"line_number":1058,"context_line":"            LOG.debug(\u0027Only available volumes can be migrated using backend \u0027"},{"line_number":1059,"context_line":"                      \u0027assisted migration. Falling back to generic migration.\u0027)"},{"line_number":1060,"context_line":"            return refuse_to_migrate"},{"line_number":1061,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a71b18c_f85cb71a","line":1058,"in_reply_to":"9a629dbe_0330e455","updated":"2016-12-06 16:02:06.000000000","message":"I could but I dont see much value in it.  If you can provide an example I\u0027ll be happy to reconsider.","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"5e003ab1aeacb8753c536f252cc4f843e830eb8b","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"            return refuse_to_migrate"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        try:"},{"line_number":1078,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_user, rbd_pool) \u003d \\"},{"line_number":1079,"context_line":"                loc_info.split(\u0027:\u0027)"},{"line_number":1080,"context_line":"        except ValueError:"},{"line_number":1081,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"},{"line_number":1082,"context_line":"                          \u0027migration not in correct format: %s. Falling back \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a629dbe_f61d6267","line":1079,"range":{"start_line":1078,"start_character":12,"end_line":1079,"end_character":35},"updated":"2016-11-07 20:18:01.000000000","message":"to avoid \\ you could split line like this:\n\n(rbd_cluster_name, rbd_ceph_conf, \n rbd_user, rbd_pool) \u003d loc_info.split(\u0027:\u0027)\n\nor use parenthesis around whole expression and format it however you like.","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"9da241ecc9a2e1efcd1d34ddd020c6d4f21ac182","unresolved":false,"context_lines":[{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"        try:"},{"line_number":1078,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_user, rbd_pool) \u003d \\"},{"line_number":1079,"context_line":"                loc_info.split(\u0027:\u0027)"},{"line_number":1080,"context_line":"        except ValueError:"},{"line_number":1081,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"},{"line_number":1082,"context_line":"                          \u0027migration not in correct format: %s. Falling back \u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a71b18c_b8db1f7b","line":1079,"in_reply_to":"9a629dbe_f61d6267","updated":"2016-12-06 16:02:06.000000000","message":"Yes, the assumption is that a backslash is undesierable.  Is that the case?","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"5e003ab1aeacb8753c536f252cc4f843e830eb8b","unresolved":false,"context_lines":[{"line_number":1103,"context_line":"        try:"},{"line_number":1104,"context_line":"            self.delete_volume(volume)"},{"line_number":1105,"context_line":"        except Exception:"},{"line_number":1106,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1107,"context_line":"                ctxt.reraise \u003d False"},{"line_number":1108,"context_line":"                LOG.warning(_LW(\u0027Failed to delete migration source volume %s\u0027),"},{"line_number":1109,"context_line":"                            volume.id)"},{"line_number":1110,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a629dbe_760f92e1","line":1107,"range":{"start_line":1106,"start_character":12,"end_line":1107,"end_character":36},"updated":"2016-11-07 20:18:01.000000000","message":"What is the point of using save_and_reraise_exception when exception is not reraised?","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"5e003ab1aeacb8753c536f252cc4f843e830eb8b","unresolved":false,"context_lines":[{"line_number":1105,"context_line":"        except Exception:"},{"line_number":1106,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1107,"context_line":"                ctxt.reraise \u003d False"},{"line_number":1108,"context_line":"                LOG.warning(_LW(\u0027Failed to delete migration source volume %s\u0027),"},{"line_number":1109,"context_line":"                            volume.id)"},{"line_number":1110,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a629dbe_1601f6ae","line":1108,"updated":"2016-11-07 20:18:01.000000000","message":"nit: Dot at the and.","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7be7ff24b592cb4a3d81690a0103fe96a38862c2","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                        v.diff_iterate(0, v.size(), None, self._iterate_cb)"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    def _update_volume_stats(self):"},{"line_number":379,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":380,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":381,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":382,"context_line":"                                         self.configuration.rbd_pool)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a3c09a3_0a3ffeb4","line":379,"updated":"2017-01-13 21:52:21.000000000","message":"If done this way, the driver will need to complain loudly if rbd_ceph_conf has a \":\" in its path.  (I\u0027m not sure if any of the other fields could contain a colon.)","commit_id":"2aa53633b4f472232ba27e4df1f3ac8f88c0b18b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":373,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":374,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":375,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":376,"context_line":"                                         self.configuration.rbd_pool)"},{"line_number":377,"context_line":"        stats \u003d {"},{"line_number":378,"context_line":"            \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":379,"context_line":"            \u0027driver_version\u0027: self.VERSION,"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_db1e386f","line":376,"updated":"2017-01-30 15:20:42.000000000","message":"-1: You need to pass the fsid so it can be checked, otherwise we could be moving it to where we don\u0027t want to.\n\nIf we have 2 servers running Cinder with different Ceph clusters but with the same string values of cluster_name, ceph_conf and rbd_user, and pool, then everything could match in your checks of migrate_volume and think we are moving it where we were asked, when in fact we aren\u0027t...","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"b4c53799a3cb11caeaba81e68f8872df2c297b0a","unresolved":false,"context_lines":[{"line_number":373,"context_line":"        location_info \u003d \u0027%s:%s:%s:%s\u0027 % (self.configuration.rbd_cluster_name,"},{"line_number":374,"context_line":"                                         self.configuration.rbd_ceph_conf,"},{"line_number":375,"context_line":"                                         self.configuration.rbd_user,"},{"line_number":376,"context_line":"                                         self.configuration.rbd_pool)"},{"line_number":377,"context_line":"        stats \u003d {"},{"line_number":378,"context_line":"            \u0027vendor_name\u0027: \u0027Open Source\u0027,"},{"line_number":379,"context_line":"            \u0027driver_version\u0027: self.VERSION,"}],"source_content_type":"text/x-python","patch_set":16,"id":"ba2be162_3e5d1649","line":376,"in_reply_to":"3a461143_db1e386f","updated":"2017-03-06 21:05:06.000000000","message":"I check for this in migrate_volume(), see below.","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"b2cf9adffe457fe7ad44d386c31e4d74c9205a20","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"        return {\u0027_name_id\u0027: name_id, \u0027provider_location\u0027: provider_location}"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"    def migrate_volume(self, context, volume, host):"},{"line_number":1334,"context_line":"        LOG.info(_LI(\u0027Attempting RBD enabled volume migration. \u0027"},{"line_number":1335,"context_line":"                     \u0027volume: %(id)s, host: %(host)s, \u0027"},{"line_number":1336,"context_line":"                     \u0027status\u003d%(status)s.\u0027),"},{"line_number":1337,"context_line":"                 {\u0027id\u0027: volume.id,"},{"line_number":1338,"context_line":"                  \u0027host\u0027: host,"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a3905b3_36dc7545","line":1335,"range":{"start_line":1334,"start_character":22,"end_line":1335,"end_character":53},"updated":"2017-01-24 12:12:51.000000000","message":"nit: \"Attempting RBD enabled volume migration. volume: %(id)s, src host: %(src_host)s -\u003e dst host %(dst_host)s\" ...","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1333,"context_line":"    def migrate_volume(self, context, volume, host):"},{"line_number":1334,"context_line":"        LOG.info(_LI(\u0027Attempting RBD enabled volume migration. \u0027"},{"line_number":1335,"context_line":"                     \u0027volume: %(id)s, host: %(host)s, \u0027"},{"line_number":1336,"context_line":"                     \u0027status\u003d%(status)s.\u0027),"},{"line_number":1337,"context_line":"                 {\u0027id\u0027: volume.id,"},{"line_number":1338,"context_line":"                  \u0027host\u0027: host,"},{"line_number":1339,"context_line":"                  \u0027status\u0027: volume.status})"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_607593f8","line":1336,"updated":"2017-01-30 15:20:42.000000000","message":"?: Is \"enabled\" volume migration or \"assisted\" volume migration?","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1342,"context_line":""},{"line_number":1343,"context_line":"        if volume[\u0027status\u0027] not in (\u0027available\u0027, \u0027retyping\u0027, \u0027maintenance\u0027):"},{"line_number":1344,"context_line":"            LOG.debug(\u0027Only available volumes can be migrated using backend \u0027"},{"line_number":1345,"context_line":"                      \u0027assisted migration. Falling back to generic migration.\u0027)"},{"line_number":1346,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_7db45c34","line":1343,"updated":"2017-01-30 15:20:42.000000000","message":"-1: volume is an OVO instance, please use attribute notation `volume.status` since we don\u0027t allow new code to use dictionary notation on OVO instances.","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1345,"context_line":"                      \u0027assisted migration. Falling back to generic migration.\u0027)"},{"line_number":1346,"context_line":"            return refuse_to_migrate"},{"line_number":1347,"context_line":""},{"line_number":1348,"context_line":"        if (host[\u0027capabilities\u0027][\u0027vendor_name\u0027] !\u003d \u0027Open Source\u0027 or"},{"line_number":1349,"context_line":"                host[\u0027capabilities\u0027][\u0027storage_protocol\u0027] !\u003d \u0027ceph\u0027):"},{"line_number":1350,"context_line":"            LOG.debug(\u0027Source and destination drivers need to be RBD \u0027"},{"line_number":1351,"context_line":"                      \u0027to use backend assisted migration. Falling back to \u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_dd571058","line":1348,"range":{"start_line":1348,"start_character":13,"end_line":1348,"end_character":64},"updated":"2017-01-30 15:20:42.000000000","message":"?: Is it necessary to check the vendor name if we are checking the storage_protocol?  I\u0027m OK with doing it for the sake completeness.","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1367,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"},{"line_number":1368,"context_line":"                          \u0027migration not in correct format: %s. Falling back \u0027"},{"line_number":1369,"context_line":"                          \u0027to generic volume migration.\u0027), loc_info)"},{"line_number":1370,"context_line":"            return refuse_to_migrate"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        with linuxrbd.RBDClient(utils.convert_str(rbd_user),"},{"line_number":1373,"context_line":"                                utils.convert_str(rbd_pool)) as target:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_7d537c39","line":1370,"updated":"2017-01-30 15:20:42.000000000","message":"nit: Would it be interesting to move code above to a method that does the checks and returns the loc_info tuple and if it\u0027s None we return?","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1370,"context_line":"            return refuse_to_migrate"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        with linuxrbd.RBDClient(utils.convert_str(rbd_user),"},{"line_number":1373,"context_line":"                                utils.convert_str(rbd_pool)) as target:"},{"line_number":1374,"context_line":""},{"line_number":1375,"context_line":"            if self._get_fsid() !\u003d target.client.get_fsid():"},{"line_number":1376,"context_line":"                LOG.info(_LI(\u0027Migration between clusters is not supported. \u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_a083fb54","line":1373,"updated":"2017-01-30 15:20:42.000000000","message":"-1: This is missing the conffile and rbd_cluster_name parameters.","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1382,"context_line":"                    source.copy(target.ioctx, volume.name)"},{"line_number":1383,"context_line":"                except Exception:"},{"line_number":1384,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":1385,"context_line":"                        LOG.error(_LE(\u0027Error copying rbd image %s to target.\u0027),"},{"line_number":1386,"context_line":"                                  volume.name)"},{"line_number":1387,"context_line":"                        self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1388,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_20746b66","line":1385,"updated":"2017-01-30 15:20:42.000000000","message":"nit: maybe report the target pool name?","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1389,"context_line":"        try:"},{"line_number":1390,"context_line":"            self.delete_volume(volume)"},{"line_number":1391,"context_line":"        except Exception:"},{"line_number":1392,"context_line":"            LOG.warning(_LW(\u0027Failed to delete migration source volume %s.\u0027),"},{"line_number":1393,"context_line":"                        volume.id)"},{"line_number":1394,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_009ccf5b","line":1392,"range":{"start_line":1392,"start_character":16,"end_line":1392,"end_character":23},"updated":"2017-01-30 15:20:42.000000000","message":"-1: This should be logged as an error.","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e07b6b886012a339215db785c3f58bf0032654a4","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"            self.delete_volume(volume)"},{"line_number":1391,"context_line":"        except Exception:"},{"line_number":1392,"context_line":"            LOG.warning(_LW(\u0027Failed to delete migration source volume %s.\u0027),"},{"line_number":1393,"context_line":"                        volume.id)"},{"line_number":1394,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a461143_c0e3a7d7","line":1393,"updated":"2017-01-30 15:20:42.000000000","message":"-1: If we proceed like this we have a problem, because we haven\u0027t delete the source volume but we are going to lose any reference to it in Cinder leaving an \"orphaned\" image in the backend.","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"b4c53799a3cb11caeaba81e68f8872df2c297b0a","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"            self.delete_volume(volume)"},{"line_number":1391,"context_line":"        except Exception:"},{"line_number":1392,"context_line":"            LOG.warning(_LW(\u0027Failed to delete migration source volume %s.\u0027),"},{"line_number":1393,"context_line":"                        volume.id)"},{"line_number":1394,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":16,"id":"ba2be162_fe10de5b","line":1393,"in_reply_to":"3a461143_c0e3a7d7","updated":"2017-03-06 21:05:06.000000000","message":"What do you suggest?","commit_id":"e34a0969f5e969a55b1e89396de1362580ddef46"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1330,"context_line":"        return {\u0027_name_id\u0027: name_id, \u0027provider_location\u0027: provider_location}"},{"line_number":1331,"context_line":""},{"line_number":1332,"context_line":"    def migrate_volume(self, context, volume, host):"},{"line_number":1333,"context_line":"        LOG.info(_LI(\u0027Attempting RBD assisted volume migration. \u0027"},{"line_number":1334,"context_line":"                     \u0027volume: %(id)s, host: %(host)s, \u0027"},{"line_number":1335,"context_line":"                     \u0027status\u003d%(status)s.\u0027),"},{"line_number":1336,"context_line":"                 {\u0027id\u0027: volume.id,"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_4a50e85d","line":1333,"range":{"start_line":1333,"start_character":17,"end_line":1333,"end_character":20},"updated":"2017-06-01 16:07:08.000000000","message":"-1: We no longer do translation of info messages, so this method call should be removed.","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                     \u0027status\u003d%(status)s.\u0027),"},{"line_number":1336,"context_line":"                 {\u0027id\u0027: volume.id,"},{"line_number":1337,"context_line":"                  \u0027host\u0027: host,"},{"line_number":1338,"context_line":"                  \u0027status\u0027: volume.status})"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"        refuse_to_migrate \u003d (False, None)"},{"line_number":1341,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_2de26e30","line":1338,"updated":"2017-06-01 16:07:08.000000000","message":"From a logging perspective I think this could make things weird when reading logs, because we\u0027ll see the info message and if it fails before L1361 we won\u0027t see anything else since the generic migration doesn\u0027t log anything at the info level.  So I think we should probably set this to debug level or move this message to L1354 and set L1356 to be an info level message.","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1359,"context_line":"            return refuse_to_migrate"},{"line_number":1360,"context_line":""},{"line_number":1361,"context_line":"        try:"},{"line_number":1362,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_fsid, rbd_user, rbd_pool) \u003d \\"},{"line_number":1363,"context_line":"                map(utils.convert_str, loc_info.split(\u0027:\u0027))"},{"line_number":1364,"context_line":"        except ValueError:"},{"line_number":1365,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_cd41b20d","line":1362,"range":{"start_line":1362,"start_character":78,"end_line":1362,"end_character":79},"updated":"2017-06-01 16:07:08.000000000","message":"-1: Use parenthesis for the multiline assignment","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1360,"context_line":""},{"line_number":1361,"context_line":"        try:"},{"line_number":1362,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_fsid, rbd_user, rbd_pool) \u003d \\"},{"line_number":1363,"context_line":"                map(utils.convert_str, loc_info.split(\u0027:\u0027))"},{"line_number":1364,"context_line":"        except ValueError:"},{"line_number":1365,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"},{"line_number":1366,"context_line":"                          \u0027migration not in correct format: %s. Falling back \u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_edbb56c1","line":1363,"range":{"start_line":1363,"start_character":16,"end_line":1363,"end_character":59},"updated":"2017-06-01 16:07:08.000000000","message":"-1: If I understand this correctly this should be changed, as you could be trying to split a bytes object using a string as the argument, which will raise a TypeError because a bytes object requires a bytes argument.\n\nI believe this would be solved like this:\n\n utils.convert_str(loc_info).split(\u0027:\u0027)","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1362,"context_line":"            (rbd_cluster_name, rbd_ceph_conf, rbd_fsid, rbd_user, rbd_pool) \u003d \\"},{"line_number":1363,"context_line":"                map(utils.convert_str, loc_info.split(\u0027:\u0027))"},{"line_number":1364,"context_line":"        except ValueError:"},{"line_number":1365,"context_line":"            LOG.error(_LE(\u0027Location info needed for backend enabled volume \u0027"},{"line_number":1366,"context_line":"                          \u0027migration not in correct format: %s. Falling back \u0027"},{"line_number":1367,"context_line":"                          \u0027to generic volume migration.\u0027), loc_info)"},{"line_number":1368,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_6d462624","line":1365,"range":{"start_line":1365,"start_character":22,"end_line":1365,"end_character":25},"updated":"2017-06-01 16:07:08.000000000","message":"-1: Remove _LE","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"                                rbd_cluster_name\u003drbd_cluster_name) as target:"},{"line_number":1372,"context_line":"            if ((rbd_fsid !\u003d self._get_fsid() or"},{"line_number":1373,"context_line":"                 rbd_fsid !\u003d target.client.get_fsid())):"},{"line_number":1374,"context_line":"                LOG.info(_LI(\u0027Migration between clusters is not supported. \u0027"},{"line_number":1375,"context_line":"                             \u0027Falling back to generic migration.\u0027))"},{"line_number":1376,"context_line":"                return refuse_to_migrate"},{"line_number":1377,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_0a1b1081","line":1374,"range":{"start_line":1374,"start_character":25,"end_line":1374,"end_character":28},"updated":"2017-06-01 16:07:08.000000000","message":"-1: Remove _LI call","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1380,"context_line":"                    source.copy(target.ioctx, volume.name)"},{"line_number":1381,"context_line":"                except Exception:"},{"line_number":1382,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":1383,"context_line":"                        LOG.error(_LE(\u0027Error copying rbd image %(vol)s to \u0027"},{"line_number":1384,"context_line":"                                      \u0027target pool %(pool)s.\u0027),"},{"line_number":1385,"context_line":"                                  {\u0027vol\u0027: volume.name,"},{"line_number":1386,"context_line":"                                   \u0027pool\u0027: rbd_pool})"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_0d2c4a00","line":1383,"range":{"start_line":1383,"start_character":34,"end_line":1383,"end_character":37},"updated":"2017-06-01 16:07:08.000000000","message":"-1: Remove _LE","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                                      \u0027target pool %(pool)s.\u0027),"},{"line_number":1385,"context_line":"                                  {\u0027vol\u0027: volume.name,"},{"line_number":1386,"context_line":"                                   \u0027pool\u0027: rbd_pool})"},{"line_number":1387,"context_line":"                        self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"        try:"},{"line_number":1390,"context_line":"            self.delete_volume(volume)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_8d0e1a39","line":1387,"updated":"2017-06-01 16:07:08.000000000","message":"-1: Missing indentation here, this should go inside save_and_reraise_exception  (I see now that I wrote the comment on patch 6 at the wrong line)","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1389,"context_line":"        try:"},{"line_number":1390,"context_line":"            self.delete_volume(volume)"},{"line_number":1391,"context_line":"        except Exception:"},{"line_number":1392,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1393,"context_line":"                LOG.error(_LE(\u0027Failed to delete migration source volume %s.\u0027),"},{"line_number":1394,"context_line":"                          volume.id)"},{"line_number":1395,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_2d8eeecf","line":1392,"updated":"2017-06-01 16:07:08.000000000","message":"nit: A simple raise after L1393 would be enough, and it would also be faster","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"04a9950719bf411055b46dd0980dfc7ac5326a4e","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"            self.delete_volume(volume)"},{"line_number":1391,"context_line":"        except Exception:"},{"line_number":1392,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1393,"context_line":"                LOG.error(_LE(\u0027Failed to delete migration source volume %s.\u0027),"},{"line_number":1394,"context_line":"                          volume.id)"},{"line_number":1395,"context_line":""},{"line_number":1396,"context_line":"        return (True, None)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df140735_edc116e6","line":1393,"updated":"2017-06-01 16:07:08.000000000","message":"?: LOG.exception or add the exception converted to a string on the error","commit_id":"aae0b3aeb4cf555e9ac63e9136f264f43c7bc572"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"c178632a7eb4a29968759d3efa4b0fc2baf8bebc","unresolved":false,"context_lines":[{"line_number":1352,"context_line":"                      \u0027generic migration.\u0027)"},{"line_number":1353,"context_line":"            return refuse_to_migrate"},{"line_number":1354,"context_line":""},{"line_number":1355,"context_line":"        loc_info \u003d host[\u0027capabilities\u0027].get(\u0027location_info\u0027)"},{"line_number":1356,"context_line":""},{"line_number":1357,"context_line":"        LOG.debug(\u0027Attempting RBD assisted volume migration. volume: %(id)s, \u0027"},{"line_number":1358,"context_line":"                  \u0027host: %(host)s, status\u003d%(status)s.\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"ff346bd7_57b80229","line":1355,"range":{"start_line":1355,"start_character":8,"end_line":1355,"end_character":60},"updated":"2017-07-24 08:54:53.000000000","message":"This could be putted at #1360,  that will make code more clear.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"c178632a7eb4a29968759d3efa4b0fc2baf8bebc","unresolved":false,"context_lines":[{"line_number":1359,"context_line":"                  {\u0027id\u0027: volume.id, \u0027host\u0027: host, \u0027status\u0027: volume.status})"},{"line_number":1360,"context_line":""},{"line_number":1361,"context_line":"        if not loc_info:"},{"line_number":1362,"context_line":"            LOG.info(\u0027Could not find location_info in capabilities reported \u0027"},{"line_number":1363,"context_line":"                     \u0027by the destination driver. Falling back to generic \u0027"},{"line_number":1364,"context_line":"                     \u0027migration.\u0027)"},{"line_number":1365,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":19,"id":"ff346bd7_57912299","line":1362,"range":{"start_line":1362,"start_character":16,"end_line":1362,"end_character":20},"updated":"2017-07-24 08:54:53.000000000","message":"debug is better?","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b40798b3cd7d4259c6c51c879a7a956a841a6639","unresolved":false,"context_lines":[{"line_number":1359,"context_line":"                  {\u0027id\u0027: volume.id, \u0027host\u0027: host, \u0027status\u0027: volume.status})"},{"line_number":1360,"context_line":""},{"line_number":1361,"context_line":"        if not loc_info:"},{"line_number":1362,"context_line":"            LOG.info(\u0027Could not find location_info in capabilities reported \u0027"},{"line_number":1363,"context_line":"                     \u0027by the destination driver. Falling back to generic \u0027"},{"line_number":1364,"context_line":"                     \u0027migration.\u0027)"},{"line_number":1365,"context_line":"            return refuse_to_migrate"}],"source_content_type":"text/x-python","patch_set":19,"id":"7f287b81_9a9ba015","line":1362,"range":{"start_line":1362,"start_character":16,"end_line":1362,"end_character":20},"in_reply_to":"ff346bd7_57912299","updated":"2017-09-08 11:36:43.000000000","message":"I think info is appropriate, but now that we can dynamically change the log levels it would be OK if this was changed to debug.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"c178632a7eb4a29968759d3efa4b0fc2baf8bebc","unresolved":false,"context_lines":[{"line_number":1377,"context_line":"                                rbd_cluster_name\u003drbd_cluster_name) as target:"},{"line_number":1378,"context_line":"            if ((rbd_fsid !\u003d self._get_fsid() or"},{"line_number":1379,"context_line":"                 rbd_fsid !\u003d target.client.get_fsid())):"},{"line_number":1380,"context_line":"                LOG.info(\u0027Migration between clusters is not supported. \u0027"},{"line_number":1381,"context_line":"                         \u0027Falling back to generic migration.\u0027)"},{"line_number":1382,"context_line":"                return refuse_to_migrate"},{"line_number":1383,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ff346bd7_17e95a19","line":1380,"range":{"start_line":1380,"start_character":20,"end_line":1380,"end_character":24},"updated":"2017-07-24 08:54:53.000000000","message":"Should be error.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b40798b3cd7d4259c6c51c879a7a956a841a6639","unresolved":false,"context_lines":[{"line_number":1377,"context_line":"                                rbd_cluster_name\u003drbd_cluster_name) as target:"},{"line_number":1378,"context_line":"            if ((rbd_fsid !\u003d self._get_fsid() or"},{"line_number":1379,"context_line":"                 rbd_fsid !\u003d target.client.get_fsid())):"},{"line_number":1380,"context_line":"                LOG.info(\u0027Migration between clusters is not supported. \u0027"},{"line_number":1381,"context_line":"                         \u0027Falling back to generic migration.\u0027)"},{"line_number":1382,"context_line":"                return refuse_to_migrate"},{"line_number":1383,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7f287b81_3a86ecba","line":1380,"range":{"start_line":1380,"start_character":20,"end_line":1380,"end_character":24},"in_reply_to":"ff346bd7_17e95a19","updated":"2017-09-08 11:36:43.000000000","message":"No, it\u0027s not an error, because unlike on L1371 where that is something that should have happened here it just means that there\u0027s a limitation on where it\u0027s possible to do the optimized mechanism.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"c178632a7eb4a29968759d3efa4b0fc2baf8bebc","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"            reason \u003d \u0027Failed to delete migration source volume %s.\u0027, volume.id"},{"line_number":1398,"context_line":"            raise exception.VolumeMigrationFailed(reason\u003dreason)"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"        LOG.debug(\u0027Successful RBD assisted volume migration.\u0027)"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"        return (True, None)"},{"line_number":1403,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ff346bd7_97dc2a75","line":1400,"range":{"start_line":1400,"start_character":12,"end_line":1400,"end_character":17},"updated":"2017-07-24 08:54:53.000000000","message":"Info is better I think.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b40798b3cd7d4259c6c51c879a7a956a841a6639","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"            reason \u003d \u0027Failed to delete migration source volume %s.\u0027, volume.id"},{"line_number":1398,"context_line":"            raise exception.VolumeMigrationFailed(reason\u003dreason)"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"        LOG.debug(\u0027Successful RBD assisted volume migration.\u0027)"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"        return (True, None)"},{"line_number":1403,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7f287b81_1a8fb0cd","line":1400,"range":{"start_line":1400,"start_character":12,"end_line":1400,"end_character":17},"in_reply_to":"ff346bd7_97dc2a75","updated":"2017-09-08 11:36:43.000000000","message":"I agree.","commit_id":"08e2859526d4ab735c18f8080373b458ed952c62"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"42def8b4c09ad3d2ce4b751d648ddf103773a315","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from cinder.volume import configuration"},{"line_number":38,"context_line":"from cinder.volume import driver"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"from os_brick.initiator import linuxrbd"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"try:"},{"line_number":43,"context_line":"    import rados"}],"source_content_type":"text/x-python","patch_set":21,"id":"ff82abbf_68b1b08f","line":40,"updated":"2017-11-21 11:09:53.000000000","message":"This needs to move up into the third party imports group.","commit_id":"9b510cad62302a9ec087b96bc13ace7bb0225f2a"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"70db829d815e253c5c9632ebc256e906e0b5261a","unresolved":false,"context_lines":[{"line_number":1481,"context_line":"                        self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1482,"context_line":""},{"line_number":1483,"context_line":"        try:"},{"line_number":1484,"context_line":"            self.delete_volume(volume)"},{"line_number":1485,"context_line":"        except Exception:"},{"line_number":1486,"context_line":"            reason \u003d \u0027Failed to delete migration source volume %s.\u0027, volume.id"},{"line_number":1487,"context_line":"            raise exception.VolumeMigrationFailed(reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":21,"id":"1f485f77_e3bf9c5e","line":1484,"range":{"start_line":1484,"start_character":12,"end_line":1484,"end_character":38},"updated":"2017-11-16 23:21:57.000000000","message":"should there be any cleanup of the destination in this case or just let it roll and require an admin to clean up the source?","commit_id":"9b510cad62302a9ec087b96bc13ace7bb0225f2a"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"3d2e377c8e552cc662116293b8a04ac255722154","unresolved":false,"context_lines":[{"line_number":1481,"context_line":"                        self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1482,"context_line":""},{"line_number":1483,"context_line":"        try:"},{"line_number":1484,"context_line":"            self.delete_volume(volume)"},{"line_number":1485,"context_line":"        except Exception:"},{"line_number":1486,"context_line":"            reason \u003d \u0027Failed to delete migration source volume %s.\u0027, volume.id"},{"line_number":1487,"context_line":"            raise exception.VolumeMigrationFailed(reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":21,"id":"ff82abbf_57f81dc3","line":1484,"range":{"start_line":1484,"start_character":12,"end_line":1484,"end_character":38},"in_reply_to":"1f485f77_e3bf9c5e","updated":"2017-11-20 20:48:34.000000000","message":"If the source fails to delete for some reason, I wanted to leave the destination in place in case deleting it might cause a lose of data.  In general, I don\u0027t want to automate the destruction of a volume unless I\u0027m sure that\u0027s what the user wants and I don\u0027t think I can be sure of that in this context.  So my thought was to leave it for the admin to cleanup.  Is that reasonable?","commit_id":"9b510cad62302a9ec087b96bc13ace7bb0225f2a"},{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"60d94adeab48b1ef78d0c76f4fd80467baa69bbc","unresolved":false,"context_lines":[{"line_number":1481,"context_line":"                        self.RBDProxy().remove(target.ioctx, volume.name)"},{"line_number":1482,"context_line":""},{"line_number":1483,"context_line":"        try:"},{"line_number":1484,"context_line":"            self.delete_volume(volume)"},{"line_number":1485,"context_line":"        except Exception:"},{"line_number":1486,"context_line":"            reason \u003d \u0027Failed to delete migration source volume %s.\u0027, volume.id"},{"line_number":1487,"context_line":"            raise exception.VolumeMigrationFailed(reason\u003dreason)"}],"source_content_type":"text/x-python","patch_set":21,"id":"ff82abbf_f2eb3fd9","line":1484,"range":{"start_line":1484,"start_character":12,"end_line":1484,"end_character":38},"in_reply_to":"ff82abbf_57f81dc3","updated":"2017-11-22 18:03:46.000000000","message":"@Jon\nYup, I think that\u0027s fine.  Would you do me a huge favor though and add a comment that that\u0027s what you\u0027re doing here if you get a chance?  Just for future clarity because I won\u0027t remember this conversation in 2 weeks :)","commit_id":"9b510cad62302a9ec087b96bc13ace7bb0225f2a"}],"cinder/volume/manager.py":[{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"50ae0037aef2b3441c4239dcd639579e842f6dcf","unresolved":false,"context_lines":[{"line_number":1946,"context_line":""},{"line_number":1947,"context_line":"        volume.migration_status \u003d \u0027migrating\u0027"},{"line_number":1948,"context_line":"        volume.save()"},{"line_number":1949,"context_line":"        if not force_host_copy:"},{"line_number":1950,"context_line":"            try:"},{"line_number":1951,"context_line":"                LOG.debug(\"Issue driver.migrate_volume.\", resource\u003dvolume)"},{"line_number":1952,"context_line":"                moved, model_update \u003d self.driver.migrate_volume(ctxt,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a18252c_c33670e6","line":1949,"updated":"2016-04-13 18:57:54.000000000","message":"This is a problem, I\u0027m thinking about it now and will post a solution once I\u0027ve found a proper approach.","commit_id":"ad83832a7c1f165d33bfc060fa73006ad8af6769"}],"releasenotes/notes/rbd-driver-assisted-migration-2d29788243060f77.yaml":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e27ac93fc110c04223ef5e8ab2b453608b21e4d4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Added driver-assisted volume migration to RBD driver."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"bab6814e_e05aa029","line":3,"updated":"2016-05-19 09:33:23.000000000","message":"nit: I think you could mention that this means efficient migration between hosts using the same RBD cluster.","commit_id":"16ec1ce543fcddd1495045000ecc466d89de736d"},{"author":{"_account_id":22998,"name":"XueFengLiu","email":"liu.xuefeng1@zte.com.cn","username":"sgfeng"},"change_message_id":"24cfb866efe446af740bc083348cf71ebd0041cc","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Added driver-assisted volume migration to RBD driver.  This allows a"},{"line_number":4,"context_line":"    volume to be efficiently copied by Ceph from one pool to another"},{"line_number":5,"context_line":"    within the same cluster."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"9a629dbe_8365d4e2","line":3,"range":{"start_line":3,"start_character":58,"end_line":3,"end_character":59},"updated":"2016-11-07 16:11:40.000000000","message":"extra space","commit_id":"8fcd815302cbe0fb3c60c4a48c7af1a8f69c3109"}]}
