)]}'
{"nova/conductor/tasks/migrate.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"af51c876e6c5fec30817e6610cb6c29b86f4199a","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                legacy_props.pop(\u0027retry\u0027, None)"},{"line_number":198,"context_line":"                self.request_spec.retry \u003d None"},{"line_number":199,"context_line":"        else:"},{"line_number":200,"context_line":"            # FIXME(mriedem): This likely overwrites the allow_cross_cell_move"},{"line_number":201,"context_line":"            # set in the API."},{"line_number":202,"context_line":"            self.request_spec.requested_destination \u003d objects.Destination("},{"line_number":203,"context_line":"                cell\u003dinstance_mapping.cell_mapping)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_56ba2b6e","line":200,"updated":"2019-02-08 15:25:34.000000000","message":"It won\u0027t because the condition above will be true since the API will set the request_spec.requested_destination, so this else just becomes dead code.\n\nThe problem would be if conductor is old and doesn\u0027t support allow cross cell resize, and we don\u0027t do a very good job of checking for that since we pass the parameter through the request spec rather than a direct parameter on the migrate server conductor RPC API interface. I guess in general we just assume conductor is always upgraded before API...","commit_id":"2aeef80d12cc28969688c0831be1aeeff8bf5317"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"af51c876e6c5fec30817e6610cb6c29b86f4199a","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        self.request_spec.ensure_project_and_user_id(self.instance)"},{"line_number":262,"context_line":"        self.request_spec.ensure_network_metadata(self.instance)"},{"line_number":263,"context_line":"        # FIXME(mriedem): If heal_reqspec_is_bfv saves changes to the"},{"line_number":264,"context_line":"        # RequestSpec we lose the set but not persisted fields on it like"},{"line_number":265,"context_line":"        # requested_destination."},{"line_number":266,"context_line":"        compute_utils.heal_reqspec_is_bfv("}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_16aca333","line":263,"updated":"2019-02-08 15:25:34.000000000","message":"This is bug 1815153.","commit_id":"2aeef80d12cc28969688c0831be1aeeff8bf5317"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d17946b09a1bd34b53231952bb64b309902df9aa","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":169,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"},{"line_number":170,"context_line":"        # and tell the scheduler to only look at hosts in the current cell."},{"line_number":171,"context_line":"        allow_cross_cell_resize \u003d ("},{"line_number":172,"context_line":"            \u0027requested_destination\u0027 in self.request_spec and"},{"line_number":173,"context_line":"            self.request_spec.requested_destination and"},{"line_number":174,"context_line":"            self.request_spec.requested_destination.allow_cross_cell_move)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_be11d206","line":171,"updated":"2019-02-11 16:14:25.000000000","message":"Move this down where it\u0027s used and then we don\u0027t need the conditional guards.","commit_id":"e4a0c4839c0039c24dae62a2e9e8e1df8bf2d139"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d17946b09a1bd34b53231952bb64b309902df9aa","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            # Check to see if the selected host is in another cell by"},{"line_number":207,"context_line":"            # looking at the host mapping. We can only do this while"},{"line_number":208,"context_line":"            # we\u0027re in the superconductor so we don\u0027t do an up-call."},{"line_number":209,"context_line":"            hm \u003d objects.HostMapping.get_by_host("},{"line_number":210,"context_line":"                self.context, selection.service_host)"},{"line_number":211,"context_line":"            if (hm.cell_mapping.uuid !\u003d"},{"line_number":212,"context_line":"                    self.request_spec.requested_destination.cell.uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_3efde2b0","line":209,"updated":"2019-02-11 16:14:25.000000000","message":"We don\u0027t need this because the Selection object has the cell_uuid on it, and self.context has cell_uuid set as well.","commit_id":"e4a0c4839c0039c24dae62a2e9e8e1df8bf2d139"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e1b2d05f6744e698a3c61dbca251b3c5a5d4f04e","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        \"\"\""},{"line_number":205,"context_line":"        # Note that the context is already targeted to the current cell in"},{"line_number":206,"context_line":"        # which the instance exists."},{"line_number":207,"context_line":"        result \u003d selection.cell_uuid \u003d\u003d self.context.cell_uuid"},{"line_number":208,"context_line":"        if not result:"},{"line_number":209,"context_line":"            LOG.debug(\u0027Selected target host %s is in cell %s and instance is \u0027"},{"line_number":210,"context_line":"                      \u0027in cell: %s\u0027, selection.service_host,"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_e08a22ba","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":14},"updated":"2019-08-16 09:34:38.000000000","message":"nit: same_cell or host_in_same_cell","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e1b2d05f6744e698a3c61dbca251b3c5a5d4f04e","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                    self.request_spec, self._migration, self.compute_rpcapi,"},{"line_number":281,"context_line":"                    selection, self.host_list)"},{"line_number":282,"context_line":"                task.execute()"},{"line_number":283,"context_line":"                return"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            # This is a reschedule that will use the supplied alternate hosts"},{"line_number":286,"context_line":"            # in the host_list as destinations. Since the resources on these"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_40369604","line":283,"updated":"2019-08-16 09:34:38.000000000","message":"This seems to do the correct thing but it makes the flow pretty convoluted. A better structure for me would be to have  a generic schedule task. Then based on the result of the scheduling two different tasks a) SameCellMigrationTask, b) CrossCellMigrationTask.\n\nThis is not something I will block this on. But I wanted to mention it here as a possible refactor.","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fba45066273008dc647f88e19c902af938974be7","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                    self.request_spec, self._migration, self.compute_rpcapi,"},{"line_number":281,"context_line":"                    selection, self.host_list)"},{"line_number":282,"context_line":"                task.execute()"},{"line_number":283,"context_line":"                return"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            # This is a reschedule that will use the supplied alternate hosts"},{"line_number":286,"context_line":"            # in the host_list as destinations. Since the resources on these"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_b0bf4ece","line":283,"in_reply_to":"7faddb67_03be4fad","updated":"2019-08-21 15:37:36.000000000","message":"Refactoring after the functional test is the way to go. :)","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f99b60d8fd1d56eb8e0096d0e95a659b4185250f","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                    self.request_spec, self._migration, self.compute_rpcapi,"},{"line_number":281,"context_line":"                    selection, self.host_list)"},{"line_number":282,"context_line":"                task.execute()"},{"line_number":283,"context_line":"                return"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            # This is a reschedule that will use the supplied alternate hosts"},{"line_number":286,"context_line":"            # in the host_list as destinations. Since the resources on these"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_03be4fad","line":283,"in_reply_to":"7faddb67_40369604","updated":"2019-08-19 13:46:47.000000000","message":"I agree this is messy and this entire method is a mess before this, which is why I refactored out _restrict_request_spec_to_cell but that\u0027s only a small part of the mess. I don\u0027t have a clear picture in my head how I\u0027d refactor this all to do what you\u0027re suggesting, but would hope it could be worked on separately, especially after functional testing is in place so we can be sure to not regress something.","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e1b2d05f6744e698a3c61dbca251b3c5a5d4f04e","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                task.execute()"},{"line_number":283,"context_line":"                return"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            # This is a reschedule that will use the supplied alternate hosts"},{"line_number":286,"context_line":"            # in the host_list as destinations. Since the resources on these"},{"line_number":287,"context_line":"            # alternates may have been consumed and might not be able to"},{"line_number":288,"context_line":"            # support the migrated instance, we need to first claim the"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_c029269c","line":285,"range":{"start_line":285,"start_character":24,"end_line":285,"end_character":34},"updated":"2019-08-16 09:34:38.000000000","message":"Do we need to handle reschedule during cross cell resize?","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fba45066273008dc647f88e19c902af938974be7","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                task.execute()"},{"line_number":283,"context_line":"                return"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            # This is a reschedule that will use the supplied alternate hosts"},{"line_number":286,"context_line":"            # in the host_list as destinations. Since the resources on these"},{"line_number":287,"context_line":"            # alternates may have been consumed and might not be able to"},{"line_number":288,"context_line":"            # support the migrated instance, we need to first claim the"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_b092ee49","line":285,"range":{"start_line":285,"start_character":24,"end_line":285,"end_character":34},"in_reply_to":"7faddb67_433be736","updated":"2019-08-21 15:37:36.000000000","message":"Thanks for the explanation. I totally agree to do re-schedule support in separate patches. \n\nIf  re-schedule for cross cell resize will happen in PrepResizeAtDeskTask,  then in the re-schedule patch we need to enhance the documentation around this code and L255 as it also talk about re-schedule but I guess then it is only same-cell resize re-schedule.","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f99b60d8fd1d56eb8e0096d0e95a659b4185250f","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                task.execute()"},{"line_number":283,"context_line":"                return"},{"line_number":284,"context_line":"        else:"},{"line_number":285,"context_line":"            # This is a reschedule that will use the supplied alternate hosts"},{"line_number":286,"context_line":"            # in the host_list as destinations. Since the resources on these"},{"line_number":287,"context_line":"            # alternates may have been consumed and might not be able to"},{"line_number":288,"context_line":"            # support the migrated instance, we need to first claim the"}],"source_content_type":"text/x-python","patch_set":41,"id":"7faddb67_433be736","line":285,"range":{"start_line":285,"start_character":24,"end_line":285,"end_character":34},"in_reply_to":"7faddb67_c029269c","updated":"2019-08-19 13:46:47.000000000","message":"Yes, but it won\u0027t happen here, it\u0027s a TODO in the PrepResizeAtDestTask flow:\n\nhttps://review.opendev.org/#/c/627890/53/nova/conductor/tasks/cross_cell_migrate.py@472\n\nI didn\u0027t implement that yet because I had enough to write just to get things working first before adding the reschedule complexity, and reviews were moving so slow on what was working I didn\u0027t want to continue working on changing the code if it wasn\u0027t getting reviewed - I could spend my time elsewhere so I did. Now that the series is moving along I can work on adding reschedule support but will likely do it separately, i.e. not try to implement that TODO in change 627890 directly since rebasing this series gets painful (for the gate at least).","commit_id":"24903ba30972a91df8d2aaa9f37c14e7b83c7694"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"32a1303abdfc7585616313075406b9a1c754ffc0","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        \"\"\""},{"line_number":206,"context_line":"        # Note that the context is already targeted to the current cell in"},{"line_number":207,"context_line":"        # which the instance exists."},{"line_number":208,"context_line":"        result \u003d selection.cell_uuid \u003d\u003d self.context.cell_uuid"},{"line_number":209,"context_line":"        if not result:"},{"line_number":210,"context_line":"            LOG.debug(\u0027Selected target host %s is in cell %s and instance is \u0027"},{"line_number":211,"context_line":"                      \u0027in cell: %s\u0027, selection.service_host,"}],"source_content_type":"text/x-python","patch_set":50,"id":"3fa7e38b_5be399d8","line":208,"range":{"start_line":208,"start_character":8,"end_line":208,"end_character":14},"updated":"2019-11-04 16:36:14.000000000","message":"nit: this would be better named \"same_cell\".","commit_id":"bcb13851c97f10f706f7dd1ee4ba01473e797cb2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f8b72a0208a719e24bee23077bac27fba91a691","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return migration"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _restrict_request_spec_to_cell(self, legacy_props):"},{"line_number":166,"context_line":"        instance_mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":167,"context_line":"            self.context, self.instance.uuid)"},{"line_number":168,"context_line":"        if (\u0027requested_destination\u0027 in self.request_spec and"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_0a3dbf8d","line":165,"range":{"start_line":165,"start_character":8,"end_line":165,"end_character":38},"updated":"2019-11-05 16:56:53.000000000","message":"Maybe this should just be renamed to \"_set_requested_destination_cell\"?","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae9c62327f96d69c6f229a11b5b3753dab0c0255","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return migration"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _restrict_request_spec_to_cell(self, legacy_props):"},{"line_number":166,"context_line":"        instance_mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":167,"context_line":"            self.context, self.instance.uuid)"},{"line_number":168,"context_line":"        if (\u0027requested_destination\u0027 in self.request_spec and"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_4ab897e7","line":165,"range":{"start_line":165,"start_character":8,"end_line":165,"end_character":38},"in_reply_to":"3fa7e38b_0a3dbf8d","updated":"2019-11-05 17:01:50.000000000","message":"Yes.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96ff8dacc40af3c6cac26f5748f6a2a9b5d553a1","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        return migration"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _restrict_request_spec_to_cell(self, legacy_props):"},{"line_number":166,"context_line":"        instance_mapping \u003d objects.InstanceMapping.get_by_instance_uuid("},{"line_number":167,"context_line":"            self.context, self.instance.uuid)"},{"line_number":168,"context_line":"        if (\u0027requested_destination\u0027 in self.request_spec and"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_0a37ff00","line":165,"range":{"start_line":165,"start_character":8,"end_line":165,"end_character":38},"in_reply_to":"3fa7e38b_4ab897e7","updated":"2019-11-05 17:58:39.000000000","message":"Done","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f8b72a0208a719e24bee23077bac27fba91a691","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                cell\u003dinstance_mapping.cell_mapping)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        # NOTE(mriedem): If the user is allowed to perform a cross-cell resize"},{"line_number":184,"context_line":"        # then add the current cell to the request spec as \"preferred\" so the"},{"line_number":185,"context_line":"        # scheduler will weigh hosts within the current cell over hosts in"},{"line_number":186,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":187,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_cac1e783","line":184,"range":{"start_line":184,"start_character":10,"end_line":184,"end_character":55},"updated":"2019-11-05 16:56:53.000000000","message":"I can see how this is confusing. This whole NOTE should probably go above the logic above that sets request_spec.requested_destination. Does that make more sense?","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96ff8dacc40af3c6cac26f5748f6a2a9b5d553a1","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                cell\u003dinstance_mapping.cell_mapping)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        # NOTE(mriedem): If the user is allowed to perform a cross-cell resize"},{"line_number":184,"context_line":"        # then add the current cell to the request spec as \"preferred\" so the"},{"line_number":185,"context_line":"        # scheduler will weigh hosts within the current cell over hosts in"},{"line_number":186,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":187,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_2a3c3be0","line":184,"range":{"start_line":184,"start_character":10,"end_line":184,"end_character":55},"in_reply_to":"3fa7e38b_6abd53d6","updated":"2019-11-05 17:58:39.000000000","message":"Done","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae9c62327f96d69c6f229a11b5b3753dab0c0255","unresolved":false,"context_lines":[{"line_number":181,"context_line":"                cell\u003dinstance_mapping.cell_mapping)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        # NOTE(mriedem): If the user is allowed to perform a cross-cell resize"},{"line_number":184,"context_line":"        # then add the current cell to the request spec as \"preferred\" so the"},{"line_number":185,"context_line":"        # scheduler will weigh hosts within the current cell over hosts in"},{"line_number":186,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":187,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_6abd53d6","line":184,"range":{"start_line":184,"start_character":10,"end_line":184,"end_character":55},"in_reply_to":"3fa7e38b_cac1e783","updated":"2019-11-05 17:01:50.000000000","message":"Yes.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f8b72a0208a719e24bee23077bac27fba91a691","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":187,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"},{"line_number":188,"context_line":"        # and tell the scheduler to only look at hosts in the current cell."},{"line_number":189,"context_line":"        if self.request_spec.requested_destination.allow_cross_cell_move:"},{"line_number":190,"context_line":"            LOG.debug(\u0027Preferring cell %(cell)s while migrating\u0027,"},{"line_number":191,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":192,"context_line":"                      instance\u003dself.instance)"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_8abbef0d","line":189,"updated":"2019-11-05 16:56:53.000000000","message":"I can\u0027t move this above the logic above b/c self.request_spec might not have a requested_destination set yet (the else block above).","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae9c62327f96d69c6f229a11b5b3753dab0c0255","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":187,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"},{"line_number":188,"context_line":"        # and tell the scheduler to only look at hosts in the current cell."},{"line_number":189,"context_line":"        if self.request_spec.requested_destination.allow_cross_cell_move:"},{"line_number":190,"context_line":"            LOG.debug(\u0027Preferring cell %(cell)s while migrating\u0027,"},{"line_number":191,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":192,"context_line":"                      instance\u003dself.instance)"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_cacb2771","line":189,"in_reply_to":"3fa7e38b_8abbef0d","updated":"2019-11-05 17:01:50.000000000","message":"...right","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96ff8dacc40af3c6cac26f5748f6a2a9b5d553a1","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # another cell, all other things being equal. If the user is not"},{"line_number":187,"context_line":"        # allowed to perform cross-cell resize, then we limit the request spec"},{"line_number":188,"context_line":"        # and tell the scheduler to only look at hosts in the current cell."},{"line_number":189,"context_line":"        if self.request_spec.requested_destination.allow_cross_cell_move:"},{"line_number":190,"context_line":"            LOG.debug(\u0027Preferring cell %(cell)s while migrating\u0027,"},{"line_number":191,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":192,"context_line":"                      instance\u003dself.instance)"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_aa27cbc7","line":189,"in_reply_to":"3fa7e38b_cacb2771","updated":"2019-11-05 17:58:39.000000000","message":"Done","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"18d58fa3a2a54df849e09b50577b06362831ef88","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        else:"},{"line_number":194,"context_line":"            LOG.debug(\u0027Restricting to cell %(cell)s while migrating\u0027,"},{"line_number":195,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":196,"context_line":"                      instance\u003dself.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _is_selected_host_in_source_cell(self, selection):"},{"line_number":199,"context_line":"        \"\"\"Checks if the given Selection is in the same cell as the instance"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_e71adc16","line":196,"updated":"2019-11-05 16:03:42.000000000","message":"I\u0027m confused about this. The method here is named to indicate its function which is \"restrict...to cell\". You removed a comment and debug message, and added conditional debug messages. I guess this is to reflect what you\u0027ve already changed in host_manager to optionally not restrict the list to same-cell if the flag is set.\n\nThe old log message and comment seems more contextually-relevant, but this comment and these messages don\u0027t sit right with me. This seems like the wrong place to log these two hints, and the method probably needs to change to indicate a new meaning of \"find preferred target cell\" or something. The comment and conditional makes it look like you\u0027re doing the control here. If I re-read this in a year or so, I would probably assume that there used to be some functional code in each clause of the conditional and it got removed in a refactor. Your comment, which seems to describe code below it, says \"if $cond, then add cell as preferred\". But, that\u0027s always done above and the conditional is actually elsewhere.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96ff8dacc40af3c6cac26f5748f6a2a9b5d553a1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        else:"},{"line_number":194,"context_line":"            LOG.debug(\u0027Restricting to cell %(cell)s while migrating\u0027,"},{"line_number":195,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":196,"context_line":"                      instance\u003dself.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _is_selected_host_in_source_cell(self, selection):"},{"line_number":199,"context_line":"        \"\"\"Checks if the given Selection is in the same cell as the instance"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_0d05d910","line":196,"in_reply_to":"3fa7e38b_4a9df76a","updated":"2019-11-05 17:58:39.000000000","message":"Done","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9ab50a39e0c244f680ca117616ce5ac7f52df796","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        else:"},{"line_number":194,"context_line":"            LOG.debug(\u0027Restricting to cell %(cell)s while migrating\u0027,"},{"line_number":195,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":196,"context_line":"                      instance\u003dself.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _is_selected_host_in_source_cell(self, selection):"},{"line_number":199,"context_line":"        \"\"\"Checks if the given Selection is in the same cell as the instance"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_ead383d2","line":196,"in_reply_to":"3fa7e38b_4a9df76a","updated":"2019-11-05 17:34:39.000000000","message":"I think the restricting log message is useful and clear and aligned with what was logged before, since we know in that case we\u0027re not going to another cell. The \"preferring cell\" one is more dubious since by default the CrossCellWeigher will prefer the same source cell but could be configured to prefer other cells if you\u0027re trying to drain a given cell via aggregates. So maybe I\u0027ll changing \"Preferring\" to \"Allowing cross-cell move\".","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f8b72a0208a719e24bee23077bac27fba91a691","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        else:"},{"line_number":194,"context_line":"            LOG.debug(\u0027Restricting to cell %(cell)s while migrating\u0027,"},{"line_number":195,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":196,"context_line":"                      instance\u003dself.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _is_selected_host_in_source_cell(self, selection):"},{"line_number":199,"context_line":"        \"\"\"Checks if the given Selection is in the same cell as the instance"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_eaf143e0","line":196,"in_reply_to":"3fa7e38b_e71adc16","updated":"2019-11-05 16:56:53.000000000","message":"So if I remove the comments/logic from here, does it make more sense if I move the \"if self.request_spec.requested_destination.allow_cross_cell_move\" condition above into the if block where the request_spec already has a requested_destination set by the API? In that case either an admin is cold migrating to a specific host and/or we\u0027re allowing a cross-cell resize.\n\nEither the old debug log or new log message with the \u0027restricting to cell\u0027 wording would be an either else case, i.e. there is a requested_destination already but allow_cross_cell_move is false, or there was no requested_destination set at all yet (else block above). I think I probably did the logic down below so I didn\u0027t have to duplicate the log message for those 2 else cases above.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae9c62327f96d69c6f229a11b5b3753dab0c0255","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        else:"},{"line_number":194,"context_line":"            LOG.debug(\u0027Restricting to cell %(cell)s while migrating\u0027,"},{"line_number":195,"context_line":"                      {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"},{"line_number":196,"context_line":"                      instance\u003dself.instance)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _is_selected_host_in_source_cell(self, selection):"},{"line_number":199,"context_line":"        \"\"\"Checks if the given Selection is in the same cell as the instance"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_4a9df76a","line":196,"in_reply_to":"3fa7e38b_eaf143e0","updated":"2019-11-05 17:01:50.000000000","message":"I think you could nuke the log message entirely, or put it over in host manager, which actually decides what to do with this. However, moving/changing the comment as I (and you) described would make this more clearly just a hint to the log reader.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f8b72a0208a719e24bee23077bac27fba91a691","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        \"\"\""},{"line_number":206,"context_line":"        # Note that the context is already targeted to the current cell in"},{"line_number":207,"context_line":"        # which the instance exists."},{"line_number":208,"context_line":"        result \u003d selection.cell_uuid \u003d\u003d self.context.cell_uuid"},{"line_number":209,"context_line":"        if not result:"},{"line_number":210,"context_line":"            LOG.debug(\u0027Selected target host %s is in cell %s and instance is \u0027"},{"line_number":211,"context_line":"                      \u0027in cell: %s\u0027, selection.service_host,"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_ea438310","line":208,"range":{"start_line":208,"start_character":8,"end_line":208,"end_character":14},"updated":"2019-11-05 16:56:53.000000000","message":"Rename to \u0027same_cell\u0027.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96ff8dacc40af3c6cac26f5748f6a2a9b5d553a1","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        \"\"\""},{"line_number":206,"context_line":"        # Note that the context is already targeted to the current cell in"},{"line_number":207,"context_line":"        # which the instance exists."},{"line_number":208,"context_line":"        result \u003d selection.cell_uuid \u003d\u003d self.context.cell_uuid"},{"line_number":209,"context_line":"        if not result:"},{"line_number":210,"context_line":"            LOG.debug(\u0027Selected target host %s is in cell %s and instance is \u0027"},{"line_number":211,"context_line":"                      \u0027in cell: %s\u0027, selection.service_host,"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_cd0ee132","line":208,"range":{"start_line":208,"start_character":8,"end_line":208,"end_character":14},"in_reply_to":"3fa7e38b_6aa233b0","updated":"2019-11-05 17:58:39.000000000","message":"Done","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae9c62327f96d69c6f229a11b5b3753dab0c0255","unresolved":false,"context_lines":[{"line_number":205,"context_line":"        \"\"\""},{"line_number":206,"context_line":"        # Note that the context is already targeted to the current cell in"},{"line_number":207,"context_line":"        # which the instance exists."},{"line_number":208,"context_line":"        result \u003d selection.cell_uuid \u003d\u003d self.context.cell_uuid"},{"line_number":209,"context_line":"        if not result:"},{"line_number":210,"context_line":"            LOG.debug(\u0027Selected target host %s is in cell %s and instance is \u0027"},{"line_number":211,"context_line":"                      \u0027in cell: %s\u0027, selection.service_host,"}],"source_content_type":"text/x-python","patch_set":51,"id":"3fa7e38b_6aa233b0","line":208,"range":{"start_line":208,"start_character":8,"end_line":208,"end_character":14},"in_reply_to":"3fa7e38b_ea438310","updated":"2019-11-05 17:01:50.000000000","message":"Agree.","commit_id":"06593b0d4f7416701fbee50a04bbfaa917c9b3fa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"26e956e7c061aaa62a86d96f516f1a0df3f49029","unresolved":false,"context_lines":[{"line_number":183,"context_line":"            if \u0027host\u0027 in self.request_spec.requested_destination:"},{"line_number":184,"context_line":"                legacy_props.pop(\u0027retry\u0027, None)"},{"line_number":185,"context_line":"                self.request_spec.retry \u003d None"},{"line_number":186,"context_line":"            # Deal with cross-cell move logic."},{"line_number":187,"context_line":"            if self.request_spec.requested_destination.allow_cross_cell_move:"},{"line_number":188,"context_line":"                LOG.debug(\u0027Allowing migration from cell %(cell)s\u0027,"},{"line_number":189,"context_line":"                          {\u0027cell\u0027: instance_mapping.cell_mapping.identity},"}],"source_content_type":"text/x-python","patch_set":52,"id":"3fa7e38b_8dec69f5","line":186,"updated":"2019-11-05 18:18:31.000000000","message":"Note that I do this below setting requested_destination.cell because later in the series https://review.opendev.org/#/c/642591/ needs to potentially change that for a targeted cold migration across cells.","commit_id":"9ee2d4337fe6faad20708a5296712c69e1f01b62"}]}