)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":true,"context_lines":[{"line_number":10,"context_line":"instances. Added the logic to handle the removing of cinder"},{"line_number":11,"context_line":"volume from multiple instances."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Close-Bug: #2110274"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibb4d71868106226b5513e4c825f769008a446727"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9124b3d3_2f21f696","line":13,"updated":"2025-06-02 15:24:02.000000000","message":"nit: \"Closes-Bug\"","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[{"line_number":10,"context_line":"instances. Added the logic to handle the removing of cinder"},{"line_number":11,"context_line":"volume from multiple instances."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Close-Bug: #2110274"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ibb4d71868106226b5513e4c825f769008a446727"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"33c7aa40_a9ee50ea","line":13,"in_reply_to":"9124b3d3_2f21f696","updated":"2025-06-09 15:49:45.000000000","message":"Done","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"ad76ce5b4e6bc7d75916532c62188e37c59f1599","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"62bdcdc8_0303308a","updated":"2025-06-04 18:13:43.000000000","message":"And while you\u0027re addressing Alan\u0027s comments, there is trailing white space in the release note.","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b90c50be_228931be","updated":"2025-06-02 15:24:02.000000000","message":"I have several comments that mostly focus on the unit test code, specifically about the idea of enhancing the existing fake.test_volume class.\n\nThe core change to the driver code looks solid, but there seems to be a lot of duplication in block_base.py for iSCSI versus FC volumes. I\u0027m guessing with a little thought the two code paths (and functions for each) could be condensed into a single set of code that handles both protocols. I\u0027d be OK merging the current code and deferring the cleanup to a follow-up patch.","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"80e081ee_7a58853c","updated":"2025-06-09 15:49:45.000000000","message":"Thanks Alan for you review comments, I have incorporated the same. Let me know if you have any further review comments","commit_id":"6909d2ba524e0bb3ae1d9d723cdf8dbce206d5f9"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"a648ad3e2c9d0d61a1fcb5cf0d57926a7f28f009","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e7854f86_22cf710e","updated":"2025-06-12 03:42:28.000000000","message":"recheck","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"c9e00e1d1ceeb2a1f39a812b45aa344e85e261c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8c6263b8_713049cf","updated":"2025-06-12 12:30:51.000000000","message":"recheck\n\nPOST_FAILURE","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"dd7acdcc6602980913a1c3879aa3329cce0c6927","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ba12cac0_d5608d34","updated":"2025-07-10 15:27:41.000000000","message":"I\u0027m seeing quite a bit of code changes between PS3 (which only had a single comment from Eric about a typo in the release note) and subsequent patch sets. The entire patch requires a fresh review, and to optimize reviewer\u0027s time, it would help to know when the authors are satisfied with the patch, and don\u0027t intend to upload another patch set.\n\nTo summarize, please let us know when you feel the patch is ready for review.","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"65f464f6a5eb158ce9a9019b8726243450735f06","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"0fa2fc2e_d9085356","updated":"2025-07-18 02:13:38.000000000","message":"recheck","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"bdb4cdb45e15df95f332f819b62f6bc3b75dcf82","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"546d3607_29c391a0","updated":"2025-07-11 04:12:53.000000000","message":"recheck","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"a435bfebe2280ccc9ba39e3f9bd2c6c0835e377e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"6d0b386b_5985c259","updated":"2025-07-16 15:02:38.000000000","message":"recheck","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"fe6f43976f4dc2259ad4fdd7cfb86d424cc0044c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"8d6c4874_b824b3fa","updated":"2025-07-16 11:13:49.000000000","message":"recheck","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"4c04cc390b84e4f133fef45b19e0f78565b38a0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b2df76f7_306ec4dd","updated":"2025-07-11 08:40:47.000000000","message":"recheck","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"d8cff336ca638d713b9066486aedf992de288c36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"dcb520cb_c7f87a0f","updated":"2025-07-13 12:09:57.000000000","message":"recheck","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"f62c7fc5_a696a555","updated":"2025-07-30 16:47:09.000000000","message":"-1 for the initiator check but I also don\u0027t find the UTs helpful and feels like they were just written in a way to make the code pass not testing any real world scenario logic.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"f1ffbbbbde4a84c4a92830b2e64f5b35fc0c9e54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"42e69230_907e9035","updated":"2025-07-30 07:08:05.000000000","message":"Addressed suggestions. Thank you so much Eric and Alan for your valuable comments.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0c5744778effb3927fefce88eaf16398a43e9d2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"14ade62d_c35c91a5","updated":"2025-08-05 18:00:26.000000000","message":"I still have my doubts for some of the unit tests but overall my major concerns are addressed. LGTM.\n(Since the NetApp CI states node failure in past couple of runs, we rely on vendor testing to confirm that the fix is properly tested).","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"10da29a8903812330c4f2bb5a6a9b965c4c20fdd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"2def9242_34bd8827","updated":"2025-08-05 19:50:39.000000000","message":"I think this is very close, but I\u0027d like to see another review by Rajat.","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"0272e72099c4bf1441cd71fa315e6b992013a62c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"8b9baa7d_8115644c","updated":"2025-08-05 21:09:02.000000000","message":"Speaking of multiattach and NVMe-TCP, what is the significance of [1]?\n\n[1] https://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/netapp/dataontap/nvme_library.py#L526","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cb916089_14590457","updated":"2025-08-05 16:46:36.000000000","message":"Thanks Rajat for your review comments, I have incorporated most of the comments. Feel free to re-open the comment if you are not agree on my response.","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"59ca447955dab2758ff401398c8a5d8049db3399","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"ff6c3c47_a85e8b2c","in_reply_to":"2def9242_34bd8827","updated":"2025-08-05 19:53:21.000000000","message":"Well, it seems Rajat added his review while I was doing mine, and so I\u0027m updating mine.","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"a4c260d4af9b1f4d8a5ec3268d82b79a34b18ee5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4f71234d_8d7d96be","updated":"2025-08-08 06:20:24.000000000","message":"recheck","commit_id":"fb8349c2e0c4b9d8610651318ccfd53e07ff84e1"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"e294011405df953f75658f7f5ef7b3f0e9491152","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"8faaa7f0_352f0f2e","updated":"2025-08-08 10:35:29.000000000","message":"recheck","commit_id":"fb8349c2e0c4b9d8610651318ccfd53e07ff84e1"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"75aa5bc259fc9be1927fcf143207f1004dfc368a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"dd96b9a4_6138ae76","updated":"2025-08-07 09:45:14.000000000","message":"recheck","commit_id":"fb8349c2e0c4b9d8610651318ccfd53e07ff84e1"}],"cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":true,"context_lines":[{"line_number":793,"context_line":"test_iscsi_attachment.connector \u003d ISCSI_CONNECTOR"},{"line_number":794,"context_line":""},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"class DummyVolume:"},{"line_number":797,"context_line":"    def __init__(self, multiattach, volume_attachment):"},{"line_number":798,"context_line":"        self.multiattach \u003d multiattach"},{"line_number":799,"context_line":"        self.volume_attachment \u003d volume_attachment"}],"source_content_type":"text/x-python","patch_set":1,"id":"e4b2356a_9ea8b768","line":796,"updated":"2025-06-02 15:24:02.000000000","message":"Would it make sense (and be cleaner) to enhance the existing test_volume() class on L745? The fakes.py code already has a test_volume() class and a test_volume object, so adding an extra DummyVolume() class seems unnecessary.\n\nTo retain compatibility with existing unit test code, you could default the multiattach and volume_attachment parameters to False and [] (or None)","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[{"line_number":793,"context_line":"test_iscsi_attachment.connector \u003d ISCSI_CONNECTOR"},{"line_number":794,"context_line":""},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"class DummyVolume:"},{"line_number":797,"context_line":"    def __init__(self, multiattach, volume_attachment):"},{"line_number":798,"context_line":"        self.multiattach \u003d multiattach"},{"line_number":799,"context_line":"        self.volume_attachment \u003d volume_attachment"}],"source_content_type":"text/x-python","patch_set":1,"id":"a977c257_7663fefe","line":796,"in_reply_to":"e4b2356a_9ea8b768","updated":"2025-06-09 15:49:45.000000000","message":"Make sense. I remove this and enhance the test_volume() class","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"5f5ae65654fec0e230e28f21289500bbc29a6cc8","unresolved":true,"context_lines":[{"line_number":768,"context_line":"test_volume.host \u003d \u0027fakehost@backbackend#fakepool\u0027"},{"line_number":769,"context_line":"test_volume.name \u003d \u0027fakename\u0027"},{"line_number":770,"context_line":"test_volume.size \u003d SIZE"},{"line_number":771,"context_line":"test_volume.multiattach \u003d True"},{"line_number":772,"context_line":"test_volume.volume_attachment \u003d ["},{"line_number":773,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"},{"line_number":774,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"},{"line_number":775,"context_line":"]"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"class test_snapshot(object):"},{"line_number":779,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"1bee01c6_5bb936ec","line":776,"range":{"start_line":771,"start_character":0,"end_line":776,"end_character":1},"updated":"2025-06-10 03:53:58.000000000","message":"I guess this is OK as long as it doesn\u0027t break existing unit tests, which don\u0027t expect to be dealing with a multiattach volume.\n\nI was thinking you\u0027d enhance the class definition at L745 by adding an __init__ function with 2 parameters with default values: multiattach\u003dFalse and volume_attachment\u003dNone.\n\nThe existing unit tests could use the test_volume declared at L751, and new unit tests could use their own variant, something like:\n\n    ma_test_volume \u003d fakes.test_volume(\n        multiattach\u003dTrue,\n        volume_attachment \u003d [\n            {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},\n            {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},\n         ])\n\nBut if the code you added at L771..L775 works for both old and new unit tests, then there\u0027s no need to change your code. I just wanted to fully explain my thoughts.","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"48e6cb2f7b3cd734dec368056effbba03490b7d5","unresolved":false,"context_lines":[{"line_number":768,"context_line":"test_volume.host \u003d \u0027fakehost@backbackend#fakepool\u0027"},{"line_number":769,"context_line":"test_volume.name \u003d \u0027fakename\u0027"},{"line_number":770,"context_line":"test_volume.size \u003d SIZE"},{"line_number":771,"context_line":"test_volume.multiattach \u003d True"},{"line_number":772,"context_line":"test_volume.volume_attachment \u003d ["},{"line_number":773,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"},{"line_number":774,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"},{"line_number":775,"context_line":"]"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"class test_snapshot(object):"},{"line_number":779,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"f5400eac_e7c0591c","line":776,"range":{"start_line":771,"start_character":0,"end_line":776,"end_character":1},"in_reply_to":"1bee01c6_5bb936ec","updated":"2025-06-11 17:47:53.000000000","message":"I have executed, all the test and all the tests are passing with my changes.\nYes, can pass these parameter during the object initialization but then again we have to pass these parameter from test class.\n\nYes, code is working fine for old and new test cases","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":768,"context_line":"test_volume.host \u003d \u0027fakehost@backbackend#fakepool\u0027"},{"line_number":769,"context_line":"test_volume.name \u003d \u0027fakename\u0027"},{"line_number":770,"context_line":"test_volume.size \u003d SIZE"},{"line_number":771,"context_line":"test_volume.multiattach \u003d True"},{"line_number":772,"context_line":"test_volume.volume_attachment \u003d ["},{"line_number":773,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"},{"line_number":774,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"}],"source_content_type":"text/x-python","patch_set":8,"id":"c1159845_8d6c3908","line":771,"range":{"start_line":771,"start_character":0,"end_line":771,"end_character":30},"updated":"2025-07-30 16:47:09.000000000","message":"this will make all the volumes multiattach which are consumed by test using the test_volume. I would rather prefer setting this multiattach property explicitly in the UTs writing specifically for multiattach","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":768,"context_line":"test_volume.host \u003d \u0027fakehost@backbackend#fakepool\u0027"},{"line_number":769,"context_line":"test_volume.name \u003d \u0027fakename\u0027"},{"line_number":770,"context_line":"test_volume.size \u003d SIZE"},{"line_number":771,"context_line":"test_volume.multiattach \u003d True"},{"line_number":772,"context_line":"test_volume.volume_attachment \u003d ["},{"line_number":773,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"},{"line_number":774,"context_line":"    {\u0027attach_status\u0027: ATTACHED, \u0027attached_host\u0027: HOST_NAME},"}],"source_content_type":"text/x-python","patch_set":8,"id":"82d1da1e_9273e488","line":771,"range":{"start_line":771,"start_character":0,"end_line":771,"end_character":30},"in_reply_to":"c1159845_8d6c3908","updated":"2025-08-05 16:46:36.000000000","message":"Done, Setting it\u0027s value to false. overriding this value for multi-attached test.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"}],"cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":true,"context_lines":[{"line_number":544,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":545,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":546,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":547,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":548,"context_line":"        volume.multiattach \u003d False"},{"line_number":549,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":550,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"        self.assertDictEqual(target_info, fake.FC_TARGET_INFO_EMPTY)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1806f370_1a2b5d6e","line":549,"range":{"start_line":547,"start_character":0,"end_line":549,"end_character":0},"updated":"2025-06-02 15:24:02.000000000","message":"I think you can avoid doing this by adding the \u0027multiattach\u0027 property to the fake.test_volume object.\n\nIn fact, I think the code that initializes the test_volume object could be moved into the corresponding test_volume() class\u0027s __init__ code.","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[{"line_number":544,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":545,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":546,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":547,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":548,"context_line":"        volume.multiattach \u003d False"},{"line_number":549,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":550,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":551,"context_line":""},{"line_number":552,"context_line":"        self.assertDictEqual(target_info, fake.FC_TARGET_INFO_EMPTY)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1222fc9e_291a8b77","line":549,"range":{"start_line":547,"start_character":0,"end_line":549,"end_character":0},"in_reply_to":"1806f370_1a2b5d6e","updated":"2025-06-09 15:49:45.000000000","message":"Done","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":true,"context_lines":[{"line_number":571,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d False"},{"line_number":572,"context_line":"        mock_build_initiator_target_map.return_value \u003d (fake.FC_TARGET_WWPNS,"},{"line_number":573,"context_line":"                                                        fake.FC_I_T_MAP, 4)"},{"line_number":574,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":575,"context_line":"        volume.multiattach \u003d False"},{"line_number":576,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":577,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":578,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"38502b63_eb85e4fb","line":575,"range":{"start_line":574,"start_character":0,"end_line":575,"end_character":34},"updated":"2025-06-02 15:24:02.000000000","message":"Ditto.","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[{"line_number":571,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d False"},{"line_number":572,"context_line":"        mock_build_initiator_target_map.return_value \u003d (fake.FC_TARGET_WWPNS,"},{"line_number":573,"context_line":"                                                        fake.FC_I_T_MAP, 4)"},{"line_number":574,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":575,"context_line":"        volume.multiattach \u003d False"},{"line_number":576,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":577,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":578,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7c4c2ee6_50079aa9","line":575,"range":{"start_line":574,"start_character":0,"end_line":575,"end_character":34},"in_reply_to":"38502b63_eb85e4fb","updated":"2025-06-09 15:49:45.000000000","message":"Done","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":true,"context_lines":[{"line_number":590,"context_line":"            mock_unmap_lun,"},{"line_number":591,"context_line":"            mock_has_luns_mapped_to_initiators):"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":594,"context_line":"        volume.multiattach \u003d True"},{"line_number":595,"context_line":"        volume.volume_attachment \u003d ["},{"line_number":596,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":597,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":598,"context_line":"        ]"},{"line_number":599,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":600,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":601,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":602,"context_line":"        self.library.terminate_connection_fc(volume, fake.FC_CONNECTOR)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e3f066e_487caa7a","line":599,"range":{"start_line":593,"start_character":0,"end_line":599,"end_character":0},"updated":"2025-06-02 15:24:02.000000000","message":"Per the suggestion I noted about the DummyVolume() class, you could enhance the test_volume() class so that L593..L598 could be condensed to a single line:\n\n        volume \u003d fake.test_volume (\n            multiattach \u003d True,\n            volume_attachment \u003d [\n                {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},\n                {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},\n            ])","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[{"line_number":590,"context_line":"            mock_unmap_lun,"},{"line_number":591,"context_line":"            mock_has_luns_mapped_to_initiators):"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":594,"context_line":"        volume.multiattach \u003d True"},{"line_number":595,"context_line":"        volume.volume_attachment \u003d ["},{"line_number":596,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":597,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":598,"context_line":"        ]"},{"line_number":599,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":600,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":601,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":602,"context_line":"        self.library.terminate_connection_fc(volume, fake.FC_CONNECTOR)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8e7887d5_40218852","line":599,"range":{"start_line":593,"start_character":0,"end_line":599,"end_character":0},"in_reply_to":"6e3f066e_487caa7a","updated":"2025-06-09 15:49:45.000000000","message":"I removed DummyVolume() class and using test_volume() class","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"967b4fb32f5ff05ec608546bc10b486bffa7157d","unresolved":true,"context_lines":[{"line_number":628,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"    def test__is_multiattach_to_host_multiattach_disabled(self):"},{"line_number":631,"context_line":"        volume \u003d fake.DummyVolume(multiattach\u003dFalse, volume_attachment\u003d["},{"line_number":632,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":633,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":634,"context_line":"        ])"}],"source_content_type":"text/x-python","patch_set":1,"id":"0ab683d4_e8def35c","line":631,"updated":"2025-06-02 15:24:02.000000000","message":"Here you seem to switch to using the DummyVolume() class, but the above tests don\u0027t use it. Maybe you introduced the DummyVolume with these tests, but didn\u0027t update the above tests\u0027 code to use it?\n\nBut as I previously noted, I think you can enhance the test_volume() class without having to add a new DummyVolume() class.","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"b44b4cba9239bf01da205ce690ce636fb00f7637","unresolved":false,"context_lines":[{"line_number":628,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"    def test__is_multiattach_to_host_multiattach_disabled(self):"},{"line_number":631,"context_line":"        volume \u003d fake.DummyVolume(multiattach\u003dFalse, volume_attachment\u003d["},{"line_number":632,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":633,"context_line":"            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: \u0027host1\u0027},"},{"line_number":634,"context_line":"        ])"}],"source_content_type":"text/x-python","patch_set":1,"id":"0d3b369d_1ba515f6","line":631,"in_reply_to":"0ab683d4_e8def35c","updated":"2025-06-09 15:49:45.000000000","message":"I am using test_volume().","commit_id":"1cc93619c1d017ea7eec90073aa8bf37d11f2fa5"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"5f5ae65654fec0e230e28f21289500bbc29a6cc8","unresolved":true,"context_lines":[{"line_number":544,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":545,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":546,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":547,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":548,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":549,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":550,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5ba510a4_1b7e43e8","line":547,"updated":"2025-06-10 03:53:58.000000000","message":"I don\u0027t know which would be more efficient, deep copying the test_volume object in fakes.py, or allocating a new one:\n\n        volume \u003d fake.test_volume()","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":544,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":545,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":546,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":547,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":548,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":549,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":550,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"2f0e5103_11fbf29b","line":547,"in_reply_to":"0dc80894_613ff608","updated":"2025-08-05 16:46:36.000000000","message":"Resolving this comment.","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"48e6cb2f7b3cd734dec368056effbba03490b7d5","unresolved":true,"context_lines":[{"line_number":544,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":545,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":546,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":547,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":548,"context_line":"        target_info \u003d self.library.terminate_connection_fc(volume,"},{"line_number":549,"context_line":"                                                           fake.FC_CONNECTOR)"},{"line_number":550,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"0dc80894_613ff608","line":547,"in_reply_to":"5ba510a4_1b7e43e8","updated":"2025-06-11 17:47:53.000000000","message":"I think deep copy is little slow compare to initializing the object but it safe to use the deep copy in case you wanted to modify the object.","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":592,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":593,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":594,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":595,"context_line":"        self.library.terminate_connection_fc(volume, fake.FC_CONNECTOR)"},{"line_number":596,"context_line":"        mock_unmap_lun.assert_called_once_with(fake.LUN_PATH,"},{"line_number":597,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":598,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"b9a8ab10_db13cf90","line":595,"range":{"start_line":595,"start_character":0,"end_line":595,"end_character":71},"updated":"2025-07-30 16:47:09.000000000","message":"I was under the assumption that this will test a multiattach volume with two attachments on the same compute host.\n\nBut the FC_CONNECTOR has \u0027fake_host\u0027 as the host value and the volume attachments have \u0027fake.host.name\u0027 which fails the check in utils and we continue to terminate volume connection normally which is already tested elsewhere so I\u0027m not sure what\u0027s the purpose of this test.\n\nIf we make the host values same, this will fail on the \u0027initiator\u0027 check as the FC connector doesn\u0027t contain that value (which proves my point that the initiator check is invalid for FC and NVMe)\n\n    connector\n    {\u0027ip\u0027: \u00271.1.1.1\u0027, \u0027host\u0027: \u0027fake_host\u0027, \u0027wwnns\u0027: [\u002720000024ff406cc3\u0027, \n     \u002720000024ff406cc2\u0027], \u0027wwpns\u0027: [\u002721000024ff406cc3\u0027, \u002721000024ff406cc2\u0027]}","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":592,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":593,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":594,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":595,"context_line":"        self.library.terminate_connection_fc(volume, fake.FC_CONNECTOR)"},{"line_number":596,"context_line":"        mock_unmap_lun.assert_called_once_with(fake.LUN_PATH,"},{"line_number":597,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":598,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"f50cee67_72540ca4","line":595,"range":{"start_line":595,"start_character":0,"end_line":595,"end_character":71},"in_reply_to":"b9a8ab10_db13cf90","updated":"2025-08-05 16:46:36.000000000","message":"Now we remove the check for validating the initiator info. I have added below properties for volume. based on this it will first check if volume is multi-attached in this case also it should invoke lun unmapped method once.\n\nvolume.volume_attachment \u003d [\n            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: fake.HOST_NAME},\n            {\u0027attach_status\u0027: fake.ATTACHED, \u0027attached_host\u0027: fake.HOST_NAME},\n        ]","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0c5744778effb3927fefce88eaf16398a43e9d2b","unresolved":false,"context_lines":[{"line_number":592,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":593,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":594,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":595,"context_line":"        self.library.terminate_connection_fc(volume, fake.FC_CONNECTOR)"},{"line_number":596,"context_line":"        mock_unmap_lun.assert_called_once_with(fake.LUN_PATH,"},{"line_number":597,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":598,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"178ef163_f8dcff16","line":595,"range":{"start_line":595,"start_character":0,"end_line":595,"end_character":71},"in_reply_to":"f50cee67_72540ca4","updated":"2025-08-05 18:00:26.000000000","message":"Since the volume has 2 attachments, it shouldn\u0027t invoke LUN unmapping unless the HOST values are different.\nHere fake.HOST_NAME resolved to \u0027fake.host.name\u0027 and FC connector host value resolves to \u0027fake_host\u0027 that\u0027s why it\u0027s still unmapping the given attachment and not skipping it.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":619,"context_line":"        mock_unmap_lun.assert_called_once_with(fake.LUN_PATH,"},{"line_number":620,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary,"},{"line_number":623,"context_line":"                       \u0027_has_luns_mapped_to_initiators\u0027)"},{"line_number":624,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary, \u0027_unmap_lun\u0027)"},{"line_number":625,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary, \u0027_get_lun_attr\u0027)"},{"line_number":626,"context_line":"    def test_terminate_connection_fc_multiattach_cleanup("},{"line_number":627,"context_line":"            self, mock_get_lun_attr, mock_unmap_lun,"},{"line_number":628,"context_line":"            mock_has_luns_mapped_to_initiators):"},{"line_number":629,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":630,"context_line":"        connector \u003d fake.FC_CONNECTOR"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":633,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":634,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        def terminate_connection(*args, **kwargs):"},{"line_number":637,"context_line":"            self.library.terminate_connection_fc(volume, connector)"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"        # Run the termination operation in parallel using ThreadPoolExecutor"},{"line_number":640,"context_line":"        with ThreadPoolExecutor(max_workers\u003d2) as executor:"},{"line_number":641,"context_line":"            list(executor.map(terminate_connection, range(2)))"},{"line_number":642,"context_line":""},{"line_number":643,"context_line":"        # Ensure that the LUN maps are cleaned up correctly for both"},{"line_number":644,"context_line":"        # parallel operations"},{"line_number":645,"context_line":"        self.assertEqual(mock_unmap_lun.call_count, 2)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary,"},{"line_number":648,"context_line":"                       \u0027_get_fc_target_wwpns\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"a8085534_bb892760","line":645,"range":{"start_line":622,"start_character":0,"end_line":645,"end_character":54},"updated":"2025-07-30 16:47:09.000000000","message":"again, I don\u0027t think this is verifying anything useful here from a unit test perspective so I would avoid concurrent/parallel execution here","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":619,"context_line":"        mock_unmap_lun.assert_called_once_with(fake.LUN_PATH,"},{"line_number":620,"context_line":"                                               fake.FC_FORMATTED_INITIATORS)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary,"},{"line_number":623,"context_line":"                       \u0027_has_luns_mapped_to_initiators\u0027)"},{"line_number":624,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary, \u0027_unmap_lun\u0027)"},{"line_number":625,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary, \u0027_get_lun_attr\u0027)"},{"line_number":626,"context_line":"    def test_terminate_connection_fc_multiattach_cleanup("},{"line_number":627,"context_line":"            self, mock_get_lun_attr, mock_unmap_lun,"},{"line_number":628,"context_line":"            mock_has_luns_mapped_to_initiators):"},{"line_number":629,"context_line":"        volume \u003d copy.deepcopy(fake.test_volume)"},{"line_number":630,"context_line":"        connector \u003d fake.FC_CONNECTOR"},{"line_number":631,"context_line":""},{"line_number":632,"context_line":"        mock_get_lun_attr.return_value \u003d {\u0027Path\u0027: fake.LUN_PATH}"},{"line_number":633,"context_line":"        mock_unmap_lun.return_value \u003d None"},{"line_number":634,"context_line":"        mock_has_luns_mapped_to_initiators.return_value \u003d True"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        def terminate_connection(*args, **kwargs):"},{"line_number":637,"context_line":"            self.library.terminate_connection_fc(volume, connector)"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"        # Run the termination operation in parallel using ThreadPoolExecutor"},{"line_number":640,"context_line":"        with ThreadPoolExecutor(max_workers\u003d2) as executor:"},{"line_number":641,"context_line":"            list(executor.map(terminate_connection, range(2)))"},{"line_number":642,"context_line":""},{"line_number":643,"context_line":"        # Ensure that the LUN maps are cleaned up correctly for both"},{"line_number":644,"context_line":"        # parallel operations"},{"line_number":645,"context_line":"        self.assertEqual(mock_unmap_lun.call_count, 2)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    @mock.patch.object(block_base.NetAppBlockStorageLibrary,"},{"line_number":648,"context_line":"                       \u0027_get_fc_target_wwpns\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"b93ad22d_7a50eec3","line":645,"range":{"start_line":622,"start_character":0,"end_line":645,"end_character":54},"in_reply_to":"a8085534_bb892760","updated":"2025-08-05 16:46:36.000000000","message":"Same comment that is mentioned for FC.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"}],"cinder/tests/unit/volume/drivers/netapp/dataontap/test_nvme_library.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":936,"context_line":"            na_utils.is_multiattach_to_host.assert_called_once_with("},{"line_number":937,"context_line":"                namespace_volume, connector)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    @mock.patch.object(na_utils, \u0027is_multiattach_to_host\u0027,"},{"line_number":940,"context_line":"                       return_value\u003dFalse)"},{"line_number":941,"context_line":"    def test_terminate_connection_parallel(self,"},{"line_number":942,"context_line":"                                           mock_is_multiattach_to_host):"},{"line_number":943,"context_line":"        def execute_terminate_connection(connector):"},{"line_number":944,"context_line":"            mock_log \u003d patch(\u0027self.library.LOG\u0027).start()"},{"line_number":945,"context_line":"            self.library.terminate_connection(fake.NAMESPACE_VOLUME,"},{"line_number":946,"context_line":"                                              connector)"},{"line_number":947,"context_line":"            self.library._get_namespace_attr.assert_called_once_with("},{"line_number":948,"context_line":"                fake.NAMESPACE_NAME,"},{"line_number":949,"context_line":"                \u0027metadata\u0027)"},{"line_number":950,"context_line":"            host \u003d connector[\u0027nqn\u0027] if connector else None"},{"line_number":951,"context_line":"            self.library._unmap_namespace.assert_called_once_with("},{"line_number":952,"context_line":"                fake.PATH_NAMESPACE, host)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"            if connector:"},{"line_number":955,"context_line":"                mock_is_multiattach_to_host.assert_called_once_with("},{"line_number":956,"context_line":"                    fake.NAMESPACE_VOLUME, connector)"},{"line_number":957,"context_line":"            else:"},{"line_number":958,"context_line":"                mock_log.debug.assert_called_with(\u0027Unmapping namespace \u0027"},{"line_number":959,"context_line":"                                                  \u0027%(name)s from all hosts.\u0027,"},{"line_number":960,"context_line":"                                                  {\u0027name\u0027: fake."},{"line_number":961,"context_line":"                                                   NAMESPACE_VOLUME[\u0027name\u0027]})"},{"line_number":962,"context_line":"            mock_log.stop()"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"        connector_list \u003d [None, {\u0027nqn\u0027: fake.HOST_NQN}]"},{"line_number":965,"context_line":"        with ThreadPoolExecutor(max_workers\u003d2) as executor:"},{"line_number":966,"context_line":"            executor.map(execute_terminate_connection, connector_list)"}],"source_content_type":"text/x-python","patch_set":8,"id":"947d02c4_dd0926a1","line":966,"range":{"start_line":939,"start_character":4,"end_line":966,"end_character":70},"updated":"2025-07-30 16:47:09.000000000","message":"I don\u0027t know what we are trying to verify here.\nIf we are checking the integrity of the lock, it\u0027s a good candidate for a tempest test trying multiple attach/detach at the same time.\nHonestly, I\u0027m not a fan of running multiple threads/processes in UTs so i would avoid this scenario here.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":936,"context_line":"            na_utils.is_multiattach_to_host.assert_called_once_with("},{"line_number":937,"context_line":"                namespace_volume, connector)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    @mock.patch.object(na_utils, \u0027is_multiattach_to_host\u0027,"},{"line_number":940,"context_line":"                       return_value\u003dFalse)"},{"line_number":941,"context_line":"    def test_terminate_connection_parallel(self,"},{"line_number":942,"context_line":"                                           mock_is_multiattach_to_host):"},{"line_number":943,"context_line":"        def execute_terminate_connection(connector):"},{"line_number":944,"context_line":"            mock_log \u003d patch(\u0027self.library.LOG\u0027).start()"},{"line_number":945,"context_line":"            self.library.terminate_connection(fake.NAMESPACE_VOLUME,"},{"line_number":946,"context_line":"                                              connector)"},{"line_number":947,"context_line":"            self.library._get_namespace_attr.assert_called_once_with("},{"line_number":948,"context_line":"                fake.NAMESPACE_NAME,"},{"line_number":949,"context_line":"                \u0027metadata\u0027)"},{"line_number":950,"context_line":"            host \u003d connector[\u0027nqn\u0027] if connector else None"},{"line_number":951,"context_line":"            self.library._unmap_namespace.assert_called_once_with("},{"line_number":952,"context_line":"                fake.PATH_NAMESPACE, host)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"            if connector:"},{"line_number":955,"context_line":"                mock_is_multiattach_to_host.assert_called_once_with("},{"line_number":956,"context_line":"                    fake.NAMESPACE_VOLUME, connector)"},{"line_number":957,"context_line":"            else:"},{"line_number":958,"context_line":"                mock_log.debug.assert_called_with(\u0027Unmapping namespace \u0027"},{"line_number":959,"context_line":"                                                  \u0027%(name)s from all hosts.\u0027,"},{"line_number":960,"context_line":"                                                  {\u0027name\u0027: fake."},{"line_number":961,"context_line":"                                                   NAMESPACE_VOLUME[\u0027name\u0027]})"},{"line_number":962,"context_line":"            mock_log.stop()"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"        connector_list \u003d [None, {\u0027nqn\u0027: fake.HOST_NQN}]"},{"line_number":965,"context_line":"        with ThreadPoolExecutor(max_workers\u003d2) as executor:"},{"line_number":966,"context_line":"            executor.map(execute_terminate_connection, connector_list)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bc20e86d_e0e3ba4c","line":966,"range":{"start_line":939,"start_character":4,"end_line":966,"end_character":70},"in_reply_to":"947d02c4_dd0926a1","updated":"2025-08-05 16:46:36.000000000","message":"In one of the comment we added below code \n\n @coordination.synchronized(\u0027netapp-terminate-fc-connection-{volume.id}\u0027)\n    def terminate_connection_fc(self, volume, connector, **kwargs):\n    \nFor this code we have written the unit test.\nThe test verifies that terminate_connection behaves correctly when called concurrently with different connectors.Running in parallel ensures the method is thread-safe or behaves correctly under concurrent calls.\n\nI think it is fine to add this unit test to verify this behaviour. \nYes, we can plan to add the tempest test for this, as of now we have tested this manually and it is working fine.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"10da29a8903812330c4f2bb5a6a9b965c4c20fdd","unresolved":false,"context_lines":[{"line_number":936,"context_line":"            na_utils.is_multiattach_to_host.assert_called_once_with("},{"line_number":937,"context_line":"                namespace_volume, connector)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    @mock.patch.object(na_utils, \u0027is_multiattach_to_host\u0027,"},{"line_number":940,"context_line":"                       return_value\u003dFalse)"},{"line_number":941,"context_line":"    def test_terminate_connection_parallel(self,"},{"line_number":942,"context_line":"                                           mock_is_multiattach_to_host):"},{"line_number":943,"context_line":"        def execute_terminate_connection(connector):"},{"line_number":944,"context_line":"            mock_log \u003d patch(\u0027self.library.LOG\u0027).start()"},{"line_number":945,"context_line":"            self.library.terminate_connection(fake.NAMESPACE_VOLUME,"},{"line_number":946,"context_line":"                                              connector)"},{"line_number":947,"context_line":"            self.library._get_namespace_attr.assert_called_once_with("},{"line_number":948,"context_line":"                fake.NAMESPACE_NAME,"},{"line_number":949,"context_line":"                \u0027metadata\u0027)"},{"line_number":950,"context_line":"            host \u003d connector[\u0027nqn\u0027] if connector else None"},{"line_number":951,"context_line":"            self.library._unmap_namespace.assert_called_once_with("},{"line_number":952,"context_line":"                fake.PATH_NAMESPACE, host)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"            if connector:"},{"line_number":955,"context_line":"                mock_is_multiattach_to_host.assert_called_once_with("},{"line_number":956,"context_line":"                    fake.NAMESPACE_VOLUME, connector)"},{"line_number":957,"context_line":"            else:"},{"line_number":958,"context_line":"                mock_log.debug.assert_called_with(\u0027Unmapping namespace \u0027"},{"line_number":959,"context_line":"                                                  \u0027%(name)s from all hosts.\u0027,"},{"line_number":960,"context_line":"                                                  {\u0027name\u0027: fake."},{"line_number":961,"context_line":"                                                   NAMESPACE_VOLUME[\u0027name\u0027]})"},{"line_number":962,"context_line":"            mock_log.stop()"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"        connector_list \u003d [None, {\u0027nqn\u0027: fake.HOST_NQN}]"},{"line_number":965,"context_line":"        with ThreadPoolExecutor(max_workers\u003d2) as executor:"},{"line_number":966,"context_line":"            executor.map(execute_terminate_connection, connector_list)"}],"source_content_type":"text/x-python","patch_set":8,"id":"4291666f_c834a08d","line":966,"range":{"start_line":939,"start_character":4,"end_line":966,"end_character":70},"in_reply_to":"bc20e86d_e0e3ba4c","updated":"2025-08-05 19:50:39.000000000","message":"Hmm, the more I study this test the more that I\u0027m feeling the same as Rajat.\n\nFirst, I don\u0027t think it\u0027s necessary to add a test that simply verifies the lock decorator is working.\n\nI also don\u0027t think that using threads like this can guarantee both of them call terminate_connection() at the same time. You\u0027d need to find a way of ensuring each call will block at some point during the call, thereby enabling the other thread to simultaneously attempt to call it (which would also block on the lock until the first call eventually completes). I suspect the first thread will run to completion before the second, thereby not really testing the lock.\n\nI also don\u0027t think the connector_list values correctly test the reason for adding a lock. The scenario in question is when you have a multiattach volume with multiple connections on the same host, and concurrent calls to terminate_connection() results in BOTH of them calling na_utils.is_multiattach_to_host() and getting a True response. The purpose of adding the lock is for the first caller (who takes the lock) to see a True response, and by the time the second caller can take the lock na_utils.is_multiattach_to_host() returns False. Using \u0027None\u0027 for one of the connector values doesn\u0027t seem to fit the desired test scenario.\n\nBut going back to my first point, I\u0027d be OK just eliminating this test entirely and trusting the lock decorator doesn\u0027t need to be tested every time it\u0027s used.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0c5744778effb3927fefce88eaf16398a43e9d2b","unresolved":false,"context_lines":[{"line_number":936,"context_line":"            na_utils.is_multiattach_to_host.assert_called_once_with("},{"line_number":937,"context_line":"                namespace_volume, connector)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    @mock.patch.object(na_utils, \u0027is_multiattach_to_host\u0027,"},{"line_number":940,"context_line":"                       return_value\u003dFalse)"},{"line_number":941,"context_line":"    def test_terminate_connection_parallel(self,"},{"line_number":942,"context_line":"                                           mock_is_multiattach_to_host):"},{"line_number":943,"context_line":"        def execute_terminate_connection(connector):"},{"line_number":944,"context_line":"            mock_log \u003d patch(\u0027self.library.LOG\u0027).start()"},{"line_number":945,"context_line":"            self.library.terminate_connection(fake.NAMESPACE_VOLUME,"},{"line_number":946,"context_line":"                                              connector)"},{"line_number":947,"context_line":"            self.library._get_namespace_attr.assert_called_once_with("},{"line_number":948,"context_line":"                fake.NAMESPACE_NAME,"},{"line_number":949,"context_line":"                \u0027metadata\u0027)"},{"line_number":950,"context_line":"            host \u003d connector[\u0027nqn\u0027] if connector else None"},{"line_number":951,"context_line":"            self.library._unmap_namespace.assert_called_once_with("},{"line_number":952,"context_line":"                fake.PATH_NAMESPACE, host)"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":"            if connector:"},{"line_number":955,"context_line":"                mock_is_multiattach_to_host.assert_called_once_with("},{"line_number":956,"context_line":"                    fake.NAMESPACE_VOLUME, connector)"},{"line_number":957,"context_line":"            else:"},{"line_number":958,"context_line":"                mock_log.debug.assert_called_with(\u0027Unmapping namespace \u0027"},{"line_number":959,"context_line":"                                                  \u0027%(name)s from all hosts.\u0027,"},{"line_number":960,"context_line":"                                                  {\u0027name\u0027: fake."},{"line_number":961,"context_line":"                                                   NAMESPACE_VOLUME[\u0027name\u0027]})"},{"line_number":962,"context_line":"            mock_log.stop()"},{"line_number":963,"context_line":""},{"line_number":964,"context_line":"        connector_list \u003d [None, {\u0027nqn\u0027: fake.HOST_NQN}]"},{"line_number":965,"context_line":"        with ThreadPoolExecutor(max_workers\u003d2) as executor:"},{"line_number":966,"context_line":"            executor.map(execute_terminate_connection, connector_list)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9e90b969_91c26d36","line":966,"range":{"start_line":939,"start_character":4,"end_line":966,"end_character":70},"in_reply_to":"bc20e86d_e0e3ba4c","updated":"2025-08-05 18:00:26.000000000","message":"The locks are expected to work correctly since they are separately managed and tested so we just need to verify the functionality added in this patch.\nI don\u0027t have an issue with the unit test but this scenario is better tested by an external tool like tempest to do API calls and verify it rather than a UT. But I also don\u0027t mind keeping it here.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"}],"cinder/tests/unit/volume/drivers/netapp/test_utils.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":875,"context_line":"        volume \u003d copy.deepcopy(fakes.test_volume)"},{"line_number":876,"context_line":"        volume.volume_attachment \u003d ["},{"line_number":877,"context_line":"            {\u0027attach_status\u0027: fakes.ATTACHED, \u0027attached_host\u0027: fakes.HOST_NAME,"},{"line_number":878,"context_line":"             \u0027connector\u0027: {\u0027initiator\u0027: \u0027fake_initiator\u0027}},"},{"line_number":879,"context_line":"            {\u0027attach_status\u0027: fakes.ATTACHED, \u0027attached_host\u0027: fakes.HOST_NAME,"},{"line_number":880,"context_line":"             \u0027connector\u0027: {\u0027initiator\u0027: \u0027fake_initiator\u0027}},"},{"line_number":881,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":8,"id":"b6229323_714caa63","line":878,"range":{"start_line":878,"start_character":26,"end_line":878,"end_character":57},"updated":"2025-07-30 16:47:09.000000000","message":"As mentioned in the utils file, this will only exist if the `/etc/iscsi/initiatorname.iscsi` file exists which might not be the case for FC and NVMe connections so including initiator in all of the tests invalidates the correctness of theses tests.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":875,"context_line":"        volume \u003d copy.deepcopy(fakes.test_volume)"},{"line_number":876,"context_line":"        volume.volume_attachment \u003d ["},{"line_number":877,"context_line":"            {\u0027attach_status\u0027: fakes.ATTACHED, \u0027attached_host\u0027: fakes.HOST_NAME,"},{"line_number":878,"context_line":"             \u0027connector\u0027: {\u0027initiator\u0027: \u0027fake_initiator\u0027}},"},{"line_number":879,"context_line":"            {\u0027attach_status\u0027: fakes.ATTACHED, \u0027attached_host\u0027: fakes.HOST_NAME,"},{"line_number":880,"context_line":"             \u0027connector\u0027: {\u0027initiator\u0027: \u0027fake_initiator\u0027}},"},{"line_number":881,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3b6a55cb_97f9506c","line":878,"range":{"start_line":878,"start_character":26,"end_line":878,"end_character":57},"in_reply_to":"b6229323_714caa63","updated":"2025-08-05 16:46:36.000000000","message":"Made the changes accordingly","commit_id":"0215931370daf70651760398a9524ae6f2be7859"}],"cinder/volume/drivers/netapp/dataontap/block_base.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c9bd0566b70c459b0c268cd9c7761dcf28a4cbf3","unresolved":true,"context_lines":[{"line_number":1101,"context_line":"                         zone is to be removed, otherwise the same map with"},{"line_number":1102,"context_line":"                         an empty dict for the \u0027data\u0027 key"},{"line_number":1103,"context_line":"        \"\"\""},{"line_number":1104,"context_line":"        if connector and na_utils.is_multiattach_to_host("},{"line_number":1105,"context_line":"                volume,"},{"line_number":1106,"context_line":"                connector"},{"line_number":1107,"context_line":"        ):"}],"source_content_type":"text/x-python","patch_set":6,"id":"964b45a2_f485b333","line":1104,"updated":"2025-07-21 15:14:23.000000000","message":"I think this will race in the situation where you have two terminate requests happening at the same time for the same volume, resulting in the cleanup not happening.\n\nThis could be resolved by adding a\n\n    @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume.id)\n    \ndecorator to this terminate_connection_fc method.\n\nLet me know what you think.","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"5ebf4129eb33dfa5441ddad52fd4e32ef158a579","unresolved":false,"context_lines":[{"line_number":1101,"context_line":"                         zone is to be removed, otherwise the same map with"},{"line_number":1102,"context_line":"                         an empty dict for the \u0027data\u0027 key"},{"line_number":1103,"context_line":"        \"\"\""},{"line_number":1104,"context_line":"        if connector and na_utils.is_multiattach_to_host("},{"line_number":1105,"context_line":"                volume,"},{"line_number":1106,"context_line":"                connector"},{"line_number":1107,"context_line":"        ):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ecbd7570_ed078319","line":1104,"in_reply_to":"964b45a2_f485b333","updated":"2025-07-24 12:31:06.000000000","message":"Hi Eric, thank you for the suggestion and it makes sense. Added the code and tested locally to ensure no regressions.","commit_id":"e27058def7537efe79b568e1f11f73e1eaceaf1f"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b2d3081bd48ff074f2598a0c3df1c2cb0b32664f","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        next(same_connector, False)"},{"line_number":989,"context_line":"        return next(same_connector, False)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def terminate_connection_iscsi(self, volume, connector, **kwargs):"},{"line_number":992,"context_line":"        \"\"\"Driver entry point to unattach a volume from an instance."},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"        Unmask the LUN on the storage system so the given initiator can no"}],"source_content_type":"text/x-python","patch_set":7,"id":"c4d4d970_52bff96c","line":991,"updated":"2025-07-24 18:18:03.000000000","message":"I\u0027m glad to see you included nvme-tcp in this patch, but you should also fix iscsi.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"f5a14e9ca7965790320d483aff0309768ddd3054","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        next(same_connector, False)"},{"line_number":989,"context_line":"        return next(same_connector, False)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def terminate_connection_iscsi(self, volume, connector, **kwargs):"},{"line_number":992,"context_line":"        \"\"\"Driver entry point to unattach a volume from an instance."},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"        Unmask the LUN on the storage system so the given initiator can no"}],"source_content_type":"text/x-python","patch_set":7,"id":"781e9348_0dc2e920","line":991,"in_reply_to":"6f5d139d_83f9ccdb","updated":"2025-07-30 10:25:52.000000000","message":"Bug link: https://bugs.launchpad.net/cinder/+bug/2119084","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"ed88fdfb2a6ebe9aab30f924d6b9d1bcf7473f31","unresolved":false,"context_lines":[{"line_number":988,"context_line":"        next(same_connector, False)"},{"line_number":989,"context_line":"        return next(same_connector, False)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def terminate_connection_iscsi(self, volume, connector, **kwargs):"},{"line_number":992,"context_line":"        \"\"\"Driver entry point to unattach a volume from an instance."},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"        Unmask the LUN on the storage system so the given initiator can no"}],"source_content_type":"text/x-python","patch_set":7,"id":"5b3573f1_06782af0","line":991,"in_reply_to":"781e9348_0dc2e920","updated":"2025-08-07 05:53:31.000000000","message":"Closing this comment as we will fix this issue as part of above bug","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"ce44cb32b89a62964f2a8f5afd475cfe577abcb6","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        next(same_connector, False)"},{"line_number":989,"context_line":"        return next(same_connector, False)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def terminate_connection_iscsi(self, volume, connector, **kwargs):"},{"line_number":992,"context_line":"        \"\"\"Driver entry point to unattach a volume from an instance."},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"        Unmask the LUN on the storage system so the given initiator can no"}],"source_content_type":"text/x-python","patch_set":7,"id":"f86287a7_6a284829","line":991,"in_reply_to":"c4d4d970_52bff96c","updated":"2025-07-25 05:49:04.000000000","message":"Hi Alan. Thanks for the review comment. Yes we want to fix this for iscsi as well. We will create a bug to handle iscsi separately since we are in the last leg of development and this bug is required for our NVMe certification on priority. Please let us know if that works.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"f1ffbbbbde4a84c4a92830b2e64f5b35fc0c9e54","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        next(same_connector, False)"},{"line_number":989,"context_line":"        return next(same_connector, False)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    def terminate_connection_iscsi(self, volume, connector, **kwargs):"},{"line_number":992,"context_line":"        \"\"\"Driver entry point to unattach a volume from an instance."},{"line_number":993,"context_line":""},{"line_number":994,"context_line":"        Unmask the LUN on the storage system so the given initiator can no"}],"source_content_type":"text/x-python","patch_set":7,"id":"6f5d139d_83f9ccdb","line":991,"in_reply_to":"f86287a7_6a284829","updated":"2025-07-30 07:08:05.000000000","message":"Created bug as Anoop mentioned.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b2d3081bd48ff074f2598a0c3df1c2cb0b32664f","unresolved":true,"context_lines":[{"line_number":1104,"context_line":"        \"\"\""},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":1107,"context_line":"        def _terminate_connection_fc():"},{"line_number":1108,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":1109,"context_line":"                    volume,"},{"line_number":1110,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"c44a604d_272afb0f","line":1107,"updated":"2025-07-24 18:18:03.000000000","message":"I don\u0027t believe you need to move all the code into this inner function. You should just need to add the L1106 decorator on the outer function at L1093.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"25ca334b89db65985c959274e99e274d8b23505f","unresolved":true,"context_lines":[{"line_number":1104,"context_line":"        \"\"\""},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":1107,"context_line":"        def _terminate_connection_fc():"},{"line_number":1108,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":1109,"context_line":"                    volume,"},{"line_number":1110,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"bce472e7_59f622d3","line":1107,"in_reply_to":"4cdc88a4_6928b5c6","updated":"2025-07-28 15:11:55.000000000","message":"It\u0027s simpler to just use the @utils.synchronized() decorator on the terminate_connection_fc() method -- it will work the same as doing it like this and is more straightforward.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"f1ffbbbbde4a84c4a92830b2e64f5b35fc0c9e54","unresolved":false,"context_lines":[{"line_number":1104,"context_line":"        \"\"\""},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":1107,"context_line":"        def _terminate_connection_fc():"},{"line_number":1108,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":1109,"context_line":"                    volume,"},{"line_number":1110,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"c1dedc1f_8c3fd667","line":1107,"in_reply_to":"bce472e7_59f622d3","updated":"2025-07-30 07:08:05.000000000","message":"Added the decorator to the outer function. Thank you so much @eharney@redhat.com for offline suggestions! I appreciate it. \n\nThanks a lot @abishop@redhat.com for continuous support as always!!","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"ce44cb32b89a62964f2a8f5afd475cfe577abcb6","unresolved":true,"context_lines":[{"line_number":1104,"context_line":"        \"\"\""},{"line_number":1105,"context_line":""},{"line_number":1106,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":1107,"context_line":"        def _terminate_connection_fc():"},{"line_number":1108,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":1109,"context_line":"                    volume,"},{"line_number":1110,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"4cdc88a4_6928b5c6","line":1107,"in_reply_to":"c44a604d_272afb0f","updated":"2025-07-25 05:49:04.000000000","message":"I believe keeping the scope of synchronized block to the inner function should be recommended as we do not want the lock to be acquired at the parent function level. The block where the cleanup can get affected is already handled. Do you think we can go ahead with keeping it at the inner function level?","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"}],"cinder/volume/drivers/netapp/dataontap/nvme_library.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b2d3081bd48ff074f2598a0c3df1c2cb0b32664f","unresolved":true,"context_lines":[{"line_number":758,"context_line":"        \"\"\""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":761,"context_line":"        def _terminate_connection():"},{"line_number":762,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":763,"context_line":"                    volume,"},{"line_number":764,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"769ea40f_1b339f3e","line":761,"updated":"2025-07-24 18:18:03.000000000","message":"As per my previous comment, I don\u0027t believe you need an inner function, just add the decorator to L753. I also think volume.id is preferred over volume[\u0027id\u0027] (it\u0027s an OVO, right?)","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"25ca334b89db65985c959274e99e274d8b23505f","unresolved":true,"context_lines":[{"line_number":758,"context_line":"        \"\"\""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":761,"context_line":"        def _terminate_connection():"},{"line_number":762,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":763,"context_line":"                    volume,"},{"line_number":764,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"5fc8d50a_78a90ea0","line":761,"in_reply_to":"29d9a306_c3258642","updated":"2025-07-28 15:11:55.000000000","message":"volume.id has been the preferred method since we introduced oslo.versionedobjects Volume objects -- the dict style of access is just for backward compat of old code.  New code should access fields as attributes instead of dicts.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"3f143d84f261f6208d46714c053c768e7f2beb47","unresolved":true,"context_lines":[{"line_number":758,"context_line":"        \"\"\""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":761,"context_line":"        def _terminate_connection():"},{"line_number":762,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":763,"context_line":"                    volume,"},{"line_number":764,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"de02b32e_4d5e4165","line":761,"in_reply_to":"5fc8d50a_78a90ea0","updated":"2025-07-29 03:43:50.000000000","message":"Okay thanks for the clarification.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":38059,"name":"Anoop Kumar Shukla","display_name":"Anoop Shukla","email":"anoop.shukla@netapp.com","username":"anoop2","status":"NetApp"},"change_message_id":"ce44cb32b89a62964f2a8f5afd475cfe577abcb6","unresolved":true,"context_lines":[{"line_number":758,"context_line":"        \"\"\""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":761,"context_line":"        def _terminate_connection():"},{"line_number":762,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":763,"context_line":"                    volume,"},{"line_number":764,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"29d9a306_c3258642","line":761,"in_reply_to":"769ea40f_1b339f3e","updated":"2025-07-25 05:49:04.000000000","message":"volume[\u0027id\u0027] works for UTs and actual testing. Any reason why volume.id is preferred over volume[\u0027id\u0027]?","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"2228fb536bac4a97d08a675701b2c9f05f11bff8","unresolved":false,"context_lines":[{"line_number":758,"context_line":"        \"\"\""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":761,"context_line":"        def _terminate_connection():"},{"line_number":762,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":763,"context_line":"                    volume,"},{"line_number":764,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"32b3b924_ac1b78bc","line":761,"in_reply_to":"d27dfdc2_de982833","updated":"2025-07-30 07:09:35.000000000","message":"Done","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"5f512a11088a5b26ca1057456b996688e2725886","unresolved":true,"context_lines":[{"line_number":758,"context_line":"        \"\"\""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"        @utils.synchronized(\u0027netapp-terminate-connection-%s\u0027 % volume[\u0027id\u0027])"},{"line_number":761,"context_line":"        def _terminate_connection():"},{"line_number":762,"context_line":"            if connector and na_utils.is_multiattach_to_host("},{"line_number":763,"context_line":"                    volume,"},{"line_number":764,"context_line":"                    connector"}],"source_content_type":"text/x-python","patch_set":7,"id":"d27dfdc2_de982833","line":761,"in_reply_to":"de02b32e_4d5e4165","updated":"2025-07-30 07:09:16.000000000","message":"Done.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"25ca334b89db65985c959274e99e274d8b23505f","unresolved":true,"context_lines":[{"line_number":764,"context_line":"                    connector"},{"line_number":765,"context_line":"            ):"},{"line_number":766,"context_line":"                return"},{"line_number":767,"context_line":"            name \u003d volume[\u0027name\u0027]"},{"line_number":768,"context_line":"            host_nqn \u003d None"},{"line_number":769,"context_line":"            if connector is None:"},{"line_number":770,"context_line":"                LOG.debug(\u0027Unmapping namespace %(name)s from all hosts.\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"e236983a_0c6de08e","line":767,"range":{"start_line":767,"start_character":19,"end_line":767,"end_character":33},"updated":"2025-07-28 15:11:55.000000000","message":"Same here ^","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"5f512a11088a5b26ca1057456b996688e2725886","unresolved":false,"context_lines":[{"line_number":764,"context_line":"                    connector"},{"line_number":765,"context_line":"            ):"},{"line_number":766,"context_line":"                return"},{"line_number":767,"context_line":"            name \u003d volume[\u0027name\u0027]"},{"line_number":768,"context_line":"            host_nqn \u003d None"},{"line_number":769,"context_line":"            if connector is None:"},{"line_number":770,"context_line":"                LOG.debug(\u0027Unmapping namespace %(name)s from all hosts.\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"55c4be94_01926c96","line":767,"range":{"start_line":767,"start_character":19,"end_line":767,"end_character":33},"in_reply_to":"e236983a_0c6de08e","updated":"2025-07-30 07:09:16.000000000","message":"Done.","commit_id":"0da02a810ea8151a85c0223e642ef018308f1480"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"0272e72099c4bf1441cd71fa315e6b992013a62c","unresolved":true,"context_lines":[{"line_number":523,"context_line":"            pool.update(ssc_vol_info)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"            # Add driver capabilities and config info"},{"line_number":526,"context_line":"            pool[\u0027QoS_support\u0027] \u003d False"},{"line_number":527,"context_line":"            pool[\u0027multiattach\u0027] \u003d False"},{"line_number":528,"context_line":"            pool[\u0027online_extend_support\u0027] \u003d False"},{"line_number":529,"context_line":"            pool[\u0027consistencygroup_support\u0027] \u003d False"},{"line_number":530,"context_line":"            pool[\u0027consistent_group_snapshot_enabled\u0027] \u003d False"},{"line_number":531,"context_line":"            pool[\u0027reserved_percentage\u0027] \u003d self.reserved_percentage"},{"line_number":532,"context_line":"            pool[\u0027max_over_subscription_ratio\u0027] \u003d ("},{"line_number":533,"context_line":"                self.max_over_subscription_ratio)"},{"line_number":534,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3ae8f29e_4f93ca9a","line":531,"range":{"start_line":526,"start_character":0,"end_line":531,"end_character":0},"updated":"2025-08-05 21:09:02.000000000","message":"What\u0027s the significance of all these being False?","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"ed88fdfb2a6ebe9aab30f924d6b9d1bcf7473f31","unresolved":false,"context_lines":[{"line_number":523,"context_line":"            pool.update(ssc_vol_info)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"            # Add driver capabilities and config info"},{"line_number":526,"context_line":"            pool[\u0027QoS_support\u0027] \u003d False"},{"line_number":527,"context_line":"            pool[\u0027multiattach\u0027] \u003d False"},{"line_number":528,"context_line":"            pool[\u0027online_extend_support\u0027] \u003d False"},{"line_number":529,"context_line":"            pool[\u0027consistencygroup_support\u0027] \u003d False"},{"line_number":530,"context_line":"            pool[\u0027consistent_group_snapshot_enabled\u0027] \u003d False"},{"line_number":531,"context_line":"            pool[\u0027reserved_percentage\u0027] \u003d self.reserved_percentage"},{"line_number":532,"context_line":"            pool[\u0027max_over_subscription_ratio\u0027] \u003d ("},{"line_number":533,"context_line":"                self.max_over_subscription_ratio)"},{"line_number":534,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"e0a0c83d_b18b25ba","line":531,"range":{"start_line":526,"start_character":0,"end_line":531,"end_character":0},"in_reply_to":"3ae8f29e_4f93ca9a","updated":"2025-08-07 05:53:31.000000000","message":"There are few more patches, below changes are part of that changes where we are setting these value to true after adding those support.\n\nPlease refer below patches \nhttps://review.opendev.org/c/openstack/cinder/+/928486\nhttps://review.opendev.org/c/openstack/cinder/+/944964","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"}],"cinder/volume/drivers/netapp/utils.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":564,"context_line":""},{"line_number":565,"context_line":"def is_multiattach_to_host(volume, connector):"},{"line_number":566,"context_line":"    # With multi-attach enabled, a single volume can be connected to"},{"line_number":567,"context_line":"    # multiple instances, all of which are located on the same nova host."},{"line_number":568,"context_line":"    # The volume should remain attached to the nova host until it is"},{"line_number":569,"context_line":"    # detached from the final instance."},{"line_number":570,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"e1f4d3c4_c09d29f1","line":567,"range":{"start_line":567,"start_character":24,"end_line":567,"end_character":73},"updated":"2025-07-30 16:47:09.000000000","message":"The framing of this part is not correct.\nWe should rather say,\n\n    With multi-attach enabled, a single volume can be attached to multiple \n    instances. If all of the instances are running on the same nova host, the \n    volume should remain attached to the nova host until it is detached from the \n    last instance.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":564,"context_line":""},{"line_number":565,"context_line":"def is_multiattach_to_host(volume, connector):"},{"line_number":566,"context_line":"    # With multi-attach enabled, a single volume can be connected to"},{"line_number":567,"context_line":"    # multiple instances, all of which are located on the same nova host."},{"line_number":568,"context_line":"    # The volume should remain attached to the nova host until it is"},{"line_number":569,"context_line":"    # detached from the final instance."},{"line_number":570,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"da1981f9_9753de81","line":567,"range":{"start_line":567,"start_character":24,"end_line":567,"end_character":73},"in_reply_to":"e1f4d3c4_c09d29f1","updated":"2025-08-05 16:46:36.000000000","message":"Thanks, updated the same.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"73f53c62f455314ed3dc023aa93827af58bc8d85","unresolved":true,"context_lines":[{"line_number":576,"context_line":"        if attach_info[\u0027attach_status\u0027] \u003d\u003d fields.VolumeAttachStatus.ATTACHED"},{"line_number":577,"context_line":"        and attach_info[\u0027attached_host\u0027] \u003d\u003d connector.get(\u0027host\u0027)"},{"line_number":578,"context_line":"        and attach_info.get(\u0027connector\u0027) is not None"},{"line_number":579,"context_line":"        and attach_info[\u0027connector\u0027][\u0027initiator\u0027] \u003d\u003d"},{"line_number":580,"context_line":"        connector.get(\u0027initiator\u0027)"},{"line_number":581,"context_line":"    ]"},{"line_number":582,"context_line":"    LOG.debug(\u0027is_multiattach_to_host: attachment %s.\u0027, attachment)"},{"line_number":583,"context_line":"    return len(attachment) \u003e 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"22bcd31e_e6077255","line":580,"range":{"start_line":579,"start_character":8,"end_line":580,"end_character":34},"updated":"2025-07-30 16:47:09.000000000","message":"This check is not correct, the [\u0027initiator\u0027] property is only populated by the iscsi connector[1] if the `/etc/iscsi/initiatorname.iscsi` file exists\n\nSimilarly, FC[2] returns WWPNs and WWNNs and NVMe returns it\u0027s own set of properties[3]\n\nSince this method is generic for FC and NVMe (not sure why not used for iscsi), this will fail if the initiator file doesn\u0027t exist and it\u0027s not even logically correct to check that.\n\nI feel that the host check is sufficient to verify that the instances are on the same node and this check is not really required.\n\nThe new check should be,\n\n    attachment \u003d [\n        attach_info\n        for attach_info in volume.volume_attachment\n        if attach_info[\u0027attach_status\u0027] \u003d\u003d fields.VolumeAttachStatus.ATTACHED\n        and attach_info[\u0027attached_host\u0027] \u003d\u003d connector.get(\u0027host\u0027)\n\n[1] https://github.com/openstack/os-brick/blob/0a5a387523a449060df088182f2242765b6f2436/os_brick/initiator/connectors/iscsi.py#L80\n[2] https://github.com/openstack/os-brick/blob/0a5a387523a449060df088182f2242765b6f2436/os_brick/initiator/connectors/fibre_channel.py#L69-L74\n[3] https://github.com/openstack/os-brick/blob/0a5a387523a449060df088182f2242765b6f2436/os_brick/initiator/connectors/nvmeof.py#L806-L814","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"4fbabdb1275b380e1a57335887363945b468ce67","unresolved":false,"context_lines":[{"line_number":576,"context_line":"        if attach_info[\u0027attach_status\u0027] \u003d\u003d fields.VolumeAttachStatus.ATTACHED"},{"line_number":577,"context_line":"        and attach_info[\u0027attached_host\u0027] \u003d\u003d connector.get(\u0027host\u0027)"},{"line_number":578,"context_line":"        and attach_info.get(\u0027connector\u0027) is not None"},{"line_number":579,"context_line":"        and attach_info[\u0027connector\u0027][\u0027initiator\u0027] \u003d\u003d"},{"line_number":580,"context_line":"        connector.get(\u0027initiator\u0027)"},{"line_number":581,"context_line":"    ]"},{"line_number":582,"context_line":"    LOG.debug(\u0027is_multiattach_to_host: attachment %s.\u0027, attachment)"},{"line_number":583,"context_line":"    return len(attachment) \u003e 1"}],"source_content_type":"text/x-python","patch_set":8,"id":"a40154c5_96d3613e","line":580,"range":{"start_line":579,"start_character":8,"end_line":580,"end_character":34},"in_reply_to":"22bcd31e_e6077255","updated":"2025-08-05 16:46:36.000000000","message":"Thanks for pointing these files. Initially we have check only for host name but we thought compute node might have multiple HBA card, mean multiple initiator so we added this logic and we tested this for NVMe and it was working fine.\nBut it seems we don\u0027t have this value for FCP. will remove the initiator check here.\n\nThanks for pointing this and sharing the os-brick files.","commit_id":"0215931370daf70651760398a9524ae6f2be7859"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"10da29a8903812330c4f2bb5a6a9b965c4c20fdd","unresolved":true,"context_lines":[{"line_number":563,"context_line":""},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"def is_multiattach_to_host(volume, connector):"},{"line_number":566,"context_line":"    # With multi-attach enabled, a single volume can be attached to multiple"},{"line_number":567,"context_line":"    # instances. If all the instances are running on the same nova host, the"},{"line_number":568,"context_line":"    # volume should remain attached to the nova host until it is detached"},{"line_number":569,"context_line":"    # from the last instance."},{"line_number":570,"context_line":"    if not volume.multiattach or not volume.volume_attachment:"},{"line_number":571,"context_line":"        return False"},{"line_number":572,"context_line":"    attachment \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"2e6fcc24_cec4c604","line":569,"range":{"start_line":566,"start_character":4,"end_line":569,"end_character":29},"updated":"2025-08-05 19:50:39.000000000","message":"This is closer but still not fully correct. Consider a volume with multiple attachments on one host AND an attachment on another host. Here\u0027s a minor rewording that I think addresses this scenario:\n\n    # With multi-attach enabled, a single volume can be attached to multiple\n    # instances. If multiple instances are running on the same nova host, the\n    # volume should remain attached to the nova host until it is detached\n    # from the last instance on that host.","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"},{"author":{"_account_id":36180,"name":"Gireesh Awasthi","display_name":"Gireesh","email":"gawasthi2010@gmail.com","username":"agireesh","status":"NetApp"},"change_message_id":"ed88fdfb2a6ebe9aab30f924d6b9d1bcf7473f31","unresolved":false,"context_lines":[{"line_number":563,"context_line":""},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"def is_multiattach_to_host(volume, connector):"},{"line_number":566,"context_line":"    # With multi-attach enabled, a single volume can be attached to multiple"},{"line_number":567,"context_line":"    # instances. If all the instances are running on the same nova host, the"},{"line_number":568,"context_line":"    # volume should remain attached to the nova host until it is detached"},{"line_number":569,"context_line":"    # from the last instance."},{"line_number":570,"context_line":"    if not volume.multiattach or not volume.volume_attachment:"},{"line_number":571,"context_line":"        return False"},{"line_number":572,"context_line":"    attachment \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"58c814cb_7abf4fd8","line":569,"range":{"start_line":566,"start_character":4,"end_line":569,"end_character":29},"in_reply_to":"2e6fcc24_cec4c604","updated":"2025-08-07 05:53:31.000000000","message":"Done.","commit_id":"f1f69cffe0834880d3f49b69c5dabe1307d9f43c"}],"releasenotes/notes/bug-2110274-fix-detach-issue-for-multiattached-volume-7202cecaeed5ecd0.yaml":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"da09406bf5d362893ab571ee0c4232a184535c50","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Volumes with multi-attach type can be connected to multiple instances."},{"line_number":5,"context_line":"    Additional logic has been implemented to handle the removal of Cinder"},{"line_number":6,"context_line":"    volumes from multiple instances. More more details, please check"},{"line_number":7,"context_line":"    `Launchpad bug #2110274 \u003chttps://bugs.launchpad.net/cinder/+bug/2110274\u003e`_ "}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3e902d4c_a34da9c2","line":6,"range":{"start_line":6,"start_character":37,"end_line":6,"end_character":41},"updated":"2025-06-24 11:51:30.000000000","message":"For*","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"},{"author":{"_account_id":36179,"name":"Saikumar Pulluri","display_name":"Saikumar Pulluri","email":"saikumar1016@gmail.com","username":"pulluri"},"change_message_id":"6ec4ac6f8bc441def84995634753018f21f54de1","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Volumes with multi-attach type can be connected to multiple instances."},{"line_number":5,"context_line":"    Additional logic has been implemented to handle the removal of Cinder"},{"line_number":6,"context_line":"    volumes from multiple instances. More more details, please check"},{"line_number":7,"context_line":"    `Launchpad bug #2110274 \u003chttps://bugs.launchpad.net/cinder/+bug/2110274\u003e`_ "}],"source_content_type":"text/x-yaml","patch_set":3,"id":"387a1852_4cbf9d14","line":6,"range":{"start_line":6,"start_character":37,"end_line":6,"end_character":41},"in_reply_to":"3e902d4c_a34da9c2","updated":"2025-07-08 11:13:13.000000000","message":"Updated. Thanks Eric.","commit_id":"c0d8daa8398e0037bc18425b6af5219cec895f11"}]}