)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"01d69c1b0d0caf3b2ccb3908ef6bec5194478edd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"71d0fa67_70120da3","updated":"2025-03-12 20:14:44.000000000","message":"A couple of nits noted inline, but otherwise code \u0026 tests LGTM.  (I do want to second Rajat\u0027s point about reducing duplicated code, though.)","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"2f2e12d54353a82359f8233fb5d6dfd1e74b8d44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"89234d2c_1f845dbc","updated":"2025-03-07 09:14:43.000000000","message":"A followup inline.\nThe Pure CI hasn\u0027t reported on the latest PS and in previous PS also it failed with different protocols but since Simon is +1 on it and the author is also from Pure, I am hopeful this is properly tested so LGTM.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"c4dd1153a38442954ad26e6f85b0686e22d36148","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"60e36d79_33741742","updated":"2025-03-05 13:35:50.000000000","message":"Looks good to me.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5eb1eb075417d044e0fcc3e37d9b61c3f1f249e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8231c2b5_74d36f63","updated":"2025-03-13 17:08:43.000000000","message":"See inline for the fix that needs to be applied to the test this patch adds.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"49dee6f85ba30d8d621c10b988a9a053b7da7721","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"623b9e0e_c4a994c6","updated":"2025-03-12 22:23:38.000000000","message":"recheck","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":35316,"name":"Keerthivasan S","email":"ksuresh@purestorage.com","username":"keerthivasan"},"change_message_id":"91b11afb5858b185c3f69249c47cc23a0231464f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ef28e15d_e2aa382f","updated":"2025-03-05 16:05:06.000000000","message":"recheck","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"479aa6417a7ce3fdccbd9fceeb87bbc70631f502","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"99e7807d_e44c6d50","in_reply_to":"89234d2c_1f845dbc","updated":"2025-03-07 17:06:05.000000000","message":"The failing protocols are non-voting deliberately because our CI has infrastructure issues we are working on.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"}],"cinder/tests/unit/volume/drivers/test_pure.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5eb1eb075417d044e0fcc3e37d9b61c3f1f249e5","unresolved":true,"context_lines":[{"line_number":1885,"context_line":"                                                     source\u003d"},{"line_number":1886,"context_line":"                                                     pure.flasharray."},{"line_number":1887,"context_line":"                                                     reference(name\u003dsrc_name),"},{"line_number":1888,"context_line":"                                                     qos\u003dqos_data)"},{"line_number":1889,"context_line":"        mock_fa.return_value \u003d mock_data"},{"line_number":1890,"context_line":"        mock_qos_specs.return_value \u003d qos"},{"line_number":1891,"context_line":"        self.driver.create_cloned_volume(vol, src_vol)"}],"source_content_type":"text/x-python","patch_set":2,"id":"856a3d87_e420e535","line":1888,"updated":"2025-03-13 17:08:43.000000000","message":"This is missing:\n\n    self.driver._get_volume_type_extra_spec \u003d mock.Mock(\n        return_value\u003d{})\n\nbefore create_cloned_volume() is called.\n\nThe failure is happening because without the mock, the real function is used and it looks for a non-existent volume type in the database. Not sure exactly why that problem didn\u0027t happen when the unit tests ran on this patch.  It only showed up when the tests added by https://review.opendev.org/c/openstack/cinder/+/933675 were run on top of these.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"}],"cinder/volume/drivers/pure.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"01d69c1b0d0caf3b2ccb3908ef6bec5194478edd","unresolved":true,"context_lines":[{"line_number":818,"context_line":"                               volume[\"size\"])"},{"line_number":819,"context_line":"        # Check if the volume_type has QoS settings and if so"},{"line_number":820,"context_line":"        # apply them to the newly created volume"},{"line_number":821,"context_line":"        qos \u003d None"},{"line_number":822,"context_line":"        qos \u003d self._get_qos_settings(volume.volume_type)"},{"line_number":823,"context_line":"        if qos:"},{"line_number":824,"context_line":"            self.set_qos(current_array, vol_name, qos)"}],"source_content_type":"text/x-python","patch_set":2,"id":"49f367e5_74a298ff","line":821,"range":{"start_line":821,"start_character":8,"end_line":821,"end_character":18},"updated":"2025-03-12 20:14:44.000000000","message":"nit: don\u0027t need to initialize this, it\u0027s about to get a value at line 822, and if that function raises an exception, we\u0027re not catching it and will never see line 823","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"2f2e12d54353a82359f8233fb5d6dfd1e74b8d44","unresolved":true,"context_lines":[{"line_number":816,"context_line":"                               vol_name,"},{"line_number":817,"context_line":"                               src_vref[\"size\"],"},{"line_number":818,"context_line":"                               volume[\"size\"])"},{"line_number":819,"context_line":"        # Check if the volume_type has QoS settings and if so"},{"line_number":820,"context_line":"        # apply them to the newly created volume"},{"line_number":821,"context_line":"        qos \u003d None"},{"line_number":822,"context_line":"        qos \u003d self._get_qos_settings(volume.volume_type)"},{"line_number":823,"context_line":"        if qos:"},{"line_number":824,"context_line":"            self.set_qos(current_array, vol_name, qos)"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"        return self._setup_volume(current_array, volume, vol_name)"},{"line_number":827,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"efaf53c1_a833030a","line":824,"range":{"start_line":819,"start_character":8,"end_line":824,"end_character":54},"updated":"2025-03-07 09:14:43.000000000","message":"this is good for now but a better followup to avoid similar issues would be to use the common _setup_volume method for the qos setting.\nAs i can see, there are 3 cases where _setup_volume method is called,\n1. create volume\n2. create volume from snapshot\n3. create cloned volume\n\nall of them have handled the QoS separately which would be better to do it in _setup_volume method\nAn example from the RBD driver is here[1]\n\n[1] https://github.com/openstack/cinder/blob/bded1ba83f33107dc20102953de7c6407bebb1f2/cinder/volume/drivers/rbd.py#L1054-L1062","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"479aa6417a7ce3fdccbd9fceeb87bbc70631f502","unresolved":false,"context_lines":[{"line_number":816,"context_line":"                               vol_name,"},{"line_number":817,"context_line":"                               src_vref[\"size\"],"},{"line_number":818,"context_line":"                               volume[\"size\"])"},{"line_number":819,"context_line":"        # Check if the volume_type has QoS settings and if so"},{"line_number":820,"context_line":"        # apply them to the newly created volume"},{"line_number":821,"context_line":"        qos \u003d None"},{"line_number":822,"context_line":"        qos \u003d self._get_qos_settings(volume.volume_type)"},{"line_number":823,"context_line":"        if qos:"},{"line_number":824,"context_line":"            self.set_qos(current_array, vol_name, qos)"},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"        return self._setup_volume(current_array, volume, vol_name)"},{"line_number":827,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"f2841e2d_96d73f58","line":824,"range":{"start_line":819,"start_character":8,"end_line":824,"end_character":54},"in_reply_to":"efaf53c1_a833030a","updated":"2025-03-07 17:06:05.000000000","message":"We are going to do some refactoring soon and this is part of the plan.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a76a06a38a5000658ac7f5dec4d854756ee5dc55","unresolved":true,"context_lines":[{"line_number":2104,"context_line":""},{"line_number":2105,"context_line":"            qos[\u0027maxIOPS\u0027] \u003d iops_qos"},{"line_number":2106,"context_line":"            qos[\u0027maxBWS\u0027] \u003d bw_qos"},{"line_number":2107,"context_line":"        return qos"},{"line_number":2108,"context_line":""},{"line_number":2109,"context_line":"    def _generate_purity_vol_name(self, volume):"},{"line_number":2110,"context_line":"        \"\"\"Return the name of the volume Purity will use."}],"source_content_type":"text/x-python","patch_set":2,"id":"bd474e5a_93417bcc","line":2107,"range":{"start_line":2107,"start_character":0,"end_line":2107,"end_character":18},"updated":"2025-03-07 09:29:59.000000000","message":"one question, shouldn\u0027t we check if the qos is backend/both and then only return it? If the QoS are only intended for front end then we are setting it wrong here.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"479aa6417a7ce3fdccbd9fceeb87bbc70631f502","unresolved":false,"context_lines":[{"line_number":2104,"context_line":""},{"line_number":2105,"context_line":"            qos[\u0027maxIOPS\u0027] \u003d iops_qos"},{"line_number":2106,"context_line":"            qos[\u0027maxBWS\u0027] \u003d bw_qos"},{"line_number":2107,"context_line":"        return qos"},{"line_number":2108,"context_line":""},{"line_number":2109,"context_line":"    def _generate_purity_vol_name(self, volume):"},{"line_number":2110,"context_line":"        \"\"\"Return the name of the volume Purity will use."}],"source_content_type":"text/x-python","patch_set":2,"id":"c3d4e5c6_bee48fe2","line":2107,"range":{"start_line":2107,"start_character":0,"end_line":2107,"end_character":18},"in_reply_to":"bd474e5a_93417bcc","updated":"2025-03-07 17:06:05.000000000","message":"The QoS parameters used by Pure do not match any frontend parameters so there can be no conflict.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"}],"releasenotes/notes/pure_fix_clone_qos-be824feb707d8223.yaml":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"01d69c1b0d0caf3b2ccb3908ef6bec5194478edd","unresolved":true,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Pure Storage driver `bug #2100547"},{"line_number":5,"context_line":"    \u003chttps://bugs.launchpad.net/cinder/+bug/2100547\u003e`_: Fixed issue where"},{"line_number":6,"context_line":"    volumes created as clones from a source image volume do get"},{"line_number":7,"context_line":"    the defined QoS settings associated with the volume type used."}],"source_content_type":"text/x-yaml","patch_set":2,"id":"2dcf3405_5a003bfd","line":5,"range":{"start_line":5,"start_character":56,"end_line":5,"end_character":73},"updated":"2025-03-12 20:14:44.000000000","message":"nit: \"Fixed issue where\" leads me to think you\u0027re about to say what is wrong (volumes did not get qos), but you say what is right.  Not going to hold this up over that, though.","commit_id":"47908f9f4e3d7e956cb257d4327ad625c6e3fd69"}]}