)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":13,"context_line":"provider tree. To be able to do that we have to re-calculate the request"},{"line_number":14,"context_line":"group - resource provider mapping for the source host based on the"},{"line_number":15,"context_line":"resource requests in the neutron ports of the instance and the resource"},{"line_number":16,"context_line":"allocation of the instance on the source host. Alternatively we could"},{"line_number":17,"context_line":"store such mapping in the MigrationContext during the move operation."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"blueprint: support-move-ops-with-qos-ports"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"5faad753_b42137b7","line":17,"range":{"start_line":16,"start_character":47,"end_line":17,"end_character":69},"updated":"2019-09-09 22:34:28.000000000","message":"This seems like a lot less work than re-calculating the mappings on revert doesn\u0027t it? Save off the port-\u003eallocation provider mapping on resize in the migration context and then on revert pull it out. I know we\u0027re getting late to make that kind of change, but how hard would it be or would we be able to do it later (though doing it later means RPC changes on the MigrationContext object).","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":13,"context_line":"provider tree. To be able to do that we have to re-calculate the request"},{"line_number":14,"context_line":"group - resource provider mapping for the source host based on the"},{"line_number":15,"context_line":"resource requests in the neutron ports of the instance and the resource"},{"line_number":16,"context_line":"allocation of the instance on the source host. Alternatively we could"},{"line_number":17,"context_line":"store such mapping in the MigrationContext during the move operation."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"blueprint: support-move-ops-with-qos-ports"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"5faad753_c1caea18","line":17,"range":{"start_line":16,"start_character":47,"end_line":17,"end_character":69},"in_reply_to":"5faad753_b42137b7","updated":"2019-09-10 09:03:51.000000000","message":"When we do the forward move then the mapping calculation can be replaced with the usage of the new placement api version 1.34 [1]. But for the revert we already lost the original allocation dict returned by the scheduler. However as the allocation key is in the binding-profile, neutron multiple binding api could be used to remember the source host binding including the source host mapping. I think this is what will happen in the live migration code path for the bandwidth feature. But for cold migrate nova does not use  that neutron API yet. \n\nHonestly I don\u0027t really want to add a new object and RPC change to the series in Train days before the feature freeze. Not just because it is questionable if we can land it, but because I think the proper solution is to use the multiple binding neutron API and let neutron remember the mapping. Also the current re-calculating solution is only expensive for instances that are using the bandwidth feature. So this not affect the majority of the deployments. \n\nI do have a TODO and intention to start using placement 1.34 in the feature and also to at least try to use the multiple binding api for cold migrate and resize. So that nova can fully get rid of the mapping calculation code. This was the original agreement a year ago that the mapping calculation code is temporary in nova.\n\n[1]https://docs.openstack.org/placement/latest/placement-api-microversion-history.html#request-group-mappings-in-allocation-candidates","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3448,"context_line":"            context, instance.uuid)"},{"line_number":3449,"context_line":"        reqspec.requested_resources \u003d port_res_req"},{"line_number":3450,"context_line":""},{"line_number":3451,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":3452,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_141a8b7c","line":3449,"updated":"2019-09-09 22:34:28.000000000","message":"It would be good to have a comment that explains what this is needed for and why we\u0027re doing it here - looks like we have to do it here (rather than conductor for example) because the API calls compute directly to trigger the revert flow.","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3448,"context_line":"            context, instance.uuid)"},{"line_number":3449,"context_line":"        reqspec.requested_resources \u003d port_res_req"},{"line_number":3450,"context_line":""},{"line_number":3451,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":3452,"context_line":"        instance.save(expected_task_state\u003d[None])"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_81bb9270","line":3449,"in_reply_to":"5faad753_141a8b7c","updated":"2019-09-10 09:03:51.000000000","message":"Done","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":3443,"context_line":""},{"line_number":3444,"context_line":"        # TODO(gibi): do not directly overwrite the"},{"line_number":3445,"context_line":"        # RequestSpec.requested_resources as others like cyborg might added"},{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_ecbfd6f8","line":3446,"updated":"2019-09-10 12:58:42.000000000","message":"do they plan to also use the same field ?","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aedc63fa3f8a9ad2d9d5c7838e6786211c9cbb8c","unresolved":false,"context_lines":[{"line_number":3443,"context_line":""},{"line_number":3444,"context_line":"        # TODO(gibi): do not directly overwrite the"},{"line_number":3445,"context_line":"        # RequestSpec.requested_resources as others like cyborg might added"},{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_efc90878","line":3446,"in_reply_to":"5faad753_ecbfd6f8","updated":"2019-09-10 13:06:53.000000000","message":"Yes, they do https://review.opendev.org/#/c/631243/38/nova/compute/api.py@1088","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fbc7c8ffe11e86de6ddc48439044a927dfc40b5b","unresolved":false,"context_lines":[{"line_number":3445,"context_line":"        # RequestSpec.requested_resources as others like cyborg might added"},{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_5e280b85","line":3448,"range":{"start_line":3448,"start_character":62,"end_line":3448,"end_character":64},"updated":"2019-09-10 15:48:33.000000000","message":"nix","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2d7b5f931e0e3005e2336ad6abe3b575036ed24","unresolved":false,"context_lines":[{"line_number":3445,"context_line":"        # RequestSpec.requested_resources as others like cyborg might added"},{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_e13b58ce","line":3448,"range":{"start_line":3448,"start_character":62,"end_line":3448,"end_character":64},"in_reply_to":"5faad753_5e280b85","updated":"2019-09-10 16:42:18.000000000","message":"Done","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fbc7c8ffe11e86de6ddc48439044a927dfc40b5b","unresolved":false,"context_lines":[{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_9e3523dd","line":3449,"range":{"start_line":3449,"start_character":32,"end_line":3449,"end_character":36},"updated":"2019-09-10 15:48:33.000000000","message":"nix","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2d7b5f931e0e3005e2336ad6abe3b575036ed24","unresolved":false,"context_lines":[{"line_number":3446,"context_line":"        # to things there already"},{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_a13560dc","line":3449,"range":{"start_line":3449,"start_character":32,"end_line":3449,"end_character":36},"in_reply_to":"5faad753_9e3523dd","updated":"2019-09-10 16:42:18.000000000","message":"Done","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fbc7c8ffe11e86de6ddc48439044a927dfc40b5b","unresolved":false,"context_lines":[{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_7e50670b","line":3450,"range":{"start_line":3450,"start_character":10,"end_line":3450,"end_character":21},"updated":"2019-09-10 15:48:33.000000000","message":"s/compute_api/dest compute service/","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_4c462a24","line":3450,"updated":"2019-09-10 12:58:42.000000000","message":"well, ok. thanks for the comment.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2d7b5f931e0e3005e2336ad6abe3b575036ed24","unresolved":false,"context_lines":[{"line_number":3447,"context_line":"        # NOTE(gibi): We need to collect the requested resource again as it is"},{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_21573005","line":3450,"range":{"start_line":3450,"start_character":10,"end_line":3450,"end_character":21},"in_reply_to":"5faad753_7e50670b","updated":"2019-09-10 16:42:18.000000000","message":"Done","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fbc7c8ffe11e86de6ddc48439044a927dfc40b5b","unresolved":false,"context_lines":[{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"},{"line_number":3454,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_fe8f772b","line":3451,"updated":"2019-09-10 15:48:33.000000000","message":"nit: for the sake of performance when an instance doesn\u0027t have ports with requested resources, wouldn\u0027t it be easy to first check if there are any vifs in the info cache with a binding:profile with an allocation set and if not, bypass this call to neutron? Barring the vif info cache being corrupted somehow in between the time the server is resized and when it\u0027s reverted. Anyway, it\u0027s a nit.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"baf1a196df569cbf461aba5107012483557b35fc","unresolved":false,"context_lines":[{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"},{"line_number":3454,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_33352ff8","line":3451,"in_reply_to":"5faad753_840872d9","updated":"2019-09-11 15:03:03.000000000","message":"Done in a followup https://review.opendev.org/681513","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"060fb2dc53cf9f2004d43a70de2e0e3c030f21b7","unresolved":false,"context_lines":[{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"},{"line_number":3454,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_840872d9","line":3451,"in_reply_to":"5faad753_e173587f","updated":"2019-09-10 16:48:44.000000000","message":"Ack, good idea.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2d7b5f931e0e3005e2336ad6abe3b575036ed24","unresolved":false,"context_lines":[{"line_number":3448,"context_line":"        # intentionally not persisted in nova. Note that this is needs to be"},{"line_number":3449,"context_line":"        # done here as the nova REST API code directly calls revert on the"},{"line_number":3450,"context_line":"        # compute_api skipping the conductor."},{"line_number":3451,"context_line":"        port_res_req \u003d self.network_api.get_requested_resource_for_instance("},{"line_number":3452,"context_line":"            context, instance.uuid)"},{"line_number":3453,"context_line":"        reqspec.requested_resources \u003d port_res_req"},{"line_number":3454,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_e173587f","line":3451,"in_reply_to":"5faad753_fe8f772b","updated":"2019-09-10 16:42:18.000000000","message":"Let me do a separate follow up for this optimization as it needs a bit more changes than the rest of the nits in this patch.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":3465,"context_line":"        # against going over quota during a race at this time because the"},{"line_number":3466,"context_line":"        # resource consumption for this operation is written to the database"},{"line_number":3467,"context_line":"        # by compute."},{"line_number":3468,"context_line":"        self.compute_rpcapi.revert_resize(context, instance,"},{"line_number":3469,"context_line":"                                          migration,"},{"line_number":3470,"context_line":"                                          migration.dest_compute,"},{"line_number":3471,"context_line":"                                          reqspec)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_cc63fab0","line":3468,"updated":"2019-09-10 12:58:42.000000000","message":"yeah, we directly call the compute service...","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":4297,"context_line":"            # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4298,"context_line":"            # fetched the original allocation of the instance so we could"},{"line_number":4299,"context_line":"            # avoid this second call to placement"},{"line_number":4300,"context_line":"            allocs \u003d self.reportclient.get_allocations_for_consumer("},{"line_number":4301,"context_line":"                context, instance.uuid)"},{"line_number":4302,"context_line":"            scheduler_utils.fill_provider_mapping_based_on_allocation("},{"line_number":4303,"context_line":"                context, self.reportclient, request_spec, allocs)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_cf569a07","line":4300,"updated":"2019-09-09 22:34:28.000000000","message":"A comment about what we\u0027re doing here would be nice. And I guess we get the allocations for the instance consumer because we\u0027ve already called _revert_allocation and moved the migration based allocations for the source provider tree back to the instance and dropped the migration consumer allocations.","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":4297,"context_line":"            # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4298,"context_line":"            # fetched the original allocation of the instance so we could"},{"line_number":4299,"context_line":"            # avoid this second call to placement"},{"line_number":4300,"context_line":"            allocs \u003d self.reportclient.get_allocations_for_consumer("},{"line_number":4301,"context_line":"                context, instance.uuid)"},{"line_number":4302,"context_line":"            scheduler_utils.fill_provider_mapping_based_on_allocation("},{"line_number":4303,"context_line":"                context, self.reportclient, request_spec, allocs)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_41091a2f","line":4300,"in_reply_to":"5faad753_cf569a07","updated":"2019-09-10 09:03:51.000000000","message":"Added a comment. \n\nYes the allocation is already fetched above in the _revert_allocation. Actually there is a patch [1] later in the series that solves this TODO by returning the source allocation from _revert_allocation()\n\n[1]https://review.opendev.org/#/c/678827/12/nova/compute/manager.py","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":4300,"context_line":"            allocs \u003d self.reportclient.get_allocations_for_consumer("},{"line_number":4301,"context_line":"                context, instance.uuid)"},{"line_number":4302,"context_line":"            scheduler_utils.fill_provider_mapping_based_on_allocation("},{"line_number":4303,"context_line":"                context, self.reportclient, request_spec, allocs)"},{"line_number":4304,"context_line":"            provider_mappings \u003d self._get_request_group_mapping(request_spec)"},{"line_number":4305,"context_line":""},{"line_number":4306,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_ef0d9637","line":4303,"range":{"start_line":4303,"start_character":44,"end_line":4303,"end_character":56},"updated":"2019-09-09 22:34:28.000000000","message":"For an old compute this is going to be None and that\u0027s why the grenade-multinode job is failing:\n\nhttps://zuul.opendev.org/t/openstack/build/5898a069eefa4b788e4a39de399b0d63/log/logs/screen-n-cpu.txt.gz#8393\n\ntempest-MigrationsAdminTest-1542746540] [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac] Setting instance vm_state to ERROR: AttributeError: \u0027NoneType\u0027 object has no attribute \u0027maps_requested_resources\u0027\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac] Traceback (most recent call last):\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]   File \"/opt/stack/new/nova/nova/compute/manager.py\", line 8508, in _error_out_instance_on_exception\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]     yield\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]   File \"/opt/stack/new/nova/nova/compute/manager.py\", line 4303, in finish_revert_resize\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]     context, self.reportclient, request_spec, allocs)\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]   File \"/opt/stack/new/nova/nova/scheduler/utils.py\", line 1163, in fill_provider_mapping_based_on_allocation\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]     if not request_spec.maps_requested_resources:\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac] AttributeError: \u0027NoneType\u0027 object has no attribute \u0027maps_requested_resources\u0027\nSep 09 16:47:59.348539 ubuntu-bionic-rax-ord-0011008637 nova-compute[19038]: ERROR nova.compute.manager [instance: c23048ac-1fe2-4c8c-8cbc-2d8756fa77ac]","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":4300,"context_line":"            allocs \u003d self.reportclient.get_allocations_for_consumer("},{"line_number":4301,"context_line":"                context, instance.uuid)"},{"line_number":4302,"context_line":"            scheduler_utils.fill_provider_mapping_based_on_allocation("},{"line_number":4303,"context_line":"                context, self.reportclient, request_spec, allocs)"},{"line_number":4304,"context_line":"            provider_mappings \u003d self._get_request_group_mapping(request_spec)"},{"line_number":4305,"context_line":""},{"line_number":4306,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_7ca87dda","line":4303,"range":{"start_line":4303,"start_character":44,"end_line":4303,"end_character":56},"in_reply_to":"5faad753_ef0d9637","updated":"2019-09-10 09:03:51.000000000","message":"Thanks for making me adding the extra test coverage to grenade.\n\nI added an extra condition here initing provider_mappings to None. This way the update_port_binding code will raise a proper exception (similarly to the case when a forward move fails in a pinned situation). \n\nI don\u0027t think we can add a not too hackish functional test for this case. If you think this would worth the effort then I can try to add that functional test in a follow up.","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fbc7c8ffe11e86de6ddc48439044a927dfc40b5b","unresolved":false,"context_lines":[{"line_number":4294,"context_line":"                           \u0027migration_uuid\u0027: migration.uuid})"},{"line_number":4295,"context_line":"                raise"},{"line_number":4296,"context_line":""},{"line_number":4297,"context_line":"            if request_spec:"},{"line_number":4298,"context_line":"                # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4299,"context_line":"                # fetched the original allocation of the instance so we could"},{"line_number":4300,"context_line":"                # avoid this second call to placement"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_fe92179f","line":4297,"updated":"2019-09-10 15:48:33.000000000","message":"This finish_revert_resize method is already big. In a follow up I\u0027d like to move this code to a private method, similar to _get_request_group_mapping. I initially thought we could just use _get_request_group_mapping here since it looks similar but I guess doesn\u0027t work because we need to use the source allocs and the request spec requested_resources would only have allocations for the dest host at this point? Anyway, you could call the new private method _fill_provider_mapping_based_on_allocation I guess.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"060fb2dc53cf9f2004d43a70de2e0e3c030f21b7","unresolved":false,"context_lines":[{"line_number":4294,"context_line":"                           \u0027migration_uuid\u0027: migration.uuid})"},{"line_number":4295,"context_line":"                raise"},{"line_number":4296,"context_line":""},{"line_number":4297,"context_line":"            if request_spec:"},{"line_number":4298,"context_line":"                # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4299,"context_line":"                # fetched the original allocation of the instance so we could"},{"line_number":4300,"context_line":"                # avoid this second call to placement"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_e4c5a68f","line":4297,"in_reply_to":"5faad753_4117cc52","updated":"2019-09-10 16:48:44.000000000","message":"I meant move the whole if/else block. Even if the extra get_allocations_for_consumer call goes away, there are a lot of comments here and this is already a big method so I\u0027d like to break out chunks of it where it makes sense.\n\nIn general our ability to bloat the size of any method related to a cold or live migration is astounding and it would be nice if we can avoid that by using smaller methods - this is obvious from the number of mocks on your new unit test just to hit this code.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b0aa80ac657279df729b4e6c0bdca72a236ba7ae","unresolved":false,"context_lines":[{"line_number":4294,"context_line":"                           \u0027migration_uuid\u0027: migration.uuid})"},{"line_number":4295,"context_line":"                raise"},{"line_number":4296,"context_line":""},{"line_number":4297,"context_line":"            if request_spec:"},{"line_number":4298,"context_line":"                # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4299,"context_line":"                # fetched the original allocation of the instance so we could"},{"line_number":4300,"context_line":"                # avoid this second call to placement"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_91a8a877","line":4297,"in_reply_to":"5faad753_e4c5a68f","updated":"2019-09-11 15:01:37.000000000","message":"Done in a followup https://review.opendev.org/#/c/681490/","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2d7b5f931e0e3005e2336ad6abe3b575036ed24","unresolved":false,"context_lines":[{"line_number":4294,"context_line":"                           \u0027migration_uuid\u0027: migration.uuid})"},{"line_number":4295,"context_line":"                raise"},{"line_number":4296,"context_line":""},{"line_number":4297,"context_line":"            if request_spec:"},{"line_number":4298,"context_line":"                # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4299,"context_line":"                # fetched the original allocation of the instance so we could"},{"line_number":4300,"context_line":"                # avoid this second call to placement"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_4117cc52","line":4297,"in_reply_to":"5faad753_fe92179f","updated":"2019-09-10 16:42:18.000000000","message":"Do I understand correctly that you want to extract the get_allocations_for_consumer and the fill_provider_mapping_based_on_allocation calls into a private helper? Or you mean that I should move the whole \n\n  if request_spec:\n      ...\n  else:\n      ...\n\nblock to a separate helper?\nIf the former then please note that the extra get_allocations_for_consumer call is removed in https://review.opendev.org/#/c/678827/13/nova/compute/manager.py","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":4297,"context_line":"            if request_spec:"},{"line_number":4298,"context_line":"                # TODO(gibi): the _revert_allocation() call above already"},{"line_number":4299,"context_line":"                # fetched the original allocation of the instance so we could"},{"line_number":4300,"context_line":"                # avoid this second call to placement"},{"line_number":4301,"context_line":"                # NOTE(gibi): We need to re-calculate the resource provider -"},{"line_number":4302,"context_line":"                # port mapping as we have to have the neutron ports allocate"},{"line_number":4303,"context_line":"                # from the source compute after revert."}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_8c5d02eb","line":4300,"updated":"2019-09-10 12:58:42.000000000","message":"*nods*","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":4302,"context_line":"                # port mapping as we have to have the neutron ports allocate"},{"line_number":4303,"context_line":"                # from the source compute after revert."},{"line_number":4304,"context_line":"                allocs \u003d self.reportclient.get_allocations_for_consumer("},{"line_number":4305,"context_line":"                    context, instance.uuid)"},{"line_number":4306,"context_line":"                scheduler_utils.fill_provider_mapping_based_on_allocation("},{"line_number":4307,"context_line":"                    context, self.reportclient, request_spec, allocs)"},{"line_number":4308,"context_line":"                provider_mappings \u003d self._get_request_group_mapping("}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_ac055ec4","line":4305,"updated":"2019-09-10 12:58:42.000000000","message":"that\u0027s a bit unfortunate we need to call again the placement API, but okay.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aedc63fa3f8a9ad2d9d5c7838e6786211c9cbb8c","unresolved":false,"context_lines":[{"line_number":4302,"context_line":"                # port mapping as we have to have the neutron ports allocate"},{"line_number":4303,"context_line":"                # from the source compute after revert."},{"line_number":4304,"context_line":"                allocs \u003d self.reportclient.get_allocations_for_consumer("},{"line_number":4305,"context_line":"                    context, instance.uuid)"},{"line_number":4306,"context_line":"                scheduler_utils.fill_provider_mapping_based_on_allocation("},{"line_number":4307,"context_line":"                    context, self.reportclient, request_spec, allocs)"},{"line_number":4308,"context_line":"                provider_mappings \u003d self._get_request_group_mapping("}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_4f1dbc0d","line":4305,"in_reply_to":"5faad753_ac055ec4","updated":"2019-09-10 13:06:53.000000000","message":"There is a separate patch higher in the series that removes the TODO. https://review.opendev.org/#/c/678827/","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":4313,"context_line":"                # the provider mappings. If the instance has ports with"},{"line_number":4314,"context_line":"                # resource request then the port update will fail in"},{"line_number":4315,"context_line":"                # _update_port_binding_for_instance() called via"},{"line_number":4316,"context_line":"                # _finish_revert_resize_network_migrate_finish() below."},{"line_number":4317,"context_line":"                provider_mappings \u003d None"},{"line_number":4318,"context_line":""},{"line_number":4319,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_2c22ce53","line":4316,"updated":"2019-09-10 12:58:42.000000000","message":"*nods*","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fbc7c8ffe11e86de6ddc48439044a927dfc40b5b","unresolved":false,"context_lines":[{"line_number":4314,"context_line":"                # resource request then the port update will fail in"},{"line_number":4315,"context_line":"                # _update_port_binding_for_instance() called via"},{"line_number":4316,"context_line":"                # _finish_revert_resize_network_migrate_finish() below."},{"line_number":4317,"context_line":"                provider_mappings \u003d None"},{"line_number":4318,"context_line":""},{"line_number":4319,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"},{"line_number":4320,"context_line":"                                                    migration.source_compute)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_5e37abb1","line":4317,"updated":"2019-09-10 15:48:33.000000000","message":"I don\u0027t love the tight coupling to something so far removed from this code but it works:\n\nhttps://github.com/openstack/nova/blob/f4ca3e70852c0a7ed7904a9f2d7177c9118d3d1c/nova/compute/manager.py#L4243\n\nhttps://github.com/openstack/nova/blob/f4ca3e70852c0a7ed7904a9f2d7177c9118d3d1c/nova/network/neutronv2/api.py#L2780\n\nhttps://github.com/openstack/nova/blob/f4ca3e70852c0a7ed7904a9f2d7177c9118d3d1c/nova/network/neutronv2/api.py#L3322\n\nAnd this really shouldn\u0027t happen anyway, right? Meaning if our min compute and pinning checks are in place, a resize of a server with ports with requested resources shouldn\u0027t be allowed to even get this far, so to get here, the operator would have had to do something like upgrade the computes to Train with compute rpc unpinned, the user performs a resize, then the compute rpc is pinned and then the user tries to revert the resize and things blow up. That shouldn\u0027t happen unless someone screws up the upgrade.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b0aa80ac657279df729b4e6c0bdca72a236ba7ae","unresolved":false,"context_lines":[{"line_number":4314,"context_line":"                # resource request then the port update will fail in"},{"line_number":4315,"context_line":"                # _update_port_binding_for_instance() called via"},{"line_number":4316,"context_line":"                # _finish_revert_resize_network_migrate_finish() below."},{"line_number":4317,"context_line":"                provider_mappings \u003d None"},{"line_number":4318,"context_line":""},{"line_number":4319,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"},{"line_number":4320,"context_line":"                                                    migration.source_compute)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_11459817","line":4317,"in_reply_to":"5faad753_046fa292","updated":"2019-09-11 15:01:37.000000000","message":"Now I got it. The difference is that the problem what grenade hit is that RequestSpec is None. While the problem that could not happen is that the RequestSpec is None _and_ we would need a non empty provider_mapping. The second cannot happen as that would fail the forward move already.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a2d7b5f931e0e3005e2336ad6abe3b575036ed24","unresolved":false,"context_lines":[{"line_number":4314,"context_line":"                # resource request then the port update will fail in"},{"line_number":4315,"context_line":"                # _update_port_binding_for_instance() called via"},{"line_number":4316,"context_line":"                # _finish_revert_resize_network_migrate_finish() below."},{"line_number":4317,"context_line":"                provider_mappings \u003d None"},{"line_number":4318,"context_line":""},{"line_number":4319,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"},{"line_number":4320,"context_line":"                                                    migration.source_compute)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_e121184f","line":4317,"in_reply_to":"5faad753_5e37abb1","updated":"2019-09-10 16:42:18.000000000","message":"But it happened in the grenade run last night. So either grande does not do the upgrade well. Or we are mistaken.\n\n\nI don\u0027t think this is really high coupling. We don\u0027t get the request spec so we don\u0027t calculate the pinning. Later on if the pinning would be needed but it is not provided then we blow. The _update_port_binding_for_instance() has no problem with provider_mapping\u003dNone if there is no port with resource request attached to the instance. \n\nI think what this solution does is it only enforces that there is a provider_mapping if that is really needed.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"060fb2dc53cf9f2004d43a70de2e0e3c030f21b7","unresolved":false,"context_lines":[{"line_number":4314,"context_line":"                # resource request then the port update will fail in"},{"line_number":4315,"context_line":"                # _update_port_binding_for_instance() called via"},{"line_number":4316,"context_line":"                # _finish_revert_resize_network_migrate_finish() below."},{"line_number":4317,"context_line":"                provider_mappings \u003d None"},{"line_number":4318,"context_line":""},{"line_number":4319,"context_line":"            self.network_api.setup_networks_on_host(context, instance,"},{"line_number":4320,"context_line":"                                                    migration.source_compute)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_046fa292","line":4317,"in_reply_to":"5faad753_e121184f","updated":"2019-09-10 16:48:44.000000000","message":"I meant we shouldn\u0027t hit the PortUpdateFailed because we shouldn\u0027t be migrating an instance with a port that has resource requests to begin with. The grenade failure was correct because nova is configured (in grenade) with [upgrade_levels]/compute\u003dauto and since there is a stein compute, it adjusts and pins the compute rpc api version to stein, so that\u0027s why on revert the request spec wasn\u0027t passed down to the compute and we blew up.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":6613,"context_line":""},{"line_number":6614,"context_line":"        migration_uuid \u003d self.get_migration_uuid_for_instance(server[\u0027id\u0027])"},{"line_number":6615,"context_line":""},{"line_number":6616,"context_line":"        # check that server is allocates from the new host properly"},{"line_number":6617,"context_line":"        self._check_allocation("},{"line_number":6618,"context_line":"            server, self.compute2_rp_uuid, non_qos_port, qos_port,"},{"line_number":6619,"context_line":"            migration_uuid, source_compute_rp_uuid\u003dself.compute1_rp_uuid)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_cf21baa1","line":6616,"range":{"start_line":6616,"start_character":28,"end_line":6616,"end_character":30},"updated":"2019-09-09 22:34:28.000000000","message":"nix","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":6613,"context_line":""},{"line_number":6614,"context_line":"        migration_uuid \u003d self.get_migration_uuid_for_instance(server[\u0027id\u0027])"},{"line_number":6615,"context_line":""},{"line_number":6616,"context_line":"        # check that server is allocates from the new host properly"},{"line_number":6617,"context_line":"        self._check_allocation("},{"line_number":6618,"context_line":"            server, self.compute2_rp_uuid, non_qos_port, qos_port,"},{"line_number":6619,"context_line":"            migration_uuid, source_compute_rp_uuid\u003dself.compute1_rp_uuid)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_9c845960","line":6616,"range":{"start_line":6616,"start_character":28,"end_line":6616,"end_character":30},"in_reply_to":"5faad753_cf21baa1","updated":"2019-09-10 09:03:51.000000000","message":"Done","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":6620,"context_line":""},{"line_number":6621,"context_line":"        self.api.post_server_action(server[\u0027id\u0027], {\u0027revertResize\u0027: None})"},{"line_number":6622,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":6623,"context_line":"        self._wait_for_migration_status(server, [\u0027reverted\u0027])"},{"line_number":6624,"context_line":""},{"line_number":6625,"context_line":"        # check that allocation is moved back to the source host"},{"line_number":6626,"context_line":"        self._check_allocation("}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_0f4d9271","line":6623,"updated":"2019-09-09 22:34:28.000000000","message":"nit: this is likely not necessary and would be racy if you didn\u0027t also first wait for the server status to be ACTIVE because the migration.status is changed to \u0027reverted\u0027 on the dest revert_resize method before casting to finish_revert_resize on the source. Anyway, you can leave it or not, up to you.","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":6620,"context_line":""},{"line_number":6621,"context_line":"        self.api.post_server_action(server[\u0027id\u0027], {\u0027revertResize\u0027: None})"},{"line_number":6622,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":6623,"context_line":"        self._wait_for_migration_status(server, [\u0027reverted\u0027])"},{"line_number":6624,"context_line":""},{"line_number":6625,"context_line":"        # check that allocation is moved back to the source host"},{"line_number":6626,"context_line":"        self._check_allocation("}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_5cf3410b","line":6623,"in_reply_to":"5faad753_0f4d9271","updated":"2019-09-10 09:03:51.000000000","message":"Hm. This would create a bad exmaple. Removed.","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d911ff5d2e3fffbdcc53bc6b3ea351496eac8e5","unresolved":false,"context_lines":[{"line_number":6635,"context_line":"        self.assertEqual({}, migration_allocations)"},{"line_number":6636,"context_line":""},{"line_number":6637,"context_line":"        self._delete_server_and_check_allocations(qos_port, server)"},{"line_number":6638,"context_line":""},{"line_number":6639,"context_line":""},{"line_number":6640,"context_line":"class PortResourceRequestReSchedulingTest("},{"line_number":6641,"context_line":"        PortResourceRequestBasedSchedulingTestBase):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_0c73524f","line":6638,"updated":"2019-09-10 12:58:42.000000000","message":"shouldn\u0027t we also test a resize revert with a specific older compute version so we would see that it\u0027s not updating the provider mappings ?","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aedc63fa3f8a9ad2d9d5c7838e6786211c9cbb8c","unresolved":false,"context_lines":[{"line_number":6635,"context_line":"        self.assertEqual({}, migration_allocations)"},{"line_number":6636,"context_line":""},{"line_number":6637,"context_line":"        self._delete_server_and_check_allocations(qos_port, server)"},{"line_number":6638,"context_line":""},{"line_number":6639,"context_line":""},{"line_number":6640,"context_line":"class PortResourceRequestReSchedulingTest("},{"line_number":6641,"context_line":"        PortResourceRequestBasedSchedulingTestBase):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_6f0f58e1","line":6638,"in_reply_to":"5faad753_0c73524f","updated":"2019-09-10 13:06:53.000000000","message":"See my reply for Matt in: https://review.opendev.org/#/c/676140/18/nova/compute/manager.py@4303 \n\nI can try to build such a functional test if needed and we will see how hackish it will be. But I\u0027d like to do that as a follow up. We have grenade coverage (that actually caught the problem).","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"39dca363dc46dc8e32ac5dfe9a5a5516d2bb55da","unresolved":false,"context_lines":[{"line_number":6635,"context_line":"        self.assertEqual({}, migration_allocations)"},{"line_number":6636,"context_line":""},{"line_number":6637,"context_line":"        self._delete_server_and_check_allocations(qos_port, server)"},{"line_number":6638,"context_line":""},{"line_number":6639,"context_line":""},{"line_number":6640,"context_line":"class PortResourceRequestReSchedulingTest("},{"line_number":6641,"context_line":"        PortResourceRequestBasedSchedulingTestBase):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5faad753_4f84dcb1","line":6638,"in_reply_to":"5faad753_6f0f58e1","updated":"2019-09-10 13:16:13.000000000","message":"I\u0027m okay with having grenade covering this as of now, but yeah I think a follow-up change would be nice to make sure we don\u0027t miss anything.","commit_id":"981e999b94fc4a2879f9765110550b7c0cf701e0"}],"nova/tests/unit/scheduler/test_utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4efc75c54209cc62a1ec29cff2ce90896ca92108","unresolved":false,"context_lines":[{"line_number":1164,"context_line":"            utils.get_weight_multiplier(host1, \u0027cpu_weight_multiplier\u0027, 1.0)"},{"line_number":1165,"context_line":"        )"},{"line_number":1166,"context_line":""},{"line_number":1167,"context_line":"    @mock.patch(\u0027nova.scheduler.utils.\u0027"},{"line_number":1168,"context_line":"                \u0027fill_provider_mapping_based_on_allocation\u0027)"},{"line_number":1169,"context_line":"    def test_fill_provider_mapping_returns_early_if_nothing_to_do("},{"line_number":1170,"context_line":"            self, mock_fill_provider):"},{"line_number":1171,"context_line":"        context \u003d nova_context.RequestContext()"},{"line_number":1172,"context_line":"        request_spec \u003d objects.RequestSpec()"},{"line_number":1173,"context_line":"        # set up the request that there is nothing to do"},{"line_number":1174,"context_line":"        request_spec.requested_resources \u003d []"},{"line_number":1175,"context_line":"        report_client \u003d mock.sentinel.report_client"},{"line_number":1176,"context_line":"        selection \u003d objects.Selection()"},{"line_number":1177,"context_line":""},{"line_number":1178,"context_line":"        utils.fill_provider_mapping("},{"line_number":1179,"context_line":"            context, report_client, request_spec, selection)"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"        mock_fill_provider.assert_not_called()"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    @mock.patch(\u0027nova.scheduler.utils.\u0027"},{"line_number":1184,"context_line":"                \u0027fill_provider_mapping_based_on_allocation\u0027)"},{"line_number":1185,"context_line":"    def test_fill_provider_mapping(self, mock_fill_provider):"},{"line_number":1186,"context_line":"        context \u003d nova_context.RequestContext()"},{"line_number":1187,"context_line":"        request_spec \u003d objects.RequestSpec()"},{"line_number":1188,"context_line":"        request_spec.requested_resources \u003d [objects.RequestGroup()]"},{"line_number":1189,"context_line":"        report_client \u003d mock.sentinel.report_client"},{"line_number":1190,"context_line":"        allocs \u003d {"},{"line_number":1191,"context_line":"            uuids.rp_uuid: {"},{"line_number":1192,"context_line":"                \u0027resources\u0027: {"},{"line_number":1193,"context_line":"                    \u0027NET_BW_EGR_KILOBIT_PER_SEC\u0027: 1,"},{"line_number":1194,"context_line":"                }"},{"line_number":1195,"context_line":"            }"},{"line_number":1196,"context_line":"        }"},{"line_number":1197,"context_line":"        allocation_req \u003d {\u0027allocations\u0027: allocs}"},{"line_number":1198,"context_line":"        selection \u003d objects.Selection("},{"line_number":1199,"context_line":"            allocation_request\u003djsonutils.dumps(allocation_req))"},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":"        utils.fill_provider_mapping("},{"line_number":1202,"context_line":"            context, report_client, request_spec, selection)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        mock_fill_provider.assert_called_once_with("},{"line_number":1205,"context_line":"            context, report_client, request_spec, allocs)"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    @mock.patch.object(objects.RequestSpec,"},{"line_number":1208,"context_line":"                       \u0027map_requested_resources_to_providers\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_cf7d3a6f","line":1205,"range":{"start_line":1167,"start_character":0,"end_line":1205,"end_character":57},"updated":"2019-09-09 22:34:28.000000000","message":"I\u0027m surprised we didn\u0027t already have unit tests for this existing method but I don\u0027t see any direct tests for this, but surely it\u0027s already tested indirectly somewhere? Maybe functional tests?","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d5a8651eac9e55750f862538f833a2c733de744b","unresolved":false,"context_lines":[{"line_number":1164,"context_line":"            utils.get_weight_multiplier(host1, \u0027cpu_weight_multiplier\u0027, 1.0)"},{"line_number":1165,"context_line":"        )"},{"line_number":1166,"context_line":""},{"line_number":1167,"context_line":"    @mock.patch(\u0027nova.scheduler.utils.\u0027"},{"line_number":1168,"context_line":"                \u0027fill_provider_mapping_based_on_allocation\u0027)"},{"line_number":1169,"context_line":"    def test_fill_provider_mapping_returns_early_if_nothing_to_do("},{"line_number":1170,"context_line":"            self, mock_fill_provider):"},{"line_number":1171,"context_line":"        context \u003d nova_context.RequestContext()"},{"line_number":1172,"context_line":"        request_spec \u003d objects.RequestSpec()"},{"line_number":1173,"context_line":"        # set up the request that there is nothing to do"},{"line_number":1174,"context_line":"        request_spec.requested_resources \u003d []"},{"line_number":1175,"context_line":"        report_client \u003d mock.sentinel.report_client"},{"line_number":1176,"context_line":"        selection \u003d objects.Selection()"},{"line_number":1177,"context_line":""},{"line_number":1178,"context_line":"        utils.fill_provider_mapping("},{"line_number":1179,"context_line":"            context, report_client, request_spec, selection)"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"        mock_fill_provider.assert_not_called()"},{"line_number":1182,"context_line":""},{"line_number":1183,"context_line":"    @mock.patch(\u0027nova.scheduler.utils.\u0027"},{"line_number":1184,"context_line":"                \u0027fill_provider_mapping_based_on_allocation\u0027)"},{"line_number":1185,"context_line":"    def test_fill_provider_mapping(self, mock_fill_provider):"},{"line_number":1186,"context_line":"        context \u003d nova_context.RequestContext()"},{"line_number":1187,"context_line":"        request_spec \u003d objects.RequestSpec()"},{"line_number":1188,"context_line":"        request_spec.requested_resources \u003d [objects.RequestGroup()]"},{"line_number":1189,"context_line":"        report_client \u003d mock.sentinel.report_client"},{"line_number":1190,"context_line":"        allocs \u003d {"},{"line_number":1191,"context_line":"            uuids.rp_uuid: {"},{"line_number":1192,"context_line":"                \u0027resources\u0027: {"},{"line_number":1193,"context_line":"                    \u0027NET_BW_EGR_KILOBIT_PER_SEC\u0027: 1,"},{"line_number":1194,"context_line":"                }"},{"line_number":1195,"context_line":"            }"},{"line_number":1196,"context_line":"        }"},{"line_number":1197,"context_line":"        allocation_req \u003d {\u0027allocations\u0027: allocs}"},{"line_number":1198,"context_line":"        selection \u003d objects.Selection("},{"line_number":1199,"context_line":"            allocation_request\u003djsonutils.dumps(allocation_req))"},{"line_number":1200,"context_line":""},{"line_number":1201,"context_line":"        utils.fill_provider_mapping("},{"line_number":1202,"context_line":"            context, report_client, request_spec, selection)"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        mock_fill_provider.assert_called_once_with("},{"line_number":1205,"context_line":"            context, report_client, request_spec, allocs)"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"    @mock.patch.object(objects.RequestSpec,"},{"line_number":1208,"context_line":"                       \u0027map_requested_resources_to_providers\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5faad753_9c60d9a7","line":1205,"range":{"start_line":1167,"start_character":0,"end_line":1205,"end_character":57},"in_reply_to":"5faad753_cf7d3a6f","updated":"2019-09-10 09:03:51.000000000","message":"There was some branching to introduce the utils.fill_provider_mapping_based_on_allocation() so I added extra coverage.","commit_id":"a89b491c7749e80847347e6bcd834273758a2182"}]}