)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4b802ddc2a5ab3cc12ef7dc8384adf404729e711","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ddc979dc_de9008c1","updated":"2025-07-28 15:46:27.000000000","message":"Change looks good, except maybe it should be called in a try block?  See comment inline.","commit_id":"b51486cf5dc9ba27ab3ef428e0838e1fefd908fb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"9b0f56b1d8d13ff5196223ee23fe087a972c02e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9af852ee_7edfc765","updated":"2025-07-21 17:26:26.000000000","message":"Thanks Eric, makes sense to keep it out of client block.","commit_id":"b51486cf5dc9ba27ab3ef428e0838e1fefd908fb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9e4e42dbc34fb166d99324563a73fff74feaf491","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"efb40b66_76b524b0","updated":"2025-07-28 17:31:00.000000000","message":"Code and tests LGTM.","commit_id":"eabae5cd404f1651852e7073cc1b5b7b68d3eb30"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ca21a96ef74140bcd023553a798f194f3785d9ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6dcefa6d_fa9f86e0","updated":"2025-07-28 16:27:05.000000000","message":"Thanks Brian, updated the logic and added a UT to validate it.","commit_id":"eabae5cd404f1651852e7073cc1b5b7b68d3eb30"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ce6f3f579f28548d763c1e1c303ea3f6c29f97b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"74d42e9e_15cfd57a","updated":"2025-07-28 19:44:40.000000000","message":"recheck tempest-integrated-storage-ubuntu-jammy failed with unrelated test\n\ntempest.api.compute.servers.test_server_actions.ServerActionsTestOtherA","commit_id":"eabae5cd404f1651852e7073cc1b5b7b68d3eb30"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e8936d61c70fd696f26900ea42365771fc153344","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"eb3ba316_1a4f8db6","updated":"2025-07-28 20:56:53.000000000","message":"This is definitely better, but see comment inline about the unit test.","commit_id":"6cc434b1af5dbb9147050a123e65015b7cc3f2b2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f7e78a434fa940fea7c4af456ccee8ef11326edb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"75e95fb5_0a3d7d32","updated":"2025-07-29 11:43:24.000000000","message":"Revisions LGTM.","commit_id":"dc8a59c918981cd94c9e80eab299d93079bdfff6"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b3ea61539ab8a585daeef3d16aef53169a28e4fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6437ae91_378d2200","updated":"2025-07-29 09:39:00.000000000","message":"Thanks Brian","commit_id":"dc8a59c918981cd94c9e80eab299d93079bdfff6"}],"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e8936d61c70fd696f26900ea42365771fc153344","unresolved":true,"context_lines":[{"line_number":814,"context_line":"            extra_specs\u003dextra_specs)"},{"line_number":815,"context_line":""},{"line_number":816,"context_line":"        with mock.patch.object(self.driver.rbd.RBD(), \u0027rename\u0027) as \\"},{"line_number":817,"context_line":"                mock_rbd_image_rename:"},{"line_number":818,"context_line":"            exist_volume \u003d \u0027vol-exist\u0027"},{"line_number":819,"context_line":"            existing_ref \u003d {\u0027source-name\u0027: exist_volume}"},{"line_number":820,"context_line":"            mock_rbd_image_rename.return_value \u003d 0"}],"source_content_type":"text/x-python","patch_set":6,"id":"4c541562_93976675","line":817,"updated":"2025-07-28 20:56:53.000000000","message":"This test won\u0027t hit the rename any more, but instead of removing the mock, it might be worth making sure it\u0027s not called, in case someone refactors the code?  Although I guess if someone refactors the code and this test is no longer mocking the rename, this test will break, so maybe the simpler thing to do is just remove the mock.\n\nLet\u0027s see what Eric thinks.","commit_id":"6cc434b1af5dbb9147050a123e65015b7cc3f2b2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b3ea61539ab8a585daeef3d16aef53169a28e4fc","unresolved":false,"context_lines":[{"line_number":814,"context_line":"            extra_specs\u003dextra_specs)"},{"line_number":815,"context_line":""},{"line_number":816,"context_line":"        with mock.patch.object(self.driver.rbd.RBD(), \u0027rename\u0027) as \\"},{"line_number":817,"context_line":"                mock_rbd_image_rename:"},{"line_number":818,"context_line":"            exist_volume \u003d \u0027vol-exist\u0027"},{"line_number":819,"context_line":"            existing_ref \u003d {\u0027source-name\u0027: exist_volume}"},{"line_number":820,"context_line":"            mock_rbd_image_rename.return_value \u003d 0"}],"source_content_type":"text/x-python","patch_set":6,"id":"e50075f9_406cc580","line":817,"in_reply_to":"4c541562_93976675","updated":"2025-07-29 09:39:00.000000000","message":"This is really a good suggestion to ensure that rename was not called and we really failed fast here. Will address it in next PS.","commit_id":"6cc434b1af5dbb9147050a123e65015b7cc3f2b2"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ab239d8b5da7deec535196c77bffec04de583368","unresolved":true,"context_lines":[{"line_number":2161,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":2162,"context_line":"                                   utils.convert_str(rbd_name),"},{"line_number":2163,"context_line":"                                   volume.name)"},{"line_number":2164,"context_line":"            return self._setup_volume(volume)"},{"line_number":2165,"context_line":""},{"line_number":2166,"context_line":"    def manage_existing_get_size(self,"},{"line_number":2167,"context_line":"                                 volume: Volume,"}],"source_content_type":"text/x-python","patch_set":3,"id":"e4fa03ab_73da211f","line":2164,"updated":"2025-07-21 15:36:40.000000000","message":"Since none of the operations inside of _setup_volume() reuse the client from line 2159, I think it would be better to move this outside of the \"with\" block and let that client close rather than calling _setup_volume() with it open.","commit_id":"e2cec379deb9599ea486cce864921c59e8bf5a62"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"9b0f56b1d8d13ff5196223ee23fe087a972c02e6","unresolved":false,"context_lines":[{"line_number":2161,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":2162,"context_line":"                                   utils.convert_str(rbd_name),"},{"line_number":2163,"context_line":"                                   volume.name)"},{"line_number":2164,"context_line":"            return self._setup_volume(volume)"},{"line_number":2165,"context_line":""},{"line_number":2166,"context_line":"    def manage_existing_get_size(self,"},{"line_number":2167,"context_line":"                                 volume: Volume,"}],"source_content_type":"text/x-python","patch_set":3,"id":"baf4fb4f_04a4f6fa","line":2164,"in_reply_to":"e4fa03ab_73da211f","updated":"2025-07-21 17:26:26.000000000","message":"Done","commit_id":"e2cec379deb9599ea486cce864921c59e8bf5a62"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4b802ddc2a5ab3cc12ef7dc8384adf404729e711","unresolved":true,"context_lines":[{"line_number":2161,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":2162,"context_line":"                                   utils.convert_str(rbd_name),"},{"line_number":2163,"context_line":"                                   volume.name)"},{"line_number":2164,"context_line":"        return self._setup_volume(volume)"},{"line_number":2165,"context_line":""},{"line_number":2166,"context_line":"    def manage_existing_get_size(self,"},{"line_number":2167,"context_line":"                                 volume: Volume,"}],"source_content_type":"text/x-python","patch_set":4,"id":"0f5902e7_f7b90942","line":2164,"range":{"start_line":2164,"start_character":15,"end_line":2164,"end_character":41},"updated":"2025-07-28 15:46:27.000000000","message":"This could raise a RBDDriverException at line 1069 due to incompatible properties in the volume type ... given the contract for this method [0], I think we should call this in a try and raise a ManageExistingVolumeTypeMismatch in that case.\n\n[0] https://opendev.org/openstack/cinder/src/commit/f54c9a23cd332aed1d625980b0b19e4e3f41222c/cinder/interface/volume_manageable_driver.py#L62","commit_id":"b51486cf5dc9ba27ab3ef428e0838e1fefd908fb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ca21a96ef74140bcd023553a798f194f3785d9ca","unresolved":false,"context_lines":[{"line_number":2161,"context_line":"            self.RBDProxy().rename(client.ioctx,"},{"line_number":2162,"context_line":"                                   utils.convert_str(rbd_name),"},{"line_number":2163,"context_line":"                                   volume.name)"},{"line_number":2164,"context_line":"        return self._setup_volume(volume)"},{"line_number":2165,"context_line":""},{"line_number":2166,"context_line":"    def manage_existing_get_size(self,"},{"line_number":2167,"context_line":"                                 volume: Volume,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ace421ac_1286060e","line":2164,"range":{"start_line":2164,"start_character":15,"end_line":2164,"end_character":41},"in_reply_to":"0f5902e7_f7b90942","updated":"2025-07-28 16:27:05.000000000","message":"Done","commit_id":"b51486cf5dc9ba27ab3ef428e0838e1fefd908fb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e8936d61c70fd696f26900ea42365771fc153344","unresolved":true,"context_lines":[{"line_number":2166,"context_line":"        # check if the volume type is valid and fail fast if not"},{"line_number":2167,"context_line":"        if not self._is_valid_type(volume.volume_type):"},{"line_number":2168,"context_line":"            msg \u003d _(\u0027Replication and Multiattach are mutually exclusive.\u0027)"},{"line_number":2169,"context_line":"            raise exception.ManageExistingVolumeTypeMismatch(reason\u003dmsg)"},{"line_number":2170,"context_line":"        # Raise an exception if we didn\u0027t find a suitable rbd image."},{"line_number":2171,"context_line":"        with RADOSClient(self) as client:"},{"line_number":2172,"context_line":"            rbd_name \u003d existing_ref[\u0027source-name\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"81b767a2_5b666967","line":2169,"updated":"2025-07-28 20:56:53.000000000","message":"Good catch Rajat to notice the rename at line 2173 ... the try I suggested for this on PS4 would require the name to be changed back if an exception occurred; i agree it\u0027s much better to check the volume type first and fail fast so we can avert any cleanup.","commit_id":"6cc434b1af5dbb9147050a123e65015b7cc3f2b2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b3ea61539ab8a585daeef3d16aef53169a28e4fc","unresolved":false,"context_lines":[{"line_number":2166,"context_line":"        # check if the volume type is valid and fail fast if not"},{"line_number":2167,"context_line":"        if not self._is_valid_type(volume.volume_type):"},{"line_number":2168,"context_line":"            msg \u003d _(\u0027Replication and Multiattach are mutually exclusive.\u0027)"},{"line_number":2169,"context_line":"            raise exception.ManageExistingVolumeTypeMismatch(reason\u003dmsg)"},{"line_number":2170,"context_line":"        # Raise an exception if we didn\u0027t find a suitable rbd image."},{"line_number":2171,"context_line":"        with RADOSClient(self) as client:"},{"line_number":2172,"context_line":"            rbd_name \u003d existing_ref[\u0027source-name\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"d259ec56_70335b6d","line":2169,"in_reply_to":"81b767a2_5b666967","updated":"2025-07-29 09:39:00.000000000","message":"Acknowledged","commit_id":"6cc434b1af5dbb9147050a123e65015b7cc3f2b2"}]}