)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"99bb8232d55e4ebfb1b41fb017220a05c6a69120","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The related bug https://bugs.launchpad.net/nova/+bug/1735407."},{"line_number":10,"context_line":"Evacuation does not respect anti-affinity rules."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fixed in related patch https://review.openstack.org/#/c/649953/"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I136b2a8a38c36ea9717d592c748d8758f15d6d28"},{"line_number":15,"context_line":"Closes-Bug: 1763181"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_305c024a","line":12,"updated":"2019-04-08 16:07:56.000000000","message":"You should just squash the changes into a single patch.","commit_id":"6332d698982f1315b3868a6fceff8daae102b121"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"4bead81abcaefffba5b4a970442b07796e9d0543","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make evacuation respects anti-affinity rule"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The change I75261 has added the late group check for evacuation."},{"line_number":10,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":11,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":12,"context_line":"which are in-migration will be migrated to the host."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3fce034c_affed802","line":9,"range":{"start_line":9,"start_character":11,"end_line":9,"end_character":17},"updated":"2019-04-12 06:58:21.000000000","message":"sorry, this one is unclickable, so don\u0027t know what it leads to","commit_id":"179eb9851763f7669956b8723d60e97b54b28dc3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The change I752617066bb2167b49239ab9d17b0c89754a3e12 has added the late"},{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_30d36a03","line":11,"range":{"start_line":11,"start_character":49,"end_line":11,"end_character":65},"updated":"2019-07-01 09:30:05.000000000","message":"only checks","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The change I752617066bb2167b49239ab9d17b0c89754a3e12 has added the late"},{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_96e1dfd2","line":11,"range":{"start_line":11,"start_character":49,"end_line":11,"end_character":65},"in_reply_to":"9fb8cfa7_30d36a03","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The change I752617066bb2167b49239ab9d17b0c89754a3e12 has added the late"},{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_90fd766f","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":52},"updated":"2019-07-01 09:30:05.000000000","message":"does not check","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The change I752617066bb2167b49239ab9d17b0c89754a3e12 has added the late"},{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_f00ab287","line":12,"range":{"start_line":12,"start_character":28,"end_line":12,"end_character":35},"updated":"2019-07-01 09:30:05.000000000","message":"on the host","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The change I752617066bb2167b49239ab9d17b0c89754a3e12 has added the late"},{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_36e4b3df","line":12,"range":{"start_line":12,"start_character":39,"end_line":12,"end_character":52},"in_reply_to":"9fb8cfa7_90fd766f","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":9,"context_line":"The change I752617066bb2167b49239ab9d17b0c89754a3e12 has added the late"},{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_56e7e7e3","line":12,"range":{"start_line":12,"start_character":28,"end_line":12,"end_character":35},"in_reply_to":"9fb8cfa7_f00ab287","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"},{"line_number":16,"context_line":"other instances will be migrated to the same host/node."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_b000ba67","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":52},"updated":"2019-07-01 09:30:05.000000000","message":"which are being migrated to the host.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":10,"context_line":"group check for evacuation."},{"line_number":11,"context_line":"But the function _validate_instance_group_policy is only to check"},{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"},{"line_number":16,"context_line":"other instances will be migrated to the same host/node."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_f6d13bfc","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":52},"in_reply_to":"9fb8cfa7_b000ba67","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"},{"line_number":16,"context_line":"other instances will be migrated to the same host/node."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Use the functional test test_parallel_evacuate_with_server_group now."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_f02f12f3","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":12},"updated":"2019-07-01 09:30:05.000000000","message":"So this patch adds","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the instances which are now on host but not to check the instances"},{"line_number":13,"context_line":"which are in-migration will be migrated to the host."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So that add the instance_uuids from migration table to check whether"},{"line_number":16,"context_line":"other instances will be migrated to the same host/node."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Use the functional test test_parallel_evacuate_with_server_group now."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_16cdef5b","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":12},"in_reply_to":"9fb8cfa7_f02f12f3","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"}],"nova/compute/manager.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a90891720dd63e5fa046f6f0bf5b830276cd354a","unresolved":false,"context_lines":[{"line_number":1385,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1386,"context_line":"                ins_uuids \u003d set()"},{"line_number":1387,"context_line":"                if migration:"},{"line_number":1388,"context_line":"                    migrations_same_host_and_node \u003d objects.MigrationList.\\"},{"line_number":1389,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1390,"context_line":"                            context, host\u003dself.host,"},{"line_number":1391,"context_line":"                            node\u003dmigration.dest_node)"},{"line_number":1392,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1393,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1394,"context_line":"                members_on_host \u003d (ins_on_host | ins_uuids) \\"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_352c9246","line":1391,"range":{"start_line":1388,"start_character":20,"end_line":1391,"end_character":53},"updated":"2019-04-10 21:49:04.000000000","message":"I\u0027m don\u0027t think we should filter by node here.  The code at line 1376 only looks at the host, not the node.","commit_id":"15db925dc771892cb8de3d8ce6fa75b1d7459ebb"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"cb898d2aa47682a8a8c7ac6d0d85b1687b982454","unresolved":false,"context_lines":[{"line_number":1385,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1386,"context_line":"                ins_uuids \u003d set()"},{"line_number":1387,"context_line":"                if migration:"},{"line_number":1388,"context_line":"                    migrations_same_host_and_node \u003d objects.MigrationList.\\"},{"line_number":1389,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1390,"context_line":"                            context, host\u003dself.host,"},{"line_number":1391,"context_line":"                            node\u003dmigration.dest_node)"},{"line_number":1392,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1393,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1394,"context_line":"                members_on_host \u003d (ins_on_host | ins_uuids) \\"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_6136dcb7","line":1391,"range":{"start_line":1388,"start_character":20,"end_line":1391,"end_character":53},"in_reply_to":"3fce034c_352c9246","updated":"2019-04-11 13:33:32.000000000","message":"\u003e I\u0027m don\u0027t think we should filter by node here.  The code at line\n \u003e 1376 only looks at the host, not the node.\n\nIn my understanding, for compute_node object, we can get one compute_node(which vm will be on) with host[1] or with host and node[2].\nSo that, if we can get both host and node from migration info, we can use both of them.\n\n[1] https://github.com/openstack/nova/blob/master/nova/objects/compute_node.py#L275\n[2] https://github.com/openstack/nova/blob/master/nova/objects/compute_node.py#L268","commit_id":"15db925dc771892cb8de3d8ce6fa75b1d7459ebb"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"d44b0d31c53c76511c4463d6ee9614d0dd7818de","unresolved":false,"context_lines":[{"line_number":1385,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1386,"context_line":"                ins_uuids \u003d set()"},{"line_number":1387,"context_line":"                if migration:"},{"line_number":1388,"context_line":"                    migrations_same_host_and_node \u003d objects.MigrationList.\\"},{"line_number":1389,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1390,"context_line":"                            context, host\u003dself.host,"},{"line_number":1391,"context_line":"                            node\u003dmigration.dest_node)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fce034c_bcf0e6fe","line":1388,"range":{"start_line":1388,"start_character":74,"end_line":1388,"end_character":75},"updated":"2019-04-18 08:10:30.000000000","message":"nit: It is preferred to wrap long lines in parentheses and not a backslash for line continuation.\nLinks：https://docs.openstack.org/hacking/latest/user/hacking.html#general","commit_id":"330ef36ed1e892b4ad786d7b9af90a9c8a922b6c"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"89ab81c90b61a8072704967b29e5db1a6f81ba8f","unresolved":false,"context_lines":[{"line_number":1385,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1386,"context_line":"                ins_uuids \u003d set()"},{"line_number":1387,"context_line":"                if migration:"},{"line_number":1388,"context_line":"                    migrations_same_host_and_node \u003d objects.MigrationList.\\"},{"line_number":1389,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1390,"context_line":"                            context, host\u003dself.host,"},{"line_number":1391,"context_line":"                            node\u003dmigration.dest_node)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fce034c_b70c7f41","line":1388,"range":{"start_line":1388,"start_character":74,"end_line":1388,"end_character":75},"in_reply_to":"3fce034c_bcf0e6fe","updated":"2019-04-18 09:19:10.000000000","message":"Done","commit_id":"330ef36ed1e892b4ad786d7b9af90a9c8a922b6c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"d44b0d31c53c76511c4463d6ee9614d0dd7818de","unresolved":false,"context_lines":[{"line_number":1391,"context_line":"                            node\u003dmigration.dest_node)"},{"line_number":1392,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1393,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1394,"context_line":"                members_on_host \u003d (ins_on_host | ins_uuids) \\"},{"line_number":1395,"context_line":"                    \u0026 members - set([instance.uuid])"},{"line_number":1396,"context_line":"                rules \u003d group.rules"},{"line_number":1397,"context_line":"                if rules and \u0027max_server_per_host\u0027 in rules:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fce034c_dcf5b2ee","line":1394,"range":{"start_line":1394,"start_character":60,"end_line":1394,"end_character":61},"updated":"2019-04-18 08:10:30.000000000","message":"ditto.","commit_id":"330ef36ed1e892b4ad786d7b9af90a9c8a922b6c"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"89ab81c90b61a8072704967b29e5db1a6f81ba8f","unresolved":false,"context_lines":[{"line_number":1391,"context_line":"                            node\u003dmigration.dest_node)"},{"line_number":1392,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1393,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1394,"context_line":"                members_on_host \u003d (ins_on_host | ins_uuids) \\"},{"line_number":1395,"context_line":"                    \u0026 members - set([instance.uuid])"},{"line_number":1396,"context_line":"                rules \u003d group.rules"},{"line_number":1397,"context_line":"                if rules and \u0027max_server_per_host\u0027 in rules:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fce034c_57051b13","line":1394,"range":{"start_line":1394,"start_character":60,"end_line":1394,"end_character":61},"in_reply_to":"3fce034c_dcf5b2ee","updated":"2019-04-18 09:19:10.000000000","message":"Done","commit_id":"330ef36ed1e892b4ad786d7b9af90a9c8a922b6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"        return [_decode(f) for f in injected_files]"},{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"    def _validate_instance_group_policy(self, context, instance,"},{"line_number":1367,"context_line":"                                        scheduler_hints, migration\u003dNone):"},{"line_number":1368,"context_line":"        # NOTE(russellb) Instance group policy is enforced by the scheduler."},{"line_number":1369,"context_line":"        # However, there is a race condition with the enforcement of"},{"line_number":1370,"context_line":"        # the policy.  Since more than one instance may be scheduled at the"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_5057fe58","line":1367,"updated":"2019-07-01 09:30:05.000000000","message":"As the parameter list of this function grows It would be nice to add a proper docstring to this function explaining the parameters.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"        return [_decode(f) for f in injected_files]"},{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"    def _validate_instance_group_policy(self, context, instance,"},{"line_number":1367,"context_line":"                                        scheduler_hints, migration\u003dNone):"},{"line_number":1368,"context_line":"        # NOTE(russellb) Instance group policy is enforced by the scheduler."},{"line_number":1369,"context_line":"        # However, there is a race condition with the enforcement of"},{"line_number":1370,"context_line":"        # the policy.  Since more than one instance may be scheduled at the"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_1685efac","line":1367,"in_reply_to":"9fb8cfa7_5057fe58","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":1398,"context_line":"                # Also consider group members that are migrating to avoid"},{"line_number":1399,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1400,"context_line":"                ins_uuids \u003d set()"},{"line_number":1401,"context_line":"                if migration:"},{"line_number":1402,"context_line":"                    migrations_same_host_and_node \u003d (objects.MigrationList."},{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_308b6ac0","line":1401,"updated":"2019-07-01 09:30:05.000000000","message":"Nesting is getting crazy here. Could you pull out a function that returns the instance uuids that are being migrated to this host?","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":7543,"name":"Yongli He","email":"yongli.he@intel.com","username":"yongli.he"},"change_message_id":"a24adcafe229e9c2d94697fd2f8b1909d1c84c52","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1400,"context_line":"                ins_uuids \u003d set()"},{"line_number":1401,"context_line":"                if migration:"},{"line_number":1402,"context_line":"                    migrations_same_host_and_node \u003d (objects.MigrationList."},{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_bb75b1ce","line":1402,"range":{"start_line":1402,"start_character":20,"end_line":1402,"end_character":49},"updated":"2019-06-19 08:46:00.000000000","message":"not my flavor, it\u0027s so long variable name -:)\nmgs_to_same_dst?","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1400,"context_line":"                ins_uuids \u003d set()"},{"line_number":1401,"context_line":"                if migration:"},{"line_number":1402,"context_line":"                    migrations_same_host_and_node \u003d (objects.MigrationList."},{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_f677fbd2","line":1402,"range":{"start_line":1402,"start_character":20,"end_line":1402,"end_character":49},"in_reply_to":"9fb8cfa7_bb75b1ce","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f7a51c78c5ba947a02947fe21f1e706e54ddd4d6","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1400,"context_line":"                ins_uuids \u003d set()"},{"line_number":1401,"context_line":"                if migration:"},{"line_number":1402,"context_line":"                    migrations_same_host_and_node \u003d (objects.MigrationList."},{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_31bac009","line":1402,"range":{"start_line":1402,"start_character":20,"end_line":1402,"end_character":49},"in_reply_to":"9fb8cfa7_bb75b1ce","updated":"2019-07-16 08:38:57.000000000","message":"I tend to disagree. I like written out variable names over random contractions. I think what makes this problematic here is the too deep indentation.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":1402,"context_line":"                    migrations_same_host_and_node \u003d (objects.MigrationList."},{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"},{"line_number":1406,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1407,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1408,"context_line":"                members_on_host \u003d ((ins_on_host | ins_uuids)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_5005be44","line":1405,"range":{"start_line":1405,"start_character":33,"end_line":1405,"end_character":54},"updated":"2019-07-01 09:30:05.000000000","message":"don\u0027t we have the instance.node already set to the dest node at this point? If yest then we don\u0027t have to pass in the Migration instance.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":1402,"context_line":"                    migrations_same_host_and_node \u003d (objects.MigrationList."},{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"},{"line_number":1406,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1407,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1408,"context_line":"                members_on_host \u003d ((ins_on_host | ins_uuids)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_b6a9837c","line":1405,"range":{"start_line":1405,"start_character":33,"end_line":1405,"end_character":54},"in_reply_to":"9fb8cfa7_5005be44","updated":"2019-07-16 09:35:05.000000000","message":"No. The node of instance here is still the source node. We will update the node of instance at last[0]. Right?\n\n[0] https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3178-L3181","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"},{"line_number":1406,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1407,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1408,"context_line":"                members_on_host \u003d ((ins_on_host | ins_uuids)"},{"line_number":1409,"context_line":"                    \u0026 members - set([instance.uuid]))"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_f099521e","line":1406,"range":{"start_line":1406,"start_character":32,"end_line":1406,"end_character":37},"updated":"2019-07-01 09:30:05.000000000","message":"you can write this directly as a set comprehension:\ninst_uuids \u003d {mgs.instance_uuid for ...}","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                        get_in_progress_by_host_and_node("},{"line_number":1404,"context_line":"                            context, host\u003dself.host,"},{"line_number":1405,"context_line":"                            node\u003dmigration.dest_node))"},{"line_number":1406,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1407,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1408,"context_line":"                members_on_host \u003d ((ins_on_host | ins_uuids)"},{"line_number":1409,"context_line":"                    \u0026 members - set([instance.uuid]))"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_1673afe8","line":1406,"range":{"start_line":1406,"start_character":32,"end_line":1406,"end_character":37},"in_reply_to":"9fb8cfa7_f099521e","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":7543,"name":"Yongli He","email":"yongli.he@intel.com","username":"yongli.he"},"change_message_id":"a24adcafe229e9c2d94697fd2f8b1909d1c84c52","unresolved":false,"context_lines":[{"line_number":1406,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1407,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1408,"context_line":"                members_on_host \u003d ((ins_on_host | ins_uuids)"},{"line_number":1409,"context_line":"                    \u0026 members - set([instance.uuid]))"},{"line_number":1410,"context_line":"                rules \u003d group.rules"},{"line_number":1411,"context_line":"                if rules and \u0027max_server_per_host\u0027 in rules:"},{"line_number":1412,"context_line":"                    max_server \u003d rules[\u0027max_server_per_host\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_fbb6c98a","line":1409,"range":{"start_line":1409,"start_character":20,"end_line":1409,"end_character":53},"updated":"2019-06-19 08:46:00.000000000","message":"minor: indent seems could be better.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":1406,"context_line":"                    ins_uuids \u003d set([mgs.instance_uuid"},{"line_number":1407,"context_line":"                                     for mgs in migrations_same_host_and_node])"},{"line_number":1408,"context_line":"                members_on_host \u003d ((ins_on_host | ins_uuids)"},{"line_number":1409,"context_line":"                    \u0026 members - set([instance.uuid]))"},{"line_number":1410,"context_line":"                rules \u003d group.rules"},{"line_number":1411,"context_line":"                if rules and \u0027max_server_per_host\u0027 in rules:"},{"line_number":1412,"context_line":"                    max_server \u003d rules[\u0027max_server_per_host\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_b67d03b3","line":1409,"range":{"start_line":1409,"start_character":20,"end_line":1409,"end_character":53},"in_reply_to":"9fb8cfa7_fbb6c98a","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f7a51c78c5ba947a02947fe21f1e706e54ddd4d6","unresolved":false,"context_lines":[{"line_number":1413,"context_line":""},{"line_number":1414,"context_line":"        :param context: `nova.RequestContext` object"},{"line_number":1415,"context_line":"        :param instance: Instance object"},{"line_number":1416,"context_line":"        :param scheduler_hints: scheduler_hints object of request_spec"},{"line_number":1417,"context_line":"        :param dest_node: Name of destination node"},{"line_number":1418,"context_line":"        :raises: RescheduledException if action will break server group policy"},{"line_number":1419,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_d1ea4c0d","line":1416,"range":{"start_line":1416,"start_character":48,"end_line":1416,"end_character":54},"updated":"2019-07-16 08:38:57.000000000","message":"dict","commit_id":"6a3f9acf2a7c262fde7100fa82d46ab57f3f8ce8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f7a51c78c5ba947a02947fe21f1e706e54ddd4d6","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"                # Also consider group members that are migrating to avoid"},{"line_number":1451,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1452,"context_line":"                ins_uuids \u003d set()"},{"line_number":1453,"context_line":"                if dest_node:"},{"line_number":1454,"context_line":"                    mgs_to_same_dst \u003d ("},{"line_number":1455,"context_line":"                        objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":1456,"context_line":"                            context, host\u003dself.host, node\u003ddest_node))"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_11e4643b","line":1453,"updated":"2019-07-16 08:38:57.000000000","message":"Nesting is getting crazy here. Could you pull out a function that returns the instance uuids that are being migrated to this host?\nOr move the _do_validation to the object level with a proper name?","commit_id":"6a3f9acf2a7c262fde7100fa82d46ab57f3f8ce8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f7a51c78c5ba947a02947fe21f1e706e54ddd4d6","unresolved":false,"context_lines":[{"line_number":1453,"context_line":"                if dest_node:"},{"line_number":1454,"context_line":"                    mgs_to_same_dst \u003d ("},{"line_number":1455,"context_line":"                        objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":1456,"context_line":"                            context, host\u003dself.host, node\u003ddest_node))"},{"line_number":1457,"context_line":"                    ins_uuids \u003d {mgs.instance_uuid for mgs in mgs_to_same_dst}"},{"line_number":1458,"context_line":"                members_on_host \u003d ("},{"line_number":1459,"context_line":"                        (ins_on_host | ins_uuids) \u0026"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_115224df","line":1456,"range":{"start_line":1456,"start_character":58,"end_line":1456,"end_character":67},"updated":"2019-07-16 08:38:57.000000000","message":"I already asked in PS13 that: \ndon\u0027t we have the instance.node already set to the dest node at this point? \n\nIf yest then we don\u0027t have to pass the dest_node to this function.","commit_id":"6a3f9acf2a7c262fde7100fa82d46ab57f3f8ce8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"27add131648aebcb1c412e86439129c3b8e313af","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"                # which are not the instance in question. This is used to"},{"line_number":1443,"context_line":"                # determine how many other members from the same anti-affinity"},{"line_number":1444,"context_line":"                # group can be on this host."},{"line_number":1445,"context_line":"                # Also consider group members that are migrating to avoid"},{"line_number":1446,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1447,"context_line":"                ins_uuids \u003d set()"},{"line_number":1448,"context_line":"                if dest_node:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5faad753_c8562b2b","line":1445,"range":{"start_line":1445,"start_character":55,"end_line":1445,"end_character":64},"updated":"2019-09-13 18:06:45.000000000","message":"to this host?","commit_id":"2b4fb9612b2ec5427f5d558d29660c053b3a3231"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"cf0282d2276871e8b0e9a08e50ea14cea065652b","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"                # which are not the instance in question. This is used to"},{"line_number":1443,"context_line":"                # determine how many other members from the same anti-affinity"},{"line_number":1444,"context_line":"                # group can be on this host."},{"line_number":1445,"context_line":"                # Also consider group members that are migrating to avoid"},{"line_number":1446,"context_line":"                # breaking the policy of group if it is a migration action."},{"line_number":1447,"context_line":"                ins_uuids \u003d set()"},{"line_number":1448,"context_line":"                if dest_node:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5faad753_a40665cd","line":1445,"range":{"start_line":1445,"start_character":55,"end_line":1445,"end_character":64},"in_reply_to":"5faad753_c8562b2b","updated":"2019-09-16 03:59:54.000000000","message":"\u003e to this host?\n\nYeah, we should consider group members that servers are migrating to this host(dest host).","commit_id":"2b4fb9612b2ec5427f5d558d29660c053b3a3231"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"27add131648aebcb1c412e86439129c3b8e313af","unresolved":false,"context_lines":[{"line_number":1448,"context_line":"                if dest_node:"},{"line_number":1449,"context_line":"                    mgs_to_same_dst \u003d ("},{"line_number":1450,"context_line":"                        objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":1451,"context_line":"                            context, host\u003dself.host, node\u003ddest_node))"},{"line_number":1452,"context_line":"                    ins_uuids \u003d {mgs.instance_uuid for mgs in mgs_to_same_dst}"},{"line_number":1453,"context_line":"                members_on_host \u003d ("},{"line_number":1454,"context_line":"                        (ins_on_host | ins_uuids) \u0026"}],"source_content_type":"text/x-python","patch_set":24,"id":"5faad753_480b1b7c","line":1451,"range":{"start_line":1451,"start_character":37,"end_line":1451,"end_character":67},"updated":"2019-09-13 18:06:45.000000000","message":"This combination doesn\u0027t look right. At the time of this check, self.host will be the source compute host [1] while dest_node will be the destination compute host, which is a mismatch that should result in no Migrations ever being returned because host and node are treated as a pair of source host/node or destination host/node [2]. But not a combination of source host and destination node.\n\nSome background about host and node: the \u0027host\u0027 represents the hostname of the compute service (nova-compute) and the \u0027node\u0027 represents the ironic node name in the ironic service. When ironic is not being used, host \u003d\u003d node but when ironic _is_ being used, host !\u003d node. The point is that host/node are a pair that refer to the same location essentially.\n\nBy doing a query for host\u003dsource, node\u003ddestination, I think you will not find anything. Which makes me wonder, are the func tests not exercising this code?\n\nAm I missing something here?\n\n[1] https://bugs.launchpad.net/nova/+bug/1763181/comments/1\n[2] https://github.com/openstack/nova/blob/33d3c74229f969c942df8dcda621eb93d8c4fb3a/nova/db/sqlalchemy/api.py#L4407","commit_id":"2b4fb9612b2ec5427f5d558d29660c053b3a3231"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"cf0282d2276871e8b0e9a08e50ea14cea065652b","unresolved":false,"context_lines":[{"line_number":1448,"context_line":"                if dest_node:"},{"line_number":1449,"context_line":"                    mgs_to_same_dst \u003d ("},{"line_number":1450,"context_line":"                        objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":1451,"context_line":"                            context, host\u003dself.host, node\u003ddest_node))"},{"line_number":1452,"context_line":"                    ins_uuids \u003d {mgs.instance_uuid for mgs in mgs_to_same_dst}"},{"line_number":1453,"context_line":"                members_on_host \u003d ("},{"line_number":1454,"context_line":"                        (ins_on_host | ins_uuids) \u0026"}],"source_content_type":"text/x-python","patch_set":24,"id":"5faad753_a4d42521","line":1451,"range":{"start_line":1451,"start_character":37,"end_line":1451,"end_character":67},"in_reply_to":"5faad753_480b1b7c","updated":"2019-09-16 03:59:54.000000000","message":"\u003e This combination doesn\u0027t look right. At the time of this check,\n \u003e self.host will be the source compute host [1] while dest_node will\n\nNo no, it is not correct. when we do late validation of instance group policy, the self.host here is the destination host. After we call the *rebuild_instance* function in rpcapi.py[0] here(from source host), so we have called the *rebuild_instance* function in manager.py[1](in destination host)\n\n[0] https://github.com/openstack/nova/blob/33d3c74229f969c942df8dcda621eb93d8c4fb3a/nova/compute/rpcapi.py#L851-L859\n[1] https://github.com/openstack/nova/blob/33d3c74229f969c942df8dcda621eb93d8c4fb3a/nova/compute/manager.py#L3073\n\n \u003e be the destination compute host, which is a mismatch that should\n \u003e result in no Migrations ever being returned because host and node\n \u003e are treated as a pair of source host/node or destination host/node\n \u003e [2]. But not a combination of source host and destination node.\n\nSo as described before, the pair of host and node here is destination host and node.\n\n \u003e \n \u003e Some background about host and node: the \u0027host\u0027 represents the\n \u003e hostname of the compute service (nova-compute) and the \u0027node\u0027\n \u003e represents the ironic node name in the ironic service. When ironic\n \u003e is not being used, host \u003d\u003d node but when ironic _is_ being used,\n \u003e host !\u003d node. The point is that host/node are a pair that refer to\n \u003e the same location essentially.\n \u003e \n\nYeah, you\u0027re right, If we deploy the ironic, the host:node is 1:M. So we should supply both host and node to query the in-progress migration :)\n\n \u003e By doing a query for host\u003dsource, node\u003ddestination, I think you\n \u003e will not find anything. Which makes me wonder, are the func tests\n \u003e not exercising this code?\n \u003e \n \u003e Am I missing something here?\n \u003e \n \u003e [1] https://bugs.launchpad.net/nova/+bug/1763181/comments/1\n \u003e [2] https://github.com/openstack/nova/blob/33d3c74229f969c942df8dcda621eb93d8c4fb3a/nova/db/sqlalchemy/api.py#L4407","commit_id":"2b4fb9612b2ec5427f5d558d29660c053b3a3231"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"5d3b0364133c568edbd6045c31c3583a316a9b69","unresolved":false,"context_lines":[{"line_number":1607,"context_line":"        :param context: `nova.RequestContext` object"},{"line_number":1608,"context_line":"        :param instance: Instance object"},{"line_number":1609,"context_line":"        :param scheduler_hints: scheduler_hints object of request_spec"},{"line_number":1610,"context_line":"        :param dest_node: Name of destination node"},{"line_number":1611,"context_line":"        :raises: RescheduledException if action will break server group policy"},{"line_number":1612,"context_line":"        \"\"\""},{"line_number":1613,"context_line":"        # NOTE(russellb) Instance group policy is enforced by the scheduler."}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_4dc37f07","line":1610,"range":{"start_line":1610,"start_character":8,"end_line":1610,"end_character":50},"updated":"2019-12-03 11:07:27.000000000","message":"Why we only check this for the evacuate, I\u0027m thinking maybe we should check this for new boot instance also.","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"a122e8ad564a49a097c3320f021859e12f259511","unresolved":false,"context_lines":[{"line_number":1607,"context_line":"        :param context: `nova.RequestContext` object"},{"line_number":1608,"context_line":"        :param instance: Instance object"},{"line_number":1609,"context_line":"        :param scheduler_hints: scheduler_hints object of request_spec"},{"line_number":1610,"context_line":"        :param dest_node: Name of destination node"},{"line_number":1611,"context_line":"        :raises: RescheduledException if action will break server group policy"},{"line_number":1612,"context_line":"        \"\"\""},{"line_number":1613,"context_line":"        # NOTE(russellb) Instance group policy is enforced by the scheduler."}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_4524b360","line":1610,"range":{"start_line":1610,"start_character":8,"end_line":1610,"end_character":50},"in_reply_to":"3fa7e38b_4dc37f07","updated":"2019-12-03 11:08:37.000000000","message":"oops, I saw you did this in the next patch, that is prefect.","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"5d3b0364133c568edbd6045c31c3583a316a9b69","unresolved":false,"context_lines":[{"line_number":1647,"context_line":"                if dest_node:"},{"line_number":1648,"context_line":"                    mgs_to_same_dst \u003d ("},{"line_number":1649,"context_line":"                        objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":1650,"context_line":"                            context, host\u003dself.host, node\u003ddest_node))"},{"line_number":1651,"context_line":"                    ins_uuids \u003d {mgs.instance_uuid for mgs in mgs_to_same_dst}"},{"line_number":1652,"context_line":"                members_on_host \u003d ("},{"line_number":1653,"context_line":"                        (ins_on_host | ins_uuids) \u0026"}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_650e8fe5","line":1650,"updated":"2019-12-03 11:07:27.000000000","message":"There are few flaws, like at here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4866\n\nThis may lead to instance migrating before this host failed the migration. That isn\u0027t fair since we should failed the second migration. But it is fine.\n\nAlso I\u0027m think maybe we should do the validation inside the claim in the future, that isn\u0027t scope of this patch.","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"5d3b0364133c568edbd6045c31c3583a316a9b69","unresolved":false,"context_lines":[{"line_number":3497,"context_line":"                # cause the evacuate to fail as rebuild does not implement"},{"line_number":3498,"context_line":"                # reschedule."},{"line_number":3499,"context_line":"                hints \u003d self._get_scheduler_hints({}, request_spec)"},{"line_number":3500,"context_line":"                dest_node \u003d migration.dest_node if migration else None"},{"line_number":3501,"context_line":"                self._validate_instance_group_policy(context, instance,"},{"line_number":3502,"context_line":"                                                     hints, dest_node)"},{"line_number":3503,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_ad18f3cc","line":3500,"range":{"start_line":3500,"start_character":51,"end_line":3500,"end_character":60},"updated":"2019-12-03 11:07:27.000000000","message":"We should be able to ensure we have migration if this is evacuate.","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"0cc84272107e2c9d172e79671901458a7a32e0de","unresolved":false,"context_lines":[{"line_number":3497,"context_line":"                # cause the evacuate to fail as rebuild does not implement"},{"line_number":3498,"context_line":"                # reschedule."},{"line_number":3499,"context_line":"                hints \u003d self._get_scheduler_hints({}, request_spec)"},{"line_number":3500,"context_line":"                dest_node \u003d migration.dest_node if migration else None"},{"line_number":3501,"context_line":"                self._validate_instance_group_policy(context, instance,"},{"line_number":3502,"context_line":"                                                     hints, dest_node)"},{"line_number":3503,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_f7cb03a1","line":3500,"range":{"start_line":3500,"start_character":51,"end_line":3500,"end_character":60},"in_reply_to":"3fa7e38b_ad18f3cc","updated":"2019-12-04 02:23:59.000000000","message":"hi Alex, from [1], the migration for rebuild/evacuate can be None.\n\n[1] https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L1048-L1054","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"34a7988e9856061c7ed52e93e9842955eff9fe09","unresolved":false,"context_lines":[{"line_number":3497,"context_line":"                # cause the evacuate to fail as rebuild does not implement"},{"line_number":3498,"context_line":"                # reschedule."},{"line_number":3499,"context_line":"                hints \u003d self._get_scheduler_hints({}, request_spec)"},{"line_number":3500,"context_line":"                dest_node \u003d migration.dest_node if migration else None"},{"line_number":3501,"context_line":"                self._validate_instance_group_policy(context, instance,"},{"line_number":3502,"context_line":"                                                     hints, dest_node)"},{"line_number":3503,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_2b1c21c1","line":3500,"range":{"start_line":3500,"start_character":51,"end_line":3500,"end_character":60},"in_reply_to":"3fa7e38b_dfa2fc3f","updated":"2019-12-17 08:31:20.000000000","message":"Yeah, you\u0027re right. So I add this check here\nhttps://review.opendev.org/#/c/649963/30/nova/conductor/manager.py","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"5f0267e6162a38fc88c580f08e4c9657fffca8b8","unresolved":false,"context_lines":[{"line_number":3497,"context_line":"                # cause the evacuate to fail as rebuild does not implement"},{"line_number":3498,"context_line":"                # reschedule."},{"line_number":3499,"context_line":"                hints \u003d self._get_scheduler_hints({}, request_spec)"},{"line_number":3500,"context_line":"                dest_node \u003d migration.dest_node if migration else None"},{"line_number":3501,"context_line":"                self._validate_instance_group_policy(context, instance,"},{"line_number":3502,"context_line":"                                                     hints, dest_node)"},{"line_number":3503,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"3fa7e38b_dfa2fc3f","line":3500,"range":{"start_line":3500,"start_character":51,"end_line":3500,"end_character":60},"in_reply_to":"3fa7e38b_f7cb03a1","updated":"2019-12-16 07:35:24.000000000","message":"But we always create it https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4932","commit_id":"4cfe43955e15a6a9cc60acef975f7c44a8cfce43"}],"nova/tests/functional/regressions/test_bug_1735407.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ab59517bec955f2e030bc0923431e73df63acd73","unresolved":false,"context_lines":[{"line_number":162,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":163,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":164,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":165,"context_line":"        # But all situations are not to breack the server group policy."},{"line_number":166,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":167,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":168,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_9bfe3703","line":165,"range":{"start_line":165,"start_character":40,"end_line":165,"end_character":46},"updated":"2019-04-08 16:09:09.000000000","message":"break","commit_id":"6332d698982f1315b3868a6fceff8daae102b121"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"4723eb6b338c4bfff2750bd3cdd37f279eda2e56","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        # are two types of situations:"},{"line_number":160,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":161,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":162,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":163,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":164,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":165,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":166,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":167,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":168,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fce034c_af2d7847","line":165,"range":{"start_line":162,"start_character":8,"end_line":165,"end_character":70},"updated":"2019-04-12 07:15:08.000000000","message":"might be better to mention the behavior of notification as before","commit_id":"2c4429bbaa4947e68158a3364ac70b1d0d831e02"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"0a9ea976e9a9c9c033e88a078cc783634ab8515e","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        # are two types of situations:"},{"line_number":160,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":161,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":162,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":163,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":164,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":165,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":166,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":167,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":168,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fce034c_30c9e477","line":165,"range":{"start_line":162,"start_character":8,"end_line":165,"end_character":70},"in_reply_to":"3fce034c_af2d7847","updated":"2019-04-12 08:52:16.000000000","message":"Done","commit_id":"2c4429bbaa4947e68158a3364ac70b1d0d831e02"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"0a9ea976e9a9c9c033e88a078cc783634ab8515e","unresolved":false,"context_lines":[{"line_number":177,"context_line":"            self.assertEqual(\u0027ACTIVE\u0027, server2[\u0027status\u0027])"},{"line_number":178,"context_line":"            self.assertEqual(\u0027host3\u0027, server2[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        # NOTE(boxiang): We will get 1 or 0 rebuild.start notification here"},{"line_number":181,"context_line":"        # because we validate server group policy (and therefore fail) before"},{"line_number":182,"context_line":"        # emitting rebuild.start."},{"line_number":183,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027 and server2[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":184,"context_line":"            fake_notifier.wait_for_versioned_notifications("},{"line_number":185,"context_line":"                \u0027instance.rebuild.start\u0027, n_events\u003d0)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fce034c_70944c45","line":182,"range":{"start_line":180,"start_character":8,"end_line":182,"end_character":33},"updated":"2019-04-12 08:52:16.000000000","message":"Notification notes here.","commit_id":"330ef36ed1e892b4ad786d7b9af90a9c8a922b6c"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"e101868c522a3b9ead568ab904969152de1034b5","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        # because we validate server group policy (and therefore fail) before"},{"line_number":182,"context_line":"        # emitting rebuild.start."},{"line_number":183,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027 and server2[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":184,"context_line":"            fake_notifier.wait_for_versioned_notifications("},{"line_number":185,"context_line":"                \u0027instance.rebuild.start\u0027, n_events\u003d0)"},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            fake_notifier.wait_for_versioned_notifications("},{"line_number":188,"context_line":"                \u0027instance.rebuild.start\u0027, n_events\u003d1)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fce034c_e4ad65e9","line":185,"range":{"start_line":184,"start_character":12,"end_line":185,"end_character":53},"updated":"2019-04-19 02:02:58.000000000","message":"This line does not check that the notification is not emitted.\nIt does not make sense.\n\nhttps://github.com/openstack/nova/blob/b7a018f1265d9e0354e26822d32cbdc789819c35/nova/tests/unit/fake_notifier.py#L52","commit_id":"58269b378aa3492e960e7e615cafc2b78db4b73e"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"333edc1d8b66d76b3d468f08b33862becddfb1dc","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        self.image_id \u003d self.api.get_images()[0][\u0027id\u0027]"},{"line_number":71,"context_line":"        self.flavor_id \u003d self.api.get_flavors()[0][\u0027id\u0027]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        manager_class \u003d nova.compute.manager.ComputeManager"},{"line_number":74,"context_line":"        original_rebuild \u003d manager_class._do_rebuild_instance"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        def fake_rebuild(self_, context, instance, *args, **kwargs):"},{"line_number":77,"context_line":"            # Simulate that the rebuild request of one of the instances"},{"line_number":78,"context_line":"            # reaches the target compute manager significantly later so the"},{"line_number":79,"context_line":"            # rebuild of the other instance can finish before the late"},{"line_number":80,"context_line":"            # validation of the first rebuild."},{"line_number":81,"context_line":"            # We cannot simply delay the virt driver\u0027s rebuild or the"},{"line_number":82,"context_line":"            # manager\u0027s _rebuild_default_impl as those run after the late"},{"line_number":83,"context_line":"            # validation"},{"line_number":84,"context_line":"            if instance.host \u003d\u003d \u0027host1\u0027:"},{"line_number":85,"context_line":"                # wait for the other instance rebuild to start"},{"line_number":86,"context_line":"                fake_notifier.wait_for_versioned_notifications("},{"line_number":87,"context_line":"                    \u0027instance.rebuild.start\u0027, n_events\u003d1)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"            original_rebuild(self_, context, instance, *args, **kwargs)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.stub_out(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":92,"context_line":"                      \u0027_do_rebuild_instance\u0027, fake_rebuild)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_parallel_evacuate_with_server_group(self):"},{"line_number":95,"context_line":"        self.skipTest(\u0027Skipped until bug 1763181 is fixed\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ffb9cba7_40838705","side":"PARENT","line":92,"range":{"start_line":73,"start_character":8,"end_line":92,"end_character":59},"updated":"2019-04-22 05:42:02.000000000","message":"Remove this fake.\nIf we parallelly evacuate two server with anti-affinity group and both of them are in migration table before they do late group policy check, they will all fail to evacuate.\nSo the fake makes no sense.","commit_id":"fc3890667e4971e3f0f35ac921c2a6c25f72adec"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # NOTE(boxiang): In the case of concurrent request evacuation, there"},{"line_number":136,"context_line":"        # are two types of situations:"},{"line_number":137,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":138,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_9043366c","line":138,"range":{"start_line":137,"start_character":0,"end_line":138,"end_character":75},"updated":"2019-07-01 09:30:05.000000000","message":"Does it mean both server1 and server2 doing the late check in parallel and both see the other server is being migrated to the same host and therefore both check fails?","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1c26406291f9e2fe2ba46e08259b54fc85fbedf9","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # NOTE(boxiang): In the case of concurrent request evacuation, there"},{"line_number":136,"context_line":"        # are two types of situations:"},{"line_number":137,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":138,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_dfe1c09c","line":138,"range":{"start_line":137,"start_character":0,"end_line":138,"end_character":75},"in_reply_to":"9fb8cfa7_160acf24","updated":"2019-08-06 09:29:38.000000000","message":"Thanks for the explanation. \n\nSo case A is when two late check runs in parallel and both sees the the other ongoing evacuation and therefore both fails.\n\nCould you extend the code comment a bit based on what we discussed in the review?","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # NOTE(boxiang): In the case of concurrent request evacuation, there"},{"line_number":136,"context_line":"        # are two types of situations:"},{"line_number":137,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":138,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_160acf24","line":138,"range":{"start_line":137,"start_character":0,"end_line":138,"end_character":75},"in_reply_to":"9fb8cfa7_9043366c","updated":"2019-07-16 09:35:05.000000000","message":"Yeah. eg:\nWe have 3 compute nodes named node01, node02 and node03.\nWe have 2 vms named vm01 and vm02 in these compute nodes.\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nvm01 -\u003e node01\nvm02 -\u003e node02\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nNow nova-compute service in node01 and node02 is stopped. We need to evacuate these 2 vms. Both 2 vms are doing later check in parallel and see the other server is being migrated to the same host. So both of them check failure.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":138,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_b0467a7a","line":140,"range":{"start_line":140,"start_character":43,"end_line":140,"end_character":49},"updated":"2019-07-01 09:30:05.000000000","message":"I don\u0027t get this note.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1c26406291f9e2fe2ba46e08259b54fc85fbedf9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":138,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_5f3870ec","line":140,"range":{"start_line":140,"start_character":43,"end_line":140,"end_character":49},"in_reply_to":"9fb8cfa7_79b9bcc9","updated":"2019-08-06 09:29:38.000000000","message":"Thanks for the explanation.\n\nSo case B is when the first evacuation succeeds as no parallel evacuation running at that time and the second evacuation is scheduled _before_ the first evacuation finished, so the scheduler can select the same host for the second evacuation too. Then the first evacuation finishes. Then second evacuation reaches the late check and fails as the target compute host already has the first instance from the same anti affinity group. \n\nCould you extend the code comment a bit based on what we discussed in the review?","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        # A: When doing the late check instance group policy, all migrations"},{"line_number":138,"context_line":"        # data have been saved into table, they will be all set into ERROR."},{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_79b9bcc9","line":140,"range":{"start_line":140,"start_character":43,"end_line":140,"end_character":49},"in_reply_to":"9fb8cfa7_b0467a7a","updated":"2019-07-16 09:35:05.000000000","message":"same compute nodes and vms as describe before.\n\nNow nova-compute service in node01 and node02 is stopped. We need to evacuate these 2 vms.\n\nNow vm01 is doing later check at first and do not see any other servers are being migrated to the same host. vm01 has finished evacuation. When vm02 is doing later check, it found no other servers are being migrated to the same host but target host has one server. So vm02 fails to evacuate.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":144,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":145,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_3027eac9","line":142,"updated":"2019-07-01 09:30:05.000000000","message":"How this test case now ensures that the two evacuation happens in parallel?\n\nAs there is a third situation. One of the evacuation finished before the second is scheduled and therefore the second scheduling stops before the race can even reproduced. If this happens then this test case does not reproduce the original problem.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"6e9393034649f588e82f5e33922d250431ae2135","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":144,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":145,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_7914614d","line":142,"in_reply_to":"7faddb67_827e8b67","updated":"2019-08-06 16:06:56.000000000","message":"Yeah, you\u0027re right. The B2 is not the goal for our testcase.\nBut now I have no good idea to seperate or control only to trigger B1. Any idea? thanks.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":144,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":145,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_59007866","line":142,"in_reply_to":"9fb8cfa7_3027eac9","updated":"2019-07-16 09:35:05.000000000","message":"\u003e How this test case now ensures that the two evacuation happens in\n \u003e parallel?\n\nWe just insure that we call the evacuate api in parallel. But the internal logic call may exist successively. So we have two situations as described before.\n\n \u003e \n \u003e As there is a third situation. One of the evacuation finished\n \u003e before the second is scheduled and therefore the second scheduling\n \u003e stops before the race can even reproduced. If this happens then\n \u003e this test case does not reproduce the original problem.\n\nSorry. I don\u0027t fully understand this situation. I find it is like my second situation. :(","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1c26406291f9e2fe2ba46e08259b54fc85fbedf9","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":140,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":141,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":142,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":143,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":144,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":145,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"7faddb67_827e8b67","line":142,"in_reply_to":"9fb8cfa7_59007866","updated":"2019-08-06 09:29:38.000000000","message":"See my answer in the above comment about the case B situation. In that case the you need that the second scheduling happens _before_ the first evac finishes. As if the first evac finishes before the second scheduling then the second scheduling already see the first vm on the compute and wont select that compute for the second vm at all. Therefore it is not the late check of the second vm that prevent the evacuation but the scheduler. \n\nYour current testcase in case B allows both situation to happen:\n* B1: second vm scheduling happens _before the first evacuation finishes. The late check for the second vm will be called and will fail the second evacuation.\n* B2: the second scheduling happens _after_ the first vm finished evacuating therefore the scheduling of the second evacuation finds no valid host. In this case the late check does not even called for the second vm.\n\nSo each run of the current test case might trigger B1 or B2 but we don\u0027t know which one. But we want to always trigger B1 instead of B2 as only B1 calls the late check that this test case want to validate.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"253574135a1a77458ea017490271c74c34fb849d","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            self.assertEqual(\u0027ACTIVE\u0027, server2[\u0027status\u0027])"},{"line_number":155,"context_line":"            self.assertEqual(\u0027host3\u0027, server2[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        # NOTE(boxiang): We will get 1 or 0 rebuild.start notification here"},{"line_number":158,"context_line":"        # because we validate server group policy (and therefore fail) before"},{"line_number":159,"context_line":"        # emitting rebuild.start."},{"line_number":160,"context_line":"        # Here we do not wait for 0 versioned notifications because it makes"},{"line_number":161,"context_line":"        # no sence."},{"line_number":162,"context_line":"        if server1[\u0027status\u0027] !\u003d \u0027ERROR\u0027 or server2[\u0027status\u0027] !\u003d \u0027ERROR\u0027:"},{"line_number":163,"context_line":"            fake_notifier.wait_for_versioned_notifications("},{"line_number":164,"context_line":"                \u0027instance.rebuild.start\u0027, n_events\u003d1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_d0320e10","line":164,"range":{"start_line":157,"start_character":0,"end_line":164,"end_character":53},"updated":"2019-07-01 09:30:05.000000000","message":"In the original test waiting for the notification was use to catch the proper timing. It is not used for timing here so I think you can simply drop this.","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"523115af480fa8d81446b9df572d9ce4b61d5b16","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            self.assertEqual(\u0027ACTIVE\u0027, server2[\u0027status\u0027])"},{"line_number":155,"context_line":"            self.assertEqual(\u0027host3\u0027, server2[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        # NOTE(boxiang): We will get 1 or 0 rebuild.start notification here"},{"line_number":158,"context_line":"        # because we validate server group policy (and therefore fail) before"},{"line_number":159,"context_line":"        # emitting rebuild.start."},{"line_number":160,"context_line":"        # Here we do not wait for 0 versioned notifications because it makes"},{"line_number":161,"context_line":"        # no sence."},{"line_number":162,"context_line":"        if server1[\u0027status\u0027] !\u003d \u0027ERROR\u0027 or server2[\u0027status\u0027] !\u003d \u0027ERROR\u0027:"},{"line_number":163,"context_line":"            fake_notifier.wait_for_versioned_notifications("},{"line_number":164,"context_line":"                \u0027instance.rebuild.start\u0027, n_events\u003d1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_6c61fca8","line":164,"range":{"start_line":157,"start_character":0,"end_line":164,"end_character":53},"in_reply_to":"9fb8cfa7_d0320e10","updated":"2019-07-16 09:35:05.000000000","message":"Done","commit_id":"6d21ccff92b713a8a268f1d41413cc36be3a6aa3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f7a51c78c5ba947a02947fe21f1e706e54ddd4d6","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        # B: When doing the late check instance group policy, maybe one of"},{"line_number":135,"context_line":"        # them will get one migration info(itself), so that, one will succeed"},{"line_number":136,"context_line":"        # to evacuate and another will be set into ERROR."},{"line_number":137,"context_line":"        # But all situations are not to break the server group policy."},{"line_number":138,"context_line":"        if server1[\u0027status\u0027] \u003d\u003d \u0027ERROR\u0027:"},{"line_number":139,"context_line":"            self.assertEqual(\u0027ERROR\u0027, server1[\u0027status\u0027])"},{"line_number":140,"context_line":"            self.assertNotEqual(\u0027host3\u0027, server1[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":19,"id":"7faddb67_9196143e","line":137,"updated":"2019-07-16 08:38:57.000000000","message":"I have still open questions from PS13.\n\nhttps://review.opendev.org/#/c/649963/13/nova/tests/functional/regressions/test_bug_1735407.py@138\n\nhttps://review.opendev.org/#/c/649963/13/nova/tests/functional/regressions/test_bug_1735407.py@140\n\nand especially https://review.opendev.org/#/c/649963/13/nova/tests/functional/regressions/test_bug_1735407.py@142","commit_id":"6a3f9acf2a7c262fde7100fa82d46ab57f3f8ce8"}]}