)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a44fc9b9fc99ca452c8cd8bf3997e8c95fa10725","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"caf348cb_0c27d5ee","updated":"2026-07-31 17:23:17.000000000","message":"Aside from preferring that split-out helper and understanding that the current query optimization is the best we can do for a backportable fix, I think this is good to go. The new test fails without the functional change and the other test change is required after it.","commit_id":"5e1a6f55559f6a677842b2c6d5e4229ce12af54d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"928ae357bcbfc548d5d8d55c61cfa1108477af08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5fb98fd7_46b9b24b","updated":"2026-07-31 01:48:25.000000000","message":"recheck\n\nfailure seems unrelated","commit_id":"5e1a6f55559f6a677842b2c6d5e4229ce12af54d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d7b554c4410e60f47245f851ef341727fcb0610","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3af3b815_c41a1e84","in_reply_to":"caf348cb_0c27d5ee","updated":"2026-07-31 20:46:27.000000000","message":"Thanks for checking.. \n\nThis build here verified this change: https://zuul.opendev.org/t/openstack/build/c81ad280553f4ef4aa43cba8eaed6e3e/logs\n\nand these are the tests we ran: \n\nhttps://review.opendev.org/c/openstack/manila-tempest-plugin/+/984884/\n\n\nthey\u0027re running on a single-host so they don\u0027t actually run into this bug. but useful to see if i\u0027m introducing regressions...\n\nwe\u0027re adding multi-node tests too: \n\nhttps://review.opendev.org/c/openstack/manila-tempest-plugin/+/989634\n\nand \n\nhere\u0027re the new test results: https://zuul.opendev.org/t/openstack/build/95021bf9c7ad451fae24927331812433\n\nthey include this bug fix, the share_management refactor, and the cold migration code..","commit_id":"5e1a6f55559f6a677842b2c6d5e4229ce12af54d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0ef1c70d13e24b45c3e751fbbf2eae97b156993a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"0ec29bd8_54d534d0","updated":"2026-08-03 13:52:16.000000000","message":"The logic didn\u0027t change so I\u0027ll +2 but it seems weird to remove the comment about why this logic makes sense (if anything more explanation would have been nice).","commit_id":"5437738f0e594c542c1f6c181345774b3ffae70d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"c95a71d458e50e11e8870b412f5791d13fe57f98","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"77396aa8_14fe0576","in_reply_to":"0ec29bd8_54d534d0","updated":"2026-08-03 17:41:56.000000000","message":"I\u0027ll restore it.","commit_id":"5437738f0e594c542c1f6c181345774b3ffae70d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"26a2faffe6e5465ec5f77af5640da133d68f77ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"a23e6353_8e8d8f60","in_reply_to":"77396aa8_14fe0576","updated":"2026-08-03 18:13:39.000000000","message":"Done","commit_id":"5437738f0e594c542c1f6c181345774b3ffae70d"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ed50f42983749a25016e64372cf857a11e1bec8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"f20276c4_4372d24f","updated":"2026-08-06 16:02:55.000000000","message":"Looks OK to me, thanks","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"8b4663fd81ec23905ce6589ca2590aaf02f8e3c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"7b580e5e_02d8184d","updated":"2026-08-07 04:47:18.000000000","message":"recheck\n\n\ninfra/flaky issues","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"6bbe9f3490a6087ab2507b3a0363cafd3c5b8e06","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d35b3be4_6fa91478","updated":"2026-08-04 20:38:04.000000000","message":"recheck\n\n\nnova-multi-cell\u0027s being flaky","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eafc500d5f20270eb6f30767e81e4ae607608972","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a4c83b09_2df9f225","updated":"2026-08-06 20:22:21.000000000","message":"recheck\n\na timeout in the shelve/unshelve test in the grenade job.. passes elsewhere.\n\n```\nDetails: (ServerActionsTestOtherB:tearDown) Server bf1b3752-3b6e-406c-b888-3f95be566b6e failed to reach ACTIVE status and task state \"None\" within the required time (196 s). Current status: SHELVED_OFFLOADED. Current task state: None.\n```","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"bc1c387bddd9f818143a1ab4fab0b30fa879744d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a4018ce2_46a1fa9f","updated":"2026-08-08 03:48:40.000000000","message":"recheck\n\nfingers","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"6caa7dad6b79af091e9eb67ebd8b233c88811a9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9bc788e8_27b0e05c","updated":"2026-08-07 16:39:24.000000000","message":"recheck\n\njob whack-a-mole. The grenade job passed earlier, and in subsequent commits; unshelving timeout.","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e87d20687449703e021188340081d8b7cdbce821","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"7ddc9221_22ab8920","updated":"2026-08-06 22:25:38.000000000","message":"recheck\n\nlots of network issues, post failures","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7ef63402c854cdf13894b2e8b2eccebadca3ad65","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0cb69485_160cf740","updated":"2026-08-07 20:57:04.000000000","message":"recheck\n\none of these times i\u0027ll get lucky","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"6d6b240a502ec76db8a782c3e1d9c40a3c624482","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e200832b_2fb7be8f","updated":"2026-08-07 18:30:33.000000000","message":"recheck\n\ntest_block_storage_cleanup timeout","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"460c41f330ae71fd4aa8560632f9f3e3e77f6869","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"a5697eca_315ad7be","updated":"2026-08-08 06:00:53.000000000","message":"recheck\n\nusual suspects at least","commit_id":"79a01e4df36da2f1753614e7c449c645b9c433f9"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8a0c5f337cc69722ce98909a574c86455662e8","unresolved":true,"context_lines":[{"line_number":4921,"context_line":"                            other \u003d objects.Instance.get_by_uuid("},{"line_number":4922,"context_line":"                                context, sm.instance_uuid,"},{"line_number":4923,"context_line":"                                expected_attrs\u003d[])"},{"line_number":4924,"context_line":"                            if other.host \u003d\u003d instance.host:"},{"line_number":4925,"context_line":"                                same_host_uuids.add("},{"line_number":4926,"context_line":"                                    sm.instance_uuid)"},{"line_number":4927,"context_line":"                        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":4,"id":"d60bbf8f_2045df31","line":4924,"updated":"2026-07-29 14:15:33.000000000","message":"To help me grok this, you\u0027re pulling out all the share mappings for this share, then for each one of those, looking up the instance for the mapping to see if it\u0027s on this host? That seems really expensive to me, as that\u0027s N+1 DB lookups (and thus round trips to conductor).\n\nIt seems to me like the best thing to do would be to create a `ShareMappingList.get_by_share_id_and_host()` which joins those two tables and gives you a list of just the share mappings in use on this (or any specific) host.\n\nSimpler, but more efficient than what you have here is to just call `self._get_instances_on_driver()` which will just fetch a list of all the instances we\u0027re running here in a single query, and then you can iterate the list as you do here. A larger single query, but a single query is almost always going to be less expensive than N queries.\n\nI dunno how many instances might be attached to a share, but I would expect most people use this _because_ it\u0027s very lightweight and they could have hundreds of instances sharing a common filesystem of reference material or something and this routine would get pretty expensive for something like that.","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d7b554c4410e60f47245f851ef341727fcb0610","unresolved":true,"context_lines":[{"line_number":4921,"context_line":"                            other \u003d objects.Instance.get_by_uuid("},{"line_number":4922,"context_line":"                                context, sm.instance_uuid,"},{"line_number":4923,"context_line":"                                expected_attrs\u003d[])"},{"line_number":4924,"context_line":"                            if other.host \u003d\u003d instance.host:"},{"line_number":4925,"context_line":"                                same_host_uuids.add("},{"line_number":4926,"context_line":"                                    sm.instance_uuid)"},{"line_number":4927,"context_line":"                        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":4,"id":"e095bab2_967ba5fe","line":4924,"in_reply_to":"26d3978d_0197b456","updated":"2026-07-31 20:46:27.000000000","message":"the nova conductor db design/MQ load is great perspective for me to learn; i\u0027ll attempt this follow up.","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d2213e928394b901bd562e50a1bb0a2dd3abf91","unresolved":true,"context_lines":[{"line_number":4921,"context_line":"                            other \u003d objects.Instance.get_by_uuid("},{"line_number":4922,"context_line":"                                context, sm.instance_uuid,"},{"line_number":4923,"context_line":"                                expected_attrs\u003d[])"},{"line_number":4924,"context_line":"                            if other.host \u003d\u003d instance.host:"},{"line_number":4925,"context_line":"                                same_host_uuids.add("},{"line_number":4926,"context_line":"                                    sm.instance_uuid)"},{"line_number":4927,"context_line":"                        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dd8143d5_eaffc5be","line":4924,"in_reply_to":"d60bbf8f_2045df31","updated":"2026-07-30 19:52:27.000000000","message":"ah, thanks for noting this; I dropped the \"get_by_uuid\" calls for each instance, and replaced that with a \"get me all instances matching this list of uuids\"; it sure is more efficient.. \n\n`_get_instances_on_driver()` is filtering by host, yes, but also has its own uuid listing going on.  \n\n`ShareMappingList.get_by_share_id_and_host()` could be useful; but it\u0027d be future proofing i think? because i\u0027m not yet sure if we need it anywhere else, introducing it would require me to think through the OVO migrations which might be overkill for this bug fix (which i intend to seek backports for). WDYT?","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a7d3ef7889273e1be65144d8a33c49048e55c7d9","unresolved":true,"context_lines":[{"line_number":4921,"context_line":"                            other \u003d objects.Instance.get_by_uuid("},{"line_number":4922,"context_line":"                                context, sm.instance_uuid,"},{"line_number":4923,"context_line":"                                expected_attrs\u003d[])"},{"line_number":4924,"context_line":"                            if other.host \u003d\u003d instance.host:"},{"line_number":4925,"context_line":"                                same_host_uuids.add("},{"line_number":4926,"context_line":"                                    sm.instance_uuid)"},{"line_number":4927,"context_line":"                        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":4,"id":"26d3978d_0197b456","line":4924,"in_reply_to":"dd8143d5_eaffc5be","updated":"2026-07-31 16:10:31.000000000","message":"\u003e ah, thanks for noting this; I dropped the \"get_by_uuid\" calls for each instance, and replaced that with a \"get me all instances matching this list of uuids\"; it sure is more efficient.. \n\nYes, a definite improvement.\n\n\u003e `_get_instances_on_driver()` is filtering by host, yes, but also has its own uuid listing going on.  \n\nRight, but..it\u0027s also filtering something you\u0027re filtering right after you query out all the instances for a given share...\n\n\u003e `ShareMappingList.get_by_share_id_and_host()` could be useful; but it\u0027d be future proofing i think? because i\u0027m not yet sure if we need it anywhere else, introducing it would require me to think through the OVO migrations which might be overkill for this bug fix (which i intend to seek backports for). WDYT?\n\nFuture proofing? I\u0027m not sure what you mean. It\u0027d be further optimizing what we\u0027re doing, and the fact that it\u0027s only used in one place is irrelevant. Those query methods are where we optimize server-side to limit the need for multiple queries, MQ round trips, data transferred over MQ and laborious post-processing. It doesn\u0027t matter how many places it\u0027d be used. I dunno how many instances could be using a single share on a large cloud, but I\u0027m guessing in the thousands for a read-only config store or similar? A thousand instance records is a lot of MQ data, especially when the goal is the throw away 90% of them a couple lines later.\n\nIf this really needs to be backported, then a remoteable object change is off the table anyway. It still seems like a good optimization going forward if we care about these scaling well and being lightweight.","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8a0c5f337cc69722ce98909a574c86455662e8","unresolved":true,"context_lines":[{"line_number":4929,"context_line":"                    share_mappings_used_by_share \u003d ["},{"line_number":4930,"context_line":"                        sm for sm in share_mappings_used_by_share"},{"line_number":4931,"context_line":"                        if sm.instance_uuid in same_host_uuids"},{"line_number":4932,"context_line":"                    ]"},{"line_number":4933,"context_line":""},{"line_number":4934,"context_line":"                return not all("},{"line_number":4935,"context_line":"                    ("}],"source_content_type":"text/x-python","patch_set":4,"id":"0f392fee_d64b452e","line":4932,"updated":"2026-07-29 14:15:33.000000000","message":"`compute/manager.py` is already way too long. This entire method adds substantially to it (especially with the black-inspired syntax that wastes so much vertical space) and is very very specific to share stuff. Can we not break most of this out to a separate `compute/share_management.py` or something? The fact that you\u0027re having to wrap everything so tightly in this new section is the 80-column limit doing its job and telling you that you\u0027re far (far) too deeply nested at this point. You\u0027re inside the *second* closure defined in an object method inside a class, inside a conditional inside a try..except inside a for loop inside a conditional.\n\nI know you\u0027re just trying to fix something, but some cleanup here would make this a lot easier to read and review.","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d2213e928394b901bd562e50a1bb0a2dd3abf91","unresolved":true,"context_lines":[{"line_number":4929,"context_line":"                    share_mappings_used_by_share \u003d ["},{"line_number":4930,"context_line":"                        sm for sm in share_mappings_used_by_share"},{"line_number":4931,"context_line":"                        if sm.instance_uuid in same_host_uuids"},{"line_number":4932,"context_line":"                    ]"},{"line_number":4933,"context_line":""},{"line_number":4934,"context_line":"                return not all("},{"line_number":4935,"context_line":"                    ("}],"source_content_type":"text/x-python","patch_set":4,"id":"89a53402_4f9ee106","line":4932,"in_reply_to":"0f392fee_d64b452e","updated":"2026-07-30 19:52:27.000000000","message":"Ack! i think it\u0027d improve this greatly.. i can propose this as a separate clean up change. It\u0027d help me contain the blast surface of this bug fix to just this stuff to help with backporting...","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a7d3ef7889273e1be65144d8a33c49048e55c7d9","unresolved":true,"context_lines":[{"line_number":4929,"context_line":"                    share_mappings_used_by_share \u003d ["},{"line_number":4930,"context_line":"                        sm for sm in share_mappings_used_by_share"},{"line_number":4931,"context_line":"                        if sm.instance_uuid in same_host_uuids"},{"line_number":4932,"context_line":"                    ]"},{"line_number":4933,"context_line":""},{"line_number":4934,"context_line":"                return not all("},{"line_number":4935,"context_line":"                    ("}],"source_content_type":"text/x-python","patch_set":4,"id":"e1fc4010_79105ffd","line":4932,"in_reply_to":"1d405d4a_c2eec4a1","updated":"2026-07-31 16:10:31.000000000","message":"Okay, I\u0027m not sure how important this backport is given it seems it was only noticed while you were doing additional work here. But either way, surely you can pull this out to L4892 with a little helper that just calculates `share_mappings_used_by_share` right?","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"466717590388fcd779d7c79d30dd16e4c5165e3c","unresolved":false,"context_lines":[{"line_number":4929,"context_line":"                    share_mappings_used_by_share \u003d ["},{"line_number":4930,"context_line":"                        sm for sm in share_mappings_used_by_share"},{"line_number":4931,"context_line":"                        if sm.instance_uuid in same_host_uuids"},{"line_number":4932,"context_line":"                    ]"},{"line_number":4933,"context_line":""},{"line_number":4934,"context_line":"                return not all("},{"line_number":4935,"context_line":"                    ("}],"source_content_type":"text/x-python","patch_set":4,"id":"8d3f5f9b_c327b2d6","line":4932,"in_reply_to":"3ccc7536_e410fd89","updated":"2026-07-31 20:57:55.000000000","message":"I moved the logic that computed `share_mappings_used_by_share` (`_check_share_usage`) to make the closures a bit more readable/maintainable and backport-friendly. I\u0027ll come back to this again in the extraction commit i\u0027m working on...","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"75197d61c77e3950fb59bc75b23fa9d7b8e3c81c","unresolved":true,"context_lines":[{"line_number":4929,"context_line":"                    share_mappings_used_by_share \u003d ["},{"line_number":4930,"context_line":"                        sm for sm in share_mappings_used_by_share"},{"line_number":4931,"context_line":"                        if sm.instance_uuid in same_host_uuids"},{"line_number":4932,"context_line":"                    ]"},{"line_number":4933,"context_line":""},{"line_number":4934,"context_line":"                return not all("},{"line_number":4935,"context_line":"                    ("}],"source_content_type":"text/x-python","patch_set":4,"id":"1d405d4a_c2eec4a1","line":4932,"in_reply_to":"89a53402_4f9ee106","updated":"2026-07-31 04:40:11.000000000","message":"I\u0027m doing this as a follow up.. https://review.opendev.org/c/openstack/nova/+/999349","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d7b554c4410e60f47245f851ef341727fcb0610","unresolved":true,"context_lines":[{"line_number":4929,"context_line":"                    share_mappings_used_by_share \u003d ["},{"line_number":4930,"context_line":"                        sm for sm in share_mappings_used_by_share"},{"line_number":4931,"context_line":"                        if sm.instance_uuid in same_host_uuids"},{"line_number":4932,"context_line":"                    ]"},{"line_number":4933,"context_line":""},{"line_number":4934,"context_line":"                return not all("},{"line_number":4935,"context_line":"                    ("}],"source_content_type":"text/x-python","patch_set":4,"id":"3ccc7536_e410fd89","line":4932,"in_reply_to":"e1fc4010_79105ffd","updated":"2026-07-31 20:46:27.000000000","message":"Yes, I discovered the issue while working on migration, but the bug can occur whenever multiple instances across different hosts attach to the same NFS\nshare; something that was possible since Epoxy. \n\nTheoretically, tests discovered this because we try to clean up the manila share and notice locks held by nova stop us. In real life, the remediation is that an admin can enumerate these orphaned locks and clean them up and help users delete their shares. Shares once created tend to live long, so it\u0027s possible no one has realized this is a problem yet :)","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3c8a0c5f337cc69722ce98909a574c86455662e8","unresolved":true,"context_lines":[{"line_number":4960,"context_line":""},{"line_number":4961,"context_line":"                share_mapping.set_access_according_to_protocol()"},{"line_number":4962,"context_line":""},{"line_number":4963,"context_line":"                still_used \u003d check_share_usage(context, instance.uuid)"},{"line_number":4964,"context_line":""},{"line_number":4965,"context_line":"                if not still_used:"},{"line_number":4966,"context_line":"                    # self.manila_api.unlock(share_mapping.share_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"a1fa3fb3_7c99cdfe","line":4963,"updated":"2026-07-29 14:15:33.000000000","message":"I assume this move is important so that `check_share_usage()` sees the post-change mappings? Might be good to put a comment here to that effect ot make sure it\u0027s never re-ordered again.","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9d2213e928394b901bd562e50a1bb0a2dd3abf91","unresolved":false,"context_lines":[{"line_number":4960,"context_line":""},{"line_number":4961,"context_line":"                share_mapping.set_access_according_to_protocol()"},{"line_number":4962,"context_line":""},{"line_number":4963,"context_line":"                still_used \u003d check_share_usage(context, instance.uuid)"},{"line_number":4964,"context_line":""},{"line_number":4965,"context_line":"                if not still_used:"},{"line_number":4966,"context_line":"                    # self.manila_api.unlock(share_mapping.share_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"59ef8a0a_f3cad0f3","line":4963,"in_reply_to":"a1fa3fb3_7c99cdfe","updated":"2026-07-30 19:52:27.000000000","message":"agreed; i added that comment","commit_id":"7921b22c1972810d97a4e1943dac1099fb7bdae5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0ef1c70d13e24b45c3e751fbbf2eae97b156993a","unresolved":true,"context_lines":[{"line_number":4939,"context_line":"                sm.status \u003d\u003d fields.ShareMappingStatus.DETACHING"},{"line_number":4940,"context_line":"            )"},{"line_number":4941,"context_line":"            for sm in share_mappings_used_by_share"},{"line_number":4942,"context_line":"        )"},{"line_number":4943,"context_line":""},{"line_number":4944,"context_line":"    @messaging.expected_exceptions(NotImplementedError)"},{"line_number":4945,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":6,"id":"fea7dc17_ed600800","line":4942,"updated":"2026-08-03 13:52:16.000000000","message":"Just reading this last condition again.. I think it\u0027s saying \"the share is still needed if none of _this_ instance\u0027s share mappings are INACTIVE,ERROR and none of any other instances are in DETACHING state\". Is that right?\n\nThis makes me wonder if (a) one share can be attached to the same instance multiple times (seems like that wouldn\u0027t make sense, but that\u0027s what you\u0027re looking for here?) and (b) do we really need the logic difference between this and other instances?\n\n...later...\n\nI just realized that you removed the \"logic explanation\" comment which would reinforce that this logic is what you want (although it doesn\u0027t explain why that logic makes sense). Why did you remove that comment?","commit_id":"5437738f0e594c542c1f6c181345774b3ffae70d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b2c8e4e4b90f06b2f210ccb3e5cdb09058a5a209","unresolved":false,"context_lines":[{"line_number":4939,"context_line":"                sm.status \u003d\u003d fields.ShareMappingStatus.DETACHING"},{"line_number":4940,"context_line":"            )"},{"line_number":4941,"context_line":"            for sm in share_mappings_used_by_share"},{"line_number":4942,"context_line":"        )"},{"line_number":4943,"context_line":""},{"line_number":4944,"context_line":"    @messaging.expected_exceptions(NotImplementedError)"},{"line_number":4945,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":6,"id":"03c38631_045ab46e","line":4942,"in_reply_to":"865b3f1b_eecbe153","updated":"2026-08-03 18:12:19.000000000","message":"I restored the comment, thanks for noting this!","commit_id":"5437738f0e594c542c1f6c181345774b3ffae70d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"c95a71d458e50e11e8870b412f5791d13fe57f98","unresolved":true,"context_lines":[{"line_number":4939,"context_line":"                sm.status \u003d\u003d fields.ShareMappingStatus.DETACHING"},{"line_number":4940,"context_line":"            )"},{"line_number":4941,"context_line":"            for sm in share_mappings_used_by_share"},{"line_number":4942,"context_line":"        )"},{"line_number":4943,"context_line":""},{"line_number":4944,"context_line":"    @messaging.expected_exceptions(NotImplementedError)"},{"line_number":4945,"context_line":"    @wrap_exception()"}],"source_content_type":"text/x-python","patch_set":6,"id":"865b3f1b_eecbe153","line":4942,"in_reply_to":"fea7dc17_ed600800","updated":"2026-08-03 17:41:56.000000000","message":"Hmm, you\u0027re right, it\u0027s worth keeping this for future maintainers.","commit_id":"5437738f0e594c542c1f6c181345774b3ffae70d"}]}