)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"50fe67f600984ab9c7d6844927ce44660dc3760f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"462c2b82_d4688590","updated":"2025-09-24 20:24:55.000000000","message":"Code LGTM; see analysis inline.  Minor nit about the test, but it\u0027s a nit.","commit_id":"0d486da2f6983e130ab214493e5f522a81d80821"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"64ede2b18a023f0a5a624c49c16cad9e08d4a0f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"025dbb5a_383a7e6b","updated":"2025-09-25 13:26:29.000000000","message":"Thanks for the analysis Brian, looks good to me.","commit_id":"0d486da2f6983e130ab214493e5f522a81d80821"}],"cinder/tests/unit/volume/flows/test_create_volume_flow.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"50fe67f600984ab9c7d6844927ce44660dc3760f","unresolved":true,"context_lines":[{"line_number":1505,"context_line":"        fake_driver \u003d mock.MagicMock()"},{"line_number":1506,"context_line":"        fake_driver.capabilities \u003d {}"},{"line_number":1507,"context_line":"        if clone_across_pools:"},{"line_number":1508,"context_line":"            fake_driver.capabilities[\u0027clone_across_pools\u0027] \u003d ("},{"line_number":1509,"context_line":"                clone_across_pools)"},{"line_number":1510,"context_line":"        fake_manager \u003d create_volume_manager.CreateVolumeFromSpecTask("},{"line_number":1511,"context_line":"            mock.MagicMock(), fake_db, fake_driver)"},{"line_number":1512,"context_line":"        fake_image_service \u003d fake_image.FakeImageService()"}],"source_content_type":"text/x-python","patch_set":3,"id":"2d597c68_5be9a9e8","line":1509,"range":{"start_line":1508,"start_character":61,"end_line":1509,"end_character":35},"updated":"2025-09-24 20:24:55.000000000","message":"nit: could just set this to True since you\u0027re checking at line 1507.  Alternatively, you could leave this as is and get rid of line 1507. Not sure which would be better.","commit_id":"0d486da2f6983e130ab214493e5f522a81d80821"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c6d23ceaad0ba2cb2c57687730c7d6b6f1bcb05b","unresolved":true,"context_lines":[{"line_number":1505,"context_line":"        fake_driver \u003d mock.MagicMock()"},{"line_number":1506,"context_line":"        fake_driver.capabilities \u003d {}"},{"line_number":1507,"context_line":"        if clone_across_pools:"},{"line_number":1508,"context_line":"            fake_driver.capabilities[\u0027clone_across_pools\u0027] \u003d ("},{"line_number":1509,"context_line":"                clone_across_pools)"},{"line_number":1510,"context_line":"        fake_manager \u003d create_volume_manager.CreateVolumeFromSpecTask("},{"line_number":1511,"context_line":"            mock.MagicMock(), fake_db, fake_driver)"},{"line_number":1512,"context_line":"        fake_image_service \u003d fake_image.FakeImageService()"}],"source_content_type":"text/x-python","patch_set":3,"id":"c9628270_e9d613a7","line":1509,"range":{"start_line":1508,"start_character":61,"end_line":1509,"end_character":35},"in_reply_to":"2d597c68_5be9a9e8","updated":"2025-09-28 07:28:11.000000000","message":"I completely missed this.\nSo we can\u0027t get rid of L#1507 since currently not all drivers report it and the ones that do report it as True. If we remove L#1507, we will simulate the behavior of drivers returning this as a False value (which will make sense if we implement this as a common capability among drivers) but currently that\u0027s not correct.\nThe right thing here would\u0027ve been to set this to True as you already mentioned which i missed updating before this merged.","commit_id":"0d486da2f6983e130ab214493e5f522a81d80821"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"50fe67f600984ab9c7d6844927ce44660dc3760f","unresolved":true,"context_lines":[{"line_number":749,"context_line":"            else:"},{"line_number":750,"context_line":"                filters[\u0027host\u0027] \u003d volume.host"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"        image_volumes \u003d self.db.volume_get_all(context, filters\u003dfilters)"},{"line_number":753,"context_line":""},{"line_number":754,"context_line":"        for image_volume in image_volumes:"},{"line_number":755,"context_line":"            # For the case image volume is stored in the service tenant,"}],"source_content_type":"text/x-python","patch_set":3,"id":"f1031aa6_003da17b","line":752,"updated":"2025-09-24 20:24:55.000000000","message":"Here\u0027s the history of this part of the code.\n\n1. Change-Id: I2cb68749f194d0cd597b7258a317afb982236aea (Liberty, original change)\n\"Efficient image transfer for Glance cinder store\"\n```\n        image_volumes \u003d self.db.volume_get_all_by_host(\n            context, volume[\u0027host\u0027], filters\u003d{\u0027id\u0027: image_volume_ids})\n```\n2. Change-Id: Idbac4dae00f9fa03fb14547a204666721bf67d96 (Dalmatian)\n\"Add the clone_across_pools driver capability\"\n```\n        if self.driver.capabilities.get(\u0027clone_across_pools\u0027):\n            image_volumes \u003d self.db.volume_get_all(\n                context, filters\u003d{\u0027id\u0027: image_volume_ids})\n        else:\n            image_volumes \u003d self.db.volume_get_all_by_host(\n                context, volume[\u0027host\u0027], filters\u003d{\u0027id\u0027: image_volume_ids})\n```\n3. Change-Id: Ic0abdbb0cf3e0cad36015fa5545ba6acce18b81a (Flamingo)\n\"Fix volume clone across cluster members\"\n(meant to address a limitation of the Liberty change)\n```\n        filters \u003d {\u0027id\u0027: image_volume_ids}\n        if volume.cluster_name:\n            filters[\u0027cluster_name\u0027] \u003d volume.cluster_name\n        else:\n            filters[\u0027host\u0027] \u003d volume.host\n\n        if self.driver.capabilities.get(\u0027clone_across_pools\u0027):\n            # BUG: Dalmatian change only used the \u0027id\u0027 filter, now\n            # we have an additional cluster_name or host filter that\n            # we don\u0027t want\n            image_volumes \u003d self.db.volume_get_all(\n                context, filters\u003dfilters)\n        else:\n            # ANOTHER BUG? If the driver doesn\u0027t support clone_across_pools\n            # but we\u0027re in A/A (different hosts but same cluster), we\u0027re\n            # going to pass the cluster_name filter, but are also now going\n            # to filter by host (which we don\u0027t want ... that was the point\n            # of this patch)\n            image_volumes \u003d self.db.volume_get_all_by_host(\n                context, volume[\u0027host\u0027], filters\u003dfilters)\n```\n4. Change-Id: I49a00636658318622b5ede2717757cf83076c1fc (this patch)\n```\n        filters \u003d {\u0027id\u0027: image_volume_ids}\n        # Only add host/cluster filter if cross pool cloning is not supported\n        if not self.driver.capabilities.get(\u0027clone_across_pools\u0027):\n            if volume.cluster_name:\n                filters[\u0027cluster_name\u0027] \u003d volume.cluster_name\n            else:\n                filters[\u0027host\u0027] \u003d volume.host\n\n        image_volumes \u003d self.db.volume_get_all(context, filters\u003dfilters)\n```\nOK, so it looks to me like Rajat\u0027s patch (#4) fixes both problems introduced\nby #3 (which I had +2\u0027d by the way, which is why I wanted to look at this carefully).","commit_id":"0d486da2f6983e130ab214493e5f522a81d80821"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6e7103d2967cdfd662d36f16108f5e7f79a58258","unresolved":false,"context_lines":[{"line_number":749,"context_line":"            else:"},{"line_number":750,"context_line":"                filters[\u0027host\u0027] \u003d volume.host"},{"line_number":751,"context_line":""},{"line_number":752,"context_line":"        image_volumes \u003d self.db.volume_get_all(context, filters\u003dfilters)"},{"line_number":753,"context_line":""},{"line_number":754,"context_line":"        for image_volume in image_volumes:"},{"line_number":755,"context_line":"            # For the case image volume is stored in the service tenant,"}],"source_content_type":"text/x-python","patch_set":3,"id":"592b070a_e236895e","line":752,"in_reply_to":"f1031aa6_003da17b","updated":"2025-09-25 13:52:51.000000000","message":"Thanks Brian for the detailed analysis, i didn\u0027t realize the history that originally it worked as expected but change #3 introduced the regression.","commit_id":"0d486da2f6983e130ab214493e5f522a81d80821"}]}