)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"29b988b6dce95b2c9005bf0ba4b6b4b5b31b8150","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1ede7b5c_190e7e93","updated":"2022-07-29 18:24:31.000000000","message":"I have a suggestion for simplification inline.","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"119e1f9c12da7e092854b909ac3a19b782fd2849","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"633f3328_d3dba4ac","updated":"2023-10-18 14:04:52.000000000","message":"Still reading up the stack...","commit_id":"11710cd1c43d865ba5df7e02e62ea30595500aea"}],"nova/tests/functional/regressions/test_bug_1982497.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e9e8f8cce6276d00e11de3364c09e7e7acdf1f94","unresolved":true,"context_lines":[{"line_number":18,"context_line":"from nova.tests.functional.libvirt import base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestLiveMigrateRollbackAtDestRaceDropClaim("},{"line_number":22,"context_line":"    base.ServersTestBase, integrated_helpers.InstanceHelperMixin"},{"line_number":23,"context_line":"):"},{"line_number":24,"context_line":"    api_major_version \u003d \u0027v2.1\u0027"},{"line_number":25,"context_line":"    microversion \u003d \u0027latest\u0027"},{"line_number":26,"context_line":"    ADDITIONAL_FILTERS \u003d [\u0027NUMATopologyFilter\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"e97e0ffa_627ff65c","line":23,"range":{"start_line":21,"start_character":0,"end_line":23,"end_character":2},"updated":"2022-07-22 11:41:16.000000000","message":"if we intend to allow/do this more often going forward we should also update\nteh readmen for the regression folder.\n\nwhen libvirt is requried it make more sense then duplicating all the xtra fixutre that requires to make it work","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f1083179f62371b71fc881e1dedf60143a7c2dae","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from nova.tests.functional.libvirt import base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestLiveMigrateRollbackAtDestRaceDropClaim("},{"line_number":22,"context_line":"    base.ServersTestBase, integrated_helpers.InstanceHelperMixin"},{"line_number":23,"context_line":"):"},{"line_number":24,"context_line":"    api_major_version \u003d \u0027v2.1\u0027"},{"line_number":25,"context_line":"    microversion \u003d \u0027latest\u0027"},{"line_number":26,"context_line":"    ADDITIONAL_FILTERS \u003d [\u0027NUMATopologyFilter\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"17b446d9_a7cc164d","line":23,"range":{"start_line":21,"start_character":0,"end_line":23,"end_character":2},"in_reply_to":"e97e0ffa_627ff65c","updated":"2022-08-02 09:37:15.000000000","message":"Ack","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b0e674d2265fda6292cef1d3ac5d1bcad8ed2035","unresolved":true,"context_lines":[{"line_number":26,"context_line":"    ADDITIONAL_FILTERS \u003d [\u0027NUMATopologyFilter\u0027]"},{"line_number":27,"context_line":"    ADMIN_API \u003d True"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # This is important as we need drop_move_claim_at_destination to act as a"},{"line_number":30,"context_line":"    # cast, so it can be run asynchronously from the rest of the rollback flow"},{"line_number":31,"context_line":"    # to reproduce the race condition"},{"line_number":32,"context_line":"    CAST_AS_CALL \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c818d44_020c4305","line":29,"range":{"start_line":29,"start_character":35,"end_line":29,"end_character":65},"updated":"2022-07-22 11:31:09.000000000","message":"rollback_live_migration_at_destination","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f1083179f62371b71fc881e1dedf60143a7c2dae","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    ADDITIONAL_FILTERS \u003d [\u0027NUMATopologyFilter\u0027]"},{"line_number":27,"context_line":"    ADMIN_API \u003d True"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # This is important as we need drop_move_claim_at_destination to act as a"},{"line_number":30,"context_line":"    # cast, so it can be run asynchronously from the rest of the rollback flow"},{"line_number":31,"context_line":"    # to reproduce the race condition"},{"line_number":32,"context_line":"    CAST_AS_CALL \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"c12cba17_aafd2b24","line":29,"range":{"start_line":29,"start_character":35,"end_line":29,"end_character":65},"in_reply_to":"3c818d44_020c4305","updated":"2022-08-02 09:37:15.000000000","message":"Done","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b0e674d2265fda6292cef1d3ac5d1bcad8ed2035","unresolved":true,"context_lines":[{"line_number":98,"context_line":"           5.4) drop_move_claim_at_destination runs on dest"},{"line_number":99,"context_line":"           5.5) rollback_live_migration_at_destination runs on dest"},{"line_number":100,"context_line":"        6) Assert that the live migration failed"},{"line_number":101,"context_line":"        7) Try to delete the instance and observe that is fails as it tries to"},{"line_number":102,"context_line":"           unpin the 2,3 CPUs that was never pinned on the source"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        Note that the order of the triggering of"}],"source_content_type":"text/x-python","patch_set":1,"id":"b7f6cf79_ff6c2861","line":101,"range":{"start_line":101,"start_character":55,"end_line":101,"end_character":57},"updated":"2022-07-22 11:31:09.000000000","message":"nit: it","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f1083179f62371b71fc881e1dedf60143a7c2dae","unresolved":false,"context_lines":[{"line_number":98,"context_line":"           5.4) drop_move_claim_at_destination runs on dest"},{"line_number":99,"context_line":"           5.5) rollback_live_migration_at_destination runs on dest"},{"line_number":100,"context_line":"        6) Assert that the live migration failed"},{"line_number":101,"context_line":"        7) Try to delete the instance and observe that is fails as it tries to"},{"line_number":102,"context_line":"           unpin the 2,3 CPUs that was never pinned on the source"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        Note that the order of the triggering of"}],"source_content_type":"text/x-python","patch_set":1,"id":"58ee7e1b_16214a5c","line":101,"range":{"start_line":101,"start_character":55,"end_line":101,"end_character":57},"in_reply_to":"b7f6cf79_ff6c2861","updated":"2022-08-02 09:37:15.000000000","message":"Done","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"29b988b6dce95b2c9005bf0ba4b6b4b5b31b8150","unresolved":true,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        host_b \u003d self.computes[\u0027host_b\u0027]"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        drop_move_claim_done \u003d threading.Condition()"},{"line_number":135,"context_line":"        orig_drop_move_claim_at_destination \u003d ("},{"line_number":136,"context_line":"            host_b.manager.drop_move_claim_at_destination)"},{"line_number":137,"context_line":"        self.drop_move_claim_has_run \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"0c4d5dd9_2a68936f","line":134,"updated":"2022-07-29 18:24:31.000000000","message":"Could we not replace the two Condition()s with a single lock? I\u0027m thinking something similar to what Alexey did in https://review.opendev.org/c/openstack/nova/+/845748/3/nova/tests/functional/libvirt/test_live_migration.py\n\n1. Declare self.lock \u003d threading.lock()\n2. Acquire it at the beginning of the test.\n3. wrapped_rollback_live_migration_at_destination() waits on the lock before calling orig_rollback_live_migration_at_destination().\n4. wrapped_drop_move_claim_at_dest() releases the lock after calling orig_drop_move_claim_at_dest.\n\nIf I\u0027ve understood everything correctly, it would create the sequence that we need - rollback_live_migration_at_destination() running after drop_move_claim_at_destination() with a single (and to my mind, easier to read and understand) Lock().","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f1083179f62371b71fc881e1dedf60143a7c2dae","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        host_b \u003d self.computes[\u0027host_b\u0027]"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"        drop_move_claim_done \u003d threading.Condition()"},{"line_number":135,"context_line":"        orig_drop_move_claim_at_destination \u003d ("},{"line_number":136,"context_line":"            host_b.manager.drop_move_claim_at_destination)"},{"line_number":137,"context_line":"        self.drop_move_claim_has_run \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"e7c538e4_7e2a4e82","line":134,"in_reply_to":"0c4d5dd9_2a68936f","updated":"2022-08-02 09:37:15.000000000","message":"In general a condition can be implemented with a lock (and some state guarded by the lock). You are right as in this test we can use the assumption that the test code runs first before any of the parallel action can happen so the test code can take the lock. I think we need two locks as we need to serialize 3 events:\n* drop_move_claim_at_dest\n* rollback_live_migration_at_destination\n* delete\n\nI changed the conditions to locks and that allowed to remove of the extra flags.","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b0e674d2265fda6292cef1d3ac5d1bcad8ed2035","unresolved":true,"context_lines":[{"line_number":178,"context_line":"                lambda: self.rollback_live_migration_at_destination_run)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        self.api.delete_server(self.server_a[\u0027id\u0027])"},{"line_number":181,"context_line":"        # This is bug 1982497 where nova tries to"},{"line_number":182,"context_line":"        self._wait_for_action_fail_completion("},{"line_number":183,"context_line":"            self.server_a, \"delete\", \"compute_terminate_instance\")"},{"line_number":184,"context_line":"        server \u003d self.api.get_server(self.server_a[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"c6925af5_13ab7f28","line":181,"range":{"start_line":181,"start_character":47,"end_line":181,"end_character":49},"updated":"2022-07-22 11:31:09.000000000","message":"to ...","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f1083179f62371b71fc881e1dedf60143a7c2dae","unresolved":false,"context_lines":[{"line_number":178,"context_line":"                lambda: self.rollback_live_migration_at_destination_run)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        self.api.delete_server(self.server_a[\u0027id\u0027])"},{"line_number":181,"context_line":"        # This is bug 1982497 where nova tries to"},{"line_number":182,"context_line":"        self._wait_for_action_fail_completion("},{"line_number":183,"context_line":"            self.server_a, \"delete\", \"compute_terminate_instance\")"},{"line_number":184,"context_line":"        server \u003d self.api.get_server(self.server_a[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"d560f8f7_472bcafa","line":181,"range":{"start_line":181,"start_character":47,"end_line":181,"end_character":49},"in_reply_to":"c6925af5_13ab7f28","updated":"2022-08-02 09:37:15.000000000","message":"Done","commit_id":"cf4caef35b5e9472af73f85ed86436deaa3ae47d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"119e1f9c12da7e092854b909ac3a19b782fd2849","unresolved":true,"context_lines":[{"line_number":183,"context_line":"            server[\"fault\"][\"message\"],"},{"line_number":184,"context_line":"            \"CPU set to unpin [2, 3] must be a subset of pinned CPU set \""},{"line_number":185,"context_line":"            \"[0, 1]\","},{"line_number":186,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":3,"id":"545e2f47_fc2d6a9d","line":186,"updated":"2023-10-18 14:04:52.000000000","message":"The rest of the style you\u0027re adding here matches most of the rest of the codebase, in that there are no trailing parens on lines by themselves, wasting vertical space. Starting on L164 you switched to that, which looks weird.","commit_id":"11710cd1c43d865ba5df7e02e62ea30595500aea"}]}