)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"70b21f4c47043482e4a661a8e018ec8f49f69ff3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"991d2f08_4ed7edf4","updated":"2022-09-06 19:41:07.000000000","message":"One question inline regarding provider_location","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"a03f172a296af3ac40640ebb09af84d0edee3fff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"488a2c7e_ae9cfcf7","updated":"2022-08-24 15:35:22.000000000","message":"Patch set #6: Use volume name_id to resolve Infinidat volume name.","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"bbbafa571ad73feef406034d0369996422a8330d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"83fe84a4_67bfbee6","updated":"2022-08-23 13:37:04.000000000","message":"recheck openstack-tox-py38","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"3444670ec43b67be93e643ebb1401779ce6f7c3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"24d7e38a_f66071e2","updated":"2022-11-23 13:39:51.000000000","message":"Looks good to me and passing Infinidat CI","commit_id":"ba25c0ea2cc6be5954f7f0df73e67e72b9cf8754"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"f253e804cc7aee8792f2ba6a6a08515fdc6871ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"75612b29_32a5a89b","updated":"2022-12-08 14:08:59.000000000","message":"Comments have been addressed and CI is passing and testing cinder plugin tests as well","commit_id":"9a29d57a61cfb845eabc2cc7f72480b48b9a68e9"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"a536983b62efcfefe84ba9be5750951dd57e83a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"d6acd960_b8f0c234","updated":"2022-12-01 17:00:59.000000000","message":"Tests cover code changes and CI is passing, looks good to me.\n","commit_id":"9a29d57a61cfb845eabc2cc7f72480b48b9a68e9"}],"cinder/volume/drivers/infinidat.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"70b21f4c47043482e4a661a8e018ec8f49f69ff3","unresolved":true,"context_lines":[{"line_number":644,"context_line":"        # we need a cinder-volume-like object to map the clone by name"},{"line_number":645,"context_line":"        # (which is derived from the cinder id) but the clone is internal"},{"line_number":646,"context_line":"        # so there is no such object. mock one"},{"line_number":647,"context_line":"        clone \u003d mock.Mock(name_id\u003dstr(volume.name_id) + \u0027-internal\u0027)"},{"line_number":648,"context_line":"        try:"},{"line_number":649,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"},{"line_number":650,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"4e353af4_0cf0a3cf","line":647,"range":{"start_line":647,"start_character":16,"end_line":647,"end_character":25},"updated":"2022-09-06 19:41:07.000000000","message":"The mock seems suspicious here, would be good to use something else but can be addressed later","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"e1d958cf2d0af87823da10eb3c5638c2827b4956","unresolved":true,"context_lines":[{"line_number":644,"context_line":"        # we need a cinder-volume-like object to map the clone by name"},{"line_number":645,"context_line":"        # (which is derived from the cinder id) but the clone is internal"},{"line_number":646,"context_line":"        # so there is no such object. mock one"},{"line_number":647,"context_line":"        clone \u003d mock.Mock(name_id\u003dstr(volume.name_id) + \u0027-internal\u0027)"},{"line_number":648,"context_line":"        try:"},{"line_number":649,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"},{"line_number":650,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"a32fa261_825998fe","line":647,"range":{"start_line":647,"start_character":16,"end_line":647,"end_character":25},"in_reply_to":"4e353af4_0cf0a3cf","updated":"2022-09-06 22:01:11.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e The mock seems suspicious here\nYes, I agree and we have already refactored this code and replaced:\nmock -\u003e collections.namedtuple in the next patch (https://review.opendev.org/c/openstack/cinder/+/851640)\n \nThank you!","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"5d89e90c0e2321581a6e28c5f18423edd5fb4038","unresolved":false,"context_lines":[{"line_number":644,"context_line":"        # we need a cinder-volume-like object to map the clone by name"},{"line_number":645,"context_line":"        # (which is derived from the cinder id) but the clone is internal"},{"line_number":646,"context_line":"        # so there is no such object. mock one"},{"line_number":647,"context_line":"        clone \u003d mock.Mock(name_id\u003dstr(volume.name_id) + \u0027-internal\u0027)"},{"line_number":648,"context_line":"        try:"},{"line_number":649,"context_line":"            infinidat_volume \u003d self._create_volume(volume)"},{"line_number":650,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"00881444_c403d585","line":647,"range":{"start_line":647,"start_character":16,"end_line":647,"end_character":25},"in_reply_to":"a32fa261_825998fe","updated":"2022-09-07 10:06:07.000000000","message":"Done","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"70b21f4c47043482e4a661a8e018ec8f49f69ff3","unresolved":true,"context_lines":[{"line_number":869,"context_line":"        :returns: model_update to update DB with any needed changes"},{"line_number":870,"context_line":"        \"\"\""},{"line_number":871,"context_line":"        model_update \u003d {\u0027_name_id\u0027: new_volume.name_id,"},{"line_number":872,"context_line":"                        \u0027provider_location\u0027: None}"},{"line_number":873,"context_line":"        new_volume_name \u003d self._make_volume_name(new_volume, migration\u003dTrue)"},{"line_number":874,"context_line":"        new_infinidat_volume \u003d self._get_infinidat_volume(new_volume)"},{"line_number":875,"context_line":"        self._set_cinder_object_metadata(new_infinidat_volume, volume)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1b804345_f06ee92d","line":872,"range":{"start_line":872,"start_character":24,"end_line":872,"end_character":49},"updated":"2022-09-06 19:41:07.000000000","message":"we are not updating the provider_location, is this intentional?","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"e1d958cf2d0af87823da10eb3c5638c2827b4956","unresolved":true,"context_lines":[{"line_number":869,"context_line":"        :returns: model_update to update DB with any needed changes"},{"line_number":870,"context_line":"        \"\"\""},{"line_number":871,"context_line":"        model_update \u003d {\u0027_name_id\u0027: new_volume.name_id,"},{"line_number":872,"context_line":"                        \u0027provider_location\u0027: None}"},{"line_number":873,"context_line":"        new_volume_name \u003d self._make_volume_name(new_volume, migration\u003dTrue)"},{"line_number":874,"context_line":"        new_infinidat_volume \u003d self._get_infinidat_volume(new_volume)"},{"line_number":875,"context_line":"        self._set_cinder_object_metadata(new_infinidat_volume, volume)"}],"source_content_type":"text/x-python","patch_set":6,"id":"2230e4d2_ea8580c9","line":872,"range":{"start_line":872,"start_character":24,"end_line":872,"end_character":49},"in_reply_to":"1b804345_f06ee92d","updated":"2022-09-06 22:01:11.000000000","message":"Hello Rajat,\n\nThank you very much for the review!\n\n\u003e\u003e we are not updating the provider_location, is this intentional?\n\nYes. The Infinidat driver does not use the provider_location (iSCSI/FC host/target/port/LUN information is defined dynamically and may change when the storage system is reconfigured).\n\nBut we change the provider_location value to None to clear the current value during volume migration. For example, in case of migration a volume from the generic NFS driver, the volume has an existing non-empty provider_location value, and we must clear it when we migrate the volume to the Infinidat backend:\n\n$ cinder create --volume-type nfs 1\n$ cinder list --fields id,volume_type\n+--------------------------------------+-------------+\n| ID                                   | Volume_Type |\n+--------------------------------------+-------------+\n| 901e8129-fb67-4495-b111-e8df2eb94ea7 | nfs         |\n+--------------------------------------+-------------+\nmysql\u003e select provider_location from volumes where id\u003d\u0027901e8129-fb67-4495-b111-e8df2eb94ea7\u0027;\n+---------------------+\n| provider_location   |\n+---------------------+\n| 172.10.20.123:/nfs  |\n+---------------------+\n1 row in set (0.00 sec)\n\n$ cinder retype --migration-policy on-demand 901e8129-fb67-4495-b111-e8df2eb94ea7 iscsi\n$ cinder list --fields id,volume_type\n+--------------------------------------+-------------+\n| ID                                   | Volume_Type |\n+--------------------------------------+-------------+\n| 901e8129-fb67-4495-b111-e8df2eb94ea7 | iscsi       |\n+--------------------------------------+-------------+\nmysql\u003e select provider_location from volumes where id\u003d\u0027901e8129-fb67-4495-b111-e8df2eb94ea7\u0027;\n+-------------------+\n| provider_location |\n+-------------------+\n| NULL              |\n+-------------------+\n1 row in set (0.00 sec)\n\nAnd in the absence of {\u0027provider_location\u0027: None}, we will have the old value from the NFS driver (172.10.20.123:/nfs)\n\nThank you very much!","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"5d89e90c0e2321581a6e28c5f18423edd5fb4038","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        :returns: model_update to update DB with any needed changes"},{"line_number":870,"context_line":"        \"\"\""},{"line_number":871,"context_line":"        model_update \u003d {\u0027_name_id\u0027: new_volume.name_id,"},{"line_number":872,"context_line":"                        \u0027provider_location\u0027: None}"},{"line_number":873,"context_line":"        new_volume_name \u003d self._make_volume_name(new_volume, migration\u003dTrue)"},{"line_number":874,"context_line":"        new_infinidat_volume \u003d self._get_infinidat_volume(new_volume)"},{"line_number":875,"context_line":"        self._set_cinder_object_metadata(new_infinidat_volume, volume)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e7c46283_1dc5e38e","line":872,"range":{"start_line":872,"start_character":24,"end_line":872,"end_character":49},"in_reply_to":"2230e4d2_ea8580c9","updated":"2022-09-07 10:06:07.000000000","message":"Done","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"f253e804cc7aee8792f2ba6a6a08515fdc6871ff","unresolved":false,"context_lines":[{"line_number":869,"context_line":"        :returns: model_update to update DB with any needed changes"},{"line_number":870,"context_line":"        \"\"\""},{"line_number":871,"context_line":"        model_update \u003d {\u0027_name_id\u0027: new_volume.name_id,"},{"line_number":872,"context_line":"                        \u0027provider_location\u0027: None}"},{"line_number":873,"context_line":"        new_volume_name \u003d self._make_volume_name(new_volume, migration\u003dTrue)"},{"line_number":874,"context_line":"        new_infinidat_volume \u003d self._get_infinidat_volume(new_volume)"},{"line_number":875,"context_line":"        self._set_cinder_object_metadata(new_infinidat_volume, volume)"}],"source_content_type":"text/x-python","patch_set":6,"id":"64c739e5_7cf19f9f","line":872,"range":{"start_line":872,"start_character":24,"end_line":872,"end_character":49},"in_reply_to":"e7c46283_1dc5e38e","updated":"2022-12-08 14:08:59.000000000","message":"Thanks for the explanation","commit_id":"0c742799bff176d399afddcfdd8b73a20c756e5f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5d3e0addf06bee7ff792c864bd611b9a20473ed3","unresolved":true,"context_lines":[{"line_number":206,"context_line":"    def _make_volume_name(self, cinder_volume, migration\u003dFalse):"},{"line_number":207,"context_line":"        \"\"\"Return the Infinidat volume name."},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        Use Cinder volume id in case of volume migration"},{"line_number":210,"context_line":"        and use Cinder volume name_id for all other cases."},{"line_number":211,"context_line":"        \"\"\""},{"line_number":212,"context_line":"        if migration:"},{"line_number":213,"context_line":"            key \u003d cinder_volume.id"}],"source_content_type":"text/x-python","patch_set":8,"id":"349c0bc8_5c57b542","line":210,"range":{"start_line":209,"start_character":8,"end_line":210,"end_character":58},"updated":"2022-12-19 13:51:11.000000000","message":"nit: would be better to explain why we are doing this\n\nI assume name_id is specific to backend so when performing migration, we want to use the common thing between backends i.e. the volume ID","commit_id":"9a29d57a61cfb845eabc2cc7f72480b48b9a68e9"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"f253e804cc7aee8792f2ba6a6a08515fdc6871ff","unresolved":false,"context_lines":[{"line_number":1225,"context_line":"        new_volume_name \u003d self._make_volume_name(new_volume, migration\u003dTrue)"},{"line_number":1226,"context_line":"        new_infinidat_volume \u003d self._get_infinidat_volume(new_volume)"},{"line_number":1227,"context_line":"        self._set_cinder_object_metadata(new_infinidat_volume, volume)"},{"line_number":1228,"context_line":"        volume_name \u003d self._make_volume_name(volume, migration\u003dTrue)"},{"line_number":1229,"context_line":"        try:"},{"line_number":1230,"context_line":"            infinidat_volume \u003d self._get_infinidat_volume(volume)"},{"line_number":1231,"context_line":"        except exception.VolumeNotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"b0a5bebf_5ce28e05","line":1228,"range":{"start_line":1228,"start_character":52,"end_line":1228,"end_character":68},"updated":"2022-12-08 14:08:59.000000000","message":"Looks like using `migration\u003dTrue` here keeps backwards compatibility with old volumes.","commit_id":"9a29d57a61cfb845eabc2cc7f72480b48b9a68e9"}]}