)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"72954b2b72c75990d5fb0faf5bbb58f046164f60","unresolved":false,"context_lines":[{"line_number":23,"context_line":"The API\u0027s @check_instance_state decorator already enforces"},{"line_number":24,"context_line":"task_state\u003dNone for attach_volume, so any concurrent request"},{"line_number":25,"context_line":"arriving while we hold the task_state will be rejected at the API"},{"line_number":26,"context_line":"layer before it reaches the conductor."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"After reserve_block_device_name returns, task_state is reset to"},{"line_number":29,"context_line":"None in a finally block. From that point, the BDM\u0027s existence"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"a11c4353_a492bc6a","line":26,"updated":"2026-09-24 06:52:34.000000000","message":"yepp","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"72954b2b72c75990d5fb0faf5bbb58f046164f60","unresolved":false,"context_lines":[{"line_number":29,"context_line":"None in a finally block. From that point, the BDM\u0027s existence"},{"line_number":30,"context_line":"prevents a duplicate attach of the same volume: both the API\u0027s"},{"line_number":31,"context_line":"_check_volume_already_attached and compute\u0027s do_reserve check for"},{"line_number":32,"context_line":"an existing BDM."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"This change only touches the conductor, which is expected to be"},{"line_number":35,"context_line":"upgraded atomically, so there is no upgrade compatibility concern"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fb216c91_6d004577","line":32,"updated":"2026-09-24 06:52:34.000000000","message":"yepp","commit_id":"9c961486b79703c6fd1b59001004238525911b86"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c694c1d7ba0818d2bd4e74520d548db232a308a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"086586eb_02c596dc","updated":"2026-09-23 08:33:01.000000000","message":"I will review in details but the functional test failures are relevant but probably not blocking we just needs to adapt them to the new task state","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"dad7908b431523dcb3deb108904863ce66dc798e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b6e007bd_2f1e4560","updated":"2026-09-24 13:27:49.000000000","message":"two small improvement request but I think this is matching what I suggested. Would be nice to have some functional reproducer of the overlap proving the fix but I won\u0027t block on that","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"cea53ca6fa9fc831320e2882db37bf36cf5d2099","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"42460981_4761d464","updated":"2026-09-29 09:26:39.000000000","message":"the functional tests failures are still relevant but fixable. I think they are failing as we are having a different task_state now in the notification sent.\n\nOther than the functional test failures I OK with the change.","commit_id":"e92e79d2cb9f65d732e85b65b62cf3965794df9d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b85bbcd8ed31b13ec4d3e9910617b7c8ea8c7e8f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"830ef8c0_4e5c55ab","updated":"2026-09-30 12:31:47.000000000","message":"thanks for the update. This made me realize that at least the .end notification change is unexpected. See inline","commit_id":"1d3499d67221d940dfed78aeb004db577a0bc084"}],"nova/conductor/manager.py":[{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"119223114191f00212e0a39700a183157d1e0cfc","unresolved":true,"context_lines":[{"line_number":2294,"context_line":"            raise exception.InstanceInvalidState("},{"line_number":2295,"context_line":"                instance_uuid\u003dinstance.uuid,"},{"line_number":2296,"context_line":"                attr\u003d\u0027task_state\u0027,"},{"line_number":2297,"context_line":"                state\u003dinstance.task_state,"},{"line_number":2298,"context_line":"                method\u003d\u0027attach_volume\u0027)"},{"line_number":2299,"context_line":""},{"line_number":2300,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b2855d50_aa700fa9","line":2297,"updated":"2026-09-25 12:50:35.000000000","message":"The state passed here is the local value we just set on line 2290. Shouldn\u0027t we report the actual database state instead, using something like state\u003de.kwargs.get(\u0027actual\u0027, instance.task_state)?","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ca248f06db20d70965978a9d6bbb85cedb4a5a2f","unresolved":false,"context_lines":[{"line_number":2294,"context_line":"            raise exception.InstanceInvalidState("},{"line_number":2295,"context_line":"                instance_uuid\u003dinstance.uuid,"},{"line_number":2296,"context_line":"                attr\u003d\u0027task_state\u0027,"},{"line_number":2297,"context_line":"                state\u003dinstance.task_state,"},{"line_number":2298,"context_line":"                method\u003d\u0027attach_volume\u0027)"},{"line_number":2299,"context_line":""},{"line_number":2300,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"68e25c8a_f30de21b","line":2297,"in_reply_to":"b2855d50_aa700fa9","updated":"2026-09-28 13:19:47.000000000","message":"Done","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"72954b2b72c75990d5fb0faf5bbb58f046164f60","unresolved":true,"context_lines":[{"line_number":2296,"context_line":"                attr\u003d\u0027task_state\u0027,"},{"line_number":2297,"context_line":"                state\u003dinstance.task_state,"},{"line_number":2298,"context_line":"                method\u003d\u0027attach_volume\u0027)"},{"line_number":2299,"context_line":""},{"line_number":2300,"context_line":"        try:"},{"line_number":2301,"context_line":"            try:"},{"line_number":2302,"context_line":"                volume_bdm \u003d self._create_volume_bdm("},{"line_number":2303,"context_line":"                    context, instance, device, volume, disk_bus\u003ddisk_bus,"},{"line_number":2304,"context_line":"                    device_type\u003ddevice_type, tag\u003dtag,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5bb12c94_d2e6f9fd","line":2301,"range":{"start_line":2299,"start_character":0,"end_line":2301,"end_character":16},"updated":"2026-09-24 06:52:34.000000000","message":"can we pull out a helper function to avoid this double nesting in a single function?","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ca248f06db20d70965978a9d6bbb85cedb4a5a2f","unresolved":false,"context_lines":[{"line_number":2296,"context_line":"                attr\u003d\u0027task_state\u0027,"},{"line_number":2297,"context_line":"                state\u003dinstance.task_state,"},{"line_number":2298,"context_line":"                method\u003d\u0027attach_volume\u0027)"},{"line_number":2299,"context_line":""},{"line_number":2300,"context_line":"        try:"},{"line_number":2301,"context_line":"            try:"},{"line_number":2302,"context_line":"                volume_bdm \u003d self._create_volume_bdm("},{"line_number":2303,"context_line":"                    context, instance, device, volume, disk_bus\u003ddisk_bus,"},{"line_number":2304,"context_line":"                    device_type\u003ddevice_type, tag\u003dtag,"}],"source_content_type":"text/x-python","patch_set":1,"id":"c25589df_eaa8975c","line":2301,"range":{"start_line":2299,"start_character":0,"end_line":2301,"end_character":16},"in_reply_to":"5bb12c94_d2e6f9fd","updated":"2026-09-28 13:19:47.000000000","message":"Done","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"72954b2b72c75990d5fb0faf5bbb58f046164f60","unresolved":true,"context_lines":[{"line_number":2362,"context_line":"            try:"},{"line_number":2363,"context_line":"                instance.task_state \u003d None"},{"line_number":2364,"context_line":"                instance.save("},{"line_number":2365,"context_line":"                    expected_task_state\u003d[task_states.BLOCK_DEVICE_MAPPING])"},{"line_number":2366,"context_line":"            except exception.UnexpectedTaskStateError:"},{"line_number":2367,"context_line":"                LOG.warning("},{"line_number":2368,"context_line":"                    \"Could not reset task_state after attach_volume \""}],"source_content_type":"text/x-python","patch_set":1,"id":"4d5c2bea_43a73a5e","line":2365,"updated":"2026-09-24 06:52:34.000000000","message":"if the first instance.save failed to set it to BLOCK_DEVICE_MAPPING then this save will fail too. I think we can relax the condition to allow this save from both Non and BLOCK_DEVICE_MAPPING task_states.","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ca248f06db20d70965978a9d6bbb85cedb4a5a2f","unresolved":false,"context_lines":[{"line_number":2362,"context_line":"            try:"},{"line_number":2363,"context_line":"                instance.task_state \u003d None"},{"line_number":2364,"context_line":"                instance.save("},{"line_number":2365,"context_line":"                    expected_task_state\u003d[task_states.BLOCK_DEVICE_MAPPING])"},{"line_number":2366,"context_line":"            except exception.UnexpectedTaskStateError:"},{"line_number":2367,"context_line":"                LOG.warning("},{"line_number":2368,"context_line":"                    \"Could not reset task_state after attach_volume \""}],"source_content_type":"text/x-python","patch_set":1,"id":"2e4177d0_5b698afb","line":2365,"in_reply_to":"4d5c2bea_43a73a5e","updated":"2026-09-28 13:19:47.000000000","message":"Done","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"119223114191f00212e0a39700a183157d1e0cfc","unresolved":true,"context_lines":[{"line_number":2366,"context_line":"            except exception.UnexpectedTaskStateError:"},{"line_number":2367,"context_line":"                LOG.warning("},{"line_number":2368,"context_line":"                    \"Could not reset task_state after attach_volume \""},{"line_number":2369,"context_line":"                    \"for instance %s; another operation may have \""},{"line_number":2370,"context_line":"                    \"changed it.\", instance.uuid)"},{"line_number":2371,"context_line":""},{"line_number":2372,"context_line":"        return volume_bdm.device_name"}],"source_content_type":"text/x-python","patch_set":1,"id":"302f941d_2b3943a2","line":2369,"updated":"2026-09-25 12:50:35.000000000","message":"nit: IMO, this could be more informative, e.g.:LOG.warning(\n    \"Could not reset task_state (currently %(ts)s) after \"\n    \"attach_volume for instance %(inst)s; another operation \"\n    \"may have changed it.\",\n    {\u0027ts\u0027: instance.task_state, \u0027inst\u0027: instance.uuid})","commit_id":"9c961486b79703c6fd1b59001004238525911b86"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ca248f06db20d70965978a9d6bbb85cedb4a5a2f","unresolved":false,"context_lines":[{"line_number":2366,"context_line":"            except exception.UnexpectedTaskStateError:"},{"line_number":2367,"context_line":"                LOG.warning("},{"line_number":2368,"context_line":"                    \"Could not reset task_state after attach_volume \""},{"line_number":2369,"context_line":"                    \"for instance %s; another operation may have \""},{"line_number":2370,"context_line":"                    \"changed it.\", instance.uuid)"},{"line_number":2371,"context_line":""},{"line_number":2372,"context_line":"        return volume_bdm.device_name"}],"source_content_type":"text/x-python","patch_set":1,"id":"8bb024ca_09e072c8","line":2369,"in_reply_to":"302f941d_2b3943a2","updated":"2026-09-28 13:19:47.000000000","message":"Done","commit_id":"9c961486b79703c6fd1b59001004238525911b86"}],"nova/tests/functional/notification_sample_tests/test_instance.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b85bbcd8ed31b13ec4d3e9910617b7c8ea8c7e8f","unresolved":true,"context_lines":[{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        Conductor sets task_state to BLOCK_DEVICE_MAPPING around"},{"line_number":392,"context_line":"        reserve_block_device_name. Functional tests run the"},{"line_number":393,"context_line":"        attach_volume CAST in-process, so compute still sees that"},{"line_number":394,"context_line":"        task_state when it emits volume_attach start/end/error."},{"line_number":395,"context_line":"        \"\"\""},{"line_number":396,"context_line":"        replacements \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"a4238ae6_255ff220","line":393,"updated":"2026-09-30 12:31:47.000000000","message":"are we? I don\u0027t see CAST_AS_CALL in effect in these tests. I think what happens is instead that we send the CAST to the compute with the instance.task_state is still having block_device_mapping set. Then after the CAST sent we revert that to None in the DB. But the RPC carries the serialized instance object with the old value. It is not refreshed on the compute side so the compute always sees the old value even if in the meantime conductor reverted.\n\nIf conductor would revert the task_state to None *before* sending the attach_volume CAST then the compute would get the same instance object task_state \u003d None as before this patch. It seem that right now compute does not care that we changed the task_state value precondition from None to block_device_mapping of attach_volume. The only visible effect is that both .start and .end notifications sending now task_state \u003d block_device_mapping instead of None. The .start would be OK to change in my mind that is a more correct value there. But the .end should not carry an active task state, especially here that we know that such task_state already reset by the conductor in the DB. \nOptions:\n1. We could change the compute manager to refresh the instance from the DB before .end sent. This will mean a timeperiod during upgrade that .end is sent with block_device_mapping from not updated computes. I can live with that.\n2. We could move the task_state reset to None to happen *before* the attach_volume CAST. This would not affect update.\n\nGiven that we have options to fix this I would suggest to try to fix this. For me #2 is cleaner from external behavior perspective as notifications does not change at all.","commit_id":"1d3499d67221d940dfed78aeb004db577a0bc084"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b85bbcd8ed31b13ec4d3e9910617b7c8ea8c7e8f","unresolved":true,"context_lines":[{"line_number":396,"context_line":"        replacements \u003d {"},{"line_number":397,"context_line":"            \u0027reservation_id\u0027: server[\u0027reservation_id\u0027],"},{"line_number":398,"context_line":"            \u0027uuid\u0027: server[\u0027id\u0027],"},{"line_number":399,"context_line":"            \u0027task_state\u0027: \u0027block_device_mapping\u0027,"},{"line_number":400,"context_line":"        }"},{"line_number":401,"context_line":"        replacements.update(extra)"},{"line_number":402,"context_line":"        return replacements"}],"source_content_type":"text/x-python","patch_set":3,"id":"4b03043a_77ba7299","line":399,"updated":"2026-09-30 12:31:47.000000000","message":"we should not use replacement for static value. Please modify the sample file to show this value.\n\nE.g.\n```\ndiff --git a/doc/notification_samples/instance-volume_attach-start.json b/doc/notification_samples/instance-volume_attach-start.json\nindex 9fff9d2e56..f5d6ba601f 100644\n--- a/doc/notification_samples/instance-volume_attach-start.json\n+++ b/doc/notification_samples/instance-volume_attach-start.json\n@@ -1,7 +1,10 @@\n {\n     \"event_type\": \"instance.volume_attach.start\",\n     \"payload\": {\n-        \"$ref\": \"common_payloads/InstanceActionVolumePayload.json#\"\n+        \"$ref\": \"common_payloads/InstanceActionVolumePayload.json#\",\n+        \"nova_object.data\": {\n+            \"task_state\": \"block_device_mapping\"\n+        }\n     },\n     \"priority\": \"INFO\",\n     \"publisher_id\": \"nova-compute:compute\"\n```\n\n//later\nor if you go with #2 then this will not be needed.","commit_id":"1d3499d67221d940dfed78aeb004db577a0bc084"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":11082,"name":"Kamil Sambor","email":"ksambor@redhat.com","username":"ksambor"},"change_message_id":"119223114191f00212e0a39700a183157d1e0cfc","unresolved":true,"context_lines":[{"line_number":4898,"context_line":"                              self.context, instance, volume,"},{"line_number":4899,"context_line":"                              None, None, None)"},{"line_number":4900,"context_line":""},{"line_number":4901,"context_line":"        # task_state must have been reset to None in the finally block."},{"line_number":4902,"context_line":"        self.assertIsNone(instance.task_state)"},{"line_number":4903,"context_line":""},{"line_number":4904,"context_line":"    @mock.patch.object(compute_rpcapi.ComputeAPI, \u0027reserve_block_device_name\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5caab628_0430181d","line":4901,"updated":"2026-09-25 12:50:35.000000000","message":"Since save() is mocked here, shouldn\u0027t we use a similar assertion pattern as above (e.g., self.assertEqual([None], save_))? IMO using this pattern would make the test more robust","commit_id":"9c961486b79703c6fd1b59001004238525911b86"}]}