)]}'
{"nova/conductor/tasks/live_migrate.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":356,"context_line":"            return {}"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    @staticmethod"},{"line_number":359,"context_line":"    def _get_port_profile_form_provider_mapping(port_id, provider_mappings):"},{"line_number":360,"context_line":"        if port_id in provider_mappings:"},{"line_number":361,"context_line":"            # NOTE(gibi): In the resource provider mapping there can be"},{"line_number":362,"context_line":"            # more than one RP fulfilling a request group. But resource"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_93f23105","line":359,"range":{"start_line":359,"start_character":26,"end_line":359,"end_character":30},"updated":"2019-12-16 19:57:17.000000000","message":"from","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":356,"context_line":"            return {}"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    @staticmethod"},{"line_number":359,"context_line":"    def _get_port_profile_form_provider_mapping(port_id, provider_mappings):"},{"line_number":360,"context_line":"        if port_id in provider_mappings:"},{"line_number":361,"context_line":"            # NOTE(gibi): In the resource provider mapping there can be"},{"line_number":362,"context_line":"            # more than one RP fulfilling a request group. But resource"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_0769ed8a","line":359,"range":{"start_line":359,"start_character":26,"end_line":359,"end_character":30},"in_reply_to":"3fa7e38b_93f23105","updated":"2019-12-19 13:17:28.000000000","message":"Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":383,"context_line":"            # perform port binding against the requested profile"},{"line_number":384,"context_line":"            ports_profile \u003d {}"},{"line_number":385,"context_line":"            for mig_vif in self.migrate_data.vifs:"},{"line_number":386,"context_line":"                profile \u003d self._get_port_profile_from_migration(mig_vif)"},{"line_number":387,"context_line":"                # NOTE(gibi): provider_mappings also contribute to the"},{"line_number":388,"context_line":"                # binding profile of the ports if the port has resource"},{"line_number":389,"context_line":"                # request. So we need to merge the profile information from"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_538fd975","line":386,"updated":"2019-12-16 19:57:17.000000000","message":"nit: this could just be:\n\nprofile \u003d mig_vif.profile if \u0027profile_json\u0027 in mig_vif else {}\n\nThen you don\u0027t need the extra _get_port_profile_from_migration method.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":383,"context_line":"            # perform port binding against the requested profile"},{"line_number":384,"context_line":"            ports_profile \u003d {}"},{"line_number":385,"context_line":"            for mig_vif in self.migrate_data.vifs:"},{"line_number":386,"context_line":"                profile \u003d self._get_port_profile_from_migration(mig_vif)"},{"line_number":387,"context_line":"                # NOTE(gibi): provider_mappings also contribute to the"},{"line_number":388,"context_line":"                # binding profile of the ports if the port has resource"},{"line_number":389,"context_line":"                # request. So we need to merge the profile information from"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_a75819f9","line":386,"in_reply_to":"3fa7e38b_538fd975","updated":"2019-12-19 13:17:28.000000000","message":"Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                # binding profile of the ports if the port has resource"},{"line_number":389,"context_line":"                # request. So we need to merge the profile information from"},{"line_number":390,"context_line":"                # both sources."},{"line_number":391,"context_line":"                profile.update("},{"line_number":392,"context_line":"                    self._get_port_profile_form_provider_mapping("},{"line_number":393,"context_line":"                        mig_vif.port_id, provider_mappings))"},{"line_number":394,"context_line":"                if profile:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_53bd9901","line":391,"range":{"start_line":391,"start_character":24,"end_line":391,"end_character":30},"updated":"2019-12-16 19:57:17.000000000","message":"Yeah. I\u0027m reminded of the heal_allocations bug where the port\u0027s binding profile was being overwritten rather than updated and when I first looked at _get_port_profile_form_provider_mapping that\u0027s what I thought you were going to be doing but then noticed the update().","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                # binding profile of the ports if the port has resource"},{"line_number":389,"context_line":"                # request. So we need to merge the profile information from"},{"line_number":390,"context_line":"                # both sources."},{"line_number":391,"context_line":"                profile.update("},{"line_number":392,"context_line":"                    self._get_port_profile_form_provider_mapping("},{"line_number":393,"context_line":"                        mig_vif.port_id, provider_mappings))"},{"line_number":394,"context_line":"                if profile:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_870abdce","line":391,"range":{"start_line":391,"start_character":24,"end_line":391,"end_character":30},"in_reply_to":"3fa7e38b_53bd9901","updated":"2019-12-19 13:17:28.000000000","message":"yeah the whole reason of the rewrite of_bind_ports_on_destination and the code comment above is to state that we have now multiple sources of profile information that needs to be merged.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":461,"context_line":"        # similar non-nova resources then here we have to collect"},{"line_number":462,"context_line":"        # all the external resource requests in a single list and"},{"line_number":463,"context_line":"        # add them to the RequestSpec."},{"line_number":464,"context_line":"        request_spec.requested_resources \u003d port_res_req"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"        scheduler_utils.setup_instance_group(self.context, request_spec)"},{"line_number":467,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_735455c7","line":464,"updated":"2019-12-16 19:57:17.000000000","message":"Yup same as resize/cold migrate:\n\nhttps://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/conductor/tasks/migrate.py#L350-L355","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":515,"context_line":""},{"line_number":516,"context_line":"            scheduler_utils.fill_provider_mapping(request_spec, selection)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"            provider_mapping \u003d request_spec.get_request_group_mapping()"},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            if provider_mapping:"},{"line_number":521,"context_line":"                compute_utils.\\"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_b3ff2db0","line":518,"range":{"start_line":518,"start_character":44,"end_line":518,"end_character":69},"updated":"2019-12-16 19:57:17.000000000","message":"I should have noticed this before when reviewing the refactor to add this method to the RequestSpec, but noticed now with the comment in _get_port_profile_form_provider_mapping - the get_request_group_mapping docstring says the return value is:\n\n\"A dict keyed by RequestGroup requester_id, currently Neutron\nport_id, to resource provider UUID that provides resource for that RequestGroup.\"\n\nBut it\u0027s not a 1:1 dict, it\u0027s a 1:\u003clist\u003e where the list in the neutron port case has 1 entry. So we should get that docstring updated at some point.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":515,"context_line":""},{"line_number":516,"context_line":"            scheduler_utils.fill_provider_mapping(request_spec, selection)"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"            provider_mapping \u003d request_spec.get_request_group_mapping()"},{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            if provider_mapping:"},{"line_number":521,"context_line":"                compute_utils.\\"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_07fecdae","line":518,"range":{"start_line":518,"start_character":44,"end_line":518,"end_character":69},"in_reply_to":"3fa7e38b_b3ff2db0","updated":"2019-12-19 13:17:28.000000000","message":"Good point. I will fix that up in an independent commit.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            if provider_mapping:"},{"line_number":521,"context_line":"                compute_utils.\\"},{"line_number":522,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":523,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":524,"context_line":"                    provider_mapping)"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_93aa3182","line":522,"range":{"start_line":522,"start_character":20,"end_line":522,"end_character":73},"updated":"2019-12-16 19:57:17.000000000","message":"So this actually updates instance.pci_requests and saves the changes to the DB per host that we try.\n\nWhat happens if we eventually can\u0027t find any suitable host due to all of the pre-checks failing and the task raising MaxRetriesExceeded? Shouldn\u0027t we save off the instance.pci_requests early in the task and on rollback set them back onto the instance? Otherwise we could have a modified instance with updated pci_requests for a dest host it never moved it, right?\n\nI remember from the code review on moving that refactored method to compute.utils that Dan suggested we move the actual instance.save() from that method to the caller so it has context and can determine if the instance should be updated, which it easily can do with something like this:\n\nif instance.obj_what_changed():\n    instance.save()\n\nPoint being that could be useful here because we could opt to only save the changes to the instance once we\u0027ve selected a host and will exit this method. But regardless of that it\u0027s probably worthwhile to just save off the instance.pci_requests early in the task and re-apply them on rollback in case they happened to change during host selection.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"94733ae70769c341fbd25ddafea919e2b2eb99d6","unresolved":false,"context_lines":[{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            if provider_mapping:"},{"line_number":521,"context_line":"                compute_utils.\\"},{"line_number":522,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":523,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":524,"context_line":"                    provider_mapping)"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_a18c8280","line":522,"range":{"start_line":522,"start_character":20,"end_line":522,"end_character":73},"in_reply_to":"3fa7e38b_87d87d52","updated":"2019-12-19 15:21:11.000000000","message":"Rather than unit tests maybe functional tests for testing rollback would be cleaner? In general the unit tests on these conductor tasks are generally terrible because there is so much content crammed into each method (they really need to be broken down for unit testability) and that results in unit tests have more mocks than the actual code they are trying to test. That\u0027s why I\u0027ve generally just relied on functional testing for stuff like this in recent years than trying to unit test everything because it\u0027s hard to say if the test is even working properly or if we have false positives due to mocks.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            if provider_mapping:"},{"line_number":521,"context_line":"                compute_utils.\\"},{"line_number":522,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":523,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":524,"context_line":"                    provider_mapping)"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_87d87d52","line":522,"range":{"start_line":522,"start_character":20,"end_line":522,"end_character":73},"in_reply_to":"3fa7e38b_93aa3182","updated":"2019-12-19 13:17:28.000000000","message":"I did the rollback part of your comment here. I will try to do the surgery of moving the instance.save out of update_pci_request_spec_with_allocated_interface_name in a separate commit as that impacts multiple compute/manager calls as well.\n\n// later\n\nAdding unit test to the rollback change is a bit of a nightmare. I move the rollback code change to the re-schedule patch and I cover the code with functional test asserts there instead.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b78f345d6a20f9ba811027b43465f0575af0685d","unresolved":false,"context_lines":[{"line_number":519,"context_line":""},{"line_number":520,"context_line":"            if provider_mapping:"},{"line_number":521,"context_line":"                compute_utils.\\"},{"line_number":522,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":523,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":524,"context_line":"                    provider_mapping)"},{"line_number":525,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_7915d64b","line":522,"range":{"start_line":522,"start_character":20,"end_line":522,"end_character":73},"in_reply_to":"3fa7e38b_a18c8280","updated":"2019-12-20 09:40:21.000000000","message":"yepp. Covered in functional test here https://review.opendev.org/#/c/699015/4/nova/tests/functional/test_servers.py@7407","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":521,"context_line":"                compute_utils.\\"},{"line_number":522,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":523,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":524,"context_line":"                    provider_mapping)"},{"line_number":525,"context_line":"            try:"},{"line_number":526,"context_line":"                self._check_compatible_with_source_hypervisor(host)"},{"line_number":527,"context_line":"                self._call_livem_checks_on_host(host, provider_mapping)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_7309b5da","line":524,"updated":"2019-12-16 19:57:17.000000000","message":"nit: align this with the other args","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":521,"context_line":"                compute_utils.\\"},{"line_number":522,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":523,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":524,"context_line":"                    provider_mapping)"},{"line_number":525,"context_line":"            try:"},{"line_number":526,"context_line":"                self._check_compatible_with_source_hypervisor(host)"},{"line_number":527,"context_line":"                self._call_livem_checks_on_host(host, provider_mapping)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_47c7e547","line":524,"in_reply_to":"3fa7e38b_7309b5da","updated":"2019-12-19 13:17:28.000000000","message":"Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64c06e3397acd6728208dca42e335545315ab889","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            # resource provider. So we only pass that single RP UUID"},{"line_number":359,"context_line":"            # here."},{"line_number":360,"context_line":"            return {\u0027allocation\u0027: provider_mappings[port_id][0]}"},{"line_number":361,"context_line":"        else:"},{"line_number":362,"context_line":"            return {}"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":"    def _bind_ports_on_destination(self, destination, provider_mappings):"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_28fe1504","line":361,"range":{"start_line":361,"start_character":0,"end_line":361,"end_character":13},"updated":"2019-12-23 13:03:42.000000000","message":"nit: unnecessary","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b2090f0cdbd7bb975c157da1a1f31111626a1b95","unresolved":false,"context_lines":[{"line_number":512,"context_line":""},{"line_number":513,"context_line":"            if provider_mapping:"},{"line_number":514,"context_line":"                compute_utils.\\"},{"line_number":515,"context_line":"                    update_pci_request_spec_with_allocated_interface_name("},{"line_number":516,"context_line":"                        self.context, self.report_client, self.instance,"},{"line_number":517,"context_line":"                        provider_mapping)"},{"line_number":518,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_b646f421","line":515,"updated":"2019-12-20 15:46:16.000000000","message":"Would have been nice to have a TODO/FIXME or something here to make sure to rollback instance.pci_requests on failure so people reviewing this know it\u0027s coming up and should be handled, but I won\u0027t block on it.","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":3334,"context_line":"                  migration.get(\u0027status\u0027) \u003d\u003d \u0027reverted\u0027)"},{"line_number":3335,"context_line":"        return instance.migration_context.get_pci_mapping_for_migration(revert)"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    def _update_port_binding_for_instance("},{"line_number":3338,"context_line":"            self, context, instance, host, migration\u003dNone,"},{"line_number":3339,"context_line":"            provider_mappings\u003dNone):"},{"line_number":3340,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_d305097b","line":3337,"updated":"2019-12-16 19:57:17.000000000","message":"And this is called from migrate_instance_finish.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":3395,"context_line":"                        reason\u003d_(\"Unable to correlate PCI slot %s\") %"},{"line_number":3396,"context_line":"                                 pci_slot)"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3399,"context_line":"            # allocation key in the port binding"},{"line_number":3400,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3401,"context_line":"                    migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_9334714b","line":3398,"updated":"2019-12-16 19:57:17.000000000","message":"Is there any reason to not do that in conductor and just let this code update the dest host port binding? I guess the reason is that if we did that, then the provider mapping has to get from conductor (or be built in compute?) to here:\n\nhttps://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/compute/manager.py#L8511\n\nBut isn\u0027t that what we do during resize?\n\nhttps://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/compute/manager.py#L5706\n\nI guess resize is done that way since it doesn\u0027t use multiple port bindings like live migration does.\n\nSo I guess two things:\n\n1. If we are going to leave this code and let conductor update the dest host port binding profile, then shouldn\u0027t we remove the TODO at https://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/compute/manager.py#L8511 ?\n\n2. Is there a reason we can\u0027t make the compute manager code consistent with respect to when the dest host port binding profile gets updated so that we are consistent between live and cold migrate? Otherwise it\u0027s kind of weird to be doing it different ways and have this special case live-migration-only condition down in the bowels of the neutron API code.\n\nI\u0027m -1 for other smaller reasons on this patch, but this is something worth thinking through for long-term maintainability.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"94733ae70769c341fbd25ddafea919e2b2eb99d6","unresolved":false,"context_lines":[{"line_number":3395,"context_line":"                        reason\u003d_(\"Unable to correlate PCI slot %s\") %"},{"line_number":3396,"context_line":"                                 pci_slot)"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3399,"context_line":"            # allocation key in the port binding"},{"line_number":3400,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3401,"context_line":"                    migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_61968aa3","line":3398,"in_reply_to":"3fa7e38b_47ccc561","updated":"2019-12-19 15:21:11.000000000","message":"So tl;dr using multiple port bindings is the future so we want live migration to continue using that way rather than regress to how resize works today. I can buy that.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b78f345d6a20f9ba811027b43465f0575af0685d","unresolved":false,"context_lines":[{"line_number":3395,"context_line":"                        reason\u003d_(\"Unable to correlate PCI slot %s\") %"},{"line_number":3396,"context_line":"                                 pci_slot)"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3399,"context_line":"            # allocation key in the port binding"},{"line_number":3400,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3401,"context_line":"                    migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_9918d283","line":3398,"in_reply_to":"3fa7e38b_61968aa3","updated":"2019-12-20 09:40:21.000000000","message":"yepp","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":3395,"context_line":"                        reason\u003d_(\"Unable to correlate PCI slot %s\") %"},{"line_number":3396,"context_line":"                                 pci_slot)"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"            # NOTE(gibi): during live migration the conductor already sets the"},{"line_number":3399,"context_line":"            # allocation key in the port binding"},{"line_number":3400,"context_line":"            if (p.get(\u0027resource_request\u0027) and"},{"line_number":3401,"context_line":"                    migration[\u0027migration_type\u0027] !\u003d constants.LIVE_MIGRATION):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_47ccc561","line":3398,"in_reply_to":"3fa7e38b_9334714b","updated":"2019-12-19 13:17:28.000000000","message":"\u003e Is there any reason to not do that in conductor and just let this\n \u003e code update the dest host port binding? I guess the reason is that\n \u003e if we did that, then the provider mapping has to get from conductor\n \u003e (or be built in compute?) to here:\n \u003e \n \u003e https://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/compute/manager.py#L8511\n \u003e \n \u003e But isn\u0027t that what we do during resize?\n \u003e \n \u003e https://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/compute/manager.py#L5706\n \u003e \n \u003e I guess resize is done that way since it doesn\u0027t use multiple port\n \u003e bindings like live migration does.\n \u003e \n \u003e So I guess two things:\n \u003e \n \u003e 1. If we are going to leave this code and let conductor update the\n \u003e dest host port binding profile, then shouldn\u0027t we remove the TODO\n \u003e at https://github.com/openstack/nova/blob/d052e72040258b6ab91f53b43e8d45b330b898e1/nova/compute/manager.py#L8511\n \u003e ?\n\nDone.\n\n \u003e \n \u003e 2. Is there a reason we can\u0027t make the compute manager code\n \u003e consistent with respect to when the dest host port binding profile\n \u003e gets updated so that we are consistent between live and cold\n \u003e migrate? Otherwise it\u0027s kind of weird to be doing it different ways\n \u003e and have this special case live-migration-only condition down in\n \u003e the bowels of the neutron API code.\n\nTo avoid the recalculation of the mapping in case of revert resize we anyhow need to change the resize code path to use multiple port binding. That way the conductor can set the binding both for cold and for live migration. This could remove the difference between the two cases here and solve the revert resize problem at the same time.\n\nAs a secondary reason. From the start we planned to use the multiple binding to hold the source allocation key in the source binding in live migration case so I did not add the request_spec to the post_live_migration_at_destination when the RPC was bumped for the qos move operations. So now it would need another RPC bump to pass down the request spec.\n\n \u003e \n \u003e I\u0027m -1 for other smaller reasons on this patch, but this is\n \u003e something worth thinking through for long-term maintainability.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":7291,"context_line":"    # TODO(gibi): add tests for live migration cases:"},{"line_number":7292,"context_line":"    # * with target host"},{"line_number":7293,"context_line":"    # * re-schedule success"},{"line_number":7294,"context_line":"    # * re-schedule fail -\u003e pci request cleanup?"},{"line_number":7295,"context_line":"    # * abort / cancel -\u003e dest / pci request cleanup?"},{"line_number":7296,"context_line":""},{"line_number":7297,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_134721a6","line":7294,"range":{"start_line":7294,"start_character":28,"end_line":7294,"end_character":48},"updated":"2019-12-16 19:57:17.000000000","message":"This is probably what I was getting at with my concerns in the conductor task code.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":7291,"context_line":"    # TODO(gibi): add tests for live migration cases:"},{"line_number":7292,"context_line":"    # * with target host"},{"line_number":7293,"context_line":"    # * re-schedule success"},{"line_number":7294,"context_line":"    # * re-schedule fail -\u003e pci request cleanup?"},{"line_number":7295,"context_line":"    # * abort / cancel -\u003e dest / pci request cleanup?"},{"line_number":7296,"context_line":""},{"line_number":7297,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_27d189f9","line":7294,"range":{"start_line":7294,"start_character":28,"end_line":7294,"end_character":48},"in_reply_to":"3fa7e38b_134721a6","updated":"2019-12-19 13:17:28.000000000","message":"sort of. I will add a unit test now and add an extra assert to the functional test in the later patch that covers the re-schedul fail case.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64c06e3397acd6728208dca42e335545315ab889","unresolved":false,"context_lines":[{"line_number":7266,"context_line":"            server, self.compute1_rp_uuid, non_qos_normal_port,"},{"line_number":7267,"context_line":"            qos_normal_port, qos_sriov_port, self.flavor_with_group_policy)"},{"line_number":7268,"context_line":""},{"line_number":7269,"context_line":"        self.api.post_server_action("},{"line_number":7270,"context_line":"            server[\u0027id\u0027],"},{"line_number":7271,"context_line":"            {"},{"line_number":7272,"context_line":"                \u0027os-migrateLive\u0027: {"},{"line_number":7273,"context_line":"                    \u0027host\u0027: None,"},{"line_number":7274,"context_line":"                    \u0027block_migration\u0027: \u0027auto\u0027"},{"line_number":7275,"context_line":"                }"},{"line_number":7276,"context_line":"            }"},{"line_number":7277,"context_line":"        )"},{"line_number":7278,"context_line":""},{"line_number":7279,"context_line":"        self._wait_for_server_parameter("},{"line_number":7280,"context_line":"            server,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_48183175","line":7277,"range":{"start_line":7269,"start_character":0,"end_line":7277,"end_character":9},"updated":"2019-12-23 13:03:42.000000000","message":"weird that we don\u0027t have a helper for this in \u0027nova/tests/functional/api/client.py\u0027","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"}],"nova/tests/unit/conductor/tasks/test_live_migrate.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":628,"context_line":"            self.context, self.fake_spec, [self.instance.uuid],"},{"line_number":629,"context_line":"            return_objects\u003dTrue, return_alternates\u003dFalse)"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"    @mock.patch("},{"line_number":632,"context_line":"        \u0027nova.network.neutronv2.api.API.get_requested_resource_for_instance\u0027,"},{"line_number":633,"context_line":"        return_value\u003d[])"},{"line_number":634,"context_line":"    @mock.patch(\"nova.utils.get_image_from_system_metadata\")"},{"line_number":635,"context_line":"    @mock.patch(\"nova.scheduler.utils.build_request_spec\")"},{"line_number":636,"context_line":"    @mock.patch(\"nova.scheduler.utils.setup_instance_group\")"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_73e455a9","line":633,"range":{"start_line":631,"start_character":4,"end_line":633,"end_character":24},"updated":"2019-12-16 19:57:17.000000000","message":"This seems like a lot of copy/paste for something we don\u0027t care about in most tests. Maybe consider mocking this in the test class setUp and only override for tests that actually care about the return value of the mock? Similar to heal_reqspec_is_bfv and ensure_network_metadata.","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":628,"context_line":"            self.context, self.fake_spec, [self.instance.uuid],"},{"line_number":629,"context_line":"            return_objects\u003dTrue, return_alternates\u003dFalse)"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"    @mock.patch("},{"line_number":632,"context_line":"        \u0027nova.network.neutronv2.api.API.get_requested_resource_for_instance\u0027,"},{"line_number":633,"context_line":"        return_value\u003d[])"},{"line_number":634,"context_line":"    @mock.patch(\"nova.utils.get_image_from_system_metadata\")"},{"line_number":635,"context_line":"    @mock.patch(\"nova.scheduler.utils.build_request_spec\")"},{"line_number":636,"context_line":"    @mock.patch(\"nova.scheduler.utils.setup_instance_group\")"}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_a20c8793","line":633,"range":{"start_line":631,"start_character":4,"end_line":633,"end_character":24},"in_reply_to":"3fa7e38b_73e455a9","updated":"2019-12-19 13:17:28.000000000","message":"Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":722,"context_line":""},{"line_number":723,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.bind_ports_to_host\u0027)"},{"line_number":724,"context_line":"    def test_bind_ports_on_destination_migration_data(self, mock_bind_ports):"},{"line_number":725,"context_line":"        \"\"\"Assert that if only the migration_data and contains binding profile"},{"line_number":726,"context_line":"        related information then that is sent to neutron."},{"line_number":727,"context_line":"        \"\"\""},{"line_number":728,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_f3f7e559","line":725,"range":{"start_line":725,"start_character":50,"end_line":725,"end_character":53},"updated":"2019-12-16 19:57:17.000000000","message":"nix","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":722,"context_line":""},{"line_number":723,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.bind_ports_to_host\u0027)"},{"line_number":724,"context_line":"    def test_bind_ports_on_destination_migration_data(self, mock_bind_ports):"},{"line_number":725,"context_line":"        \"\"\"Assert that if only the migration_data and contains binding profile"},{"line_number":726,"context_line":"        related information then that is sent to neutron."},{"line_number":727,"context_line":"        \"\"\""},{"line_number":728,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_827e2be1","line":725,"range":{"start_line":725,"start_character":50,"end_line":725,"end_character":53},"in_reply_to":"3fa7e38b_f3f7e559","updated":"2019-12-19 13:17:28.000000000","message":"Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":744,"context_line":""},{"line_number":745,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.bind_ports_to_host\u0027)"},{"line_number":746,"context_line":"    def test_bind_ports_on_destination_provider_mapping(self, mock_bind_ports):"},{"line_number":747,"context_line":"        \"\"\"Assert that if only the provider mapping and contains binding"},{"line_number":748,"context_line":"        profile related information then that is sent to neutron."},{"line_number":749,"context_line":"        \"\"\""},{"line_number":750,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_939eb104","line":747,"range":{"start_line":747,"start_character":52,"end_line":747,"end_character":55},"updated":"2019-12-16 19:57:17.000000000","message":"nix","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":744,"context_line":""},{"line_number":745,"context_line":"    @mock.patch(\u0027nova.network.neutronv2.api.API.bind_ports_to_host\u0027)"},{"line_number":746,"context_line":"    def test_bind_ports_on_destination_provider_mapping(self, mock_bind_ports):"},{"line_number":747,"context_line":"        \"\"\"Assert that if only the provider mapping and contains binding"},{"line_number":748,"context_line":"        profile related information then that is sent to neutron."},{"line_number":749,"context_line":"        \"\"\""},{"line_number":750,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_a281e7c4","line":747,"range":{"start_line":747,"start_character":52,"end_line":747,"end_character":55},"in_reply_to":"3fa7e38b_939eb104","updated":"2019-12-19 13:17:28.000000000","message":"Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14e9bd79533480ae03c8c1992562357e97fbe685","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        self.assertIsNotNone(self.fake_spec.requested_destination.cell)"},{"line_number":791,"context_line":"        # Make sure the spec was updated to include the project_id."},{"line_number":792,"context_line":"        self.assertEqual(self.fake_spec.project_id, self.instance.project_id)"},{"line_number":793,"context_line":"        # Make sure that requested_resources are added to the request spec"},{"line_number":794,"context_line":"        self.assertEqual("},{"line_number":795,"context_line":"            resource_req, self.task.request_spec.requested_resources)"},{"line_number":796,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_b3b88d78","line":793,"updated":"2019-12-16 19:57:17.000000000","message":"Isn\u0027t this redundant with test_get_request_spec_for_select_destinations_update_resource_req and if so, can\u0027t we just remove test_get_request_spec_for_select_destinations_update_resource_req?","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        self.assertIsNotNone(self.fake_spec.requested_destination.cell)"},{"line_number":791,"context_line":"        # Make sure the spec was updated to include the project_id."},{"line_number":792,"context_line":"        self.assertEqual(self.fake_spec.project_id, self.instance.project_id)"},{"line_number":793,"context_line":"        # Make sure that requested_resources are added to the request spec"},{"line_number":794,"context_line":"        self.assertEqual("},{"line_number":795,"context_line":"            resource_req, self.task.request_spec.requested_resources)"},{"line_number":796,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3fa7e38b_02f9db4c","line":793,"in_reply_to":"3fa7e38b_b3b88d78","updated":"2019-12-19 13:17:28.000000000","message":"You are right test_get_request_spec_for_select_destinations_update_resource covers a subset of code that also covered here. Done","commit_id":"1e7835a3531f41602c1b8be35ef30fd8e43ffa09"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d511d53e09c40098b224457e1158832c41a6af27","unresolved":false,"context_lines":[{"line_number":602,"context_line":"    @mock.patch(\"nova.scheduler.utils.setup_instance_group\")"},{"line_number":603,"context_line":"    @mock.patch(\"nova.objects.RequestSpec.from_primitives\")"},{"line_number":604,"context_line":"    def test_find_destination_with_remoteError("},{"line_number":605,"context_line":"            self, m_from_primitives, m_setup_instance_group,"},{"line_number":606,"context_line":"            m_build_request_spec, m_get_image_from_system_metadata):"},{"line_number":607,"context_line":"        m_get_image_from_system_metadata.return_value \u003d {\u0027properties\u0027: {}}"},{"line_number":608,"context_line":"        m_build_request_spec.return_value \u003d {}"}],"source_content_type":"text/x-python","patch_set":17,"id":"3fa7e38b_18b62a27","line":605,"updated":"2019-12-19 13:17:28.000000000","message":"unnecessary change","commit_id":"61a65e35b44864c0c911b5fd8eaf003815d23761"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3833d65a35d6b3f90e29cf81feeaa7dc8dc4a480","unresolved":false,"context_lines":[{"line_number":602,"context_line":"    @mock.patch(\"nova.scheduler.utils.setup_instance_group\")"},{"line_number":603,"context_line":"    @mock.patch(\"nova.objects.RequestSpec.from_primitives\")"},{"line_number":604,"context_line":"    def test_find_destination_with_remoteError("},{"line_number":605,"context_line":"            self, m_from_primitives, m_setup_instance_group,"},{"line_number":606,"context_line":"            m_build_request_spec, m_get_image_from_system_metadata):"},{"line_number":607,"context_line":"        m_get_image_from_system_metadata.return_value \u003d {\u0027properties\u0027: {}}"},{"line_number":608,"context_line":"        m_build_request_spec.return_value \u003d {}"}],"source_content_type":"text/x-python","patch_set":17,"id":"3fa7e38b_f8fd4eb2","line":605,"in_reply_to":"3fa7e38b_18b62a27","updated":"2019-12-19 13:20:53.000000000","message":"Done","commit_id":"61a65e35b44864c0c911b5fd8eaf003815d23761"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64c06e3397acd6728208dca42e335545315ab889","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        self.ensure_network_metadata_mock \u003d _p.start()"},{"line_number":76,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        _p \u003d mock.patch("},{"line_number":79,"context_line":"            \u0027nova.network.neutronv2.api.API.\u0027"},{"line_number":80,"context_line":"            \u0027get_requested_resource_for_instance\u0027,"},{"line_number":81,"context_line":"            return_value\u003d[])"},{"line_number":82,"context_line":"        self.mock_get_res_req \u003d _p.start()"},{"line_number":83,"context_line":"        self.addCleanup(_p.stop)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def _generate_task(self):"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_6813ad5a","line":82,"range":{"start_line":78,"start_character":0,"end_line":82,"end_character":42},"updated":"2019-12-23 13:03:42.000000000","message":"Any reason this logic doesn\u0027t live in the NeutronFixture?\n\nLater: wait, these are unit tests and we don\u0027t use NeutronFixture here (though we should probably think about it at this point)","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64c06e3397acd6728208dca42e335545315ab889","unresolved":false,"context_lines":[{"line_number":741,"context_line":"    @mock.patch.object(scheduler_utils, \u0027setup_instance_group\u0027)"},{"line_number":742,"context_line":"    def test_find_destination_with_resource_request("},{"line_number":743,"context_line":"            self, mock_setup, mock_reset, mock_select, mock_check, mock_call,"},{"line_number":744,"context_line":"            mock_fill_provider_mapping, mock_update_pci_req):"},{"line_number":745,"context_line":"        resource_req \u003d [objects.RequestGroup(requester_id\u003duuids.port_id)]"},{"line_number":746,"context_line":"        self.mock_get_res_req.return_value \u003d resource_req"},{"line_number":747,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_e820fd9c","line":744,"updated":"2019-12-23 13:03:42.000000000","message":"nit: docstring would be nice, though this is really just testing the basic flow","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64c06e3397acd6728208dca42e335545315ab889","unresolved":false,"context_lines":[{"line_number":745,"context_line":"        resource_req \u003d [objects.RequestGroup(requester_id\u003duuids.port_id)]"},{"line_number":746,"context_line":"        self.mock_get_res_req.return_value \u003d resource_req"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        self.assertEqual((\"host1\", \"node1\", fake_limits1),"},{"line_number":749,"context_line":"                         self.task._find_destination())"},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        # Make sure the request_spec was updated to include the cell"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_682c0d93","line":748,"range":{"start_line":748,"start_character":44,"end_line":748,"end_character":56},"updated":"2019-12-23 13:03:42.000000000","message":"This is an existing pattern, but it took me a while to figure out the source of this (it\u0027s the \u0027fake_selection1\u0027 Selection object, which is being returned by the \u0027select_destinations\u0027 mock above). A comment might be a good idea","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"}],"nova/tests/unit/network/test_neutronv2.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"64c06e3397acd6728208dca42e335545315ab889","unresolved":false,"context_lines":[{"line_number":4658,"context_line":"            \"are required for ports with a resource request.\","},{"line_number":4659,"context_line":"            six.text_type(ex))"},{"line_number":4660,"context_line":""},{"line_number":4661,"context_line":"    @mock.patch.object(neutronapi, \u0027get_client\u0027, return_value\u003dmock.Mock())"},{"line_number":4662,"context_line":"    def test_update_port_bindings_for_instance_with_resource_req_live_mig("},{"line_number":4663,"context_line":"            self, get_client_mock):"},{"line_number":4664,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa7e38b_c852e112","line":4661,"range":{"start_line":4661,"start_character":47,"end_line":4661,"end_character":73},"updated":"2019-12-23 13:03:42.000000000","message":"unnecessary (it\u0027s the default iirc)","commit_id":"bd8e2fe9c80dbe39bdd03fb213148b30a9dfd36e"}]}