)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28403,"name":"Naoki Saito","email":"nasaito@nec.com","username":"n-saito"},"change_message_id":"b760059f40859e05009ebaf601baad507fcd8c39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"90adaab4_427bbd3d","updated":"2023-09-07 02:00:41.000000000","message":"run-NEC V Cinder CI","commit_id":"9d4beb6e486cbf8c07aa66e5539e15bf20127e74"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"cef126429dd7adb71ed52cb21a29a51cbeebf7fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"abc1e12c_9aa11eda","updated":"2023-09-12 00:43:04.000000000","message":"recheck InvocationError in cinder-plugin-ceph-tempest","commit_id":"09fd20b3aa9788eb2141eb636a47b7cbc0623b31"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"2b8439361c0fe5dac5b9469243624ab64f55365a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c43fe8b6_904750a2","updated":"2023-09-15 00:48:40.000000000","message":"recheck cinder-plugin-ceph-tempest by Invocation error","commit_id":"09fd20b3aa9788eb2141eb636a47b7cbc0623b31"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"e2dea49a3dc76e605bdc4e046c4a17d421894875","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"af56318d_f2d434a9","updated":"2023-09-13 14:10:22.000000000","message":"recheck cinder-plugin-ceph-tempest by InvocationError","commit_id":"09fd20b3aa9788eb2141eb636a47b7cbc0623b31"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"37afc21c99dfed335fdb1da279ab2b5b54890f13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"950a7b9e_fbd1f2ee","updated":"2023-12-15 14:12:38.000000000","message":"I would assume the expected behavior is correct but one question inline to confirm","commit_id":"8b5706ac0d9bac24104c54b42cb7c397030ed5fd"},{"author":{"_account_id":31779,"name":"Jean Pierre Roquesalane","display_name":"happystacker","email":"jeanpierre.roquesalane@dell.com","username":"happystacker"},"change_message_id":"cd63ced34ee91959279c411922f47b6cd0980cae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b7f1b006_511f1439","updated":"2024-01-03 15:47:06.000000000","message":"LGTM","commit_id":"8b5706ac0d9bac24104c54b42cb7c397030ed5fd"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"3886379b879e11f36c1f24c56d19b2ee3a9e144b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"50c5c58f_99816545","updated":"2023-11-29 06:15:43.000000000","message":"recheck InvocationError in cinder-plugin-ceph-tempest","commit_id":"8b5706ac0d9bac24104c54b42cb7c397030ed5fd"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"62e20b0c3213a01a058f18fcb96680a7f1e84bc4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9de40448_59692727","updated":"2023-12-06 09:36:14.000000000","message":"recheck tempest-integrated-storage by Internal Server Error","commit_id":"8b5706ac0d9bac24104c54b42cb7c397030ed5fd"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"19f0e4b1718e5bdbb940427864cc9cdc0ae02f9d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fe1e329a_ce38bfc4","updated":"2024-01-19 14:30:11.000000000","message":"This is weird way to code: is the self.nested_count supposed to be set in _login() or outside of it? If inside, there\u0027s a pointless assignment. If outside, it needs to be done in login() too.\n\nIn view of approvals I don\u0027t want to add a -1 on this, but this looks not entirely thought through to me.","commit_id":"1f3af3396a1f3e42e94fc2308e26c16ede2116d6"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"a5a8c294ccac87857ad87c7cc8bc38f6ba6b0964","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f57d961d_696d636e","updated":"2024-01-18 04:49:46.000000000","message":"rebased by \"896412: Skip sparse copy during volume reimage | https://review.opendev.org/c/openstack/cinder/+/896412 \" is merged","commit_id":"1f3af3396a1f3e42e94fc2308e26c16ede2116d6"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"da13974628ae8258b8d5ba33ca763a40107d3f0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6eab98fa_88f36e51","updated":"2024-01-19 02:50:47.000000000","message":"recheck cinder-tempest-plugin-lvm-lio-barbican by request time out","commit_id":"1f3af3396a1f3e42e94fc2308e26c16ede2116d6"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"e88ba7967fbb3bc9d00458d84248d00756e64780","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b50249de_a5ee2bc9","in_reply_to":"fe1e329a_ce38bfc4","updated":"2024-01-19 14:38:35.000000000","message":"I wanted to post an alternative but there was a dependency. So let\u0027s leave a -1 for now. I\u0027m going to review the dependency first and then get back to this.","commit_id":"1f3af3396a1f3e42e94fc2308e26c16ede2116d6"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"d76244e8ced5db4f1b6aa552d2d460df9567d88f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"d7ef52ee_67a945b3","updated":"2024-02-02 05:20:22.000000000","message":"patchset 6 set a relation chain correctly.","commit_id":"0104f273aaf459917cbd7a36ce255f505a824fb7"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"14ffb78f3cdc969b01d50fd4a367eb0ac4d9ff37","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"57f9263e_8ecbd64f","updated":"2024-02-21 15:03:51.000000000","message":"recheck by build-openstack-releasenotes is failed","commit_id":"ccc1593fcda752e89edc281d8bc2997e9d8b7411"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"72c2658625e47e80075b56e75bfc61bb594f0df4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"f05be9e4_1976b988","updated":"2024-03-04 23:24:03.000000000","message":"I talked with over with Eric and Brian, and we decided not to help vendor programmers code better anymore. The review is only ensure the safety of this for the Cinder core. If you want to double-initialize, be my guest.","commit_id":"91bd49c4e577250866ec915594baa108edd3692a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6f4f4ce531eda1d8fffd791445955a6ba58215a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"aa266a83_e4754b35","updated":"2024-03-15 16:53:49.000000000","message":"Question inline; the -1 is to make sure you see it, not that there\u0027s necessarily a problem.","commit_id":"91bd49c4e577250866ec915594baa108edd3692a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"494b317893c5961a1e9d593095c69886d8a16377","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"f9d4db86_721480fb","updated":"2024-06-21 21:50:19.000000000","message":"Thanks for the explanation of how nested_count is being used.  There\u0027s the weird double-initialization thing that Pete pointed out, but I won\u0027t block this if you are convinced that the code is working correctly.","commit_id":"6006703944f2e122958981bd282a4c7e3897ed0c"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"a04d572d53b9fd7dd2cb6b82735071158cc34284","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"58282504_a4a61090","updated":"2024-06-04 09:42:10.000000000","message":"This patchset only fixes the relation chain.","commit_id":"6006703944f2e122958981bd282a4c7e3897ed0c"},{"author":{"_account_id":29122,"name":"Raghavendra Tilay","email":"raghavendra-uddhav.tilay@hpe.com","username":"raghavendrat"},"change_message_id":"ca7502c4e27284f5957ce8f5903b46ddd207d7ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"89a1e89e_939b7a3f","updated":"2024-07-31 06:33:55.000000000","message":"Code changes look good. Zuul and Hitachi CI have passed.","commit_id":"bf0725e62a4412964722b8da5e571a7d5c0e629b"}],"cinder/volume/drivers/hitachi/hbsd_rest_api.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"37afc21c99dfed335fdb1da279ab2b5b54890f13","unresolved":true,"context_lines":[{"line_number":537,"context_line":"                                 timeout\u003dself.conf.hitachi_lock_timeout)"},{"line_number":538,"context_line":"        if not err:"},{"line_number":539,"context_line":"            self.set_my_session(self.Session(rsp[\"sessionId\"], rsp[\"token\"]))"},{"line_number":540,"context_line":"            self.nested_count \u003d 0"},{"line_number":541,"context_line":"            return True"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"6a2a16e0_c3289acf","line":540,"range":{"start_line":540,"start_character":12,"end_line":540,"end_character":33},"updated":"2023-12-15 14:12:38.000000000","message":"I can see we don\u0027t send an unlock resource group request if the nested_count is 0\nThe associated launchpad bug mentions about not sending it during relogin which i understand but just to confirm if we want to set it here as well?","commit_id":"8b5706ac0d9bac24104c54b42cb7c397030ed5fd"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"c6f143aff9b8f2a4260d3069861c339a4d71e2f5","unresolved":false,"context_lines":[{"line_number":537,"context_line":"                                 timeout\u003dself.conf.hitachi_lock_timeout)"},{"line_number":538,"context_line":"        if not err:"},{"line_number":539,"context_line":"            self.set_my_session(self.Session(rsp[\"sessionId\"], rsp[\"token\"]))"},{"line_number":540,"context_line":"            self.nested_count \u003d 0"},{"line_number":541,"context_line":"            return True"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"f3246b81_549c8010","line":540,"range":{"start_line":540,"start_character":12,"end_line":540,"end_character":33},"in_reply_to":"6a2a16e0_c3289acf","updated":"2023-12-18 05:17:30.000000000","message":"Thank you for reviewing.\nAltohugh _login() is called by somewheres, and only one case which is called by relogin() is in multithread,why nested_count also sets 0 here is to prevent same bug by some future patches.","commit_id":"8b5706ac0d9bac24104c54b42cb7c397030ed5fd"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6f4f4ce531eda1d8fffd791445955a6ba58215a3","unresolved":true,"context_lines":[{"line_number":494,"context_line":"        executes API requests, preventing other users from performing"},{"line_number":495,"context_line":"        operations on the resources."},{"line_number":496,"context_line":"        \"\"\""},{"line_number":497,"context_line":"        with self.resource_lock:"},{"line_number":498,"context_line":"            if self.nested_count \u003c\u003d 0:"},{"line_number":499,"context_line":"                url \u003d \u0027%(url)s/resource-group-service/actions/%(action)s\u0027 % {"},{"line_number":500,"context_line":"                    \u0027url\u0027: self.service_url,"}],"source_content_type":"text/x-python","patch_set":8,"id":"69636ff8_274460b5","line":497,"updated":"2024-03-15 16:53:49.000000000","message":"I find this nested_count stuff confusing.  This resource_lock is used to protect this critical section, but whether you call the backend to lock or not is actually controlled by nested_count.  The checking of the value of nested_count is protected by resource_lock, as is its incrementation (same thing in the unlock_resource_group() function), but the code you\u0027ve added to set nested_count to zero is not protected by resource_lock.  That doesn\u0027t smell right to me.  And I don\u0027t think that you have tests verifying that the synchronization is working the way you want it to.  So I personally am a bit suspicious about this patch, although I could be wrong about that.  In any case, please review your logic to make sure this is all coded to do what you expect.","commit_id":"91bd49c4e577250866ec915594baa108edd3692a"},{"author":{"_account_id":33473,"name":"Atsushi Kawai","display_name":"akawai","email":"atsushi.kawai.bu@hitachi.com","username":"akawai"},"change_message_id":"b86a91902d6c7535f4b8e2961d8a1f4df008aa72","unresolved":true,"context_lines":[{"line_number":494,"context_line":"        executes API requests, preventing other users from performing"},{"line_number":495,"context_line":"        operations on the resources."},{"line_number":496,"context_line":"        \"\"\""},{"line_number":497,"context_line":"        with self.resource_lock:"},{"line_number":498,"context_line":"            if self.nested_count \u003c\u003d 0:"},{"line_number":499,"context_line":"                url \u003d \u0027%(url)s/resource-group-service/actions/%(action)s\u0027 % {"},{"line_number":500,"context_line":"                    \u0027url\u0027: self.service_url,"}],"source_content_type":"text/x-python","patch_set":8,"id":"6169741c_df8e288d","line":497,"in_reply_to":"69636ff8_274460b5","updated":"2024-06-03 09:47:34.000000000","message":"Excuse me to reply late.\nIt is our logic why ``nested_count`` is safe if this driver runs multi-threads:\n\n\n** premise: **\n* ``nested_count`` counts how many threads locks a resource group for Hitachi storage. The lock for the group is allocated for each REST API session, it means other thread which uses same session can unlock. Additionally, a REST API to lock/unlock the group loads on the storage. so, the driver only calls the API for locking if ``nested_count`` is ``0``, and calls the API for unlocking if ``nested_count`` is ``1``\n* ``resource_lock`` is to lock while updating/referring ``nested_count`` or calling a REST API for locking/unlocking a resource group for Hitachi storage.\n* ``login_session`` is to choose one of threads to call a REST API to generate REST API session, because the REST API puts a high load on storage.\n\n** Caller of ``_login()``: **\n* The method calls a REST API to generate a REST API session\n* ``login()`` is called from a thread to initialize the cinder driver, so, the method does not called from several threads.\n* ``relogin()`` is called from following two methods: one is ``_check_rest_api_resonse()`` and another is ``_keep_session()`` . Both methods call ``relogin()`` when the session is lost.\n\n** Case of raising ``relogin()`` while processing ``lock_resource_group()`` or ``unlock_resource_group()``: **\n* Between from ``lock_resource_group()`` to ``unlock_resource_group()`` , the trigger for thread switching is only in ``_invoke()``. ``_invoke()`` is called only if ``nested_count`` is ``0``, and ``relogin()`` sets ``nested_count`` as ``0``. It\u0027s no problem because just overwriting ``0``.\n\n** Case of raising ``lock_resource_group()`` or ``unlock_resource_group()`` while processing ``relogin()``: **\n* ``relogin()`` validates ``login_lock``, then, if the thread can`t find a valid session:\n\n  * ``relogin()`` sets ``nested_count`` as ``0``.\n  * The thread switching trigger after here is only in ``_request ()``. It can switch a thread to call either ``lock_resource_group()`` or ``unlock_resource_group()``.\n\n    * In the thread to call either methods, ``_invoke()`` is called to call the REST API for locking/unlocking the resource group when ``nested_count`` is ``0``. The API must be failed because the trigger of calling this method is REST API session is lost, then ``relogin()`` is called, but ``nested_count`` is not set because ``login_lock`` is valid.","commit_id":"91bd49c4e577250866ec915594baa108edd3692a"}]}