)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2b26dd5847f39536f2fceab6ca6fda1b3de62770","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Log information about the v2 clone API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Starting with the Mimic release and the RBD clone v2 feature, it is no"},{"line_number":10,"context_line":"longer be necessary to manage snapshot protection status. The usage of"},{"line_number":11,"context_line":"clone v2 is controlled via the “rbd default clone format” configuration"},{"line_number":12,"context_line":"option which defaults to “auto”, but can be overridden to “1” to force"},{"line_number":13,"context_line":"the legacy clone v1 behavior or “2” to force the new clone v2 behavior."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f65232a_6d9e4864","line":10,"range":{"start_line":10,"start_character":7,"end_line":10,"end_character":9},"updated":"2020-10-22 11:05:34.000000000","message":"nit: remove \"be\"","commit_id":"0cbedbab5344c5811d9590fb857f5b164eded5bc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2383ba9b9755cf6157fca90d511e246be1848824","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Sofia Enriquez \u003clsofia.enriquez@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-11-30 19:08:13 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Log information about the v2 clone API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Starting with the Mimic release and the RBD clone v2 feature, it is no"},{"line_number":10,"context_line":"longer necessary to manage snapshot protection status. The usage of"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"3957149c_0fe69314","line":7,"range":{"start_line":7,"start_character":26,"end_line":7,"end_character":28},"updated":"2020-11-30 19:49:18.000000000","message":"Ceph v2","commit_id":"b1dd6bde42cf4f591a5a1689d9f0413d1f6d2b63"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2383ba9b9755cf6157fca90d511e246be1848824","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Starting with the Mimic release and the RBD clone v2 feature, it is no"},{"line_number":10,"context_line":"longer necessary to manage snapshot protection status. The usage of"},{"line_number":11,"context_line":"clone v2 is controlled via the “rbd default clone format” configuration"},{"line_number":12,"context_line":"option which defaults to “auto”, but can be overridden to “1” to force"},{"line_number":13,"context_line":"the legacy clone v1 behavior or “2” to force the new clone v2 behavior."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"62a013e3_0ce22e7f","line":11,"range":{"start_line":11,"start_character":27,"end_line":11,"end_character":30},"updated":"2020-11-30 19:49:18.000000000","message":"the Ceph","commit_id":"b1dd6bde42cf4f591a5a1689d9f0413d1f6d2b63"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2383ba9b9755cf6157fca90d511e246be1848824","unresolved":true,"context_lines":[{"line_number":12,"context_line":"option which defaults to “auto”, but can be overridden to “1” to force"},{"line_number":13,"context_line":"the legacy clone v1 behavior or “2” to force the new clone v2 behavior."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"The idea of this patch is to notify the user wherever is using or not"},{"line_number":16,"context_line":"the V2 Clone API."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: Ib52879a270a4ae4cdd3cb5fc18f2b7bdbccd8ab5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"0f62f384_4485e7ad","line":16,"range":{"start_line":15,"start_character":0,"end_line":16,"end_character":17},"updated":"2020-11-30 19:49:18.000000000","message":"I suggest something like: Because using the Ceph v2 clone API improves performance, this patch has Cinder log a message for the operator indicating whether Ceph has been configured to use the v2 clone API so that the operator can consult with the Ceph administrator to take appropriate action.","commit_id":"b1dd6bde42cf4f591a5a1689d9f0413d1f6d2b63"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"f6ae16437c0eb898806319bf941066b1fe9d7eda","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"volume.volume.op_features() changes depending of the version:"},{"line_number":17,"context_line":"- \u003d\u003d 0 , when a volume has a clone using the v1 clone API"},{"line_number":18,"context_line":"- \u003d\u003d 1 , when a volume has a clone using the v2 clone API"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Because using the Ceph v2 clone API improves performance, this patch has"},{"line_number":21,"context_line":"Cinder log a message for the operator indicating whether Ceph has been"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"6afa1794_bdf1bd94","line":18,"range":{"start_line":18,"start_character":1,"end_line":18,"end_character":6},"updated":"2021-01-22 20:50:47.000000000","message":"This isn\u0027t correct -- it sets a bit in the returned integer to true, it doesn\u0027t return 1.","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"af631e33a0fc23d63eba12bf683c3179e10feff8","unresolved":true,"context_lines":[{"line_number":11,"context_line":"The usage of clone v2 is controlled via the Ceph “rbd default"},{"line_number":12,"context_line":"clone format” configuration option which defaults to “auto”, but"},{"line_number":13,"context_line":"can be overridden to “1” to force the legacy clone v1 behavior or"},{"line_number":14,"context_line":"“2” to force the new clonev2 behavior."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"As an alternative, setting the minimum client to mimic"},{"line_number":17,"context_line":"\"$ ceph osd set-require-min-compat-client mimic\""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"2806b910_5db04221","line":14,"updated":"2021-01-28 16:38:19.000000000","message":"Maybe this is obvious to everyone, but I suggest making it clear that this must be configured *in Ceph* -- it\u0027s not something that can be configured on the cinder side.  (I don\u0027t want operators scrolling through the changes to get the wrong impression of what this patch is doing.)","commit_id":"db470a9e05d970782f042f580a28663ddfbe0ef0"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"af631e33a0fc23d63eba12bf683c3179e10feff8","unresolved":true,"context_lines":[{"line_number":28,"context_line":"In this way the operator can consult with the Ceph administrator"},{"line_number":29,"context_line":"to take appropriate action."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"You can determine which volumes are using the clone v2 feature"},{"line_number":32,"context_line":"thanks to volume.volume.op_features(). It returns a value depending"},{"line_number":33,"context_line":"on the v2 clone api being enabled or not. Furthermore"},{"line_number":34,"context_line":"RBD_FEATURE_CLONE_PARENT is used as bitwise check. You need to keep"},{"line_number":35,"context_line":"it so that when you have a volume with a different feature flag set,"},{"line_number":36,"context_line":"the bitwise check still returns the correct result. The possible"},{"line_number":37,"context_line":"return values of op_features() are not just 0 or 1. Your current"},{"line_number":38,"context_line":"check would return true for a volume with CLONE_PARENT (1\u003c\u003c0)"},{"line_number":39,"context_line":"off but with RBD_OPERATION_FEATURE_SNAP_TRASH (1\u003c\u003c3)"},{"line_number":40,"context_line":"or RBD_OPERATION_FEATURE_GROUP (1\u003c\u003c2) set, which is incorrect."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"Co-authored-by: Eric Harney \u003ceharney@redhat.com\u003e"},{"line_number":43,"context_line":"Change-Id: Ib52879a270a4ae4cdd3cb5fc18f2b7bdbccd8ab5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"5eb19a4e_55147f39","line":40,"range":{"start_line":31,"start_character":0,"end_line":40,"end_character":62},"updated":"2021-01-28 16:38:19.000000000","message":"The first two sentences are a helpful hint to developers, but the rest of this paragraph, particularly the last sentence, don\u0027t seem appropriate for a commit message.","commit_id":"db470a9e05d970782f042f580a28663ddfbe0ef0"}],"cinder/tests/unit/volume/drivers/test_rbd.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2b26dd5847f39536f2fceab6ca6fda1b3de62770","unresolved":false,"context_lines":[{"line_number":881,"context_line":"                mock_log:"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"            self.driver.create_snapshot(self.snapshot)"},{"line_number":884,"context_line":"            self.assertTrue(mock_log.info.called)"},{"line_number":885,"context_line":""},{"line_number":886,"context_line":"    @common_mocks"},{"line_number":887,"context_line":"    @mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f65232a_6df3e89f","line":884,"updated":"2020-10-22 11:05:34.000000000","message":"nit: I think it\u0027s best to check that only one call has happened and also that we didn\u0027t call warning:\n\n  mock_log.info.assert_called_once_with(mock.ANY)\n  mock_log.warning.assert_not_called()\n\nSame for the other 2 tests","commit_id":"0cbedbab5344c5811d9590fb857f5b164eded5bc"}],"cinder/volume/drivers/rbd.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"114f0568210c8ccd0ed0f278e5a17ca553192ddd","unresolved":false,"context_lines":[{"line_number":1182,"context_line":"            volume.protect_snap(snap)"},{"line_number":1183,"context_line":""},{"line_number":1184,"context_line":"            if not self._clone_v2_api_checked:"},{"line_number":1185,"context_line":"                # op_features returning 1 means we are using clone v3 API"},{"line_number":1186,"context_line":"                if (volume.volume.op_features() \u0026"},{"line_number":1187,"context_line":"                        self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":1188,"context_line":"                    LOG.info(\u0027using v2 clone API\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_92b780f9","line":1185,"range":{"start_line":1185,"start_character":67,"end_line":1185,"end_character":69},"updated":"2020-09-29 13:38:29.000000000","message":"v2, but this comment isn\u0027t really necessary since it just explains the one-liner below it","commit_id":"d8e411609d1395f821a236ae347a8e492cba7d4e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"114f0568210c8ccd0ed0f278e5a17ca553192ddd","unresolved":false,"context_lines":[{"line_number":1189,"context_line":""},{"line_number":1190,"context_line":"                else:"},{"line_number":1191,"context_line":"                    LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":1192,"context_line":"                                \u0027 mimic+ and set the osd option for better\u0027"},{"line_number":1193,"context_line":"                                \u0027 performance, fewer deletion issues\u0027)"},{"line_number":1194,"context_line":"                self._clone_v2_api_checked \u003d True"},{"line_number":1195,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_f2c7dc64","line":1192,"range":{"start_line":1192,"start_character":53,"end_line":1192,"end_character":63},"updated":"2020-09-29 13:38:29.000000000","message":"We should specify more about what this is, something like:\n    \"set the OSD minimum client compat version to mimic\"","commit_id":"d8e411609d1395f821a236ae347a8e492cba7d4e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4207f0e1c361442a17d7f833a68f4d2cdb92f207","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        self._is_replication_enabled \u003d False"},{"line_number":247,"context_line":"        self._replication_targets \u003d []"},{"line_number":248,"context_line":"        self._target_names \u003d []"},{"line_number":249,"context_line":"        self._clone_v2_api_checked \u003d False"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        if self.rbd is not None:"},{"line_number":252,"context_line":"            self.RBD_FEATURE_LAYERING \u003d self.rbd.RBD_FEATURE_LAYERING"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_78d8a27f","line":249,"updated":"2020-10-01 17:26:35.000000000","message":"The logic of caching this check doesn\u0027t match what we actually need.\n\nIn cases where the minimum client compat version was recently changed (enabling the v2 clone API), some volumes will still be in the old format, which means the driver will only ever say that the new API is not being used.","commit_id":"c4073604a0c1f998308749cf021e5d33e1635e8b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0ab4773ac7276cacaf5d064b6affb3866724c48d","unresolved":false,"context_lines":[{"line_number":1191,"context_line":"                                \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":1192,"context_line":"                                \u0027 compat version to mimic for better\u0027"},{"line_number":1193,"context_line":"                                \u0027 performance, fewer deletion issues\u0027)"},{"line_number":1194,"context_line":"                self.clone_v3_api_checked \u003d True"},{"line_number":1195,"context_line":""},{"line_number":1196,"context_line":"    def delete_snapshot(self, snapshot):"},{"line_number":1197,"context_line":"        \"\"\"Deletes an rbd snapshot.\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_04e5c93e","line":1194,"range":{"start_line":1194,"start_character":27,"end_line":1194,"end_character":29},"updated":"2020-10-01 14:23:24.000000000","message":"Typo -- needs some unit tests.","commit_id":"c4073604a0c1f998308749cf021e5d33e1635e8b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2b26dd5847f39536f2fceab6ca6fda1b3de62770","unresolved":false,"context_lines":[{"line_number":1180,"context_line":"            volume.create_snap(snap)"},{"line_number":1181,"context_line":"            volume.protect_snap(snap)"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"            if (volume.volume.op_features() \u0026"},{"line_number":1184,"context_line":"                    self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":1185,"context_line":"                LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":1186,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f65232a_adb7c0d8","line":1183,"range":{"start_line":1183,"start_character":30,"end_line":1183,"end_character":41},"updated":"2020-10-22 11:05:34.000000000","message":"I\u0027m not thrilled to add an additional call to the Ceph cluster on each snapshot creation just to show a warning.  Can we do this on driver initialization or when we gather stats or something?\n\n-1: In any case this call can raise an exception, and I don\u0027t think we should fail the snapshot if this part fails.","commit_id":"0cbedbab5344c5811d9590fb857f5b164eded5bc"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"47de174103003a3ebaefec5988cd40a4bbe7542e","unresolved":false,"context_lines":[{"line_number":1180,"context_line":"            volume.create_snap(snap)"},{"line_number":1181,"context_line":"            volume.protect_snap(snap)"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"            if (volume.volume.op_features() \u0026"},{"line_number":1184,"context_line":"                    self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":1185,"context_line":"                LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":1186,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f65232a_f0b7cb85","line":1183,"range":{"start_line":1183,"start_character":30,"end_line":1183,"end_character":41},"in_reply_to":"3f65232a_adb7c0d8","updated":"2020-10-22 12:39:38.000000000","message":"I\u0027m not sure that doing it at driver initialization works -- if you\u0027ve recently enabled the new clone format, older images wouldn\u0027t have it set.  I believe the CLONE_PARENT flag also only shows up on an image after you have created a snapshot from that image.  It might be possible to do it at startup by looking for the last created snapshot and examining that volume?\n\nI agree about catching the exception.","commit_id":"0cbedbab5344c5811d9590fb857f5b164eded5bc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3b1b22c527d592cfa2bdf619f2f1d73a9f0370df","unresolved":true,"context_lines":[{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"},{"line_number":296,"context_line":"                pass"},{"line_number":297,"context_line":"            LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":298,"context_line":"                        \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":299,"context_line":"                        \u0027 compat version to mimic for better\u0027"},{"line_number":300,"context_line":"                        \u0027 performance, fewer deletion issues\u0027)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_target_config(self, target_id):"},{"line_number":303,"context_line":"        \"\"\"Get a replication target from known replication targets.\"\"\""}],"source_content_type":"text/x-python","patch_set":16,"id":"b4108e97_690a06e2","line":300,"range":{"start_line":297,"start_character":0,"end_line":300,"end_character":62},"updated":"2020-12-02 10:52:18.000000000","message":"I\u0027m not sure but if we want to print this warning only one time, isn\u0027t it better to do in __init__ method which will also save us from all these checks ?","commit_id":"b8a99aaa287e633f3d5d21ec1dc95dc081f5f8c3"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0ebe29a3a583923b9d305dc001ae5752d029ecfd","unresolved":true,"context_lines":[{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"},{"line_number":296,"context_line":"                pass"},{"line_number":297,"context_line":"            LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":298,"context_line":"                        \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":299,"context_line":"                        \u0027 compat version to mimic for better\u0027"},{"line_number":300,"context_line":"                        \u0027 performance, fewer deletion issues\u0027)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_target_config(self, target_id):"},{"line_number":303,"context_line":"        \"\"\"Get a replication target from known replication targets.\"\"\""}],"source_content_type":"text/x-python","patch_set":16,"id":"dbcd0e89_9b4f06a2","line":300,"range":{"start_line":297,"start_character":0,"end_line":300,"end_character":62},"in_reply_to":"b4108e97_690a06e2","updated":"2020-12-02 15:52:18.000000000","message":"Hi Rajat thanks for your comment, \nIn my point of view we only want to print this (1) when the driver initializes and (2) when the user enables the v2 clone api. \n\nThe problem of doing it at __init__ method:\n- The user can enable/disable the v2 clone api without restarting c-vol.\n- In order to do it in __init__ method I need to access the last snapshot created and then find the snap\u0027s volume to check the os_features(). And I couldn\u0027t find an efficient way to do it.Let me know if I\u0027m missing something.","commit_id":"b8a99aaa287e633f3d5d21ec1dc95dc081f5f8c3"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"57f5b3a4479e81bb5a0e88bcd339dbfa1ecaa32c","unresolved":true,"context_lines":[{"line_number":278,"context_line":"                if not (self._last_value_v2_clone_api \u003d\u003d v2):"},{"line_number":279,"context_line":"                    self._last_value_v2_clone_api \u003d v2"},{"line_number":280,"context_line":"                    return True"},{"line_number":281,"context_line":"        except AttributeError:"},{"line_number":282,"context_line":"            pass"},{"line_number":283,"context_line":"        return False"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def _show_msg_check_clone_v2_api(self, volume):"}],"source_content_type":"text/x-python","patch_set":18,"id":"1c29a6fc_8fd675ed","line":282,"range":{"start_line":281,"start_character":0,"end_line":282,"end_character":16},"updated":"2021-01-11 15:26:51.000000000","message":"in which situation will this error occur?","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c343b6b37bbfc913267b280ff4c3fcbab462da88","unresolved":true,"context_lines":[{"line_number":278,"context_line":"                if not (self._last_value_v2_clone_api \u003d\u003d v2):"},{"line_number":279,"context_line":"                    self._last_value_v2_clone_api \u003d v2"},{"line_number":280,"context_line":"                    return True"},{"line_number":281,"context_line":"        except AttributeError:"},{"line_number":282,"context_line":"            pass"},{"line_number":283,"context_line":"        return False"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def _show_msg_check_clone_v2_api(self, volume):"}],"source_content_type":"text/x-python","patch_set":18,"id":"46dc09bb_f4985ce8","line":282,"range":{"start_line":281,"start_character":0,"end_line":282,"end_character":16},"in_reply_to":"1c29a6fc_8fd675ed","updated":"2021-01-12 12:40:12.000000000","message":"As far as I tested volume.volume.op_features() always returns 0 or 1. However, I agree with you and probably volume.volume.op_features doesn\u0027t exist for very old versions. In any case, this call can raise an exception, and I don\u0027t think we should fail the clone if this part fails.","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"57f5b3a4479e81bb5a0e88bcd339dbfa1ecaa32c","unresolved":true,"context_lines":[{"line_number":283,"context_line":"        return False"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def _show_msg_check_clone_v2_api(self, volume):"},{"line_number":286,"context_line":"        if (not self._clone_v2_api_checked"},{"line_number":287,"context_line":"                or self._had_changed_v2_clone_api(volume)):"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":290,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":18,"id":"164751c2_e0939839","line":287,"range":{"start_line":286,"start_character":0,"end_line":287,"end_character":59},"updated":"2021-01-11 15:26:51.000000000","message":"I found a case breaking this logic when we do config changes (although it\u0027s rare someone will be doing it)\nset rbd default clone format \u003d 2 in ceph.conf (restart ceph services)\ncreate volume from snapshot -\u003e logs \u0027Using v2 Clone API\u0027 and sets self._clone_v2_api_checked \u003d True\n\nnow set rbd default clone format \u003d 1 in ceph.conf (restart ceph services)\nnow create volume from snapshot -\u003e doesn\u0027t log anything\n\nThis happens because self._clone_v2_api_checked is still True (since we didn\u0027t restart c-vol) and self._had_changed_v2_clone_api(volume) validates volume.volume.op_features() which is 0 (incase when we\u0027re enforcing v1 in ceph.conf) and returns False hence we don\u0027t get past this condition","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"57f5b3a4479e81bb5a0e88bcd339dbfa1ecaa32c","unresolved":true,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":290,"context_line":"            try:"},{"line_number":291,"context_line":"                if (volume.volume.op_features()"},{"line_number":292,"context_line":"                        \u0026 self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":18,"id":"9e1d4fad_9f7a36bb","line":291,"range":{"start_line":291,"start_character":20,"end_line":291,"end_character":47},"updated":"2021-01-11 15:26:51.000000000","message":"If self._last_value_v2_clone_api is not None then it points to this value.\nMaybe we can check that like:\nif self._last_value_v2_clone_api or volume.volume.op_features():\n\nThis will avoid the call to op_features() if _last_value_v2_clone_api is set","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c343b6b37bbfc913267b280ff4c3fcbab462da88","unresolved":false,"context_lines":[{"line_number":288,"context_line":""},{"line_number":289,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":290,"context_line":"            try:"},{"line_number":291,"context_line":"                if (volume.volume.op_features()"},{"line_number":292,"context_line":"                        \u0026 self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":18,"id":"d90143fd_10c26dc1","line":291,"range":{"start_line":291,"start_character":20,"end_line":291,"end_character":47},"in_reply_to":"9e1d4fad_9f7a36bb","updated":"2021-01-12 12:40:12.000000000","message":"Ack","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"57f5b3a4479e81bb5a0e88bcd339dbfa1ecaa32c","unresolved":true,"context_lines":[{"line_number":289,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":290,"context_line":"            try:"},{"line_number":291,"context_line":"                if (volume.volume.op_features()"},{"line_number":292,"context_line":"                        \u0026 self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"}],"source_content_type":"text/x-python","patch_set":18,"id":"72c6a0a2_1ea9d573","line":292,"range":{"start_line":292,"start_character":26,"end_line":292,"end_character":69},"updated":"2021-01-11 15:26:51.000000000","message":"This option is always 1 (incase of v1 or v2), I wonder if we really need this for the conditional.\nWhy we aren\u0027t doing just if volume.volume.op_features() ? (0-\u003e v1 , 1-\u003e v2)\nThis should work the same if I\u0027m not missing anything here","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c343b6b37bbfc913267b280ff4c3fcbab462da88","unresolved":false,"context_lines":[{"line_number":289,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":290,"context_line":"            try:"},{"line_number":291,"context_line":"                if (volume.volume.op_features()"},{"line_number":292,"context_line":"                        \u0026 self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"}],"source_content_type":"text/x-python","patch_set":18,"id":"84b08899_3fb67d2c","line":292,"range":{"start_line":292,"start_character":26,"end_line":292,"end_character":69},"in_reply_to":"72c6a0a2_1ea9d573","updated":"2021-01-12 12:40:12.000000000","message":"Ack","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"57f5b3a4479e81bb5a0e88bcd339dbfa1ecaa32c","unresolved":true,"context_lines":[{"line_number":292,"context_line":"                        \u0026 self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"},{"line_number":296,"context_line":"                pass"},{"line_number":297,"context_line":"            LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":298,"context_line":"                        \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":299,"context_line":"                        \u0027 compat version to mimic for better\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"f71d2627_d9a8609b","line":296,"range":{"start_line":295,"start_character":0,"end_line":296,"end_character":20},"updated":"2021-01-11 15:26:51.000000000","message":"I\u0027m suspecting volume.volume.op_features doesn\u0027t exist for versions \u003c mimic and hence this case but haven\u0027t verified it.","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c343b6b37bbfc913267b280ff4c3fcbab462da88","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                        \u0026 self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"},{"line_number":296,"context_line":"                pass"},{"line_number":297,"context_line":"            LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":298,"context_line":"                        \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":299,"context_line":"                        \u0027 compat version to mimic for better\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"1452206b_29a6891a","line":296,"range":{"start_line":295,"start_character":0,"end_line":296,"end_character":20},"in_reply_to":"f71d2627_d9a8609b","updated":"2021-01-12 12:40:12.000000000","message":"In any case this call can raise an exception, and I don\u0027t think we should fail the clone if this part fails.","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"57f5b3a4479e81bb5a0e88bcd339dbfa1ecaa32c","unresolved":true,"context_lines":[{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"},{"line_number":296,"context_line":"                pass"},{"line_number":297,"context_line":"            LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":298,"context_line":"                        \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":299,"context_line":"                        \u0027 compat version to mimic for better\u0027"},{"line_number":300,"context_line":"                        \u0027 performance, fewer deletion issues\u0027)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_target_config(self, target_id):"},{"line_number":303,"context_line":"        \"\"\"Get a replication target from known replication targets.\"\"\""}],"source_content_type":"text/x-python","patch_set":18,"id":"19795cdf_f5b5960b","line":300,"range":{"start_line":297,"start_character":24,"end_line":300,"end_character":62},"updated":"2021-01-11 15:26:51.000000000","message":"This warning can also be logged when we are enforcing using v1 in ceph.conf, should we mention that case as well?","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"c343b6b37bbfc913267b280ff4c3fcbab462da88","unresolved":true,"context_lines":[{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"},{"line_number":296,"context_line":"                pass"},{"line_number":297,"context_line":"            LOG.warning(\u0027Not using v2 clone API, please upgrade to\u0027"},{"line_number":298,"context_line":"                        \u0027 mimic+ and set the OSD minimum client\u0027"},{"line_number":299,"context_line":"                        \u0027 compat version to mimic for better\u0027"},{"line_number":300,"context_line":"                        \u0027 performance, fewer deletion issues\u0027)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def _get_target_config(self, target_id):"},{"line_number":303,"context_line":"        \"\"\"Get a replication target from known replication targets.\"\"\""}],"source_content_type":"text/x-python","patch_set":18,"id":"315f8728_8dde8b6b","line":300,"range":{"start_line":297,"start_character":24,"end_line":300,"end_character":62},"in_reply_to":"19795cdf_f5b5960b","updated":"2021-01-12 12:40:12.000000000","message":"yep, the msg is being displayed when enforcing using v1 in in ceph.conf. Are you suggesting to change the warning text?","commit_id":"4fb35c58e4fd242382f100d67430dffa1acecbba"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"53897732def09d527c11cdf667e794a749496751","unresolved":true,"context_lines":[{"line_number":247,"context_line":"        self._replication_targets \u003d []"},{"line_number":248,"context_line":"        self._target_names \u003d []"},{"line_number":249,"context_line":"        self._clone_v2_api_checked \u003d False"},{"line_number":250,"context_line":"        self._last_value_clone_api_version \u003d 0"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"        if self.rbd is not None:"},{"line_number":253,"context_line":"            self.RBD_FEATURE_LAYERING \u003d self.rbd.RBD_FEATURE_LAYERING"}],"source_content_type":"text/x-python","patch_set":20,"id":"296f6e7b_b943fcb7","line":250,"updated":"2021-01-13 02:47:35.000000000","message":"-1: In case of staring with the v2 clone api enable, the info message will be print twice.","commit_id":"5b634a7cd6e8e2d88e5912eb49da29dd6784ebb5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"36b7922adb5bfa0d9ad4309c8e178fd772e56472","unresolved":true,"context_lines":[{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    def _check_clone_api_version_diff(self, volume):"},{"line_number":274,"context_line":"        try:"},{"line_number":275,"context_line":"            api_version \u003d volume.volume.op_features()"},{"line_number":276,"context_line":"            if not (self._last_value_clone_api_version \u003d\u003d"},{"line_number":277,"context_line":"                    api_version):"},{"line_number":278,"context_line":"                self._last_value_clone_api_version \u003d api_version"}],"source_content_type":"text/x-python","patch_set":21,"id":"90c1755d_a0a29ab2","line":275,"range":{"start_line":275,"start_character":26,"end_line":275,"end_character":53},"updated":"2021-01-22 16:43:23.000000000","message":"I don\u0027t think this returns an api version?","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"57b6650129c303f74e3ecb86e87b09f17151f3c2","unresolved":true,"context_lines":[{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    def _check_clone_api_version_diff(self, volume):"},{"line_number":274,"context_line":"        try:"},{"line_number":275,"context_line":"            api_version \u003d volume.volume.op_features()"},{"line_number":276,"context_line":"            if not (self._last_value_clone_api_version \u003d\u003d"},{"line_number":277,"context_line":"                    api_version):"},{"line_number":278,"context_line":"                self._last_value_clone_api_version \u003d api_version"}],"source_content_type":"text/x-python","patch_set":21,"id":"b58c81d5_b62aef1f","line":275,"range":{"start_line":275,"start_character":26,"end_line":275,"end_character":53},"in_reply_to":"90c1755d_a0a29ab2","updated":"2021-01-22 16:56:52.000000000","message":"Ack - true, wrong name, this only returns two possible values. volume.volume.op_features() returns *0* when using v1 clone API or *1* when using the v2 clone API.","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"36b7922adb5bfa0d9ad4309c8e178fd772e56472","unresolved":true,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if volume.volume.op_features():"},{"line_number":291,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":292,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":293,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":21,"id":"1df9f3af_3997cb21","line":290,"range":{"start_line":290,"start_character":19,"end_line":290,"end_character":46},"updated":"2021-01-22 16:43:23.000000000","message":"What is this checking?  I suspect you want to check\n    volume.volume.op_features() | RBD_FEATURE_CLONE_PARENT\n\nor similar?","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0d180f4883b7ce6622485c387f942d28fb3d6a05","unresolved":true,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if volume.volume.op_features():"},{"line_number":291,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":292,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":293,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":21,"id":"8e307f75_09141b4d","line":290,"range":{"start_line":290,"start_character":19,"end_line":290,"end_character":46},"in_reply_to":"099dbef9_409f5738","updated":"2021-01-23 04:09:02.000000000","message":"Ack - I updated the patch, let me know your thoughts. Thanks!","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"57b6650129c303f74e3ecb86e87b09f17151f3c2","unresolved":true,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if volume.volume.op_features():"},{"line_number":291,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":292,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":293,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":21,"id":"7f548379_96cd7cab","line":290,"range":{"start_line":290,"start_character":19,"end_line":290,"end_character":46},"in_reply_to":"1df9f3af_3997cb21","updated":"2021-01-22 16:56:52.000000000","message":"As Rajat points out [1] this option is always 1 (incase of v1 or v2) so I removed it because I couldn\u0027t find the reason to keep it. \n\n[1]https://review.opendev.org/c/openstack/cinder/+/754849/18/cinder/volume/drivers/rbd.py#292","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"028b5dae0366cdd828912d61d4dad7bea21d022d","unresolved":true,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if volume.volume.op_features():"},{"line_number":291,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":292,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":293,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":21,"id":"099dbef9_409f5738","line":290,"range":{"start_line":290,"start_character":19,"end_line":290,"end_character":46},"in_reply_to":"7f548379_96cd7cab","updated":"2021-01-22 19:23:29.000000000","message":"I think his comment missed that this is a bitwise check.  You need to keep it so that when you have a volume with a different feature flag set, the bitwise check still returns the correct result.  The possible return values of op_features() are not just 0 or 1.  Your current check would return true for a volume with CLONE_PARENT (1\u003c\u003c0) off but with RBD_OPERATION_FEATURE_SNAP_TRASH (1\u003c\u003c3) or RBD_OPERATION_FEATURE_GROUP (1\u003c\u003c2) set, which is incorrect.","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"87bbc3dbab98ab1e92d87391ae81abfa56ff6991","unresolved":true,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if volume.volume.op_features():"},{"line_number":291,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":292,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":293,"context_line":"                    return"}],"source_content_type":"text/x-python","patch_set":21,"id":"6451de78_78a41e4c","line":290,"range":{"start_line":290,"start_character":19,"end_line":290,"end_character":46},"in_reply_to":"8e307f75_09141b4d","updated":"2021-01-25 14:41:23.000000000","message":"To clarify my comment, I didn\u0027t miss the bitwise operator but didn\u0027t know about the possible values of volume.volume.op_features().\nIt was more of a question than a suggestion to remove the clone parent condition.\nAfter checking the possible values[1] (which Eric pointed out), the check makes sense.\n\n[1] https://github.com/ceph/ceph/blob/master/src/include/rbd/features.h#L105-L108","commit_id":"e7a505c7957832e6c49643767275bd3c8accd14e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"87bbc3dbab98ab1e92d87391ae81abfa56ff6991","unresolved":true,"context_lines":[{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if (volume.volume.op_features() \u0026"},{"line_number":291,"context_line":"                        self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":292,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"}],"source_content_type":"text/x-python","patch_set":23,"id":"a0a5b96e_380d293e","line":292,"range":{"start_line":292,"start_character":25,"end_line":292,"end_character":54},"updated":"2021-01-25 14:41:23.000000000","message":"this needs to be updated to _last_value_clone_api_bit ?","commit_id":"0ee4ca8c00be3b3f87e4339aa78165b0a70d0a69"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"e99da59290f3b058babe02f885683c554559d3eb","unresolved":true,"context_lines":[{"line_number":289,"context_line":"            try:"},{"line_number":290,"context_line":"                if (volume.volume.op_features() \u0026"},{"line_number":291,"context_line":"                        self.rbd.RBD_OPERATION_FEATURE_CLONE_PARENT):"},{"line_number":292,"context_line":"                    self._last_value_clone_api_version \u003d 1"},{"line_number":293,"context_line":"                    LOG.info(\u0027Using v2 Clone API\u0027)"},{"line_number":294,"context_line":"                    return"},{"line_number":295,"context_line":"            except AttributeError:"}],"source_content_type":"text/x-python","patch_set":23,"id":"2d7ea829_5a130708","line":292,"range":{"start_line":292,"start_character":25,"end_line":292,"end_character":54},"in_reply_to":"a0a5b96e_380d293e","updated":"2021-01-25 17:49:14.000000000","message":"Thanks for reviewing this patch Rajat! \nI forgot to submit the last update. I updated the unit test to check those values and avoid this kind of problems next time 😐","commit_id":"0ee4ca8c00be3b3f87e4339aa78165b0a70d0a69"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"b021ce38d69db9f395f4d860439698c402f65615","unresolved":true,"context_lines":[{"line_number":1032,"context_line":"        if int(volume.size):"},{"line_number":1033,"context_line":"            self._resize(volume)"},{"line_number":1034,"context_line":""},{"line_number":1035,"context_line":"        with RBDVolumeProxy(self, snapshot.volume_name) as src_volume:"},{"line_number":1036,"context_line":"            self._show_msg_check_clone_v2_api(src_volume)"},{"line_number":1037,"context_line":"        return volume_update"},{"line_number":1038,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"03ec6c2c_d3abca6f","line":1035,"range":{"start_line":1035,"start_character":8,"end_line":1035,"end_character":27},"updated":"2021-01-25 18:12:51.000000000","message":"This should be inside of the _show_msg_check_clone_v2_api method (around line 288) so that RBDVolumeProxy is not opened if the check isn\u0027t actually needed.","commit_id":"0ee4ca8c00be3b3f87e4339aa78165b0a70d0a69"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b6f9c1f6b475978dd302d2c3d440bed43b6761ea","unresolved":true,"context_lines":[{"line_number":270,"context_line":"        return RBD_OPTS + additional_opts"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"    def _show_msg_check_clone_v2_api(self, volume_name):"},{"line_number":273,"context_line":"        if not self._clone_v2_api_checked:"},{"line_number":274,"context_line":"            self._clone_v2_api_checked \u003d True"},{"line_number":275,"context_line":"            with RBDVolumeProxy(self, volume_name) as volume:"},{"line_number":276,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":26,"id":"0d247b37_cf9d26dc","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":42},"updated":"2021-02-04 12:30:45.000000000","message":"This only logs right info during initialization of driver (or restart) but doesn\u0027t account for when this feature is enabled/disabled on the ceph side.\nI think the main aim here is to show this info one time for deployments working on mimic+ (or log warning if not) so it makes sense.","commit_id":"efd83675780a76422436d47c4622e22a0645cc30"}]}