)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"1e463a455dec116fcbe25935323c3fbefb471f14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"89304136_6878e588","updated":"2024-07-03 13:49:51.000000000","message":"recheck openstacksdk-functional-devstack by Client error","commit_id":"c5b764ca03411e12f4857ee60443d8afc1f2b898"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"29b1980fe64e46a3de3c02839f46b5c238430877","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e5d07eab_3a4ada59","updated":"2024-07-23 15:37:40.000000000","message":"Code looks good, though I have added one comment, because even though I think it\u0027s ok, I cannot be 100% sure.","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0e0438bee1f26d12397fdbfb9ae1ae78a979d5d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"18c71abf_1e2df5e0","updated":"2024-07-25 14:14:03.000000000","message":"My concerns have been addressed.","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"03981521db3b6f7d3c5a1ead07317729b1c7bf00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"cddfb2f3_0f8cf23c","updated":"2024-07-08 07:07:09.000000000","message":"Patchset3 fixes a typo in the release note.\ndirver ~\u003e driver","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"}],"cinder/tests/unit/volume/drivers/hitachi/test_hitachi_hbsd_rest_iscsi.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"dead1d7da4bae7e6c1e1c9f22f32426e2408a863","unresolved":true,"context_lines":[{"line_number":861,"context_line":"    def test_update_migrated_volume(self, request):"},{"line_number":862,"context_line":"        request.return_value \u003d FakeResponse(202, COMPLETED_SUCCEEDED_RESULT)"},{"line_number":863,"context_line":"        self.assertRaises("},{"line_number":864,"context_line":"            NotImplementedError,"},{"line_number":865,"context_line":"            self.driver.update_migrated_volume,"},{"line_number":866,"context_line":"            self.ctxt,"},{"line_number":867,"context_line":"            TEST_VOLUME[0],"}],"source_content_type":"text/x-python","patch_set":3,"id":"11bca392_6c112637","line":864,"updated":"2024-07-23 22:47:50.000000000","message":"This is a really strange way to handle this.  I realize that this is because you\u0027re calling super() on the driver, and the update_migrated_volume method isn\u0027t implemented higher up the chain.  But this means that you are implicitly relying on the volume manager to catch the NotImplementedError, and on what it does when it catches the exception:\nhttps://opendev.org/openstack/cinder/src/commit/834d933fa9fef6630d5df7ae278c6dad6dd82b80/cinder/volume/manager.py#L4400-L4404\n\nI think that instead of relying on the model_update_default, you should explicitly update the model appropriately in your update_migrated_volume() implementation.\n\nI suggest that you file a bug about this \"code smell\" and address it in a followup.","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9bf93a2956f6bd2d70123c54d24bc258aa69823e","unresolved":true,"context_lines":[{"line_number":861,"context_line":"    def test_update_migrated_volume(self, request):"},{"line_number":862,"context_line":"        request.return_value \u003d FakeResponse(202, COMPLETED_SUCCEEDED_RESULT)"},{"line_number":863,"context_line":"        self.assertRaises("},{"line_number":864,"context_line":"            NotImplementedError,"},{"line_number":865,"context_line":"            self.driver.update_migrated_volume,"},{"line_number":866,"context_line":"            self.ctxt,"},{"line_number":867,"context_line":"            TEST_VOLUME[0],"}],"source_content_type":"text/x-python","patch_set":3,"id":"86b6aef8_33c425a0","line":864,"in_reply_to":"11bca392_6c112637","updated":"2024-07-24 10:09:27.000000000","message":"I believe they are removing the call to super on the next patch in the series.","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"c8a7586e44fe1f97130d53a4519dfab7aed452b9","unresolved":false,"context_lines":[{"line_number":861,"context_line":"    def test_update_migrated_volume(self, request):"},{"line_number":862,"context_line":"        request.return_value \u003d FakeResponse(202, COMPLETED_SUCCEEDED_RESULT)"},{"line_number":863,"context_line":"        self.assertRaises("},{"line_number":864,"context_line":"            NotImplementedError,"},{"line_number":865,"context_line":"            self.driver.update_migrated_volume,"},{"line_number":866,"context_line":"            self.ctxt,"},{"line_number":867,"context_line":"            TEST_VOLUME[0],"}],"source_content_type":"text/x-python","patch_set":3,"id":"df92cbcc_c576f99c","line":864,"in_reply_to":"86b6aef8_33c425a0","updated":"2024-07-24 10:56:37.000000000","message":"I agree to fix it and I\u0027ve already fix it in the patch https://review.opendev.org/c/openstack/cinder/+/923618","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"}],"cinder/volume/drivers/hitachi/hbsd_common.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"29b1980fe64e46a3de3c02839f46b5c238430877","unresolved":true,"context_lines":[{"line_number":1107,"context_line":"        ldev \u003d self.get_ldev(new_volume)"},{"line_number":1108,"context_line":"        # We do not need to check if ldev is not None because it is guaranteed"},{"line_number":1109,"context_line":"        # that ldev is not None because migration has been successful so far."},{"line_number":1110,"context_line":"        self.modify_ldev_name(ldev, volume[\u0027id\u0027].replace(\"-\", \"\"))"},{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"},{"line_number":1113,"context_line":"        \"\"\"Retype the specified volume.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"780af16e_80db19f3","line":1110,"updated":"2024-07-23 15:37:40.000000000","message":"?: Wouldn\u0027t it be better to modify the ldev name of both volumes?\nWith proposed code we would have 2 different volumes with the same ldev name.\nThe old volume would have had the name assigned on creation and here we are assigning the same name to the new volume.","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"c8a7586e44fe1f97130d53a4519dfab7aed452b9","unresolved":false,"context_lines":[{"line_number":1107,"context_line":"        ldev \u003d self.get_ldev(new_volume)"},{"line_number":1108,"context_line":"        # We do not need to check if ldev is not None because it is guaranteed"},{"line_number":1109,"context_line":"        # that ldev is not None because migration has been successful so far."},{"line_number":1110,"context_line":"        self.modify_ldev_name(ldev, volume[\u0027id\u0027].replace(\"-\", \"\"))"},{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"},{"line_number":1113,"context_line":"        \"\"\"Retype the specified volume.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"cdaa7b20_ef79ebe1","line":1110,"in_reply_to":"780af16e_80db19f3","updated":"2024-07-24 10:56:37.000000000","message":"1. Availability to set same object ID but unique\n\nThe Hitachi storage specification for the maximum length of the LDEV nickname is 32 digit characters, and the length of object ID is 36 digits hex and it contains 4 `-\u0027s.\n\nTo identify the object, all 32 digits hex are required and not required all `-\u0027s.\nOnly Hitachi driver can set the 32 digit hex to LDEV nickname, and it can not to add any other unique value by the limit of the LDEV nickname length.\n\nSo, LDEV nickname cannot be set same object ID but unique\n\n2. Risk when multiple LDEVs has same LDEV nickname value\n\nWhen running the command ``cinder manage --id-type source-name``, LDEV will be specified by the value of the LDEV nickname. The command would be failed, if multiple LDEVs whose LDEV nickname values are same. But the situation is only while the operation ``host-assisted migration``, from updating LDEV nickname by this patch to removing LDEV which is the migration source. Additionally, the command ``unmanage`` can not intercept running ``migration`` command. It means the ``manage`` command must not run while the situation.\nThe cases to refer LDEV nickname are only above ``manage`` command and another patch https://review.opendev.org/c/openstack/cinder/+/923618, which is depended on this patch, so it has no risk by existing multiple LDEVs whose nickname value are same.\n\n3. update LDEV nickname for source of migration\n\nIt could avoid a situation to exist multiple LDEVs whose nickname value are same, if driver set LDEV nickname for migration source as the value for temporary object ID and set LDEV nickname for migration target at same time.\n\nBut driver can not implement to set temporary object ID to LDEV nickname for migration source, because driver method for the backend, where has LDEV for migration source, is not called when running ``host-assisted migration``.","commit_id":"1895845dcf791d37dc765503b0c3300a75b16991"}]}