)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"82630e5d1e5c99b031ad45ca5ae47f0c27168915","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8c7a6536_c9ec9915","updated":"2024-07-23 16:46:41.000000000","message":"The downvote is for updating the \"_name_id\" field.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"99cc7020c7677c4fd1d49eed638916b537d82b9f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a68797b7_1c2cf7ca","updated":"2024-07-09 07:09:30.000000000","message":"patchset2 fixes commit messages to be wrapped 72 characters.\npatchset3 removes a whitespace at the end of line in user\u0027s guide.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"be885040895446be0c6eb7bbfde368cc04c6f3ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9c765a72_a63d4148","updated":"2024-07-29 09:38:26.000000000","message":"I\u0027ve fixed for rest of unresolved comments in the latest patchset.","commit_id":"f9895230cdfdca9851a8ffbd466ac02ef6c28325"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"62db550432af60c34afbf2221f49c039eef86aaa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"dde6b231_2769105d","updated":"2024-07-30 15:21:00.000000000","message":"Syntax problem in the release note ... see comment inline.","commit_id":"0d19645e665b75aad92ecedcafbedd42b45cbab5"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"6d3a9e11ce2661de80957105af4eae21c6ba9ca9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"86eddb5e_f12d6e0f","updated":"2024-07-30 03:51:35.000000000","message":"The patchset remove a trailing space in user\u0027s guide","commit_id":"0d19645e665b75aad92ecedcafbedd42b45cbab5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b5858004dbb378d67e70288536633e11508e673e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"cc5e60cf_95abd739","updated":"2024-08-12 09:23:11.000000000","message":"@atsushi.kawai.bu@hitachi.com could you do the backports of this bug fix please?\nOn 2023.2 there are marge conflicts when doing the backports, so it would be best if you were to do them.","commit_id":"d04db6fe8874525a34e44c63b4c7a81c468c7ef9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7b2d02b6234b3dfe66db1217446029eb499c4e2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b40295f6_a0a16ef4","updated":"2024-08-08 12:53:58.000000000","message":"Thank you for addressing that issue.\nCode looks good to me, I have no additional concerns.","commit_id":"d04db6fe8874525a34e44c63b4c7a81c468c7ef9"}],"cinder/volume/drivers/hitachi/hbsd_common.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"82630e5d1e5c99b031ad45ca5ae47f0c27168915","unresolved":true,"context_lines":[{"line_number":432,"context_line":"        ldev_info \u003d self.get_ldev_info(None, ldev)"},{"line_number":433,"context_line":"        # To avoid calling the same REST API multiple times, we pass the LDEV"},{"line_number":434,"context_line":"        # info to the caller."},{"line_number":435,"context_line":"        for key in ldev_info:"},{"line_number":436,"context_line":"            ldev_info_[key] \u003d ldev_info[key]"},{"line_number":437,"context_line":"        return (\u0027label\u0027 in ldev_info"},{"line_number":438,"context_line":"                and _UUID_PATTERN.match(ldev_info[\u0027label\u0027])"},{"line_number":439,"context_line":"                and ldev_info[\u0027label\u0027] !\u003d obj.id.replace(\u0027-\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ed305309_c1e99c5f","line":436,"range":{"start_line":435,"start_character":1,"end_line":436,"end_character":44},"updated":"2024-07-23 16:46:41.000000000","message":"nit: Better to use `update`:\n\n```\nldev_info_.update(ldev_info)\n```","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"be885040895446be0c6eb7bbfde368cc04c6f3ac","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        ldev_info \u003d self.get_ldev_info(None, ldev)"},{"line_number":433,"context_line":"        # To avoid calling the same REST API multiple times, we pass the LDEV"},{"line_number":434,"context_line":"        # info to the caller."},{"line_number":435,"context_line":"        for key in ldev_info:"},{"line_number":436,"context_line":"            ldev_info_[key] \u003d ldev_info[key]"},{"line_number":437,"context_line":"        return (\u0027label\u0027 in ldev_info"},{"line_number":438,"context_line":"                and _UUID_PATTERN.match(ldev_info[\u0027label\u0027])"},{"line_number":439,"context_line":"                and ldev_info[\u0027label\u0027] !\u003d obj.id.replace(\u0027-\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"6c261d25_25ee23f1","line":436,"range":{"start_line":435,"start_character":1,"end_line":436,"end_character":44},"in_reply_to":"1dc31e18_ecebc6ee","updated":"2024-07-29 09:38:26.000000000","message":"Done","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"13f4b46565e091eb5a7b449e3d0c050ee77fb695","unresolved":true,"context_lines":[{"line_number":432,"context_line":"        ldev_info \u003d self.get_ldev_info(None, ldev)"},{"line_number":433,"context_line":"        # To avoid calling the same REST API multiple times, we pass the LDEV"},{"line_number":434,"context_line":"        # info to the caller."},{"line_number":435,"context_line":"        for key in ldev_info:"},{"line_number":436,"context_line":"            ldev_info_[key] \u003d ldev_info[key]"},{"line_number":437,"context_line":"        return (\u0027label\u0027 in ldev_info"},{"line_number":438,"context_line":"                and _UUID_PATTERN.match(ldev_info[\u0027label\u0027])"},{"line_number":439,"context_line":"                and ldev_info[\u0027label\u0027] !\u003d obj.id.replace(\u0027-\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1dc31e18_ecebc6ee","line":436,"range":{"start_line":435,"start_character":1,"end_line":436,"end_character":44},"in_reply_to":"ed305309_c1e99c5f","updated":"2024-07-26 08:59:40.000000000","message":"I\u0027ll fix it in the next patchset.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"82630e5d1e5c99b031ad45ca5ae47f0c27168915","unresolved":true,"context_lines":[{"line_number":435,"context_line":"        for key in ldev_info:"},{"line_number":436,"context_line":"            ldev_info_[key] \u003d ldev_info[key]"},{"line_number":437,"context_line":"        return (\u0027label\u0027 in ldev_info"},{"line_number":438,"context_line":"                and _UUID_PATTERN.match(ldev_info[\u0027label\u0027])"},{"line_number":439,"context_line":"                and ldev_info[\u0027label\u0027] !\u003d obj.id.replace(\u0027-\u0027, \u0027\u0027)"},{"line_number":440,"context_line":"                and (not hasattr(obj, \u0027name_id\u0027) or"},{"line_number":441,"context_line":"                     ldev_info[\u0027label\u0027] !\u003d obj.name_id.replace(\u0027-\u0027, \u0027\u0027)))"}],"source_content_type":"text/x-python","patch_set":3,"id":"93536e74_96b5d9e3","line":438,"updated":"2024-07-23 16:46:41.000000000","message":"?: Wouldn\u0027t it be better to either do a case insensitive match or change `_UUID_PATTERN` to include upper cased letters?","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"13f4b46565e091eb5a7b449e3d0c050ee77fb695","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        for key in ldev_info:"},{"line_number":436,"context_line":"            ldev_info_[key] \u003d ldev_info[key]"},{"line_number":437,"context_line":"        return (\u0027label\u0027 in ldev_info"},{"line_number":438,"context_line":"                and _UUID_PATTERN.match(ldev_info[\u0027label\u0027])"},{"line_number":439,"context_line":"                and ldev_info[\u0027label\u0027] !\u003d obj.id.replace(\u0027-\u0027, \u0027\u0027)"},{"line_number":440,"context_line":"                and (not hasattr(obj, \u0027name_id\u0027) or"},{"line_number":441,"context_line":"                     ldev_info[\u0027label\u0027] !\u003d obj.name_id.replace(\u0027-\u0027, \u0027\u0027)))"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc66365_861294fe","line":438,"in_reply_to":"93536e74_96b5d9e3","updated":"2024-07-26 08:59:40.000000000","message":"We don\u0027t think to fix it because:\nObject ID will be generated by the method ``uuid4`` in the pytho library ``uuid``. The method genarate UUID along RFC 4122, and RFC 4122 says \n`The hexadecimal values \"a\" through \"f\" are output as lower case characters\u0027\nThe value which is set by this patch must be colection of [0-9] and [a-f]. If the LDEV nickname contains [A-F], the driver should determine the value is written by user.\n\nref: P3 in https://datatracker.ietf.org/doc/html/rfc4122.html","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"82630e5d1e5c99b031ad45ca5ae47f0c27168915","unresolved":true,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5b4f3049_eb18539f","line":1188,"updated":"2024-07-23 16:46:41.000000000","message":"-1: A driver should not be modifying the `_name_id` field. That\u0027s a cinder core field.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"920bc73415751d0485dccd5a6170d0bbd55183c1","unresolved":true,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa11a62e_cb374bea","line":1188,"in_reply_to":"2a5834e3_b6f169ce","updated":"2024-08-07 13:31:38.000000000","message":"We\u0027re fixing this patch to cover the which you commented at Aug 2nd.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7e0078a2c75bad212b17e2319cd3f511c34372b4","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"eb89b196_9e13679f","line":1188,"in_reply_to":"33957436_b079be0d","updated":"2024-08-01 11:12:48.000000000","message":"OK, here\u0027s what I think doesn\u0027t look good in this method compared to the 2 examples you provided:\n\nThe code is renaming the ldev name of the `new_volume` to use the `volume.id` as the base, so **this can be a problem if both volumes share the same \"ldev namespace\"** because we\u0027ll have 2 volumes with the same ldev name.\n\nWhy I say we are trying to have 2 volumes with the same ldev:\n- Before L1181 is executed `volume` has ldev name based on `volume.id` and `new_volume` has it based on `new_volume.id`. That happened during volume creation.\n- Once L1181 is executed then `new_volume` also uses `volume.id` as the base for its ldev name, so both volumes\u0027 ldev are based on `volume.id`.\n\nDepending on your backend one of two things can happen:\n\n- It doesn\u0027t support duplicated ldev name so the method fails and the whole operation fails because the code isn\u0027t expecting a failure.\n- It succeeds and we end up with 2 volumes using the same ldev name. This is not good, because when deleting the old volume your driver will think that the ldev id doesn\u0027t match.\n\nI don\u0027t know when the \"ldev namespace\" is shared in your backend, but the scenario where this is always shared is when 2 different cinder backends share the same storage array pool.\n\nThat\u0027s why in the 2 examples you provided they try to rename the volume and if it fails (which always does on the first migration under that specific scenario because the name is already used by the old volume) then they leave the `_name_id` as it is.\n\nThe way to avoid that problem is to use a temporary name, like the 3PAR driver does [1].\n\n\nAnother thing, which is backend dependent so I don\u0027t know if it applies to your driver, is renaming volumes when a volume is attached. That\u0027s why all other drivers (including the two you provided) avoid renaming attached volumes.\nIsn\u0027t it problematic for you backend to change the ldev name when a volume is attached?\n\nThe easiest way to solve the problem is for your driver to always use `volume.name_id` (`name_id` is a property that basically ORs `_name_id` and `id` fields [2]) instead of using `volume.id` to generate or check the ldev name. In that scenario this method doesn\u0027t need to do anything except:\n\n```\n        return {\u0027_name_id\u0027: new_volume.name_id,\n                \u0027provider_location\u0027: new_volume.provider_location}\n```\n\nIn theory that shouldn\u0027t be necessary and it should be enough to return `None` or `{}`, but I see a bug in the volume manager that requires the drivers to return that at least.\n\n[1]: https://github.com/openstack/cinder/blob/0c871f587a96c35bd8edc389a89372bf70af33f6/cinder/volume/drivers/hpe/hpe_3par_common.py#L3040\n[2]: https://github.com/openstack/cinder/blob/0c871f587a96c35bd8edc389a89372bf70af33f6/cinder/objects/volume.py#L175","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"13f4b46565e091eb5a7b449e3d0c050ee77fb695","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"33957436_b079be0d","line":1188,"in_reply_to":"5b4f3049_eb18539f","updated":"2024-07-26 08:59:40.000000000","message":"We don\u0027t think so, because:\nIn the code of the method ``update_migrated_volume`` says:\n\n\u003e    Each driver implementing this method needs to be responsible for the\n\u003e    values of _name_id and provider_location.\n\nin https://opendev.org/openstack/cinder/src/commit/6565c1fd2b58047c12b5a159a3e7836ad0f36637/cinder/volume/driver.py#L1412-L1427\n\nand the methods in other vender drivers are also modify the values, like:\n- https://opendev.org/openstack/cinder/src/commit/6565c1fd2b58047c12b5a159a3e7836ad0f36637/cinder/volume/drivers/rbd.py#L2291\n- https://opendev.org/openstack/cinder/src/commit/6565c1fd2b58047c12b5a159a3e7836ad0f36637/cinder/volume/drivers/lvm.py#L430\n\nAddtionally, why ``None`` is set in the ``_name_id`` is to avoid that LDEV is not deleted by mismatching LDEV nickname and ``_name_id``. The case is host-assisted migration for host-assisted migrated volume.\n\nAt the time of issuing this method, the ID of the temporary volume is set in the ``_name_id`` attribute of the migration target volume.\n\nBy setting the ``_name_id`` to ``None`` in this method, when Cinder Volume Manager swaps the attributes of the volumes during the next host-assisted migration, the ID of the temporary volume used in the previous host-assisted migration will not be set in the ``_name_id`` of the migrated volume. \n\nThis behavior prevents a comparison mismatch between ``_name_id`` and LDEV nicknames when deleting temporary volumes in 2nd host-assisted migrations.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"64cb418f2846540de1919f818a819b2cd9d1be23","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"2a5834e3_b6f169ce","line":1188,"in_reply_to":"70ca8793_801f7b39","updated":"2024-08-02 13:55:26.000000000","message":"Thanks for the clarification. It helps a lot.\n\nThen the code will work as intended, though I still believe that having duplicated ldev names is conceptually wrong and in my opinion they should always be unique.\n\nI think there are still scenarios where we could be deleting the wrong volume.\n\nThis is one I can come up with:\n\n- Request a migraation of Cinder volumeA (id: a; ldev: 1; ldev name: a)\n\n- Migration creates Cinder volumeB (id: b; ldev 2; ldev name: b)\n\n- Final steps of migration sets Cinder volumes metadata leaving thigs as:\n  * volumeA (id: a; ldev 2; ldev name: a)\n  * volumeB (id: b; name_id: a; ldev: 1; ldev name: a) \n\n- Deletion of volumeB (id: b; name_id: a; ldev: 1; ldev name: a) fails due to timeout. Ldev is really deleted in the array even though cinder doesn\u0027t know, so the cinder DB record still exists.\n\n- Request a new migration of volumeA (id: a; ldev 2; ldev name: a)\n\n- Migration creates Cinder volumeC (id: c; ldev 1; ldev name: c) which reuses the now available ldev number 1 \n\n- Final steps of migration sets Cinder volumes metadata leaving thigs as:\n  * volumeA (id: a; ldev 1; ldev name: a)\n  * volumeC (id: c; name_id: a; ldev: 2; ldev name: a)\n\nNow if we delete the old Cinder volumeB (id: b; name_id: a; ldev: 1; ldev name: a) it will delete the contents of volumeA (id: a; ldev 1; ldev name: a) because they reference the same volume (same ldev number) and the `is_invaalid_ldev` will return `False` because the ldev name matches.\n\nIt is true that it is unlikely to happen, but it can happen, and one can never be too sure with data loss.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"ce31b794864429a18107da24fe7a82464f8c73d4","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"70ca8793_801f7b39","line":1188,"in_reply_to":"eb89b196_9e13679f","updated":"2024-08-02 13:09:04.000000000","message":"Thx for checking.\nWe think all issues you commented will not occur because:\n- As Hitachi storage permit multiple LDEVs have a same LDEV nickname value, any failures by conflicting LDEV nickname value will not occur in the operation for migration target volume.\n- Even if LDEV nickname value and volume ID are not same in the method ``is_invalid_ldev`` called when deleting LDEV, LDEV will be deleted when ``name_id`` and LDEV nickname value are same.\n- As Hitachi storage permit to change LDEV nickname while attaching LDEV, any failure will not occur by changing LDEV nickname while attaching LDEV.\nSo, we think this fixing is good and it need not to be fixed.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"db2c2186b451ea01a90b4c8bb338de21319bcbdc","unresolved":true,"context_lines":[{"line_number":1185,"context_line":"        # \u0027id\u0027, \u0027_name_id\u0027 should be cleared. This setting is necessary for the"},{"line_number":1186,"context_line":"        # next host-assisted migration to successfully delete the source LDEV."},{"line_number":1187,"context_line":"        # \u0027provider_location\u0027 does not need to change."},{"line_number":1188,"context_line":"        return {\u0027_name_id\u0027: None,"},{"line_number":1189,"context_line":"                \u0027provider_location\u0027: new_volume.provider_location}"},{"line_number":1190,"context_line":""},{"line_number":1191,"context_line":"    def retype(self, ctxt, volume, new_type, diff, host):"}],"source_content_type":"text/x-python","patch_set":3,"id":"773b2550_bd02f8e7","line":1188,"in_reply_to":"fa11a62e_cb374bea","updated":"2024-08-08 07:49:39.000000000","message":"On the patchset 7, we\u0027ve fixed as following, to cover the case you commented:\n* fixed policy\n\n  * In order to keep a one-to-one correspondence between volumes and LDEVs even when attributes are exchanged between volume objects by host-assisted migration, LDEV nickname are set only when LDEVs are created, and LDEV nicknames are compared only to volume.name_id in validation of LDEVs to be deleted.\n\n* Fixed points\n\n  * delete operation to update LDEV nickname for migration target in ``update_migrated_volume`` .\n  * change the returned value ``_name_id`` in ``update_migrated_volume`` from ``None`` to the object ID for temporary volume.\n  * While checking a correspondence between an object and a LDEV in ``is_invalid_ldev``, if the object is a volume, delete the comparison between the LDEV nickname and the volume ID and compare LDEV nickname with ``name_id`` only.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7b2d02b6234b3dfe66db1217446029eb499c4e2f","unresolved":true,"context_lines":[{"line_number":440,"context_line":"        return (\u0027label\u0027 in ldev_info"},{"line_number":441,"context_line":"                and _UUID_PATTERN.match(ldev_info[\u0027label\u0027])"},{"line_number":442,"context_line":"                and ldev_info[\u0027label\u0027] !\u003d ("},{"line_number":443,"context_line":"                    obj.name_id if hasattr(obj, \u0027name_id\u0027) else"},{"line_number":444,"context_line":"                    obj.id).replace(\u0027-\u0027, \u0027\u0027))"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def delete_volume(self, volume):"},{"line_number":447,"context_line":"        \"\"\"Delete the specified volume.\"\"\""}],"source_content_type":"text/x-python","patch_set":7,"id":"12c51b48_d009a4ad","line":444,"range":{"start_line":443,"start_character":1,"end_line":444,"end_character":45},"updated":"2024-08-08 12:53:58.000000000","message":"nit: these last 2 lines can be replaced with:\n```\ngetattr(obj, \u0027name_id\u0027, obj.id).replace(\u0027-\u0027, \u0027\u0027)\n```","commit_id":"d04db6fe8874525a34e44c63b4c7a81c468c7ef9"}],"doc/source/configuration/block-storage/drivers/hitachi-vsp-driver.rst":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"82630e5d1e5c99b031ad45ca5ae47f0c27168915","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"   * Do not change LDEV nickname for the LDEVs created by Hitachi block"},{"line_number":149,"context_line":"     storage driver. The nickname is referred when deleting a volume or"},{"line_number":150,"context_line":"     a snapshot, to avoid data-loss risk. See details in `bug #2072317`_."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"Set up Hitachi storage volume driver and volume operations"},{"line_number":153,"context_line":"----------------------------------------------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"b5cd325a_7599532c","line":150,"updated":"2024-07-23 16:46:41.000000000","message":"Great idea to have this warning in the docs!","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"13f4b46565e091eb5a7b449e3d0c050ee77fb695","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"   * Do not change LDEV nickname for the LDEVs created by Hitachi block"},{"line_number":149,"context_line":"     storage driver. The nickname is referred when deleting a volume or"},{"line_number":150,"context_line":"     a snapshot, to avoid data-loss risk. See details in `bug #2072317`_."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"Set up Hitachi storage volume driver and volume operations"},{"line_number":153,"context_line":"----------------------------------------------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"c2884b60_804756ed","line":150,"in_reply_to":"b5cd325a_7599532c","updated":"2024-07-26 08:59:40.000000000","message":"Thanks!","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"}],"releasenotes/notes/hitachi-prevent-data-loss-9ec3569d7d5b1e7d.yaml":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"82630e5d1e5c99b031ad45ca5ae47f0c27168915","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Hitachi driver `bug #2072317"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/2072317\u003e\u0027_: Fix to prevent"},{"line_number":6,"context_line":"    data-loss risk by :"},{"line_number":7,"context_line":"    When deleting object, comparing an object ID for the deleted object"},{"line_number":8,"context_line":"    and a value in ``LDEV nickname`` for the LDEV is assigned to the object."},{"line_number":9,"context_line":"    If both IDs are same, both the object and the LDEV are deleted,"},{"line_number":10,"context_line":"    If not so, only the object is deleted and the LDEV is kept."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"164e6671_61596931","line":10,"range":{"start_line":4,"start_character":0,"end_line":10,"end_character":63},"updated":"2024-07-23 16:46:41.000000000","message":"nit: I don\u0027t think it\u0027s necessary to give so much detail for end users.  Maybe summarize it a bit.  For example: \n\n```\n    Hitachi driver `bug #2072317\n    \u003chttps://bugs.launchpad.net/cinder/+bug/2072317\u003e\u0027_: Fix potential\n    data-loss due to a network issue during a volume deletion.   \n```","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"be885040895446be0c6eb7bbfde368cc04c6f3ac","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Hitachi driver `bug #2072317"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/2072317\u003e\u0027_: Fix to prevent"},{"line_number":6,"context_line":"    data-loss risk by :"},{"line_number":7,"context_line":"    When deleting object, comparing an object ID for the deleted object"},{"line_number":8,"context_line":"    and a value in ``LDEV nickname`` for the LDEV is assigned to the object."},{"line_number":9,"context_line":"    If both IDs are same, both the object and the LDEV are deleted,"},{"line_number":10,"context_line":"    If not so, only the object is deleted and the LDEV is kept."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"0ce6f401_860ad019","line":10,"range":{"start_line":4,"start_character":0,"end_line":10,"end_character":63},"in_reply_to":"1423efd8_bf31b782","updated":"2024-07-29 09:38:26.000000000","message":"Done","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"13f4b46565e091eb5a7b449e3d0c050ee77fb695","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Hitachi driver `bug #2072317"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/2072317\u003e\u0027_: Fix to prevent"},{"line_number":6,"context_line":"    data-loss risk by :"},{"line_number":7,"context_line":"    When deleting object, comparing an object ID for the deleted object"},{"line_number":8,"context_line":"    and a value in ``LDEV nickname`` for the LDEV is assigned to the object."},{"line_number":9,"context_line":"    If both IDs are same, both the object and the LDEV are deleted,"},{"line_number":10,"context_line":"    If not so, only the object is deleted and the LDEV is kept."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1423efd8_bf31b782","line":10,"range":{"start_line":4,"start_character":0,"end_line":10,"end_character":63},"in_reply_to":"164e6671_61596931","updated":"2024-07-26 08:59:40.000000000","message":"I\u0027ll fix it in the next patchset.","commit_id":"cd1c6cc8a2ce44673b9a9a8154e6affcc0ed8748"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"62db550432af60c34afbf2221f49c039eef86aaa","unresolved":true,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Hitachi driver `bug #2072317"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/2072317\u003e\u0027_: Fix potential"},{"line_number":6,"context_line":"    data-loss due to a network issue during a volume deletion."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"0aceeaa6_910629cf","line":5,"range":{"start_line":5,"start_character":52,"end_line":5,"end_character":53},"updated":"2024-07-30 15:21:00.000000000","message":"This needs to be a backtick (like you have on line 4)","commit_id":"0d19645e665b75aad92ecedcafbedd42b45cbab5"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"b66b8c97f6e40d34f634211586ab36b2ae0ea7e5","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Hitachi driver `bug #2072317"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/2072317\u003e\u0027_: Fix potential"},{"line_number":6,"context_line":"    data-loss due to a network issue during a volume deletion."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"2432f915_11b2bb53","line":5,"range":{"start_line":5,"start_character":52,"end_line":5,"end_character":53},"in_reply_to":"0aceeaa6_910629cf","updated":"2024-07-31 00:01:37.000000000","message":"Thx for your checking.\nI\u0027ve fixed it.","commit_id":"0d19645e665b75aad92ecedcafbedd42b45cbab5"}]}