)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":12670,"name":"Helen Walsh","email":"helen.walsh@emc.com","username":"walshh2"},"change_message_id":"63d14efc896b2cb48a45fc0677ff803cb2568f18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"29ea0d1d_75df8617","updated":"2021-10-12 14:56:26.000000000","message":"LGTM","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"77b3a51001dfa2e334e66ff4fb257b44195f48f2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ab008856_455e9df6","updated":"2021-12-06 18:31:54.000000000","message":"Passing CI and is documented.  Should be ok.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4e5210d1_3bf69ecf","updated":"2021-12-14 13:17:15.000000000","message":"Sorry if my questions are stupid and I\u0027m making no sense, but I\u0027m having trouble making sense of the patch due to my ignorance in this specific storage solution.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"8eb4f579025b2806f229889ee6b6450ebad13d76","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3dfc371c_a9c0cd5b","updated":"2021-10-11 11:54:09.000000000","message":"recheck","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"b7d3bd9598a5adfcb9fe4d62ff128c0289ae50a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0161b746_199bc7a5","updated":"2021-11-17 04:12:23.000000000","message":"run-IBM Storage CI","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2650c0b8_e57a17e6","updated":"2021-12-20 04:48:05.000000000","message":"Hi Gorka,\nThankyou for your detailed insight on this review.\nI have answered your queries and made the necessary code changes as per your review comments.\n\nPlease let me know if there is anything else that needs to be done on this.\n\nThank you.","commit_id":"a0365f77b2a6bbaf7dbb5b8537046ef6d211b51a"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"b2aa4e4d76ee48e29b51d9e99966da15f3044c5f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0ec6b902_641e0fe5","updated":"2021-12-20 11:58:01.000000000","message":"recheck","commit_id":"a0365f77b2a6bbaf7dbb5b8537046ef6d211b51a"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"b08285997a77dd9ba5d813ae4eccd06b31e2c2e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9fdbe563_39f956cd","updated":"2021-12-21 13:30:59.000000000","message":"run-IBM Storage CI","commit_id":"a0365f77b2a6bbaf7dbb5b8537046ef6d211b51a"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"15aa742f4cc8964a84a24e0bfa539d22b289ee46","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d29b5881_0ebf1cce","updated":"2022-01-18 18:52:10.000000000","message":"Hi Gorka and Brian, I have addressed the concerns. Please review the change and let me know if anything else is needed.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"3e6e512caa15555e39212977c648ab1faba75103","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4079372c_d2dfcca3","updated":"2022-01-07 16:34:16.000000000","message":"I\u0027m not sure I see where in the driver changes where the cleanrate value is being pulled from a volume type extra spec as described in the release note and the commit message.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"f9761d830e768229e69ebb7007c4bd32a9f1615b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"2a9e388a_95cf3bc4","updated":"2022-01-17 14:15:19.000000000","message":"Please elaborate the solution.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"a9c4ac079bb7d5a63867cfd338a936da75c4f9ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"36b4a84c_d041b963","updated":"2022-01-10 13:13:45.000000000","message":"Thank you for your review.\nHope i have answered your query. Requesting you to please please review it and provide +2.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9751f2136c557734b30b8c38cf8162710f17a52e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"42923d0a_1a6f5187","updated":"2022-01-10 17:35:41.000000000","message":"You need to address Gorka\u0027s latest comment on PS3.  Some other issues noted inline.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"a81b9b57f25686946322c043caad4a12866617c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4acaed79_55f564f5","updated":"2021-12-31 03:57:09.000000000","message":"run-DellEMC PowerFlex CI","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"ba17ec8f3fc5a8de21ffdbe024e30864588a2f67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"149938c3_60f9313a","updated":"2021-12-27 12:12:50.000000000","message":"run-IBM Storage CI","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"a9c4ac079bb7d5a63867cfd338a936da75c4f9ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"72a51985_5a7321ec","in_reply_to":"4079372c_d2dfcca3","updated":"2022-01-10 13:13:45.000000000","message":"Hi Hemna, Thank you for reviewing the code.\nThe clean_rate parameter for volume spec is add in the dictionary storwize_svc_opts. At line 126, this option will be taken from volume-type-specs.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"a4937809ff69a961903f53421d3694d23fcb5816","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"88c847b3_9bf6e34f","updated":"2022-01-18 20:19:54.000000000","message":"Reply inline.","commit_id":"21df5508ab65274a77aae433c72b69f0afdd0949"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"a9abd8605f9e376a38292e6304844e495dd01c5f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"5a51b9da_426cb2e2","updated":"2022-01-24 17:04:17.000000000","message":"Hi Brian,\nI have addressed the concern. Please let me know if there is anything else.\nThanks","commit_id":"1a167a82716c854ad9b8d0545221272e378b218b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"523e117006bbef33664625d870d2f9ca9bf820eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e205d7e4_4c6861bb","updated":"2022-01-27 20:42:02.000000000","message":"Not sure that function is quite correct yet.  (See comment inline.)","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"023f7aa1606891440c57360588249c38a98fca75","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a2b9d08e_7356ab1f","updated":"2022-01-24 20:20:31.000000000","message":"recheck","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"49115ff54e92a6a967532c34f4c9c338b9766d7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a974ea0a_c7ba370c","updated":"2022-01-25 07:45:25.000000000","message":"recheck","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"cd26a7d5838c38fc927020d426a95faa4199324f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"244961fa_81511b7f","updated":"2022-01-25 10:04:18.000000000","message":"run-IBM Storage CI","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"5533996871c57b592d1bcc084010e6ab8566d8e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"8dcc1ff9_082477eb","updated":"2022-01-28 11:29:04.000000000","message":"Hi Brian,\nI have addressed your comments. Please let me know if anything else is required.","commit_id":"2c3635b61d50cf1d3d9f040822c67c306f117dac"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d021be4aea02c2aac580e533cd22f87bc7191595","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"1eee53fa_8aa4c170","updated":"2022-01-31 15:49:35.000000000","message":"LGTM. Thank you very much for the patience.","commit_id":"2c3635b61d50cf1d3d9f040822c67c306f117dac"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"53ce113998efa68831f9d067f356c053d1a2552c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"ec26b9c2_13677132","updated":"2022-02-01 17:14:58.000000000","message":"Passing CI and it looks like the comments have been addressed.","commit_id":"2c3635b61d50cf1d3d9f040822c67c306f117dac"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"f9d68063f34e26334fda3b8125e03b8b155b0cb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"977904c3_fbe4091a","updated":"2022-01-28 18:04:50.000000000","message":"run-IBM Storage CI","commit_id":"2c3635b61d50cf1d3d9f040822c67c306f117dac"}],"cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py":[{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"16059bf7caa163fe921a78d1a34596a1bfa30a78","unresolved":true,"context_lines":[{"line_number":10143,"context_line":"                              self.storwize_svc_common.check_flashcopy_rate,"},{"line_number":10144,"context_line":"                              flashcopy_rate)"},{"line_number":10145,"context_line":""},{"line_number":10146,"context_line":"    def test_storwize_check_clean_rate_invalid1(self):"},{"line_number":10147,"context_line":"        clean_rate \u003d 170"},{"line_number":10148,"context_line":"        self.assertRaises(exception.InvalidInput,"},{"line_number":10149,"context_line":"                          self.storwize_svc_common.check_clean_rate,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3017dbb5_6555cb49","line":10146,"updated":"2021-10-01 07:44:28.000000000","message":"Add q new unittest or updated the existing unittest with the valid case","commit_id":"89682a543c56e7c08f0c5702e09a3370cafad65b"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"16059bf7caa163fe921a78d1a34596a1bfa30a78","unresolved":true,"context_lines":[{"line_number":10152,"context_line":"    @mock.patch.object(storwize_svc_common.StorwizeSSH, \u0027chfcmap\u0027)"},{"line_number":10153,"context_line":"    @mock.patch.object(storwize_svc_common.StorwizeHelpers,"},{"line_number":10154,"context_line":"                       \u0027_get_vdisk_fc_mappings\u0027)"},{"line_number":10155,"context_line":"    def test_storwize_update_clean_rate(self,"},{"line_number":10156,"context_line":"                                        chfcmap,"},{"line_number":10157,"context_line":"                                        get_vdisk_fc_mappings):"},{"line_number":10158,"context_line":"        get_vdisk_fc_mappings.return_value \u003d [\u00274\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"af11470a_a3476624","line":10155,"updated":"2021-10-01 07:44:28.000000000","message":"Update with atleast one invalid case","commit_id":"89682a543c56e7c08f0c5702e09a3370cafad65b"}],"cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py":[{"author":{"_account_id":29122,"name":"Raghavendra Tilay","email":"raghavendra-uddhav.tilay@hpe.com","username":"raghavendrat"},"change_message_id":"5331f364a9f4f5e160cedde69359bb6b1541064b","unresolved":true,"context_lines":[{"line_number":614,"context_line":"        if consistgrp:"},{"line_number":615,"context_line":"            ssh_cmd.extend([\u0027-consistgrp\u0027, consistgrp])"},{"line_number":616,"context_line":"        if clean_rate:"},{"line_number":617,"context_line":"            ssh_cmd.extend([\u0027-cleanrate\u0027, str(int(clean_rate))])"},{"line_number":618,"context_line":"        out, err \u003d self._ssh(ssh_cmd, check_exit_code\u003dFalse)"},{"line_number":619,"context_line":"        if \u0027successfully created\u0027 not in out:"},{"line_number":620,"context_line":"            msg \u003d (_(\u0027CLI Exception output:\\n command: %(cmd)s\\n \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"13fa85cb_8ca16637","line":617,"range":{"start_line":617,"start_character":42,"end_line":617,"end_character":62},"updated":"2021-10-01 09:32:54.000000000","message":"Can this be simply kept as ... clean_rate ?\nDidn\u0027t understand logic behind conversion.","commit_id":"89682a543c56e7c08f0c5702e09a3370cafad65b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":true,"context_lines":[{"line_number":123,"context_line":"               help\u003d\u0027Specifies the Storwize FlashCopy copy rate to be used \u0027"},{"line_number":124,"context_line":"               \u0027when creating a full volume copy. The default is rate \u0027"},{"line_number":125,"context_line":"               \u0027is 50, and the valid rates are 1-150.\u0027),"},{"line_number":126,"context_line":"    cfg.IntOpt(\u0027storwize_svc_clean_rate\u0027,"},{"line_number":127,"context_line":"               default\u003d50,"},{"line_number":128,"context_line":"               min\u003d0, max\u003d150,"},{"line_number":129,"context_line":"               help\u003d\u0027Specifies the Storwize cleaning rate for the mapping. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff13b4ad_a3588a68","line":126,"updated":"2021-12-14 13:17:15.000000000","message":"-1: I don\u0027t know anything about the FlashCopy functionality, its defaults, etc, but from the code and a kick glace at the docs[1] it seems like we have a number of issue that would arise from the current implementation.\n\nExisting volumes don\u0027t have a value set in the array or the volume type, yet in methods like the retype we\u0027ll \"assume\" that they do, because we use this configuration option as the default, which seems wrong.\n\nWe can also create a volume with a value set here, then change the value in the config file, restart the service and now the driver will think those volumes have a different value that what has been actually set.\n\nOr am I missing something?\n\n\n[1]: https://www.ibm.com/docs/en/flashsystem-9x00/8.3.x?topic\u003dto-copy-services-functions","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[{"line_number":123,"context_line":"               help\u003d\u0027Specifies the Storwize FlashCopy copy rate to be used \u0027"},{"line_number":124,"context_line":"               \u0027when creating a full volume copy. The default is rate \u0027"},{"line_number":125,"context_line":"               \u0027is 50, and the valid rates are 1-150.\u0027),"},{"line_number":126,"context_line":"    cfg.IntOpt(\u0027storwize_svc_clean_rate\u0027,"},{"line_number":127,"context_line":"               default\u003d50,"},{"line_number":128,"context_line":"               min\u003d0, max\u003d150,"},{"line_number":129,"context_line":"               help\u003d\u0027Specifies the Storwize cleaning rate for the mapping. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"dc4fefb0_40b8627b","line":126,"in_reply_to":"ff13b4ad_a3588a68","updated":"2021-12-20 04:48:05.000000000","message":"Firstly, cleanrate is a parameter for FC map and not for the volume.\n\nFor existing volumes, even though the cleanrate param is not in the volume_type array, storage will set it to default value as 50 for the FC map.\nAfter we include this patch, for existing volume, this patch fill fetch the current value for cleanrate which is nothing but the default value: 50. Same as the value set by storage.\n\nFor the second part:\nAfter restarting the service also, the preference will be given to the extra-spec set in the volume_type. If that extra-spec is not given only then it will take the value from configuration.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":true,"context_lines":[{"line_number":128,"context_line":"               min\u003d0, max\u003d150,"},{"line_number":129,"context_line":"               help\u003d\u0027Specifies the Storwize cleaning rate for the mapping. \u0027"},{"line_number":130,"context_line":"                    \u0027The default rate is 50, and the valid rates are \u0027"},{"line_number":131,"context_line":"                    \u00270-150.\u0027),"},{"line_number":132,"context_line":"    cfg.StrOpt(\u0027storwize_svc_mirror_pool\u0027,"},{"line_number":133,"context_line":"               default\u003dNone,"},{"line_number":134,"context_line":"               help\u003d\u0027Specifies the name of the pool in which mirrored copy \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ec71465_05519c39","line":131,"updated":"2021-12-14 13:17:15.000000000","message":"We should spell out the behavior of setting it to 0.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[{"line_number":128,"context_line":"               min\u003d0, max\u003d150,"},{"line_number":129,"context_line":"               help\u003d\u0027Specifies the Storwize cleaning rate for the mapping. \u0027"},{"line_number":130,"context_line":"                    \u0027The default rate is 50, and the valid rates are \u0027"},{"line_number":131,"context_line":"                    \u00270-150.\u0027),"},{"line_number":132,"context_line":"    cfg.StrOpt(\u0027storwize_svc_mirror_pool\u0027,"},{"line_number":133,"context_line":"               default\u003dNone,"},{"line_number":134,"context_line":"               help\u003d\u0027Specifies the name of the pool in which mirrored copy \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"11da6637_796a1dfd","line":131,"in_reply_to":"7ec71465_05519c39","updated":"2021-12-20 04:48:05.000000000","message":"This segment is used only to specify the range of values that can be set. For further details user can go through the SVC manual for \u0027mkfcmap\u0027 options.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":true,"context_lines":[{"line_number":613,"context_line":"            ssh_cmd.append(\u0027-autodelete\u0027)"},{"line_number":614,"context_line":"        if consistgrp:"},{"line_number":615,"context_line":"            ssh_cmd.extend([\u0027-consistgrp\u0027, consistgrp])"},{"line_number":616,"context_line":"        if clean_rate:"},{"line_number":617,"context_line":"            ssh_cmd.extend([\u0027-cleanrate\u0027, str(int(clean_rate))])"},{"line_number":618,"context_line":"        out, err \u003d self._ssh(ssh_cmd, check_exit_code\u003dFalse)"},{"line_number":619,"context_line":"        if \u0027successfully created\u0027 not in out:"}],"source_content_type":"text/x-python","patch_set":3,"id":"806c71d4_de57fa09","line":616,"updated":"2021-12-14 13:17:15.000000000","message":"Why aren\u0027t we passing it when it\u0027s 0?  It seems to be an acceptable value[1], right? Or am I reading that wrong?\n\nhttps://www.ibm.com/docs/en/flashsystem-9x00/8.3.x?topic\u003dff-background-copy-cleaning-rates","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[{"line_number":613,"context_line":"            ssh_cmd.append(\u0027-autodelete\u0027)"},{"line_number":614,"context_line":"        if consistgrp:"},{"line_number":615,"context_line":"            ssh_cmd.extend([\u0027-consistgrp\u0027, consistgrp])"},{"line_number":616,"context_line":"        if clean_rate:"},{"line_number":617,"context_line":"            ssh_cmd.extend([\u0027-cleanrate\u0027, str(int(clean_rate))])"},{"line_number":618,"context_line":"        out, err \u003d self._ssh(ssh_cmd, check_exit_code\u003dFalse)"},{"line_number":619,"context_line":"        if \u0027successfully created\u0027 not in out:"}],"source_content_type":"text/x-python","patch_set":3,"id":"00521232_a34fba5f","line":616,"in_reply_to":"806c71d4_de57fa09","updated":"2021-12-20 04:48:05.000000000","message":"Agreed. This code will restrict cleanrate\u003d0\n\nWe will remove this check as we have a function check_clean_rate() to validate the range for cleanrate before calling mkfcmap()\n\nOR\n\nAs per the next comment, we can change it as:\nif clean_rate is not None:","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":true,"context_lines":[{"line_number":666,"context_line":""},{"line_number":667,"context_line":"    def chfcmap(self, fc_map_id, copyrate\u003d\u002750\u0027, clean_rate\u003dNone,"},{"line_number":668,"context_line":"                autodel\u003d\u0027on\u0027):"},{"line_number":669,"context_line":"        if clean_rate:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"            self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":3,"id":"674c8e8c_0d41141a","line":669,"updated":"2021-12-14 13:17:15.000000000","message":"-1: This seems wrong, because a value of 0 should still go into this side of the if.  I think we should change this to\n\n  if clean_rate is None:","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[{"line_number":666,"context_line":""},{"line_number":667,"context_line":"    def chfcmap(self, fc_map_id, copyrate\u003d\u002750\u0027, clean_rate\u003dNone,"},{"line_number":668,"context_line":"                autodel\u003d\u0027on\u0027):"},{"line_number":669,"context_line":"        if clean_rate:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"            self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":3,"id":"906616b3_10edde8b","line":669,"in_reply_to":"674c8e8c_0d41141a","updated":"2021-12-20 04:48:05.000000000","message":"Agreed. This will block clean_rate\u003d0.\n\nThe correct condition should be:\nif clean_rate is not None:\n\nThis condition will allow clean_rate\u003d0 as well.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":true,"context_lines":[{"line_number":669,"context_line":"        if clean_rate:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"            self.run_ssh_assert_no_output(ssh_cmd)"},{"line_number":673,"context_line":"        else:"},{"line_number":674,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-copyrate\u0027, copyrate,"},{"line_number":675,"context_line":"                       \u0027-autodelete\u0027, autodel, fc_map_id]"}],"source_content_type":"text/x-python","patch_set":3,"id":"48c384c6_262e9e5e","line":672,"updated":"2021-12-14 13:17:15.000000000","message":"-1: This is common to both cases, so in the if..else we should just set the ssh_cmd and then after do a common run_ssh_assert_no_output","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        if clean_rate:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"            self.run_ssh_assert_no_output(ssh_cmd)"},{"line_number":673,"context_line":"        else:"},{"line_number":674,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-copyrate\u0027, copyrate,"},{"line_number":675,"context_line":"                       \u0027-autodelete\u0027, autodel, fc_map_id]"}],"source_content_type":"text/x-python","patch_set":3,"id":"60f6e2e5_2f616fa5","line":672,"in_reply_to":"48c384c6_262e9e5e","updated":"2021-12-20 04:48:05.000000000","message":"Agreed. I will address it in the follow-up patch.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"88a298e7201a699d6bea0fa779956d5312c061e9","unresolved":true,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"90663632_54752966","line":5524,"updated":"2021-12-14 13:17:15.000000000","message":"-1: Correct me if I\u0027m wrong, but this seems like it wouldn\u0027t work as expected for volumes that were created before this patch, right?\n\nI mean, before this patch merges there is no clean_rate set in the extra specs and existing cinder volumes in the storage don\u0027t have it set either right? But the _get_vdisk_params will return the current self.configuration.storwize_svc_clean_rate for old volumes, right?.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"15aa742f4cc8964a84a24e0bfa539d22b289ee46","unresolved":true,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"d3576454_9253a9be","line":5524,"in_reply_to":"1250f191_60f0bcae","updated":"2022-01-18 18:52:10.000000000","message":"Thanks Gorka. I have addressed the concern and tested the code. Please review this change.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b1d4f8a0cc7442e930a0860584accf8f5ecb40dc","unresolved":true,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1250f191_60f0bcae","line":5524,"in_reply_to":"7166a492_46343284","updated":"2022-01-17 17:00:13.000000000","message":"I mean that if you remove this check on the rate (L5524) and always call the update_clean_rate method (L5525-5526) then you should be able to resolve the issue.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b1dd69a7fd9e3af18eecae1cc4cd76398cda5813","unresolved":false,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"cbc463f7_7c580161","line":5524,"in_reply_to":"78a2113d_72d55826","updated":"2022-01-10 13:54:23.000000000","message":"-1: There are cases where it won\u0027t work.\n\nPremises:\n\n- We have an old volume named VOL_1 of a type that doesn\u0027t specify the clean_rate\n- Create new volume type TYPE_N setting the clean_rate to 60\n- Change storwize_svc_clean_rate config option to 60\n\nIf we ask Cinder to retype VOL_1 to TYPE_N it won\u0027t set the clean rate to 60 as it should, because the old_opts has 60 as the clean_rate (the one configured in cinder.conf) which matches the one requested in the new volume type.\n\nThe problem is that we are not differentiating when a volume type doesn\u0027t have the clean_rate defined.\n\nProbably the easiest solution (even if it\u0027s a bit slower) is to always set the clean_rate.  Another alternative would be to store the clean_rate as part of the admin_metadata so it can be extracted regardless of the volume_type.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"25bf39e95558cfad54e3fe172e3367466c05e39f","unresolved":false,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"78a2113d_72d55826","line":5524,"in_reply_to":"90663632_54752966","updated":"2021-12-20 04:48:05.000000000","message":"Consider a volume created without this patch, no cleanrate set in volume_type extra specs or  configuration. In this case, the storage will assign the default value as 50 as we didn\u0027t explicitly pass it.\n\n\nSuppose we have case where:\n\nFor new volume, extraspecs clean_rate\u003d80 : \n - get_vdiskparams will fetch from extraspecs clean_rate\u003d80\n\n\nFor old volume, created before this patchset:(will not have clean_rate extraspecs set anyway, hence it will have the default value 50 set in the storage)\n - get_vdisk params will fetch the cself.configuration.storwize_svc_clean_rate\u003d 60(suppose)\n - for old volume, it may not fetch the actual cleanrate value(which is 50), but this condition is anyway satisfied and it sets new clean_rate\u003d80.","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"f9761d830e768229e69ebb7007c4bd32a9f1615b","unresolved":false,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7166a492_46343284","line":5524,"in_reply_to":"cbc463f7_7c580161","updated":"2022-01-17 14:15:19.000000000","message":"Hi Gorka, Thanks for your insight. I have tested this scenario and you are correct it doesn\u0027t update the cleanrate for old volume in this scenario.\n\nRequesting you to please elaborate the solution where you mentioned that it can be solved by always setting the clean_rate. As in are you referring to the cinder.conf or volume-specs ?","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d021be4aea02c2aac580e533cd22f87bc7191595","unresolved":false,"context_lines":[{"line_number":5521,"context_line":"            self._helpers.update_flashcopy_rate(volume.name,"},{"line_number":5522,"context_line":"                                                new_opts[\u0027flashcopy_rate\u0027])"},{"line_number":5523,"context_line":""},{"line_number":5524,"context_line":"        if new_opts[\u0027clean_rate\u0027] !\u003d old_opts[\u0027clean_rate\u0027]:"},{"line_number":5525,"context_line":"            self._helpers.update_clean_rate(volume.name,"},{"line_number":5526,"context_line":"                                            new_opts[\u0027clean_rate\u0027])"},{"line_number":5527,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"4b6cb6df_7b9e979c","line":5524,"in_reply_to":"d3576454_9253a9be","updated":"2022-01-31 15:49:35.000000000","message":"Thanks!","commit_id":"70d5d1287c50497bd14fec79d5f37962ed99158c"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"0fbadec121b2b7c537ce8c4ac117026f7766e584","unresolved":true,"context_lines":[{"line_number":2233,"context_line":"                     {\u0027cg\u0027: cgId})"},{"line_number":2234,"context_line":"        return volume_model_updates"},{"line_number":2235,"context_line":""},{"line_number":2236,"context_line":"    def check_clean_rate(self, clean_rate):"},{"line_number":2237,"context_line":"        \"\"\"Check if the value of clean_rate is valid.\"\"\""},{"line_number":2238,"context_line":"        if clean_rate not in range(0, 151):"},{"line_number":2239,"context_line":"            raise exception.InvalidInput("}],"source_content_type":"text/x-python","patch_set":4,"id":"e3dbf806_63455e79","line":2236,"range":{"start_line":2236,"start_character":8,"end_line":2236,"end_character":24},"updated":"2021-12-22 11:20:03.000000000","message":"No need to check it. It will be validated by oslo.config","commit_id":"a0365f77b2a6bbaf7dbb5b8537046ef6d211b51a"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"cf211097e6cb0d03b14f508480f96559b1b054c7","unresolved":false,"context_lines":[{"line_number":2233,"context_line":"                     {\u0027cg\u0027: cgId})"},{"line_number":2234,"context_line":"        return volume_model_updates"},{"line_number":2235,"context_line":""},{"line_number":2236,"context_line":"    def check_clean_rate(self, clean_rate):"},{"line_number":2237,"context_line":"        \"\"\"Check if the value of clean_rate is valid.\"\"\""},{"line_number":2238,"context_line":"        if clean_rate not in range(0, 151):"},{"line_number":2239,"context_line":"            raise exception.InvalidInput("}],"source_content_type":"text/x-python","patch_set":4,"id":"4618b239_771348c2","line":2236,"range":{"start_line":2236,"start_character":8,"end_line":2236,"end_character":24},"in_reply_to":"e3dbf806_63455e79","updated":"2021-12-24 10:02:23.000000000","message":"Thankyou for the review Ivan.\nI have removed this function and you were correct, oslo.conf takes care of the range validation.","commit_id":"a0365f77b2a6bbaf7dbb5b8537046ef6d211b51a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9751f2136c557734b30b8c38cf8162710f17a52e","unresolved":true,"context_lines":[{"line_number":669,"context_line":"        if clean_rate is not None:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"        else:"},{"line_number":673,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-copyrate\u0027, copyrate,"},{"line_number":674,"context_line":"                       \u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c63886a5_b3dccfc9","line":672,"range":{"start_line":672,"start_character":8,"end_line":672,"end_character":12},"updated":"2022-01-10 17:35:41.000000000","message":"Why do you have this asymmetry with the mkfcmap function?  That one can handle both clean_rate and copy_rate in the same call, whereas this function handles clean_rate separately.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"a9abd8605f9e376a38292e6304844e495dd01c5f","unresolved":true,"context_lines":[{"line_number":669,"context_line":"        if clean_rate is not None:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"        else:"},{"line_number":673,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-copyrate\u0027, copyrate,"},{"line_number":674,"context_line":"                       \u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":6,"id":"d3f21743_4d84678b","line":672,"range":{"start_line":672,"start_character":8,"end_line":672,"end_character":12},"in_reply_to":"7e13cd6b_9bbf37fe","updated":"2022-01-24 17:04:17.000000000","message":"Hi Brian, Thanks for the insight.\nI have addressed the concern and made changes which will allow to update clean_rate and copy_rate at the same time and also separately.\nAlso, the same for autodelete parameter.\n\nPlease let me know if there is anything else.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"15aa742f4cc8964a84a24e0bfa539d22b289ee46","unresolved":true,"context_lines":[{"line_number":669,"context_line":"        if clean_rate is not None:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"        else:"},{"line_number":673,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-copyrate\u0027, copyrate,"},{"line_number":674,"context_line":"                       \u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":6,"id":"eef2ad9f_8cfc0985","line":672,"range":{"start_line":672,"start_character":8,"end_line":672,"end_character":12},"in_reply_to":"c63886a5_b3dccfc9","updated":"2022-01-18 18:52:10.000000000","message":"Hi Brian, yes this intended because not everytime we are updating both flashcopy and clean_rate together. Hence they are handled separately.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"a4937809ff69a961903f53421d3694d23fcb5816","unresolved":true,"context_lines":[{"line_number":669,"context_line":"        if clean_rate is not None:"},{"line_number":670,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-cleanrate\u0027,"},{"line_number":671,"context_line":"                       clean_rate, fc_map_id]"},{"line_number":672,"context_line":"        else:"},{"line_number":673,"context_line":"            ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027, \u0027-copyrate\u0027, copyrate,"},{"line_number":674,"context_line":"                       \u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7e13cd6b_9bbf37fe","line":672,"range":{"start_line":672,"start_character":8,"end_line":672,"end_character":12},"in_reply_to":"eef2ad9f_8cfc0985","updated":"2022-01-18 20:19:54.000000000","message":"Sure, but this design makes it *impossible* to update them at the same time.  Even worse, if the function is called like chfcmap(1234, 22, 33, \u0027off\u0027), the clean_rate will be updated, but the other parameters will be silently ignored.  That is very un-obvious behavior.\n\nIn any case, this function doesn\u0027t look well designed.  Suppose that someone makes the following sequence of calls:\n\n  chfcmap(1234, copyrate\u003d\u002722\u0027)\n  chfcmap(1234, autodel\u003d\u0027off\u0027)\n\nAfter these two calls, autodel is off (which we want), but the copyrate is set to 50 (which we don\u0027t want, we set it to 22).\n\nI think you need to re-think the design here.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9751f2136c557734b30b8c38cf8162710f17a52e","unresolved":true,"context_lines":[{"line_number":5394,"context_line":"        all_keys \u003d no_copy_keys + copy_keys"},{"line_number":5395,"context_line":"        old_opts \u003d self._get_vdisk_params(volume[\u0027volume_type_id\u0027],"},{"line_number":5396,"context_line":"                                          volume_metadata\u003d"},{"line_number":5397,"context_line":"                                          volume.get(\u0027volume_matadata\u0027))"},{"line_number":5398,"context_line":"        new_opts \u003d self._get_vdisk_params(new_type[\u0027id\u0027],"},{"line_number":5399,"context_line":"                                          volume_type\u003dnew_type)"},{"line_number":5400,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf1171e7_a0692bec","line":5397,"range":{"start_line":5397,"start_character":54,"end_line":5397,"end_character":69},"updated":"2022-01-10 17:35:41.000000000","message":"This doesn\u0027t look correct (also happens at lines 3928 and 4017).  Should be fixed in a followup.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"15aa742f4cc8964a84a24e0bfa539d22b289ee46","unresolved":true,"context_lines":[{"line_number":5394,"context_line":"        all_keys \u003d no_copy_keys + copy_keys"},{"line_number":5395,"context_line":"        old_opts \u003d self._get_vdisk_params(volume[\u0027volume_type_id\u0027],"},{"line_number":5396,"context_line":"                                          volume_metadata\u003d"},{"line_number":5397,"context_line":"                                          volume.get(\u0027volume_matadata\u0027))"},{"line_number":5398,"context_line":"        new_opts \u003d self._get_vdisk_params(new_type[\u0027id\u0027],"},{"line_number":5399,"context_line":"                                          volume_type\u003dnew_type)"},{"line_number":5400,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"88b09da5_d4d11372","line":5397,"range":{"start_line":5397,"start_character":54,"end_line":5397,"end_character":69},"in_reply_to":"bf1171e7_a0692bec","updated":"2022-01-18 18:52:10.000000000","message":"Fixed the spelling mistake.","commit_id":"73daaef7c6018ec9c29b6aa3dac1e43b5e9291af"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"523e117006bbef33664625d870d2f9ca9bf820eb","unresolved":true,"context_lines":[{"line_number":669,"context_line":"        ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027]"},{"line_number":670,"context_line":"        if clean_rate is not None:"},{"line_number":671,"context_line":"            ssh_cmd +\u003d [\u0027-cleanrate\u0027, clean_rate]"},{"line_number":672,"context_line":"        elif copyrate is not None:"},{"line_number":673,"context_line":"            ssh_cmd +\u003d [\u0027-copyrate\u0027, copyrate]"},{"line_number":674,"context_line":"        ssh_cmd +\u003d [\u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":9,"id":"2bf365b0_ed8fe504","line":672,"range":{"start_line":672,"start_character":8,"end_line":672,"end_character":12},"updated":"2022-01-27 20:42:02.000000000","message":"I don\u0027t think you want elif here ... shouldn\u0027t this be another \u0027if\u0027?  The way you have this now, if someone makes the call\n\n  chfcmap(copyrate\u003d\u002722\u0027, clean_rate\u003d\u002733\u0027)\n\nthe clean_rate will be updated, but the copyrate will be ignored.","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"5533996871c57b592d1bcc084010e6ab8566d8e9","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        ssh_cmd \u003d [\u0027svctask\u0027, \u0027chfcmap\u0027]"},{"line_number":670,"context_line":"        if clean_rate is not None:"},{"line_number":671,"context_line":"            ssh_cmd +\u003d [\u0027-cleanrate\u0027, clean_rate]"},{"line_number":672,"context_line":"        elif copyrate is not None:"},{"line_number":673,"context_line":"            ssh_cmd +\u003d [\u0027-copyrate\u0027, copyrate]"},{"line_number":674,"context_line":"        ssh_cmd +\u003d [\u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"}],"source_content_type":"text/x-python","patch_set":9,"id":"81b0aeaa_a14369b7","line":672,"range":{"start_line":672,"start_character":8,"end_line":672,"end_character":12},"in_reply_to":"2bf365b0_ed8fe504","updated":"2022-01-28 11:29:04.000000000","message":"Thanks Brian. This is fixed.","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"523e117006bbef33664625d870d2f9ca9bf820eb","unresolved":true,"context_lines":[{"line_number":671,"context_line":"            ssh_cmd +\u003d [\u0027-cleanrate\u0027, clean_rate]"},{"line_number":672,"context_line":"        elif copyrate is not None:"},{"line_number":673,"context_line":"            ssh_cmd +\u003d [\u0027-copyrate\u0027, copyrate]"},{"line_number":674,"context_line":"        ssh_cmd +\u003d [\u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"    def stopfcmap(self, fc_map_id, force\u003dFalse, split\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":9,"id":"42b73673_70e757ec","line":674,"range":{"start_line":674,"start_character":8,"end_line":674,"end_character":54},"updated":"2022-01-27 20:42:02.000000000","message":"I hate to be a pest about this, but if I make the calls:\n\n  chfcmap(autodel\u003d\u0027off\u0027)\n  chfcmap(clean_rate\u003d\u002755\u0027)\n\nthe second call is going to turn autodel back on again ... that doesn\u0027t seem correct to me.","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"},{"author":{"_account_id":32074,"name":"Harsh Ailani","email":"haailani@in.ibm.com","username":"haailani"},"change_message_id":"5533996871c57b592d1bcc084010e6ab8566d8e9","unresolved":false,"context_lines":[{"line_number":671,"context_line":"            ssh_cmd +\u003d [\u0027-cleanrate\u0027, clean_rate]"},{"line_number":672,"context_line":"        elif copyrate is not None:"},{"line_number":673,"context_line":"            ssh_cmd +\u003d [\u0027-copyrate\u0027, copyrate]"},{"line_number":674,"context_line":"        ssh_cmd +\u003d [\u0027-autodelete\u0027, autodel, fc_map_id]"},{"line_number":675,"context_line":"        self.run_ssh_assert_no_output(ssh_cmd)"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"    def stopfcmap(self, fc_map_id, force\u003dFalse, split\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":9,"id":"da07c29f_f0b134ee","line":674,"range":{"start_line":674,"start_character":8,"end_line":674,"end_character":54},"in_reply_to":"42b73673_70e757ec","updated":"2022-01-28 11:29:04.000000000","message":"Firstly the mentioned use case is not possible in our plugin, as we always want autodel \u003d\u0027on\u0027 by default.\n\nCurrently there are no scenarios where autodel is required as \u0027off\u0027. Hence the call chfcmap(autodel\u003d\u0027off\u0027) is never made.\n\nIn future, if the user wants autodel to be off, then chfcmap call should always be made by specifying autodel\u003d\u0027off\u0027","commit_id":"7dd1ca1f36640d128b3a1bb12d94e1e27d419d35"}],"releasenotes/notes/ibm-svf-add-cleanrate-support-e246a8f218d2f22e.yaml":[{"author":{"_account_id":29122,"name":"Raghavendra Tilay","email":"raghavendra-uddhav.tilay@hpe.com","username":"raghavendrat"},"change_message_id":"5331f364a9f4f5e160cedde69359bb6b1541064b","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    IBM Spectrum Virtualize Family driver: Added support for passing cleanrate parameter for volume-types."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"70a014c8_e1643756","line":4,"updated":"2021-10-01 09:32:54.000000000","message":"Max characters in a line should be 79.\nIf there are more characters, zuul may report error.","commit_id":"89682a543c56e7c08f0c5702e09a3370cafad65b"}]}