)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     LuyaoZhong \u003cluyao.zhong@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-08-17 12:46:47 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"track error migrations in resource tracker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If rollback_live_migration failed, the migration status is set to \u0027error\u0027, and"},{"line_number":10,"context_line":"there might me some resource not be cleaned up like vpmem since rollback is not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_ea5657ce","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":1},"updated":"2020-08-20 10:46:35.000000000","message":"T","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     LuyaoZhong \u003cluyao.zhong@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-08-17 12:46:47 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"track error migrations in resource tracker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If rollback_live_migration failed, the migration status is set to \u0027error\u0027, and"},{"line_number":10,"context_line":"there might me some resource not be cleaned up like vpmem since rollback is not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_7ec13754","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":1},"in_reply_to":"9f560f44_ea5657ce","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"track error migrations in resource tracker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If rollback_live_migration failed, the migration status is set to \u0027error\u0027, and"},{"line_number":10,"context_line":"there might me some resource not be cleaned up like vpmem since rollback is not"},{"line_number":11,"context_line":"completed. So we propose to track those \u0027error\u0027 migrations in resource tracker"},{"line_number":12,"context_line":"until they are cleaned up by periodic task \u0027_cleanup_incomplete_migrations\u0027."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_ca5b93f3","line":9,"range":{"start_line":9,"start_character":67,"end_line":9,"end_character":78},"updated":"2020-08-20 10:46:35.000000000","message":"wrapping (72 characters)","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"track error migrations in resource tracker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If rollback_live_migration failed, the migration status is set to \u0027error\u0027, and"},{"line_number":10,"context_line":"there might me some resource not be cleaned up like vpmem since rollback is not"},{"line_number":11,"context_line":"completed. So we propose to track those \u0027error\u0027 migrations in resource tracker"},{"line_number":12,"context_line":"until they are cleaned up by periodic task \u0027_cleanup_incomplete_migrations\u0027."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_deb20306","line":9,"range":{"start_line":9,"start_character":67,"end_line":9,"end_character":78},"in_reply_to":"9f560f44_ca5b93f3","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\u0027_cleanup_incomplete_migrations\u0027 will also handle failed rollback_live_migration"},{"line_number":19,"context_line":"cleanup except for failed resize/revert-resize."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I422a907056543f9bf95acbffdd2658438febf801"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_aa057fc4","line":20,"updated":"2020-08-20 10:46:35.000000000","message":"Link to the blueprint would be useful","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\u0027_cleanup_incomplete_migrations\u0027 will also handle failed rollback_live_migration"},{"line_number":19,"context_line":"cleanup except for failed resize/revert-resize."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I422a907056543f9bf95acbffdd2658438febf801"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_beb74ff5","line":20,"in_reply_to":"9f560f44_aa057fc4","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7aa86af28eadf68656c6e7186efc846ed9a1351","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So if rollback_live_migration succeeds, we need to set the migration"},{"line_number":16,"context_line":"status to \u0027failed\u0027 which will not be tracked in resource tracker. The"},{"line_number":17,"context_line":"\u0027failed\u0027 status already used for resize to indicated a migration"},{"line_number":18,"context_line":"finishing the cleanup."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"\u0027_cleanup_incomplete_migrations\u0027 will also handle failed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9f560f44_d89a326d","line":17,"range":{"start_line":17,"start_character":15,"end_line":17,"end_character":16},"updated":"2020-09-08 09:25:35.000000000","message":"is","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"71ccf6677b505f389086f7cd029336b2a2ca2e46","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So if rollback_live_migration succeeds, we need to set the migration"},{"line_number":16,"context_line":"status to \u0027failed\u0027 which will not be tracked in resource tracker. The"},{"line_number":17,"context_line":"\u0027failed\u0027 status already used for resize to indicated a migration"},{"line_number":18,"context_line":"finishing the cleanup."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"\u0027_cleanup_incomplete_migrations\u0027 will also handle failed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9f560f44_f420c34b","line":17,"range":{"start_line":17,"start_character":15,"end_line":17,"end_character":16},"updated":"2020-09-09 16:47:38.000000000","message":"is","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"4c047e3864cdbb891531a5959bb66e614f36b9c2","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"So if rollback_live_migration succeeds, we need to set the migration"},{"line_number":16,"context_line":"status to \u0027failed\u0027 which will not be tracked in resource tracker. The"},{"line_number":17,"context_line":"\u0027failed\u0027 status already used for resize to indicated a migration"},{"line_number":18,"context_line":"finishing the cleanup."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"\u0027_cleanup_incomplete_migrations\u0027 will also handle failed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9f560f44_042ffc3e","line":17,"range":{"start_line":17,"start_character":15,"end_line":17,"end_character":16},"in_reply_to":"9f560f44_f420c34b","updated":"2020-09-10 07:25:23.000000000","message":"Done","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"}],"nova/compute/manager.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"6a9309558a801ca88b95c8ca7d1848978f0cb5cf","unresolved":false,"context_lines":[{"line_number":10274,"context_line":""},{"line_number":10275,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10276,"context_line":"    def _run_pending_deletes(self, context):"},{"line_number":10277,"context_line":"        \"\"\"Retry any pending instance cleanup like file deletes and other"},{"line_number":10278,"context_line":"        resources cleanup.\"\"\""},{"line_number":10279,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10280,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_766c9c9b","line":10277,"updated":"2020-08-07 06:29:07.000000000","message":"pep8: H403: multi line docstrings should end on a new line","commit_id":"413c689a5eb5c576ccf5c68a97daa453e8c1e001"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"6a9309558a801ca88b95c8ca7d1848978f0cb5cf","unresolved":false,"context_lines":[{"line_number":10275,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10276,"context_line":"    def _run_pending_deletes(self, context):"},{"line_number":10277,"context_line":"        \"\"\"Retry any pending instance cleanup like file deletes and other"},{"line_number":10278,"context_line":"        resources cleanup.\"\"\""},{"line_number":10279,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10280,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"},{"line_number":10281,"context_line":"                   \u0027soft_deleted\u0027: False,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_5667d8b7","line":10278,"updated":"2020-08-07 06:29:07.000000000","message":"pep8: H403: multi line docstrings should end on a new line","commit_id":"413c689a5eb5c576ccf5c68a97daa453e8c1e001"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                # destination host, migration status will be set to \u0027failed\u0027"},{"line_number":937,"context_line":"                # at the end of rollback_live_migration_at_destination, then"},{"line_number":938,"context_line":"                # it will not be tracked in resource tracker"},{"line_number":939,"context_line":"                self.compute_rpcapi.rollback_live_migration_at_destination("},{"line_number":940,"context_line":"                    context, instance, migration.dest_compute,"},{"line_number":941,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dNone)"},{"line_number":942,"context_line":"            LOG.info(\u0027Instance found in migrating state during \u0027"},{"line_number":943,"context_line":"                     \u0027startup. Resetting task_state\u0027,"},{"line_number":944,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0a3c6b2d","line":941,"range":{"start_line":939,"start_character":0,"end_line":941,"end_character":59},"updated":"2020-08-20 10:46:35.000000000","message":"This can raise exceptions. Do we need to handle those?\n\nAlso, this would really benefit from a proper functional test demonstrating the cleanup of various resources. I see minimal changes to existing tests but nothing to verify that resources are properly cleaned up for a failed validation. fwict, I this code path is only triggered on startup when enumerating the instances on the host (\u0027_reset_live_migration\u0027 is only called by \u0027_init_instance\u0027, which is only called by \u0027init_host\u0027). A test would probably look like:\n\n1. Create an instance.\n2. Live migrate it, ensuring the migration fails.\n3. Restart the compute service.\n4. Ensure some resource (port bindings?) is cleaned up correct.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f84c34f90f9e8c5712470c8f010127eeb2bcbc49","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                # destination host, migration status will be set to \u0027failed\u0027"},{"line_number":937,"context_line":"                # at the end of rollback_live_migration_at_destination, then"},{"line_number":938,"context_line":"                # it will not be tracked in resource tracker"},{"line_number":939,"context_line":"                self.compute_rpcapi.rollback_live_migration_at_destination("},{"line_number":940,"context_line":"                    context, instance, migration.dest_compute,"},{"line_number":941,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dNone)"},{"line_number":942,"context_line":"            LOG.info(\u0027Instance found in migrating state during \u0027"},{"line_number":943,"context_line":"                     \u0027startup. Resetting task_state\u0027,"},{"line_number":944,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0d92c557","line":941,"range":{"start_line":939,"start_character":0,"end_line":941,"end_character":59},"in_reply_to":"9f560f44_0a3c6b2d","updated":"2020-08-20 10:50:21.000000000","message":"Also, the other caller of this is surrounded by a conditional based on the return value from \u0027self._live_migration_cleanup_flags\u0027. Why don\u0027t we need the same check here? Perhaps we do?","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                # destination host, migration status will be set to \u0027failed\u0027"},{"line_number":937,"context_line":"                # at the end of rollback_live_migration_at_destination, then"},{"line_number":938,"context_line":"                # it will not be tracked in resource tracker"},{"line_number":939,"context_line":"                self.compute_rpcapi.rollback_live_migration_at_destination("},{"line_number":940,"context_line":"                    context, instance, migration.dest_compute,"},{"line_number":941,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dNone)"},{"line_number":942,"context_line":"            LOG.info(\u0027Instance found in migrating state during \u0027"},{"line_number":943,"context_line":"                     \u0027startup. Resetting task_state\u0027,"},{"line_number":944,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_788f8f29","line":941,"range":{"start_line":939,"start_character":0,"end_line":941,"end_character":59},"in_reply_to":"9f560f44_0a3c6b2d","updated":"2020-08-24 09:17:11.000000000","message":"How about let it raising exceptions. Actually I didn\u0027t come up with how to \u0027handle\u0027 it here, we should not reset task_state for instance if rollback fails, and if we keep the task_state, it seems that it will be stuck at migrating forever. Both are unreasonable.\n\nI\u0027ll try to add a functional test as you suggested.Proberbly an adding follow-up patch.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"3d6df7ea0df153594db22f46f7ddea760bbf7a66","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                # destination host, migration status will be set to \u0027failed\u0027"},{"line_number":937,"context_line":"                # at the end of rollback_live_migration_at_destination, then"},{"line_number":938,"context_line":"                # it will not be tracked in resource tracker"},{"line_number":939,"context_line":"                self.compute_rpcapi.rollback_live_migration_at_destination("},{"line_number":940,"context_line":"                    context, instance, migration.dest_compute,"},{"line_number":941,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dNone)"},{"line_number":942,"context_line":"            LOG.info(\u0027Instance found in migrating state during \u0027"},{"line_number":943,"context_line":"                     \u0027startup. Resetting task_state\u0027,"},{"line_number":944,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_a3d3aa00","line":941,"range":{"start_line":939,"start_character":0,"end_line":941,"end_character":59},"in_reply_to":"9f560f44_0a3c6b2d","updated":"2020-08-25 05:46:33.000000000","message":"step 2, I think you mean ensuring the migration in \u0027running\u0027 (#line 926)","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                # destination host, migration status will be set to \u0027failed\u0027"},{"line_number":937,"context_line":"                # at the end of rollback_live_migration_at_destination, then"},{"line_number":938,"context_line":"                # it will not be tracked in resource tracker"},{"line_number":939,"context_line":"                self.compute_rpcapi.rollback_live_migration_at_destination("},{"line_number":940,"context_line":"                    context, instance, migration.dest_compute,"},{"line_number":941,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dNone)"},{"line_number":942,"context_line":"            LOG.info(\u0027Instance found in migrating state during \u0027"},{"line_number":943,"context_line":"                     \u0027startup. Resetting task_state\u0027,"},{"line_number":944,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_78c14f54","line":941,"range":{"start_line":939,"start_character":0,"end_line":941,"end_character":59},"in_reply_to":"9f560f44_0d92c557","updated":"2020-08-24 09:17:11.000000000","message":"cleanup flags are in migrate_data but we can\u0027t get migrate_data back here. we can only cleanup whatever we could get.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"26324d1a2bb663d068ffc797dc91c0f87f04a6be","unresolved":false,"context_lines":[{"line_number":936,"context_line":"                # destination host, migration status will be set to \u0027failed\u0027"},{"line_number":937,"context_line":"                # at the end of rollback_live_migration_at_destination, then"},{"line_number":938,"context_line":"                # it will not be tracked in resource tracker"},{"line_number":939,"context_line":"                self.compute_rpcapi.rollback_live_migration_at_destination("},{"line_number":940,"context_line":"                    context, instance, migration.dest_compute,"},{"line_number":941,"context_line":"                    destroy_disks\u003dFalse, migrate_data\u003dNone)"},{"line_number":942,"context_line":"            LOG.info(\u0027Instance found in migrating state during \u0027"},{"line_number":943,"context_line":"                     \u0027startup. Resetting task_state\u0027,"},{"line_number":944,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_7efbe577","line":941,"range":{"start_line":939,"start_character":0,"end_line":941,"end_character":59},"in_reply_to":"9f560f44_a3d3aa00","updated":"2020-08-25 07:34:02.000000000","message":"Functional test really helped. I found we need revert allocations also.\n\nSince this change has nothing to do with \u0027error\u0027 migration, I\u0027ll move it to another patch with functional test. We could discuss about details there.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":10274,"context_line":""},{"line_number":10275,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10276,"context_line":"    def _run_pending_deletes(self, context):"},{"line_number":10277,"context_line":"        \"\"\"Retry any pending instance cleanup like file deletes and other"},{"line_number":10278,"context_line":"        resources cleanup."},{"line_number":10279,"context_line":"        \"\"\""},{"line_number":10280,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10281,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_ea0ad776","line":10278,"range":{"start_line":10277,"start_character":45,"end_line":10278,"end_character":26},"updated":"2020-08-20 10:46:35.000000000","message":"I think this is unnecessary, personally.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":10274,"context_line":""},{"line_number":10275,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10276,"context_line":"    def _run_pending_deletes(self, context):"},{"line_number":10277,"context_line":"        \"\"\"Retry any pending instance cleanup like file deletes and other"},{"line_number":10278,"context_line":"        resources cleanup."},{"line_number":10279,"context_line":"        \"\"\""},{"line_number":10280,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10281,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_1e047b49","line":10278,"range":{"start_line":10277,"start_character":45,"end_line":10278,"end_character":26},"in_reply_to":"9f560f44_ea0ad776","updated":"2020-08-24 09:17:11.000000000","message":"I‘m ok to remove it.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0d0ca6fc4016981fb07a6c12140b1ae707920338","unresolved":false,"context_lines":[{"line_number":10332,"context_line":"            for migration in migrations:"},{"line_number":10333,"context_line":"                if instance.uuid !\u003d migration.instance_uuid:"},{"line_number":10334,"context_line":"                    continue"},{"line_number":10335,"context_line":"                prefix \u003d (\u0027old_\u0027 if migration.source_compute \u003d\u003d CONF.host"},{"line_number":10336,"context_line":"                          else \u0027new_\u0027)"},{"line_number":10337,"context_line":"                with instance.mutated_migration_context(prefix\u003dprefix):"},{"line_number":10338,"context_line":"                    cleaned \u003d self.driver.cleanup_instance(instance)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_469c29bf","line":10335,"updated":"2020-08-27 13:44:08.000000000","message":"nit: can we get a comment? It\u0027s not immediately obvious why this is happening","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"ac2f219ab47ebe7af4a111d6b8b5a2b998a98ab5","unresolved":false,"context_lines":[{"line_number":10332,"context_line":"            for migration in migrations:"},{"line_number":10333,"context_line":"                if instance.uuid !\u003d migration.instance_uuid:"},{"line_number":10334,"context_line":"                    continue"},{"line_number":10335,"context_line":"                prefix \u003d (\u0027old_\u0027 if migration.source_compute \u003d\u003d CONF.host"},{"line_number":10336,"context_line":"                          else \u0027new_\u0027)"},{"line_number":10337,"context_line":"                with instance.mutated_migration_context(prefix\u003dprefix):"},{"line_number":10338,"context_line":"                    cleaned \u003d self.driver.cleanup_instance(instance)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_6b72c9af","line":10335,"in_reply_to":"9f560f44_469c29bf","updated":"2020-08-28 09:07:17.000000000","message":"Done","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7aa86af28eadf68656c6e7186efc846ed9a1351","unresolved":false,"context_lines":[{"line_number":10299,"context_line":"        return results"},{"line_number":10300,"context_line":""},{"line_number":10301,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10302,"context_line":"    def _run_pending_instance_deletes(self, context):"},{"line_number":10303,"context_line":"        \"\"\"Retry any pending instance cleanup.\"\"\""},{"line_number":10304,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10305,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_b86f7e26","line":10302,"updated":"2020-09-08 09:25:35.000000000","message":"tbh, I don\u0027t really grasp why this exists when we have a \u0027cleanup\u0027 virt driver function that we call. Shouldn\u0027t \u0027cleanup\u0027 (or more specifically, \u0027destroy\u0027 [1]) handle instance cleanup for us? What would cause this periodic task to do anything?\n\n[1] https://github.com/openstack/nova/blob/b75e68986486df6c56e29eb2f244955352a969ac/nova/virt/libvirt/driver.py#L1327-L1331","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"22bf14fc3f3d010dfcc7d3dbdba5fa716eccdbce","unresolved":false,"context_lines":[{"line_number":10299,"context_line":"        return results"},{"line_number":10300,"context_line":""},{"line_number":10301,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10302,"context_line":"    def _run_pending_instance_deletes(self, context):"},{"line_number":10303,"context_line":"        \"\"\"Retry any pending instance cleanup.\"\"\""},{"line_number":10304,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10305,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_80e2f3d2","line":10302,"in_reply_to":"9f560f44_a951ed64","updated":"2020-09-09 01:45:03.000000000","message":"yeah, I don\u0027t know why it seems sometimes deleting instance files will fail.[1]. But anyway we need a new driver interface.\n\n[1]https://github.com/openstack/nova/blob/b75e68986486df6c56e29eb2f244955352a969ac/nova/virt/libvirt/driver.py#L1473","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a2e1ee61ffedd0c2d8852b6f3649e111585a81ec","unresolved":false,"context_lines":[{"line_number":10299,"context_line":"        return results"},{"line_number":10300,"context_line":""},{"line_number":10301,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.instance_delete_interval)"},{"line_number":10302,"context_line":"    def _run_pending_instance_deletes(self, context):"},{"line_number":10303,"context_line":"        \"\"\"Retry any pending instance cleanup.\"\"\""},{"line_number":10304,"context_line":"        LOG.debug(\u0027Cleaning up deleted instances\u0027)"},{"line_number":10305,"context_line":"        filters \u003d {\u0027deleted\u0027: True,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_a951ed64","line":10302,"in_reply_to":"9f560f44_b86f7e26","updated":"2020-09-08 15:31:26.000000000","message":"Later: Ah, so we expect that we might not be able to clean up the instance files and if we can\u0027t, we won\u0027t set the instance.cleaned attribute. Later, this periodic task will run and attempt to clean things up for us again. However, we don\u0027t want to do that for anything other than files.","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7aa86af28eadf68656c6e7186efc846ed9a1351","unresolved":false,"context_lines":[{"line_number":10321,"context_line":"                      instance\u003dinstance)"},{"line_number":10322,"context_line":"            if attempts \u003c CONF.maximum_instance_delete_attempts:"},{"line_number":10323,"context_line":"                success \u003d self.driver.cleanup_instance("},{"line_number":10324,"context_line":"                    instance, del_inst_files_only\u003dTrue)"},{"line_number":10325,"context_line":""},{"line_number":10326,"context_line":"                instance.system_metadata[\u0027clean_attempts\u0027] \u003d str(attempts + 1)"},{"line_number":10327,"context_line":"                if success:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_d8c5125e","line":10324,"range":{"start_line":10324,"start_character":28,"end_line":10324,"end_character":55},"updated":"2020-09-08 09:25:35.000000000","message":"What is the purpose of this? Why would you not want to zero out the pMEM device in this case?","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"71ccf6677b505f389086f7cd029336b2a2ca2e46","unresolved":false,"context_lines":[{"line_number":10417,"context_line":"                revert \u003d (True if migration.source_compute \u003d\u003d CONF.host"},{"line_number":10418,"context_line":"                          else False)"},{"line_number":10419,"context_line":"                with instance.mutated_migration_context(revert\u003drevert):"},{"line_number":10420,"context_line":"                    self.driver.delete_instance_files(instance)"},{"line_number":10421,"context_line":"                    self.driver.cleanup_lingering_instance_resources(instance)"},{"line_number":10422,"context_line":"                try:"},{"line_number":10423,"context_line":"                    migration.status \u003d \u0027failed\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_94dd0743","line":10420,"range":{"start_line":10420,"start_character":0,"end_line":10420,"end_character":63},"updated":"2020-09-09 16:47:38.000000000","message":"This can be done outside of the \u0027mutated_migration_context\u0027 context manager, right?","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"4c047e3864cdbb891531a5959bb66e614f36b9c2","unresolved":false,"context_lines":[{"line_number":10417,"context_line":"                revert \u003d (True if migration.source_compute \u003d\u003d CONF.host"},{"line_number":10418,"context_line":"                          else False)"},{"line_number":10419,"context_line":"                with instance.mutated_migration_context(revert\u003drevert):"},{"line_number":10420,"context_line":"                    self.driver.delete_instance_files(instance)"},{"line_number":10421,"context_line":"                    self.driver.cleanup_lingering_instance_resources(instance)"},{"line_number":10422,"context_line":"                try:"},{"line_number":10423,"context_line":"                    migration.status \u003d \u0027failed\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_e43128a4","line":10420,"range":{"start_line":10420,"start_character":0,"end_line":10420,"end_character":63},"in_reply_to":"9f560f44_94dd0743","updated":"2020-09-10 07:25:23.000000000","message":"Done","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"71ccf6677b505f389086f7cd029336b2a2ca2e46","unresolved":false,"context_lines":[{"line_number":10419,"context_line":"                with instance.mutated_migration_context(revert\u003drevert):"},{"line_number":10420,"context_line":"                    self.driver.delete_instance_files(instance)"},{"line_number":10421,"context_line":"                    self.driver.cleanup_lingering_instance_resources(instance)"},{"line_number":10422,"context_line":"                try:"},{"line_number":10423,"context_line":"                    migration.status \u003d \u0027failed\u0027"},{"line_number":10424,"context_line":"                    migration.save()"},{"line_number":10425,"context_line":"                except exception.MigrationNotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_f4ce0381","line":10422,"updated":"2020-09-09 16:47:38.000000000","message":"nit: newline before this","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"4c047e3864cdbb891531a5959bb66e614f36b9c2","unresolved":false,"context_lines":[{"line_number":10419,"context_line":"                with instance.mutated_migration_context(revert\u003drevert):"},{"line_number":10420,"context_line":"                    self.driver.delete_instance_files(instance)"},{"line_number":10421,"context_line":"                    self.driver.cleanup_lingering_instance_resources(instance)"},{"line_number":10422,"context_line":"                try:"},{"line_number":10423,"context_line":"                    migration.status \u003d \u0027failed\u0027"},{"line_number":10424,"context_line":"                    migration.save()"},{"line_number":10425,"context_line":"                except exception.MigrationNotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_4425745b","line":10422,"in_reply_to":"9f560f44_f4ce0381","updated":"2020-09-10 07:25:23.000000000","message":"Done","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f0f2022dd597cbde7633e013dc2a17bd1aff6859","unresolved":false,"context_lines":[{"line_number":888,"context_line":"            context, instances, nodename)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"        # Grab all in-progress migrations:"},{"line_number":891,"context_line":"        migrations \u003d objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":892,"context_line":"                context, self.host, nodename)"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":895,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_4f02680b","line":892,"range":{"start_line":891,"start_character":8,"end_line":892,"end_character":45},"updated":"2020-08-12 06:56:01.000000000","message":"Can we use one DB call get all the migraiton object we need here?","commit_id":"0e801c89d8638851d0ec8f5fd168ef5af2fb4fd7"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"a4e84ea549dde616c27e217f610c5170d339af9b","unresolved":false,"context_lines":[{"line_number":888,"context_line":"            context, instances, nodename)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"        # Grab all in-progress migrations:"},{"line_number":891,"context_line":"        migrations \u003d objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":892,"context_line":"                context, self.host, nodename)"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":895,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_a174eb9d","line":892,"range":{"start_line":891,"start_character":8,"end_line":892,"end_character":45},"in_reply_to":"9f560f44_4f02680b","updated":"2020-08-13 06:06:13.000000000","message":"do you mean add \u0027error\u0027 into the status filter in this function? or introduce a new db call?","commit_id":"0e801c89d8638851d0ec8f5fd168ef5af2fb4fd7"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"3527114ec4e47845c961dda312cfd2dbf6323e51","unresolved":false,"context_lines":[{"line_number":888,"context_line":"            context, instances, nodename)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"        # Grab all in-progress migrations:"},{"line_number":891,"context_line":"        migrations \u003d objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":892,"context_line":"                context, self.host, nodename)"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":895,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_c2791894","line":892,"range":{"start_line":891,"start_character":8,"end_line":892,"end_character":45},"in_reply_to":"9f560f44_a174eb9d","updated":"2020-08-13 14:10:54.000000000","message":"I guess add the filter to \u0027objects.MigrationList.get_by_filters\u0027 is workable. Anyway, it is wasteful to use two DB calls to get the data, special in large scale cluster.","commit_id":"0e801c89d8638851d0ec8f5fd168ef5af2fb4fd7"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"263614e33dc10bde665c040d41680c46292e8933","unresolved":false,"context_lines":[{"line_number":888,"context_line":"            context, instances, nodename)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"        # Grab all in-progress migrations:"},{"line_number":891,"context_line":"        migrations \u003d objects.MigrationList.get_in_progress_by_host_and_node("},{"line_number":892,"context_line":"                context, self.host, nodename)"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":895,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_3f59ef59","line":892,"range":{"start_line":891,"start_character":8,"end_line":892,"end_character":45},"in_reply_to":"9f560f44_c2791894","updated":"2020-08-17 06:44:28.000000000","message":"Yeah, I agree with that one db call is enough.\nBut I\u0027m still not sure how to define \u0027in progress\u0027, because I see there are two db functions defined about in-progress migration, they seems different.\n\n1. migration_get_in_progress_by_host_and_node\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L3151\nIt takes a complementary set of some status as in-progress status.\n\n2. migration_get_in_progress_by_instance\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L3167\nIt takes some specific status as in-progress status. But obviously, it doesn\u0027t include other status like \u0027migrating\u0027,\u0027confirming\u0027,\u0027reverting\u0027 and so on.\n\nWhich one should I take for my filters if I use get_by_filters.","commit_id":"0e801c89d8638851d0ec8f5fd168ef5af2fb4fd7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":887,"context_line":"        instance_by_uuid \u003d self._update_usage_from_instances("},{"line_number":888,"context_line":"            context, instances, nodename)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"        # Grab all in-progress migrations and error migrations:"},{"line_number":891,"context_line":"        filters \u003d {\u0027host\u0027: self.host, \u0027node\u0027: nodename, \u0027status\u0027: ["},{"line_number":892,"context_line":"            \u0027queued\u0027, \u0027preparing\u0027, \u0027running\u0027, \u0027post-migrating\u0027, \u0027accepted\u0027,"},{"line_number":893,"context_line":"            \u0027pre-migrating\u0027, \u0027migrating\u0027, \u0027confirming\u0027, \u0027reverting\u0027, \u0027error\u0027,"},{"line_number":894,"context_line":"            \u0027finished\u0027]}"},{"line_number":895,"context_line":"        migrations \u003d objects.MigrationList.get_by_filters(context, filters)"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":898,"context_line":"        self._update_usage_from_migrations(context, migrations, nodename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0ad8ebec","line":895,"range":{"start_line":890,"start_character":0,"end_line":895,"end_character":75},"updated":"2020-08-20 10:46:35.000000000","message":"Shouldn\u0027t this be a new API in the \u0027nova.db.api\u0027 and \u0027nova.db.sqlalchemy.api\u0027?\n\nThis also feels like it could be a separate change. You\u0027re stating that we should included those in the \u0027error\u0027 state because they likely haven\u0027t been cleaned up and therefore are using inventory, yeah? That seems reasonable so is it possible to do that change without any of the other changes here?","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":887,"context_line":"        instance_by_uuid \u003d self._update_usage_from_instances("},{"line_number":888,"context_line":"            context, instances, nodename)"},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"        # Grab all in-progress migrations and error migrations:"},{"line_number":891,"context_line":"        filters \u003d {\u0027host\u0027: self.host, \u0027node\u0027: nodename, \u0027status\u0027: ["},{"line_number":892,"context_line":"            \u0027queued\u0027, \u0027preparing\u0027, \u0027running\u0027, \u0027post-migrating\u0027, \u0027accepted\u0027,"},{"line_number":893,"context_line":"            \u0027pre-migrating\u0027, \u0027migrating\u0027, \u0027confirming\u0027, \u0027reverting\u0027, \u0027error\u0027,"},{"line_number":894,"context_line":"            \u0027finished\u0027]}"},{"line_number":895,"context_line":"        migrations \u003d objects.MigrationList.get_by_filters(context, filters)"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":898,"context_line":"        self._update_usage_from_migrations(context, migrations, nodename)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_780b0f8d","line":895,"range":{"start_line":890,"start_character":0,"end_line":895,"end_character":75},"in_reply_to":"9f560f44_0ad8ebec","updated":"2020-08-24 09:17:11.000000000","message":"I prefer not to seperating, since we track them then we should have other way(periodic task change) to cleanup them, these changes should be proposed together to make it work properly.\n\nintroducing a new API sounds nice, current way looks a little ugly.","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":1328,"context_line":""},{"line_number":1329,"context_line":"            try:"},{"line_number":1330,"context_line":"                if uuid not in instances:"},{"line_number":1331,"context_line":"                    read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":1332,"context_line":"                    migration._context \u003d read_deleted_context"},{"line_number":1333,"context_line":"                    instances[uuid] \u003d migration.instance"},{"line_number":1334,"context_line":"            except exception.InstanceNotFound as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_4ade63da","line":1331,"updated":"2020-08-20 10:46:35.000000000","message":"This needs a comment","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":1328,"context_line":""},{"line_number":1329,"context_line":"            try:"},{"line_number":1330,"context_line":"                if uuid not in instances:"},{"line_number":1331,"context_line":"                    read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":1332,"context_line":"                    migration._context \u003d read_deleted_context"},{"line_number":1333,"context_line":"                    instances[uuid] \u003d migration.instance"},{"line_number":1334,"context_line":"            except exception.InstanceNotFound as e:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_7ea017f5","line":1331,"in_reply_to":"9f560f44_4ade63da","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0d0ca6fc4016981fb07a6c12140b1ae707920338","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"                    # has database record. This kind of instance might be"},{"line_number":1330,"context_line":"                    # deleted during unfinished migrating but exist in the"},{"line_number":1331,"context_line":"                    # hypervisor."},{"line_number":1332,"context_line":"                    read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":1333,"context_line":"                    migration._context \u003d read_deleted_context"},{"line_number":1334,"context_line":"                    instances[uuid] \u003d migration.instance"},{"line_number":1335,"context_line":"            except exception.InstanceNotFound as e:"},{"line_number":1336,"context_line":"                # migration referencing deleted instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_c6ddd970","line":1333,"range":{"start_line":1332,"start_character":0,"end_line":1333,"end_character":61},"updated":"2020-08-27 13:44:08.000000000","message":"nit: could be one line","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"ac2f219ab47ebe7af4a111d6b8b5a2b998a98ab5","unresolved":false,"context_lines":[{"line_number":1329,"context_line":"                    # has database record. This kind of instance might be"},{"line_number":1330,"context_line":"                    # deleted during unfinished migrating but exist in the"},{"line_number":1331,"context_line":"                    # hypervisor."},{"line_number":1332,"context_line":"                    read_deleted_context \u003d context.elevated(read_deleted\u003d\u0027yes\u0027)"},{"line_number":1333,"context_line":"                    migration._context \u003d read_deleted_context"},{"line_number":1334,"context_line":"                    instances[uuid] \u003d migration.instance"},{"line_number":1335,"context_line":"            except exception.InstanceNotFound as e:"},{"line_number":1336,"context_line":"                # migration referencing deleted instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_eb16d949","line":1333,"range":{"start_line":1332,"start_character":0,"end_line":1333,"end_character":61},"in_reply_to":"9f560f44_c6ddd970","updated":"2020-08-28 09:07:17.000000000","message":"Done","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"}],"nova/db/api.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0d0ca6fc4016981fb07a6c12140b1ae707920338","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                                              values)"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"def migration_get_in_progress_and_error(context, host, node):"},{"line_number":474,"context_line":"    \"\"\"Finds all in progress migrations and error migrations for the given"},{"line_number":475,"context_line":"    host + node."},{"line_number":476,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_4661c9a8","line":473,"range":{"start_line":473,"start_character":4,"end_line":473,"end_character":39},"updated":"2020-08-27 13:44:08.000000000","message":"Would\n\n  migration_get_in_progress_and_error_by_host_and_node\n\nbe too wordy?","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"ac2f219ab47ebe7af4a111d6b8b5a2b998a98ab5","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                                              values)"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":""},{"line_number":473,"context_line":"def migration_get_in_progress_and_error(context, host, node):"},{"line_number":474,"context_line":"    \"\"\"Finds all in progress migrations and error migrations for the given"},{"line_number":475,"context_line":"    host + node."},{"line_number":476,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_6b2489b0","line":473,"range":{"start_line":473,"start_character":4,"end_line":473,"end_character":39},"in_reply_to":"9f560f44_4661c9a8","updated":"2020-08-28 09:07:17.000000000","message":"a little wordy, but more precise","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"}],"nova/objects/instance.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0d0ca6fc4016981fb07a6c12140b1ae707920338","unresolved":false,"context_lines":[{"line_number":1047,"context_line":"                setattr(self, inst_attr_name, attr_value)"},{"line_number":1048,"context_line":""},{"line_number":1049,"context_line":"    @contextlib.contextmanager"},{"line_number":1050,"context_line":"    def mutated_migration_context(self, prefix\u003dNone):"},{"line_number":1051,"context_line":"        \"\"\"Context manager to temporarily apply/revert the migration context."},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"        Calling .save() from within the context manager means that the mutated"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_263bf59b","line":1050,"range":{"start_line":1050,"start_character":40,"end_line":1050,"end_character":51},"updated":"2020-08-27 13:44:08.000000000","message":"nit: Rather than passing the prefix, could we use a boolean option? Maybe \u0027revert\u003dFalse\u0027 ?","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"ac2f219ab47ebe7af4a111d6b8b5a2b998a98ab5","unresolved":false,"context_lines":[{"line_number":1047,"context_line":"                setattr(self, inst_attr_name, attr_value)"},{"line_number":1048,"context_line":""},{"line_number":1049,"context_line":"    @contextlib.contextmanager"},{"line_number":1050,"context_line":"    def mutated_migration_context(self, prefix\u003dNone):"},{"line_number":1051,"context_line":"        \"\"\"Context manager to temporarily apply/revert the migration context."},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"        Calling .save() from within the context manager means that the mutated"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_cb3dd5bd","line":1050,"range":{"start_line":1050,"start_character":40,"end_line":1050,"end_character":51},"in_reply_to":"9f560f44_263bf59b","updated":"2020-08-28 09:07:17.000000000","message":"Done","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"}],"nova/tests/unit/compute/test_resource_tracker.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"    @mock.patch(\u0027nova.objects.Instance.get_by_uuid\u0027)"},{"line_number":1155,"context_line":"    @mock.patch(\u0027nova.objects.InstanceList.get_by_host_and_node\u0027)"},{"line_number":1156,"context_line":"    def test_no_instances_err_migration(self, get_mock, get_inst_mock,"},{"line_number":1157,"context_line":"                                         migr_mock, get_cn_mock, pci_mock,"},{"line_number":1158,"context_line":"                                         instance_pci_mock,"},{"line_number":1159,"context_line":"                                         mock_is_volume_backed_instance):"},{"line_number":1160,"context_line":"        # We test the behavior of update_available_resource() when"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0a746bce","line":1157,"range":{"start_line":1157,"start_character":40,"end_line":1157,"end_character":41},"updated":"2020-08-20 10:46:35.000000000","message":"nit.\n\nCould you actually just bring these down to a newline?\n\n   def test_no_instances_err_migration(\n       self, get_mock, get_inst_mock, ...\n       ...\n   ):","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"    @mock.patch(\u0027nova.objects.Instance.get_by_uuid\u0027)"},{"line_number":1155,"context_line":"    @mock.patch(\u0027nova.objects.InstanceList.get_by_host_and_node\u0027)"},{"line_number":1156,"context_line":"    def test_no_instances_err_migration(self, get_mock, get_inst_mock,"},{"line_number":1157,"context_line":"                                         migr_mock, get_cn_mock, pci_mock,"},{"line_number":1158,"context_line":"                                         instance_pci_mock,"},{"line_number":1159,"context_line":"                                         mock_is_volume_backed_instance):"},{"line_number":1160,"context_line":"        # We test the behavior of update_available_resource() when"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_7ed81752","line":1157,"range":{"start_line":1157,"start_character":40,"end_line":1157,"end_character":41},"in_reply_to":"9f560f44_0a746bce","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":1169,"context_line":"        # Setup virt resources to match used resources to number"},{"line_number":1170,"context_line":"        # of defined instances on the hypervisor"},{"line_number":1171,"context_line":"        virt_resources \u003d copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)"},{"line_number":1172,"context_line":"        virt_resources.update(vcpus_used\u003d2,"},{"line_number":1173,"context_line":"                              memory_mb_used\u003d256,"},{"line_number":1174,"context_line":"                              local_gb_used\u003d5)"},{"line_number":1175,"context_line":"        self._setup_rt(virt_resources\u003dvirt_resources)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_4a282396","line":1172,"range":{"start_line":1172,"start_character":8,"end_line":1172,"end_character":43},"updated":"2020-08-20 10:46:35.000000000","message":"nit:\n\n  virt_resources.update(\n      vcpus_used\u003d2, ...)","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":1169,"context_line":"        # Setup virt resources to match used resources to number"},{"line_number":1170,"context_line":"        # of defined instances on the hypervisor"},{"line_number":1171,"context_line":"        virt_resources \u003d copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)"},{"line_number":1172,"context_line":"        virt_resources.update(vcpus_used\u003d2,"},{"line_number":1173,"context_line":"                              memory_mb_used\u003d256,"},{"line_number":1174,"context_line":"                              local_gb_used\u003d5)"},{"line_number":1175,"context_line":"        self._setup_rt(virt_resources\u003dvirt_resources)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_ded96356","line":1172,"range":{"start_line":1172,"start_character":8,"end_line":1172,"end_character":43},"in_reply_to":"9f560f44_4a282396","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":1203,"context_line":"        }"},{"line_number":1204,"context_line":"        _update_compute_node(expected_resources, **vals)"},{"line_number":1205,"context_line":"        actual_resources \u003d update_mock.call_args[0][1]"},{"line_number":1206,"context_line":"        self.assertTrue(obj_base.obj_equal_prims(expected_resources,"},{"line_number":1207,"context_line":"                                                 actual_resources))"},{"line_number":1208,"context_line":"        update_mock.assert_called_once()"},{"line_number":1209,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_8a1fdb6a","line":1206,"range":{"start_line":1206,"start_character":8,"end_line":1206,"end_character":68},"updated":"2020-08-20 10:46:35.000000000","message":"nit:\n\n  self.assertTrue(\n      obj_base.obj_equal_prims(expected_resources, actual_resources))","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":1203,"context_line":"        }"},{"line_number":1204,"context_line":"        _update_compute_node(expected_resources, **vals)"},{"line_number":1205,"context_line":"        actual_resources \u003d update_mock.call_args[0][1]"},{"line_number":1206,"context_line":"        self.assertTrue(obj_base.obj_equal_prims(expected_resources,"},{"line_number":1207,"context_line":"                                                 actual_resources))"},{"line_number":1208,"context_line":"        update_mock.assert_called_once()"},{"line_number":1209,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_bede2f3b","line":1206,"range":{"start_line":1206,"start_character":8,"end_line":1206,"end_character":68},"in_reply_to":"9f560f44_8a1fdb6a","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0d0ca6fc4016981fb07a6c12140b1ae707920338","unresolved":false,"context_lines":[{"line_number":3212,"context_line":"        instance.migration_context \u003d objects.MigrationContext("},{"line_number":3213,"context_line":"            migration_id\u003dmig2.id)"},{"line_number":3214,"context_line":"        ctxt \u003d mock.MagicMock()"},{"line_number":3215,"context_line":"        self.rt._update_usage_from_migrations(ctxt, mig_list,"},{"line_number":3216,"context_line":"                                              _NODENAME)"},{"line_number":3217,"context_line":"        upd_mock.assert_called_once_with(ctxt, instance, mig2,"},{"line_number":3218,"context_line":"                                         _NODENAME)"},{"line_number":3219,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_c674b96a","line":3216,"range":{"start_line":3215,"start_character":61,"end_line":3216,"end_character":56},"updated":"2020-08-27 13:44:08.000000000","message":"nit: can be on one line now. Below too","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"ac2f219ab47ebe7af4a111d6b8b5a2b998a98ab5","unresolved":false,"context_lines":[{"line_number":3212,"context_line":"        instance.migration_context \u003d objects.MigrationContext("},{"line_number":3213,"context_line":"            migration_id\u003dmig2.id)"},{"line_number":3214,"context_line":"        ctxt \u003d mock.MagicMock()"},{"line_number":3215,"context_line":"        self.rt._update_usage_from_migrations(ctxt, mig_list,"},{"line_number":3216,"context_line":"                                              _NODENAME)"},{"line_number":3217,"context_line":"        upd_mock.assert_called_once_with(ctxt, instance, mig2,"},{"line_number":3218,"context_line":"                                         _NODENAME)"},{"line_number":3219,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_ab6341dd","line":3216,"range":{"start_line":3215,"start_character":61,"end_line":3216,"end_character":56},"in_reply_to":"9f560f44_c674b96a","updated":"2020-08-28 09:07:17.000000000","message":"Done","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0d0ca6fc4016981fb07a6c12140b1ae707920338","unresolved":false,"context_lines":[{"line_number":3252,"context_line":"                              instance\u003dinstance)"},{"line_number":3253,"context_line":"        mig_list \u003d objects.MigrationList(objects\u003d[mig1])"},{"line_number":3254,"context_line":"        ctxt \u003d mock.MagicMock()"},{"line_number":3255,"context_line":"        self.rt._update_usage_from_migrations(ctxt, mig_list,"},{"line_number":3256,"context_line":"                                              _NODENAME)"},{"line_number":3257,"context_line":"        self.assertFalse(upd_mock.called)"},{"line_number":3258,"context_line":"        self.assertEqual(mig1.status, \"error\")"},{"line_number":3259,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_a66f0590","line":3256,"range":{"start_line":3255,"start_character":61,"end_line":3256,"end_character":56},"updated":"2020-08-27 13:44:08.000000000","message":"ditto","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"ac2f219ab47ebe7af4a111d6b8b5a2b998a98ab5","unresolved":false,"context_lines":[{"line_number":3252,"context_line":"                              instance\u003dinstance)"},{"line_number":3253,"context_line":"        mig_list \u003d objects.MigrationList(objects\u003d[mig1])"},{"line_number":3254,"context_line":"        ctxt \u003d mock.MagicMock()"},{"line_number":3255,"context_line":"        self.rt._update_usage_from_migrations(ctxt, mig_list,"},{"line_number":3256,"context_line":"                                              _NODENAME)"},{"line_number":3257,"context_line":"        self.assertFalse(upd_mock.called)"},{"line_number":3258,"context_line":"        self.assertEqual(mig1.status, \"error\")"},{"line_number":3259,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_0b618dd4","line":3256,"range":{"start_line":3255,"start_character":61,"end_line":3256,"end_character":56},"in_reply_to":"9f560f44_a66f0590","updated":"2020-08-28 09:07:17.000000000","message":"Done","commit_id":"3f508c0a6f619ab0d7a4f0f8175f3c3ad8a1cb46"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"71ccf6677b505f389086f7cd029336b2a2ca2e46","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"    @mock.patch(\u0027nova.objects.InstanceList.get_by_host_and_node\u0027)"},{"line_number":1153,"context_line":"    def test_no_instances_err_migration("},{"line_number":1154,"context_line":"        self, get_mock, get_inst_mock, migr_mock, get_cn_mock, pci_mock,"},{"line_number":1155,"context_line":"        instance_pci_mock, mock_is_volume_backed_instance):"},{"line_number":1156,"context_line":"        # We test the behavior of update_available_resource() when"},{"line_number":1157,"context_line":"        # there is an error migration that involves this compute node"},{"line_number":1158,"context_line":"        # as the destination host not the source host, and the resource"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_34735b50","line":1155,"range":{"start_line":1155,"start_character":57,"end_line":1155,"end_character":59},"updated":"2020-09-09 16:47:38.000000000","message":"nit:\n\n      instance_pci_mock, mock_is_volume_backed_instance,\n  ):","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"4c047e3864cdbb891531a5959bb66e614f36b9c2","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"    @mock.patch(\u0027nova.objects.InstanceList.get_by_host_and_node\u0027)"},{"line_number":1153,"context_line":"    def test_no_instances_err_migration("},{"line_number":1154,"context_line":"        self, get_mock, get_inst_mock, migr_mock, get_cn_mock, pci_mock,"},{"line_number":1155,"context_line":"        instance_pci_mock, mock_is_volume_backed_instance):"},{"line_number":1156,"context_line":"        # We test the behavior of update_available_resource() when"},{"line_number":1157,"context_line":"        # there is an error migration that involves this compute node"},{"line_number":1158,"context_line":"        # as the destination host not the source host, and the resource"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_24284031","line":1155,"range":{"start_line":1155,"start_character":57,"end_line":1155,"end_character":59},"in_reply_to":"9f560f44_34735b50","updated":"2020-09-10 07:25:23.000000000","message":"Done","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"}],"nova/virt/driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7aa86af28eadf68656c6e7186efc846ed9a1351","unresolved":false,"context_lines":[{"line_number":1694,"context_line":"        and cleanup other specific resources or whatever we add in the future."},{"line_number":1695,"context_line":""},{"line_number":1696,"context_line":"        :param instance: nova.objects.instance.Instance"},{"line_number":1697,"context_line":"        :param del_inst_files_only: only delete instance files if True"},{"line_number":1698,"context_line":"        :returns: True if the cleanup is successful, else false."},{"line_number":1699,"context_line":"        \"\"\""},{"line_number":1700,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_98cb9a51","line":1697,"range":{"start_line":1697,"start_character":0,"end_line":1697,"end_character":70},"updated":"2020-09-08 09:25:35.000000000","message":"This seems to defeat the entire purpose of reworking this API instead of simply adding a new one [1]. Is there any reason we need to do this?\n\n[1] https://review.opendev.org/#/c/714653/5/nova/virt/driver.py@1858","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def cleanup_instance(self, instance):"},{"line_number":1462,"context_line":"        success \u003d self._delete_instance_files(instance)"},{"line_number":1463,"context_line":"        self._cleanup_specific_resources(instance)"},{"line_number":1464,"context_line":"        return success"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"    def _cleanup_specific_resources(self, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_ed1e715d","line":1463,"updated":"2020-08-20 10:46:35.000000000","message":"Shouldn\u0027t we return False if we have vpmems but failed to clean them up?\n\nLater: I guess we\u0027ll actually raise an exception. Could you note that here with a comment?","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"    def cleanup_instance(self, instance):"},{"line_number":1462,"context_line":"        success \u003d self._delete_instance_files(instance)"},{"line_number":1463,"context_line":"        self._cleanup_specific_resources(instance)"},{"line_number":1464,"context_line":"        return success"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"    def _cleanup_specific_resources(self, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_3eea1fd7","line":1463,"in_reply_to":"9f560f44_ed1e715d","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4b6238c19dd7abfd0d1fa676b6e44e1fec8a7c76","unresolved":false,"context_lines":[{"line_number":1463,"context_line":"        self._cleanup_specific_resources(instance)"},{"line_number":1464,"context_line":"        return success"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"    def _cleanup_specific_resources(self, instance):"},{"line_number":1467,"context_line":"        # zero the data on backend pmem device"},{"line_number":1468,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1469,"context_line":"        if vpmems:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_8d2d3597","line":1466,"range":{"start_line":1466,"start_character":8,"end_line":1466,"end_character":35},"updated":"2020-08-20 10:46:35.000000000","message":"Can we give this a less generic name now that we have the generic helper function (cleanup_instance). In fact, could we just fold this into \u0027cleanup_instance\u0027 since we already hve a \u0027_cleanup_vpmems\u0027 function that we\u0027re using?","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"b768af46d0bac240cb3c671536b7e206e8133dc7","unresolved":false,"context_lines":[{"line_number":1463,"context_line":"        self._cleanup_specific_resources(instance)"},{"line_number":1464,"context_line":"        return success"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"    def _cleanup_specific_resources(self, instance):"},{"line_number":1467,"context_line":"        # zero the data on backend pmem device"},{"line_number":1468,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1469,"context_line":"        if vpmems:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_9ebc0bdd","line":1466,"range":{"start_line":1466,"start_character":8,"end_line":1466,"end_character":35},"in_reply_to":"9f560f44_8d2d3597","updated":"2020-08-24 09:17:11.000000000","message":"Done","commit_id":"9c1a1ce21c3088afe7e2b171dfbb19680d0f7a20"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"9642daf6486c55c2746eaa34097e0494dfc59c65","unresolved":false,"context_lines":[{"line_number":1492,"context_line":"        self._undefine_domain(instance)"},{"line_number":1493,"context_line":""},{"line_number":1494,"context_line":"    def cleanup_instance(self, instance):"},{"line_number":1495,"context_line":"        success \u003d self._delete_instance_files(instance)"},{"line_number":1496,"context_line":"        # zero the data on backend pmem device, if fails"},{"line_number":1497,"context_line":"        # it will raise an exception"},{"line_number":1498,"context_line":"        vpmems \u003d self._get_vpmems(instance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_53a04092","line":1495,"range":{"start_line":1495,"start_character":7,"end_line":1495,"end_character":55},"updated":"2020-08-31 05:12:58.000000000","message":"should we stop vpmem cleanup if delete instance files failed?","commit_id":"27c22938da9a35ec6b57ef9ee7c3d83575d745bf"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"0b3d5d01ffe6d22184515e301ac1cf44a7f2d373","unresolved":false,"context_lines":[{"line_number":1492,"context_line":"        self._undefine_domain(instance)"},{"line_number":1493,"context_line":""},{"line_number":1494,"context_line":"    def cleanup_instance(self, instance):"},{"line_number":1495,"context_line":"        success \u003d self._delete_instance_files(instance)"},{"line_number":1496,"context_line":"        # zero the data on backend pmem device, if fails"},{"line_number":1497,"context_line":"        # it will raise an exception"},{"line_number":1498,"context_line":"        vpmems \u003d self._get_vpmems(instance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_ff3021a1","line":1495,"range":{"start_line":1495,"start_character":7,"end_line":1495,"end_character":55},"in_reply_to":"9f560f44_53a04092","updated":"2020-09-01 10:31:07.000000000","message":"If you think it\u0027s necessary I could update.","commit_id":"27c22938da9a35ec6b57ef9ee7c3d83575d745bf"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"afe4d25a2bc4ae2832ce7b66d65b9d92d68fc310","unresolved":false,"context_lines":[{"line_number":1492,"context_line":"        self._undefine_domain(instance)"},{"line_number":1493,"context_line":""},{"line_number":1494,"context_line":"    def cleanup_instance(self, instance):"},{"line_number":1495,"context_line":"        success \u003d self._delete_instance_files(instance)"},{"line_number":1496,"context_line":"        # zero the data on backend pmem device, if fails"},{"line_number":1497,"context_line":"        # it will raise an exception"},{"line_number":1498,"context_line":"        vpmems \u003d self._get_vpmems(instance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_c86e36ff","line":1495,"range":{"start_line":1495,"start_character":7,"end_line":1495,"end_character":55},"in_reply_to":"9f560f44_ff3021a1","updated":"2020-09-02 07:46:51.000000000","message":"later: we should not stop cleanup vpmem, in _cleanup_incomplete_migrations we also invoke this function, it only print a log if cleanup_instance fails, I think the reason is it\u0027s previously only deleting files, it doesn\u0027t matter leave the files there, now we should not let it pass if other cleanup not completed.","commit_id":"27c22938da9a35ec6b57ef9ee7c3d83575d745bf"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"9642daf6486c55c2746eaa34097e0494dfc59c65","unresolved":false,"context_lines":[{"line_number":1496,"context_line":"        # zero the data on backend pmem device, if fails"},{"line_number":1497,"context_line":"        # it will raise an exception"},{"line_number":1498,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1499,"context_line":"        if vpmems:"},{"line_number":1500,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1501,"context_line":"        return success"},{"line_number":1502,"context_line":""},{"line_number":1503,"context_line":"    def _cleanup_vpmems(self, vpmems):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_f39b5463","line":1500,"range":{"start_line":1499,"start_character":8,"end_line":1500,"end_character":40},"updated":"2020-08-31 05:12:58.000000000","message":"should we return false if cleanup vpmem failed?","commit_id":"27c22938da9a35ec6b57ef9ee7c3d83575d745bf"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"0b3d5d01ffe6d22184515e301ac1cf44a7f2d373","unresolved":false,"context_lines":[{"line_number":1496,"context_line":"        # zero the data on backend pmem device, if fails"},{"line_number":1497,"context_line":"        # it will raise an exception"},{"line_number":1498,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1499,"context_line":"        if vpmems:"},{"line_number":1500,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1501,"context_line":"        return success"},{"line_number":1502,"context_line":""},{"line_number":1503,"context_line":"    def _cleanup_vpmems(self, vpmems):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_df00bdae","line":1500,"range":{"start_line":1499,"start_character":8,"end_line":1500,"end_character":40},"in_reply_to":"9f560f44_f39b5463","updated":"2020-09-01 10:31:07.000000000","message":"return false is ok but we need raise exception when it\u0027s false, so I prefer let it raise here.","commit_id":"27c22938da9a35ec6b57ef9ee7c3d83575d745bf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7aa86af28eadf68656c6e7186efc846ed9a1351","unresolved":false,"context_lines":[{"line_number":1425,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1426,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed"},{"line_number":1427,"context_line":"        \"\"\""},{"line_number":1428,"context_line":"        # zero the data on backend pmem device"},{"line_number":1429,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1430,"context_line":"        if vpmems:"},{"line_number":1431,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1432,"context_line":""},{"line_number":1433,"context_line":"        if destroy_vifs:"},{"line_number":1434,"context_line":"            self._unplug_vifs(instance, network_info, True)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_b8d3fe00","line":1431,"range":{"start_line":1428,"start_character":0,"end_line":1431,"end_character":40},"updated":"2020-09-08 09:25:35.000000000","message":"Instead of doing this here...","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7aa86af28eadf68656c6e7186efc846ed9a1351","unresolved":false,"context_lines":[{"line_number":1477,"context_line":"        if cleanup_instance_dir:"},{"line_number":1478,"context_line":"            attempts \u003d int(instance.system_metadata.get(\u0027clean_attempts\u0027,"},{"line_number":1479,"context_line":"                                                        \u00270\u0027))"},{"line_number":1480,"context_line":"            success \u003d self._delete_instance_files(instance)"},{"line_number":1481,"context_line":"            # NOTE(mriedem): This is used in the _run_pending_instance_deletes"},{"line_number":1482,"context_line":"            # periodic task in the compute manager. The tight coupling is not"},{"line_number":1483,"context_line":"            # great..."}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_f80a9687","line":1480,"range":{"start_line":1480,"start_character":12,"end_line":1480,"end_character":59},"updated":"2020-09-08 09:25:35.000000000","message":"Could you change this call to \u0027cleanup_instance\u0027? does that make sense?","commit_id":"6008f7edc96d60b4c4f0e907283bea803c4aa1d3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"71ccf6677b505f389086f7cd029336b2a2ca2e46","unresolved":false,"context_lines":[{"line_number":1418,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed"},{"line_number":1419,"context_line":"        \"\"\""},{"line_number":1420,"context_line":"        # zero the data on backend pmem device"},{"line_number":1421,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1422,"context_line":"        if vpmems:"},{"line_number":1423,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1424,"context_line":""},{"line_number":1425,"context_line":"        if destroy_vifs:"},{"line_number":1426,"context_line":"            self._unplug_vifs(instance, network_info, True)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_94e7c7e3","line":1423,"range":{"start_line":1421,"start_character":0,"end_line":1423,"end_character":40},"updated":"2020-09-09 16:47:38.000000000","message":"Can you replace this with a call to \u0027cleanup_lingering_instance_resources\u0027?","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"4c047e3864cdbb891531a5959bb66e614f36b9c2","unresolved":false,"context_lines":[{"line_number":1418,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed"},{"line_number":1419,"context_line":"        \"\"\""},{"line_number":1420,"context_line":"        # zero the data on backend pmem device"},{"line_number":1421,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1422,"context_line":"        if vpmems:"},{"line_number":1423,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1424,"context_line":""},{"line_number":1425,"context_line":"        if destroy_vifs:"},{"line_number":1426,"context_line":"            self._unplug_vifs(instance, network_info, True)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_e48da831","line":1423,"range":{"start_line":1421,"start_character":0,"end_line":1423,"end_character":40},"in_reply_to":"9f560f44_94e7c7e3","updated":"2020-09-10 07:25:23.000000000","message":"Emm...since the new interface is for cleanup \"lingering\" instance, such as running deleted instance and incomplete migrations, so I\u0027d like not invoke the new function here.","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"71ccf6677b505f389086f7cd029336b2a2ca2e46","unresolved":false,"context_lines":[{"line_number":1487,"context_line":"        # it will raise an exception"},{"line_number":1488,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1489,"context_line":"        if vpmems:"},{"line_number":1490,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"    def _cleanup_vpmems(self, vpmems):"},{"line_number":1493,"context_line":"        for vpmem in vpmems:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_f43d632c","line":1490,"updated":"2020-09-09 16:47:38.000000000","message":"Why do we not have to worry about disks or VIFs or anything else? Basically, why introduce this instead of simply calling \u0027cleanup\u0027? We could harden that to handle things like failure to unplug VIFs, for example?","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"},{"author":{"_account_id":23598,"name":"Zhong Luyao","email":"luyao.zhong@intel.com","username":"ZhongLuyao"},"change_message_id":"4c047e3864cdbb891531a5959bb66e614f36b9c2","unresolved":false,"context_lines":[{"line_number":1487,"context_line":"        # it will raise an exception"},{"line_number":1488,"context_line":"        vpmems \u003d self._get_vpmems(instance)"},{"line_number":1489,"context_line":"        if vpmems:"},{"line_number":1490,"context_line":"            self._cleanup_vpmems(vpmems)"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"    def _cleanup_vpmems(self, vpmems):"},{"line_number":1493,"context_line":"        for vpmem in vpmems:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_245cc07b","line":1490,"in_reply_to":"9f560f44_f43d632c","updated":"2020-09-10 07:25:23.000000000","message":"actually I don\u0027t know why _cleanup_incomplete_migrations only call \u0027delete_instance_files\u0027, so I was just thinking  it\u0027s no need to cleanup other things like vif or volume? I introduce a new interface, because original code does not call \u0027cleanup\u0027. And how could I handle unflug VIFs fail? do you mean raise an exception? It seems \u0027cleanup\u0027 will ignore the VIFs unplug fail.[1]\n\n[1https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1181]","commit_id":"68265373759b414c0aba0bc569551f3c8c87c3ad"}]}
