)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5ea9db925d078a8619bcdb05f7c9b8f030ab097","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kevin_Zheng \u003czhengzhenyu@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-06-06 11:39:49 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Abort live migration in queue (1/2)."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch modified compute manager and rpcapi"},{"line_number":10,"context_line":"to be able to abort migration in ``queued`` and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5f7c97a3_579c6c5a","line":7,"range":{"start_line":7,"start_character":30,"end_line":7,"end_character":35},"updated":"2018-06-06 13:39:15.000000000","message":"nit: rather than do this in commit message titles, prefix the title with the part of code or service for which this change applies, e.g. \"compute: add support to abort queued live migration\".","commit_id":"2af4644805a0b5641f42a85eb467b9c75936e307"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Compute: add support to abort queued live migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch modified compute manager and rpcapi"},{"line_number":10,"context_line":"to be able to abort migration in ``queued`` and"},{"line_number":11,"context_line":"``preparing`` status. The following patch will"},{"line_number":12,"context_line":"modify the API part."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Part of blueprint abort-live-migration-in-queued-status"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"5f7c97a3_122f2bf5","line":11,"range":{"start_line":10,"start_character":33,"end_line":11,"end_character":20},"updated":"2018-07-10 20:20:00.000000000","message":"Looking at the state transitions for live migration, it looks like we have:\n\naccepted -\u003e queued -\u003e preparing -\u003e running -\u003e completed (assuming no errors occur)\n\nWe can\u0027t abort a live migration in \u0027accepted\u0027 status at this point since that\u0027s during scheduling in conductor and we don\u0027t have hooks there.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10d310e85f53e1ac19c0485e140b14fa002e71a4","unresolved":false,"context_lines":[{"line_number":4424,"context_line":"        LOG.debug(\"Going to cancel live migration %s\","},{"line_number":4425,"context_line":"                  migration.id, instance\u003dinstance)"},{"line_number":4426,"context_line":""},{"line_number":4427,"context_line":"        if migration.status !\u003d \u0027running\u0027:"},{"line_number":4428,"context_line":"            raise exception.InvalidMigrationState(migration_id\u003dmigration_id,"},{"line_number":4429,"context_line":"                    instance_uuid\u003dinstance.uuid,"},{"line_number":4430,"context_line":"                    state\u003dmigration.status,"},{"line_number":4431,"context_line":"                    method\u003d\u0027abort live migration\u0027)"},{"line_number":4432,"context_line":"        self._record_action_start(context, instance,"},{"line_number":4433,"context_line":"                                  instance_actions.LIVE_MIGRATION_CANCEL)"},{"line_number":4434,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_a07fbc67","line":4431,"range":{"start_line":4427,"start_character":8,"end_line":4431,"end_character":50},"updated":"2018-07-13 01:57:12.000000000","message":"If self.compute_rpcapi.live_migration_abort is doing this same check, I\u0027m not sure we need this here - can\u0027t we just move it?","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"bfe578cf2381118d10336891f8671f4fd795295d","unresolved":false,"context_lines":[{"line_number":4424,"context_line":"        LOG.debug(\"Going to cancel live migration %s\","},{"line_number":4425,"context_line":"                  migration.id, instance\u003dinstance)"},{"line_number":4426,"context_line":""},{"line_number":4427,"context_line":"        if migration.status !\u003d \u0027running\u0027:"},{"line_number":4428,"context_line":"            raise exception.InvalidMigrationState(migration_id\u003dmigration_id,"},{"line_number":4429,"context_line":"                    instance_uuid\u003dinstance.uuid,"},{"line_number":4430,"context_line":"                    state\u003dmigration.status,"},{"line_number":4431,"context_line":"                    method\u003d\u0027abort live migration\u0027)"},{"line_number":4432,"context_line":"        self._record_action_start(context, instance,"},{"line_number":4433,"context_line":"                                  instance_actions.LIVE_MIGRATION_CANCEL)"},{"line_number":4434,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_a30026b3","line":4431,"range":{"start_line":4427,"start_character":8,"end_line":4431,"end_character":50},"in_reply_to":"5f7c97a3_a07fbc67","updated":"2018-07-13 03:37:58.000000000","message":"Hmm, maybe, thinking again, what should we do about the record_action_start? maybe we should also move that to rpc, or maybe we should leave it as is and remove this in next cycle?","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b107d215d8cd4003c90ef1302f1d1f5d74edeb20","unresolved":false,"context_lines":[{"line_number":4429,"context_line":"                    instance_uuid\u003dinstance.uuid,"},{"line_number":4430,"context_line":"                    state\u003dmigration.status,"},{"line_number":4431,"context_line":"                    method\u003d\u0027abort live migration\u0027)"},{"line_number":4432,"context_line":"        self._record_action_start(context, instance,"},{"line_number":4433,"context_line":"                                  instance_actions.LIVE_MIGRATION_CANCEL)"},{"line_number":4434,"context_line":""},{"line_number":4435,"context_line":"        self.compute_rpcapi.live_migration_abort(context,"},{"line_number":4436,"context_line":"                instance, migration.id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_cc88ef22","side":"PARENT","line":4433,"range":{"start_line":4432,"start_character":0,"end_line":4433,"end_character":73},"updated":"2018-07-13 07:22:29.000000000","message":"I thought you removed this accidentally but after seeing nova/compute/rpcapi.py file change in this patch i got it. \n\nin compute.rpcapi only we know whether action of live migration abort is started or not. \n\n+1","commit_id":"10adbb6762a7145d0f61f67739049f1cbf4bd427"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"156ee3258bc8b5d29558170b25043be7bf09c979","unresolved":false,"context_lines":[{"line_number":6210,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6211,"context_line":"            phase\u003dfields.NotificationPhase.START)"},{"line_number":6212,"context_line":"        if instance.uuid in self._waiting_live_migrations:"},{"line_number":6213,"context_line":"            if self._waiting_live_migrations[instance.uuid][1]:"},{"line_number":6214,"context_line":"                self._waiting_live_migrations[instance.uuid][1].cancel()"},{"line_number":6215,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6216,"context_line":"            if migration.status in (\u0027queued\u0027, \u0027preparing\u0027):"},{"line_number":6217,"context_line":"                self._set_migration_status("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_a9206a9c","line":6214,"range":{"start_line":6213,"start_character":12,"end_line":6214,"end_character":72},"updated":"2018-05-15 12:49:44.000000000","message":"nit: this would be more clear if you did:\n\nfuture \u003d self._waiting_live_migrations[instance.uuid][1]\nif future:\n    future.cancel()","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"93a87c043135554175be156d7db93842199dfb58","unresolved":false,"context_lines":[{"line_number":6210,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6211,"context_line":"            phase\u003dfields.NotificationPhase.START)"},{"line_number":6212,"context_line":"        if instance.uuid in self._waiting_live_migrations:"},{"line_number":6213,"context_line":"            if self._waiting_live_migrations[instance.uuid][1]:"},{"line_number":6214,"context_line":"                self._waiting_live_migrations[instance.uuid][1].cancel()"},{"line_number":6215,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6216,"context_line":"            if migration.status in (\u0027queued\u0027, \u0027preparing\u0027):"},{"line_number":6217,"context_line":"                self._set_migration_status("}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_699d6408","line":6214,"range":{"start_line":6213,"start_character":12,"end_line":6214,"end_character":72},"in_reply_to":"5f7c97a3_a9206a9c","updated":"2018-05-16 01:23:00.000000000","message":"Done","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"156ee3258bc8b5d29558170b25043be7bf09c979","unresolved":false,"context_lines":[{"line_number":6212,"context_line":"        if instance.uuid in self._waiting_live_migrations:"},{"line_number":6213,"context_line":"            if self._waiting_live_migrations[instance.uuid][1]:"},{"line_number":6214,"context_line":"                self._waiting_live_migrations[instance.uuid][1].cancel()"},{"line_number":6215,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6216,"context_line":"            if migration.status in (\u0027queued\u0027, \u0027preparing\u0027):"},{"line_number":6217,"context_line":"                self._set_migration_status("},{"line_number":6218,"context_line":"                    self._waiting_live_migrations[instance.uuid][0],"},{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_69433243","line":6219,"range":{"start_line":6215,"start_character":12,"end_line":6219,"end_character":31},"updated":"2018-05-15 12:49:44.000000000","message":"Shouldn\u0027t this be in the if block if the Future is set? Otherwise we could have cancelled the live migration and then potentially *not* set the migration to cancelled here. I\u0027m not even sure we need the status check again here, the API is already going to be checking for that right? Also, I don\u0027t think you can cancel a running Future, so if the task is running then cancel() should return False:\n\nhttps://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.Future.cancel\n\nSo you should really have something like:\n\nif future and future.cancel():\n    self._set_migration_status(\n        self._waiting_live_migrations[instance.uuid][0],\n        \u0027canceled\u0027)","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"670fe5ad92500c3e840c7a128b0bc4976c8f2304","unresolved":false,"context_lines":[{"line_number":6212,"context_line":"        if instance.uuid in self._waiting_live_migrations:"},{"line_number":6213,"context_line":"            if self._waiting_live_migrations[instance.uuid][1]:"},{"line_number":6214,"context_line":"                self._waiting_live_migrations[instance.uuid][1].cancel()"},{"line_number":6215,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6216,"context_line":"            if migration.status in (\u0027queued\u0027, \u0027preparing\u0027):"},{"line_number":6217,"context_line":"                self._set_migration_status("},{"line_number":6218,"context_line":"                    self._waiting_live_migrations[instance.uuid][0],"},{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_ac7f2e13","line":6219,"range":{"start_line":6215,"start_character":12,"end_line":6219,"end_character":31},"in_reply_to":"5f7c97a3_298becc2","updated":"2018-05-30 21:27:39.000000000","message":"You really need some way of signaling to a migration that is in the pre-check stage before it goes to driver.live_migration(), after which the normal abort will work.\n\nSo, how about this for do_live_migration():\n\n A1. Do the pre-check stuff straight away\n A2. Before calling driver.live_migration(), attempt to pop our future out of the list. If it\u0027s there, then we can continue, then we were canceled so exit\n A3. Call driver.live_migration()\n\nFor the cancel logic here:\n\n B1. Immediately attempt to pop the future from the list. If it\u0027s not there, then the migration must be running, so call driver.live_migration_abort(). This could also happen if we\u0027re the second cancel call, but that\u0027s fine.\n B2. If we got a future, call cancel() on it. If that succeeds, we\u0027re done\n B3. If the cancel() failed, we must have raced with the startup. This must have happened before A2 above, so we know the process will stop when it gets to that step anyway.\n\nThere is still potentially a very small race between step A2 and A3+ where we have popped, but a live_migration_abort() would fail because the driver hasn\u0027t really started a live migration yet. However, in eventlet-land this probably can\u0027t/won\u0027t happen, it\u0027s a very small window, and they could just try again.","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"93a87c043135554175be156d7db93842199dfb58","unresolved":false,"context_lines":[{"line_number":6212,"context_line":"        if instance.uuid in self._waiting_live_migrations:"},{"line_number":6213,"context_line":"            if self._waiting_live_migrations[instance.uuid][1]:"},{"line_number":6214,"context_line":"                self._waiting_live_migrations[instance.uuid][1].cancel()"},{"line_number":6215,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6216,"context_line":"            if migration.status in (\u0027queued\u0027, \u0027preparing\u0027):"},{"line_number":6217,"context_line":"                self._set_migration_status("},{"line_number":6218,"context_line":"                    self._waiting_live_migrations[instance.uuid][0],"},{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_298becc2","line":6219,"range":{"start_line":6215,"start_character":12,"end_line":6219,"end_character":31},"in_reply_to":"5f7c97a3_69433243","updated":"2018-05-16 01:23:00.000000000","message":"Ah, yes, that what I miss, I was afraid of cancelling a running migration and left garbages.","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"3de411dd55cac654c4ea0a7bd8b1496ea9e96bd6","unresolved":false,"context_lines":[{"line_number":6212,"context_line":"        if instance.uuid in self._waiting_live_migrations:"},{"line_number":6213,"context_line":"            if self._waiting_live_migrations[instance.uuid][1]:"},{"line_number":6214,"context_line":"                self._waiting_live_migrations[instance.uuid][1].cancel()"},{"line_number":6215,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6216,"context_line":"            if migration.status in (\u0027queued\u0027, \u0027preparing\u0027):"},{"line_number":6217,"context_line":"                self._set_migration_status("},{"line_number":6218,"context_line":"                    self._waiting_live_migrations[instance.uuid][0],"},{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_61c7d897","line":6219,"range":{"start_line":6215,"start_character":12,"end_line":6219,"end_character":31},"in_reply_to":"5f7c97a3_ac7f2e13","updated":"2018-05-31 07:56:01.000000000","message":"Thanks alot, I think this do makes sense.","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"156ee3258bc8b5d29558170b25043be7bf09c979","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"},{"line_number":6223,"context_line":"                migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6224,"context_line":""},{"line_number":6225,"context_line":"            if migration.status \u003d\u003d \u0027running\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_a9a38aff","line":6222,"range":{"start_line":6222,"start_character":44,"end_line":6222,"end_character":49},"updated":"2018-05-15 12:49:44.000000000","message":"Why was this added?\n\nAlso, why did you add a while loop? This looks like it could be an infinite loop especially if multiple threads are trying to cancel the live migration at the same time, the migration here could theoretically be cancelled. If we did this, you\u0027d likely need a lock around all of this code, but I\u0027m not sure why we need this. I\u0027m not sure why the else block here doesn\u0027t just contain the code that already existed in this method.","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"89c9d4c651ccf6c50f482131f0ce509667f66cda","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"},{"line_number":6223,"context_line":"                migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6224,"context_line":""},{"line_number":6225,"context_line":"            if migration.status \u003d\u003d \u0027running\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_4c89ba72","line":6222,"range":{"start_line":6222,"start_character":44,"end_line":6222,"end_character":49},"in_reply_to":"5f7c97a3_49af4829","updated":"2018-05-30 21:06:34.000000000","message":"OK so in this case, the future already started running _do_live_migration which changed the migration status to \"preparing\" and removed the instance from the queue, so you\u0027re trying to honor the abort request by waiting for the migration status change to \u0027running\u0027, in which case you\u0027d abort it as before, or turns into \u0027error\u0027 status in which case we don\u0027t need to do anything. That makes sense, but you really need a code comment here to explain the logic since this is all multi-threaded across multiple methods so it gets hard to follow. Also, I\u0027d think we\u0027d want a backoff timer on the calls to get the migration status along with potentially a timeout, you could use this:\n\nhttps://github.com/openstack/oslo.service/blob/1.31.2/oslo_service/loopingcall.py#L309","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"09789f6ec2086e4526545b66e54862a5ef338396","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"},{"line_number":6223,"context_line":"                migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6224,"context_line":""},{"line_number":6225,"context_line":"            if migration.status \u003d\u003d \u0027running\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_8c4d923c","line":6222,"range":{"start_line":6222,"start_character":44,"end_line":6222,"end_character":49},"in_reply_to":"5f7c97a3_4c89ba72","updated":"2018-05-30 21:14:02.000000000","message":"Also you might need to make this generic, because if the instance is still in _waiting_live_migrations but Future.cancel() fails because it\u0027s started executing, then we\u0027ll also have the same kind of wait-to-abort loop.","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"93a87c043135554175be156d7db93842199dfb58","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"                    \u0027canceled\u0027)"},{"line_number":6220,"context_line":"        else:"},{"line_number":6221,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6222,"context_line":"            while migration.status not in (\u0027error\u0027, \u0027running\u0027):"},{"line_number":6223,"context_line":"                migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6224,"context_line":""},{"line_number":6225,"context_line":"            if migration.status \u003d\u003d \u0027running\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_49af4829","line":6222,"range":{"start_line":6222,"start_character":44,"end_line":6222,"end_character":49},"in_reply_to":"5f7c97a3_a9a38aff","updated":"2018-05-16 01:23:00.000000000","message":"I was trying to cancel also migrations in \"preparing\" status, as a preparing migration will end up in either \"error\" or \"running\" status, so what I did here is to wait it changes to those status, if it turned into \"running\" then we cancel it using the current logic, if it turned into \"error\" we can just ignore it. Thoughts?","commit_id":"37dbad081dda16ed9c8bbe736cfb3a086b79c9f8"},{"author":{"_account_id":26311,"name":"tianhui","email":"tianhui@awcloud.com","username":"tianhui"},"change_message_id":"ac476dd4b521bf9552d6f21c04a15f2b1ee2dbdd","unresolved":false,"context_lines":[{"line_number":6238,"context_line":"        try:"},{"line_number":6239,"context_line":"            self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6240,"context_line":"        except KeyError:"},{"line_number":6241,"context_line":"            LOG.debug(\u0027Migration %s aborted by another process, rollback.\u0027)"},{"line_number":6242,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6243,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6244,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f7c97a3_f8a072a7","line":6241,"range":{"start_line":6241,"start_character":22,"end_line":6241,"end_character":73},"updated":"2018-06-26 09:07:10.000000000","message":"what is the \"%s\"","commit_id":"a357902d5a3b9480c4bcf6b2c9f007cc1c0c3b2c"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"3a1bd8aa603c6205dc86fc9cf860ac5f57b20275","unresolved":false,"context_lines":[{"line_number":6238,"context_line":"        try:"},{"line_number":6239,"context_line":"            self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6240,"context_line":"        except KeyError:"},{"line_number":6241,"context_line":"            LOG.debug(\u0027Migration %s aborted by another process, rollback.\u0027)"},{"line_number":6242,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6243,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6244,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f7c97a3_d37c7972","line":6241,"range":{"start_line":6241,"start_character":22,"end_line":6241,"end_character":73},"in_reply_to":"5f7c97a3_f8a072a7","updated":"2018-06-27 02:08:00.000000000","message":"Ah, thanks, my bad.","commit_id":"a357902d5a3b9480c4bcf6b2c9f007cc1c0c3b2c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6239,"context_line":"            self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6240,"context_line":"        except KeyError:"},{"line_number":6241,"context_line":"            LOG.debug(\u0027Migration %s aborted by another process, rollback.\u0027,"},{"line_number":6242,"context_line":"                      migration.uuid)"},{"line_number":6243,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6244,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6245,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_1235ebf5","line":6242,"updated":"2018-07-10 20:20:00.000000000","message":"Add instance\u003dinstance kwarg for context in the log message.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6239,"context_line":"            self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6240,"context_line":"        except KeyError:"},{"line_number":6241,"context_line":"            LOG.debug(\u0027Migration %s aborted by another process, rollback.\u0027,"},{"line_number":6242,"context_line":"                      migration.uuid)"},{"line_number":6243,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6244,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6245,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8413f0b7","line":6242,"in_reply_to":"5f7c97a3_1235ebf5","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6241,"context_line":"            LOG.debug(\u0027Migration %s aborted by another process, rollback.\u0027,"},{"line_number":6242,"context_line":"                      migration.uuid)"},{"line_number":6243,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6244,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6245,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"},{"line_number":6246,"context_line":"            self._notify_about_instance_usage("},{"line_number":6247,"context_line":"                context, instance, \u0027live.migration.abort.end\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_92533b3a","line":6244,"range":{"start_line":6244,"start_character":17,"end_line":6244,"end_character":41},"updated":"2018-07-10 20:20:00.000000000","message":"Is this the correct thing to call? At this point, we haven\u0027t started the actual guest transfer in the hypervisor, the only thing we\u0027ve done is pre_live_migration on the destination host, so why aren\u0027t you calling _cleanup_pre_live_migration instead like the except blocks above?\n\n(later)\n\nOK I see you\u0027re not calling _cleanup_pre_live_migration because that just sets the migration status to \u0027error\u0027 and then calls _rollback_live_migration. _cleanup_pre_live_migration seems redundant anyway since _rollback_live_migration will also set the migration status to \u0027error\u0027 but that\u0027s the way that code has been for a long time so might as well not worry about changing it here.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6244,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6245,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"},{"line_number":6246,"context_line":"            self._notify_about_instance_usage("},{"line_number":6247,"context_line":"                context, instance, \u0027live.migration.abort.end\u0027)"},{"line_number":6248,"context_line":"            compute_utils.notify_about_instance_action("},{"line_number":6249,"context_line":"                context, instance, self.host,"},{"line_number":6250,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_721bff7e","line":6247,"range":{"start_line":6247,"start_character":36,"end_line":6247,"end_character":60},"updated":"2018-07-10 20:20:00.000000000","message":"Doesn\u0027t this mean we\u0027d send this notification twice, once when live_migration_abort is called and once again when we actually abort the queued migration here in this method?\n\n(later)\n\nNo, because live_migration_abort won\u0027t send the \"end\" notification unless either the Future was cancelled (meaning _do_live_migration doesn\u0027t even run and we don\u0027t get here) or we successfully popped off the queue here and don\u0027t get the KeyError.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6250,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6251,"context_line":"                phase\u003dfields.NotificationPhase.END)"},{"line_number":6252,"context_line":""},{"line_number":6253,"context_line":"        else:"},{"line_number":6254,"context_line":"            self._set_migration_status(migration, \u0027running\u0027)"},{"line_number":6255,"context_line":"            if migrate_data:"},{"line_number":6256,"context_line":"                migrate_data.migration \u003d migration"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_b27517cb","line":6253,"updated":"2018-07-10 20:20:00.000000000","message":"Can we avoid the else block? We either successfully popped the queued migration from the queue because nothing tried to abort it, or something did abort it, we hit the KeyError and can rollback and then return from this method.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6250,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6251,"context_line":"                phase\u003dfields.NotificationPhase.END)"},{"line_number":6252,"context_line":""},{"line_number":6253,"context_line":"        else:"},{"line_number":6254,"context_line":"            self._set_migration_status(migration, \u0027running\u0027)"},{"line_number":6255,"context_line":"            if migrate_data:"},{"line_number":6256,"context_line":"                migrate_data.migration \u003d migration"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_c4db28e8","line":6253,"in_reply_to":"5f7c97a3_b27517cb","updated":"2018-07-11 02:41:31.000000000","message":"yeah, that is correct, I cannot remember why I did this...","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"2b08a8965af80aa9703793fd5f9e28d3c399f298","unresolved":false,"context_lines":[{"line_number":6250,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6251,"context_line":"                phase\u003dfields.NotificationPhase.END)"},{"line_number":6252,"context_line":""},{"line_number":6253,"context_line":"        else:"},{"line_number":6254,"context_line":"            self._set_migration_status(migration, \u0027running\u0027)"},{"line_number":6255,"context_line":"            if migrate_data:"},{"line_number":6256,"context_line":"                migrate_data.migration \u003d migration"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_37ac8b87","line":6253,"in_reply_to":"5f7c97a3_c4db28e8","updated":"2018-07-11 07:54:46.000000000","message":"Ah I remembered why I did this","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6333,"context_line":"        :param migration_id: ID of in-progress live migration"},{"line_number":6334,"context_line":""},{"line_number":6335,"context_line":"        \"\"\""},{"line_number":6336,"context_line":"        def _notify_abort_end():"},{"line_number":6337,"context_line":"            self._notify_about_instance_usage("},{"line_number":6338,"context_line":"                context, instance, \u0027live.migration.abort.end\u0027)"},{"line_number":6339,"context_line":"            compute_utils.notify_about_instance_action("},{"line_number":6340,"context_line":"                context, instance, self.host,"},{"line_number":6341,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6342,"context_line":"                phase\u003dfields.NotificationPhase.END)"},{"line_number":6343,"context_line":""},{"line_number":6344,"context_line":"        def _notify_abort_start():"},{"line_number":6345,"context_line":"            self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8d679ab8","line":6342,"range":{"start_line":6336,"start_character":8,"end_line":6342,"end_character":51},"updated":"2018-07-10 20:20:00.000000000","message":"nit: maybe move this out so it\u0027s not an inner method and then _do_live_migration can also re-use this.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6333,"context_line":"        :param migration_id: ID of in-progress live migration"},{"line_number":6334,"context_line":""},{"line_number":6335,"context_line":"        \"\"\""},{"line_number":6336,"context_line":"        def _notify_abort_end():"},{"line_number":6337,"context_line":"            self._notify_about_instance_usage("},{"line_number":6338,"context_line":"                context, instance, \u0027live.migration.abort.end\u0027)"},{"line_number":6339,"context_line":"            compute_utils.notify_about_instance_action("},{"line_number":6340,"context_line":"                context, instance, self.host,"},{"line_number":6341,"context_line":"                action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6342,"context_line":"                phase\u003dfields.NotificationPhase.END)"},{"line_number":6343,"context_line":""},{"line_number":6344,"context_line":"        def _notify_abort_start():"},{"line_number":6345,"context_line":"            self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_64ea5478","line":6342,"range":{"start_line":6336,"start_character":8,"end_line":6342,"end_character":51},"in_reply_to":"5f7c97a3_8d679ab8","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6355,"context_line":"        #    succeed, then the abort action is succeed, mark the migration"},{"line_number":6356,"context_line":"        #    status to \u0027cancelled\u0027."},{"line_number":6357,"context_line":"        # 2. The selected migration is still in queue, but the future.cancel()"},{"line_number":6358,"context_line":"        #    failed, then the process is running, and the migration status is"},{"line_number":6359,"context_line":"        #    \u0027preparing\u0027, then just pop it from the queue, and the migration"},{"line_number":6360,"context_line":"        #    process will handle it latter."},{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_cd67d2e9","line":6358,"range":{"start_line":6358,"start_character":30,"end_line":6358,"end_character":37},"updated":"2018-07-10 20:20:00.000000000","message":"Specifically, _do_live_migration has started executing, correct?","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6355,"context_line":"        #    succeed, then the abort action is succeed, mark the migration"},{"line_number":6356,"context_line":"        #    status to \u0027cancelled\u0027."},{"line_number":6357,"context_line":"        # 2. The selected migration is still in queue, but the future.cancel()"},{"line_number":6358,"context_line":"        #    failed, then the process is running, and the migration status is"},{"line_number":6359,"context_line":"        #    \u0027preparing\u0027, then just pop it from the queue, and the migration"},{"line_number":6360,"context_line":"        #    process will handle it latter."},{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_c4f48858","line":6358,"range":{"start_line":6358,"start_character":30,"end_line":6358,"end_character":37},"in_reply_to":"5f7c97a3_cd67d2e9","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6356,"context_line":"        #    status to \u0027cancelled\u0027."},{"line_number":6357,"context_line":"        # 2. The selected migration is still in queue, but the future.cancel()"},{"line_number":6358,"context_line":"        #    failed, then the process is running, and the migration status is"},{"line_number":6359,"context_line":"        #    \u0027preparing\u0027, then just pop it from the queue, and the migration"},{"line_number":6360,"context_line":"        #    process will handle it latter."},{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"},{"line_number":6362,"context_line":"        #    status is \u0027running\u0027, let the driver handle it."}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_2da206fc","line":6359,"range":{"start_line":6359,"start_character":14,"end_line":6359,"end_character":23},"updated":"2018-07-10 20:20:00.000000000","message":"And the migration status couldn\u0027t be \u0027running\u0027 in this scenario because if _do_live_migration has started executing and we\u0027ve already popped it from the queue and set the migration status to \u0027running\u0027 at this point, popping it here will raise KeyError at which point we check if it\u0027s running and if so, we abort the old way.\n\nMight be worth adding that detail to the comment.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6356,"context_line":"        #    status to \u0027cancelled\u0027."},{"line_number":6357,"context_line":"        # 2. The selected migration is still in queue, but the future.cancel()"},{"line_number":6358,"context_line":"        #    failed, then the process is running, and the migration status is"},{"line_number":6359,"context_line":"        #    \u0027preparing\u0027, then just pop it from the queue, and the migration"},{"line_number":6360,"context_line":"        #    process will handle it latter."},{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"},{"line_number":6362,"context_line":"        #    status is \u0027running\u0027, let the driver handle it."}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_648fb4d3","line":6359,"range":{"start_line":6359,"start_character":14,"end_line":6359,"end_character":23},"in_reply_to":"5f7c97a3_2da206fc","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6357,"context_line":"        # 2. The selected migration is still in queue, but the future.cancel()"},{"line_number":6358,"context_line":"        #    failed, then the process is running, and the migration status is"},{"line_number":6359,"context_line":"        #    \u0027preparing\u0027, then just pop it from the queue, and the migration"},{"line_number":6360,"context_line":"        #    process will handle it latter."},{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"},{"line_number":6362,"context_line":"        #    status is \u0027running\u0027, let the driver handle it."},{"line_number":6363,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_ed62ced8","line":6360,"range":{"start_line":6360,"start_character":36,"end_line":6360,"end_character":42},"updated":"2018-07-10 20:20:00.000000000","message":"later","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6357,"context_line":"        # 2. The selected migration is still in queue, but the future.cancel()"},{"line_number":6358,"context_line":"        #    failed, then the process is running, and the migration status is"},{"line_number":6359,"context_line":"        #    \u0027preparing\u0027, then just pop it from the queue, and the migration"},{"line_number":6360,"context_line":"        #    process will handle it latter."},{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"},{"line_number":6362,"context_line":"        #    status is \u0027running\u0027, let the driver handle it."},{"line_number":6363,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_e4abc41c","line":6360,"range":{"start_line":6360,"start_character":36,"end_line":6360,"end_character":42},"in_reply_to":"5f7c97a3_ed62ced8","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6361,"context_line":"        # 3. The selected migration is not in the queue, then the migration"},{"line_number":6362,"context_line":"        #    status is \u0027running\u0027, let the driver handle it."},{"line_number":6363,"context_line":"        try:"},{"line_number":6364,"context_line":"            tuple_to_abort \u003d self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6365,"context_line":"            future \u003d tuple_to_abort[1]"},{"line_number":6366,"context_line":"            _notify_abort_start()"},{"line_number":6367,"context_line":"            if future and future.cancel():"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_0d786a7b","line":6364,"range":{"start_line":6364,"start_character":12,"end_line":6364,"end_character":26},"updated":"2018-07-10 20:20:00.000000000","message":"nit: for readability, make this:\n\nmigration, future \u003d (\n    self._waiting_live_migrations.pop(instance.uuid))","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6363,"context_line":"        try:"},{"line_number":6364,"context_line":"            tuple_to_abort \u003d self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6365,"context_line":"            future \u003d tuple_to_abort[1]"},{"line_number":6366,"context_line":"            _notify_abort_start()"},{"line_number":6367,"context_line":"            if future and future.cancel():"},{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_ed908e98","line":6366,"range":{"start_line":6366,"start_character":12,"end_line":6366,"end_character":31},"updated":"2018-07-10 20:20:00.000000000","message":"Can\u0027t we move this outside the try/except block and just call it once? Then we don\u0027t need the inner method for the start notification.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6363,"context_line":"        try:"},{"line_number":6364,"context_line":"            tuple_to_abort \u003d self._waiting_live_migrations.pop(instance.uuid)"},{"line_number":6365,"context_line":"            future \u003d tuple_to_abort[1]"},{"line_number":6366,"context_line":"            _notify_abort_start()"},{"line_number":6367,"context_line":"            if future and future.cancel():"},{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_ef915d9d","line":6366,"range":{"start_line":6366,"start_character":12,"end_line":6366,"end_character":31},"in_reply_to":"5f7c97a3_ed908e98","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6365,"context_line":"            future \u003d tuple_to_abort[1]"},{"line_number":6366,"context_line":"            _notify_abort_start()"},{"line_number":6367,"context_line":"            if future and future.cancel():"},{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"},{"line_number":6370,"context_line":"        except KeyError:"},{"line_number":6371,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8de0fa28","line":6368,"updated":"2018-07-10 20:20:00.000000000","message":"OK so if we got here, we\u0027ve successfully aborted the queued migration and _do_live_migration won\u0027t run so we need to set the migration status to cancelled and send the notification.\n\nIf Future.cancel() fails, it means _do_live_migration is running and the migration status is preparing, and _do_live_migration itself will attempt to pop the queued migration, hit a KeyError, and rollback, set the migration to cancelled and send the live.migration.abort.end notification.\n\n(scenario 2 above)\n\nThis might be good to document inline to reinforce the logic since it\u0027s confusing to have this all in your head at once.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6365,"context_line":"            future \u003d tuple_to_abort[1]"},{"line_number":6366,"context_line":"            _notify_abort_start()"},{"line_number":6367,"context_line":"            if future and future.cancel():"},{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"},{"line_number":6370,"context_line":"        except KeyError:"},{"line_number":6371,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_44139866","line":6368,"in_reply_to":"5f7c97a3_8de0fa28","updated":"2018-07-11 02:41:31.000000000","message":"Done","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6367,"context_line":"            if future and future.cancel():"},{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"},{"line_number":6370,"context_line":"        except KeyError:"},{"line_number":6371,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6372,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":6373,"context_line":"                raise exception.InvalidMigrationState("}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_edbe4efe","line":6370,"updated":"2018-07-10 20:20:00.000000000","message":"And this is scenario 3 where _do_live_migration popped the queued migration and it should be running.\n\nDo existing tests already cover this scenario? They must.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6367,"context_line":"            if future and future.cancel():"},{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"},{"line_number":6370,"context_line":"        except KeyError:"},{"line_number":6371,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6372,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":6373,"context_line":"                raise exception.InvalidMigrationState("}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_6f7ead6e","line":6370,"in_reply_to":"5f7c97a3_edbe4efe","updated":"2018-07-11 02:41:31.000000000","message":"yeah, thats the normal case, so already covered.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"},{"line_number":6370,"context_line":"        except KeyError:"},{"line_number":6371,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6372,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":6373,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":6374,"context_line":"                    migration_id\u003dmigration_id, instance_uuid\u003dinstance.uuid,"},{"line_number":6375,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":6376,"context_line":"            _notify_abort_start()"},{"line_number":6377,"context_line":"            self.driver.live_migration_abort(instance)"},{"line_number":6378,"context_line":"            _notify_abort_end()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8d4eba36","line":6375,"range":{"start_line":6371,"start_character":12,"end_line":6375,"end_character":74},"updated":"2018-07-10 20:20:00.000000000","message":"This is still racy but maybe not any worse than what already existed here, right?","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":6368,"context_line":"                self._set_migration_status(tuple_to_abort[0], \u0027cancelled\u0027)"},{"line_number":6369,"context_line":"                _notify_abort_end()"},{"line_number":6370,"context_line":"        except KeyError:"},{"line_number":6371,"context_line":"            migration \u003d objects.Migration.get_by_id(context, migration_id)"},{"line_number":6372,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":6373,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":6374,"context_line":"                    migration_id\u003dmigration_id, instance_uuid\u003dinstance.uuid,"},{"line_number":6375,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":6376,"context_line":"            _notify_abort_start()"},{"line_number":6377,"context_line":"            self.driver.live_migration_abort(instance)"},{"line_number":6378,"context_line":"            _notify_abort_end()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_0fa6190f","line":6375,"range":{"start_line":6371,"start_character":12,"end_line":6375,"end_character":74},"in_reply_to":"5f7c97a3_8d4eba36","updated":"2018-07-11 02:41:31.000000000","message":"yeah thats true.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"643e88ea521deec3bf76c1c833ac78c72786040e","unresolved":false,"context_lines":[{"line_number":6322,"context_line":"        self._notify_about_instance_usage("},{"line_number":6323,"context_line":"            context, instance, \u0027live.migration.force.complete.end\u0027)"},{"line_number":6324,"context_line":""},{"line_number":6325,"context_line":"    def _notify_abort_end(self, context, instance):"},{"line_number":6326,"context_line":"        self._notify_about_instance_usage("},{"line_number":6327,"context_line":"            context, instance, \u0027live.migration.abort.end\u0027)"},{"line_number":6328,"context_line":"        compute_utils.notify_about_instance_action("}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_e017f445","line":6325,"range":{"start_line":6325,"start_character":8,"end_line":6325,"end_character":25},"updated":"2018-07-13 01:35:56.000000000","message":"Rename _notify_live_migrate_abort_end","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"bfe578cf2381118d10336891f8671f4fd795295d","unresolved":false,"context_lines":[{"line_number":6322,"context_line":"        self._notify_about_instance_usage("},{"line_number":6323,"context_line":"            context, instance, \u0027live.migration.force.complete.end\u0027)"},{"line_number":6324,"context_line":""},{"line_number":6325,"context_line":"    def _notify_abort_end(self, context, instance):"},{"line_number":6326,"context_line":"        self._notify_about_instance_usage("},{"line_number":6327,"context_line":"            context, instance, \u0027live.migration.abort.end\u0027)"},{"line_number":6328,"context_line":"        compute_utils.notify_about_instance_action("}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_e3fdbeb2","line":6325,"range":{"start_line":6325,"start_character":8,"end_line":6325,"end_character":25},"in_reply_to":"5f7c97a3_e017f445","updated":"2018-07-13 03:37:58.000000000","message":"Done","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"643e88ea521deec3bf76c1c833ac78c72786040e","unresolved":false,"context_lines":[{"line_number":6370,"context_line":"        try:"},{"line_number":6371,"context_line":"            migration, future \u003d ("},{"line_number":6372,"context_line":"                self._waiting_live_migrations.pop(instance.uuid))"},{"line_number":6373,"context_line":"            self._notify_abort_start(context, instance)"},{"line_number":6374,"context_line":"            if future and future.cancel():"},{"line_number":6375,"context_line":"                # If we got here, we\u0027ve successfully aborted the queued"},{"line_number":6376,"context_line":"                # migration and _do_live_migration won\u0027t run so we need"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_40090827","line":6373,"range":{"start_line":6373,"start_character":17,"end_line":6373,"end_character":36},"updated":"2018-07-13 01:35:56.000000000","message":"Move this outside the try/except block and then we don\u0027t need the separate method for it.","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"bfe578cf2381118d10336891f8671f4fd795295d","unresolved":false,"context_lines":[{"line_number":6370,"context_line":"        try:"},{"line_number":6371,"context_line":"            migration, future \u003d ("},{"line_number":6372,"context_line":"                self._waiting_live_migrations.pop(instance.uuid))"},{"line_number":6373,"context_line":"            self._notify_abort_start(context, instance)"},{"line_number":6374,"context_line":"            if future and future.cancel():"},{"line_number":6375,"context_line":"                # If we got here, we\u0027ve successfully aborted the queued"},{"line_number":6376,"context_line":"                # migration and _do_live_migration won\u0027t run so we need"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_83b40af3","line":6373,"range":{"start_line":6373,"start_character":17,"end_line":6373,"end_character":36},"in_reply_to":"5f7c97a3_40090827","updated":"2018-07-13 03:37:58.000000000","message":"I didn\u0027t do it because I though we don\u0027t sent out notification start if keyerror and status!\u003drunning, but thinking again, maybe we can do it, just it never ends because of exception.","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"643e88ea521deec3bf76c1c833ac78c72786040e","unresolved":false,"context_lines":[{"line_number":6389,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":6390,"context_line":"                    migration_id\u003dmigration_id, instance_uuid\u003dinstance.uuid,"},{"line_number":6391,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":6392,"context_line":"            self._notify_abort_start(context, instance)"},{"line_number":6393,"context_line":"            self.driver.live_migration_abort(instance)"},{"line_number":6394,"context_line":"            self._notify_abort_end(context, instance)"},{"line_number":6395,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_a0481c62","line":6392,"range":{"start_line":6392,"start_character":17,"end_line":6392,"end_character":36},"updated":"2018-07-13 01:35:56.000000000","message":"remove this","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b940673325c25311625904390dc37bbb9be5de9b","unresolved":false,"context_lines":[{"line_number":6247,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6248,"context_line":"            self._rollback_live_migration(context, instance, dest,"},{"line_number":6249,"context_line":"                                          migrate_data, \u0027cancelled\u0027)"},{"line_number":6250,"context_line":"            self._notify_abort_end(context, instance)"},{"line_number":6251,"context_line":"            return"},{"line_number":6252,"context_line":""},{"line_number":6253,"context_line":"        self._set_migration_status(migration, \u0027running\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_4b091c30","line":6250,"range":{"start_line":6250,"start_character":17,"end_line":6250,"end_character":34},"updated":"2018-07-13 18:37:40.000000000","message":"wrong name","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b940673325c25311625904390dc37bbb9be5de9b","unresolved":false,"context_lines":[{"line_number":6254,"context_line":"        if migrate_data:"},{"line_number":6255,"context_line":"            migrate_data.migration \u003d migration"},{"line_number":6256,"context_line":"        LOG.debug(\u0027live_migration data is %s\u0027, migrate_data,"},{"line_number":6257,"context_line":"                  instance\u003dinstance)"},{"line_number":6258,"context_line":"        try:"},{"line_number":6259,"context_line":"            self.driver.live_migration(context, instance, dest,"},{"line_number":6260,"context_line":"                                       self._post_live_migration,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_6b04181a","line":6257,"updated":"2018-07-13 18:37:40.000000000","message":"unrelated","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b940673325c25311625904390dc37bbb9be5de9b","unresolved":false,"context_lines":[{"line_number":6330,"context_line":"            action\u003dfields.NotificationAction.LIVE_MIGRATION_ABORT,"},{"line_number":6331,"context_line":"            phase\u003dfields.NotificationPhase.END)"},{"line_number":6332,"context_line":""},{"line_number":6333,"context_line":"    def _notify_live_migrate_abort_start(self, context, instance):"},{"line_number":6334,"context_line":"        self._notify_about_instance_usage("},{"line_number":6335,"context_line":"            context, instance, \u0027live.migration.abort.start\u0027)"},{"line_number":6336,"context_line":"        compute_utils.notify_about_instance_action("}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_2bfe2006","line":6333,"updated":"2018-07-13 18:37:40.000000000","message":"There is only one thing using this so we don\u0027t need it split out.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"04d4d810aaaacd91a479705383cb52b6c134903c","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        can accept 4.x calls from Pike nodes, and can be pinned to 4.x"},{"line_number":347,"context_line":"        for Pike compatibility. All new changes should go against 5.x."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"        * 5.0  - Remove 4.x compatibility"},{"line_number":350,"context_line":"    \u0027\u0027\u0027"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    VERSION_ALIASES \u003d {"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_d5075c1e","line":349,"updated":"2018-07-10 21:11:09.000000000","message":"If you\u0027re going to bump the RPC API version to 5.1 then you should have a comment here for the change log.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":346,"context_line":"        can accept 4.x calls from Pike nodes, and can be pinned to 4.x"},{"line_number":347,"context_line":"        for Pike compatibility. All new changes should go against 5.x."},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"        * 5.0  - Remove 4.x compatibility"},{"line_number":350,"context_line":"    \u0027\u0027\u0027"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    VERSION_ALIASES \u003d {"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_6fe30d3c","line":349,"in_reply_to":"5f7c97a3_d5075c1e","updated":"2018-07-11 02:41:31.000000000","message":"Oops forgot this.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":689,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":690,"context_line":"            else:"},{"line_number":691,"context_line":"                version \u003d \u00275.0\u0027"},{"line_number":692,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_7290df42","line":689,"range":{"start_line":686,"start_character":12,"end_line":689,"end_character":74},"updated":"2018-07-10 20:20:00.000000000","message":"Why are we checking this again here? We already check it in the API (which then calls this method):\n\nhttps://review.openstack.org/#/c/568542/15/nova/compute/api.py@4418\n\nI guess it\u0027s for the case that the API is new enough to support aborting a queued live migration but the compute host on which the server is running is not new enough to support that, so you do the check again here and fail if the compute is old and the migration is not running.\n\nI don\u0027t like the duplication between the compute API and the RPC API but I\u0027m not sure that I have better ideas.\n\nMaybe the compute API could pass a flag in that requires an upgraded compute based on the status which is checked in the API? For example, this is your API check in the next change:\n\nhttps://review.openstack.org/#/c/573136/6/nova/compute/api.py@4425\n\nif (not support_abort_in_queue and migration.status !\u003d \u0027running\u0027)\\\n                or migration.status not in (\u0027queued\u0027, \u0027preparing\u0027, \u0027running\u0027)\n\nWe could do something like:\n\nrequire_abort_queued_support \u003d support_abort_in_queue and migration.status in (\u0027queued\u0027, \u0027preparing\u0027)\n\nThen pass require_abort_queued_support to the RPC API which would then do:\n\nif require_abort_queued_support and not can_send_version(): raise InvalidMigrationState","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3968c7b1af0b653a7e1b7b116736d237c46995bb","unresolved":false,"context_lines":[{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":689,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":690,"context_line":"            else:"},{"line_number":691,"context_line":"                version \u003d \u00275.0\u0027"},{"line_number":692,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_15b2b497","line":689,"range":{"start_line":686,"start_character":12,"end_line":689,"end_character":74},"in_reply_to":"5f7c97a3_0d0e2a3c","updated":"2018-07-10 21:06:49.000000000","message":"I\u0027m confused. So then what I\u0027m saying here is wrong?\n\nhttps://review.openstack.org/#/c/573136/6/nova/compute/api.py@4426\n\nNote that even if the service is fully upgraded and the RPC API isn\u0027t pinned, if the user is trying to abort a live migration that\u0027s not running, queued or preparing status, we need to fail *somewhere*.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"79f35db91a4411e3a8857bafd59b64f4a448c0e0","unresolved":false,"context_lines":[{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":689,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":690,"context_line":"            else:"},{"line_number":691,"context_line":"                version \u003d \u00275.0\u0027"},{"line_number":692,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_ef387d3d","line":689,"range":{"start_line":686,"start_character":12,"end_line":689,"end_character":74},"in_reply_to":"5f7c97a3_15b2b497","updated":"2018-07-11 02:41:31.000000000","message":"Yeah, I wasn\u0027t planning to check service versions in API layer, as rpc here can do the negotiation. But seems worth to add a log or error message about the compute version might not be new enough?","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dae0cfda0a13ad6e6153890e98af95dd32e6e483","unresolved":false,"context_lines":[{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":689,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":690,"context_line":"            else:"},{"line_number":691,"context_line":"                version \u003d \u00275.0\u0027"},{"line_number":692,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_0d0e2a3c","line":689,"range":{"start_line":686,"start_character":12,"end_line":689,"end_character":74},"in_reply_to":"5f7c97a3_7290df42","updated":"2018-07-10 20:38:48.000000000","message":"The API can check the !running condition, and compare that to the \"is the service version new enough\". However, only rpcapi here knows whether or not the rpc version pin is set lower than current (either automatically or manually by the operator). Thus, you need the can_send_version() check here regardless. IMHO, instead of checking migration status in the api and comparing to service version, then checking migration status and comparing to can_send_version() here, just do the latter here. You don\u0027t need to check service_version specifically and separately.\n\nIf the RPC API is set to let you speak 5.1 then you\u0027re good to go. If not, then you need to explode if !running. That should be all the checking you need in this case I think.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b454cb22f99866b1a983615fb54f7de69b2b6374","unresolved":false,"context_lines":[{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":689,"context_line":"                    state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":690,"context_line":"            else:"},{"line_number":691,"context_line":"                version \u003d \u00275.0\u0027"},{"line_number":692,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_62a9c3dc","line":689,"range":{"start_line":686,"start_character":12,"end_line":689,"end_character":74},"in_reply_to":"5f7c97a3_ef387d3d","updated":"2018-07-12 08:11:51.000000000","message":"what is case where RPC API isn\u0027t pinned and service upgrade done?\n\nchecking version on both side i feel is ok. so that we can error the not supported from API only when there is no service upgrade.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c255f9637151f0be0156d94348d2e6d5ba84d66","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_20aaccd2","line":685,"updated":"2018-07-13 01:42:50.000000000","message":"We still need to address this:\n\nhttps://review.openstack.org/#/c/568542/15/nova/compute/rpcapi.py@689\n\nAnd when Dan is talking about the \"pin\" he means the upgrade_levels pin which affects the can_send_version results:\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#upgrade-levels\n\nSo if we just checked the nova-compute service version in the API for this host, it could say it\u0027s 33 but can_send_version can still fail if the rpc api is pinned to queens for compatibility mode during upgrades.","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"bfe578cf2381118d10336891f8671f4fd795295d","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_43fa7244","line":685,"in_reply_to":"5f7c97a3_206f2c94","updated":"2018-07-13 03:37:58.000000000","message":"allowed_statuses \u003d [\u0027running\u0027]\nif can_send_version(\u00275.1\u0027):\n    if support_abort_in_queue:\n        # If the user has requested microversion 2.64 *and*\n        # the compute service is new enough (and RPC API \n        # isn\u0027t pinned lower), then we also support\n        # cancelling queued/preparing live migrations.\n        allowed_statues.extend([\u0027queued\u0027, \u0027preparing\u0027)\nelse:\n    version \u003d \u00275.0\u0027\nif migration.status not in allowed_statuses:\n    raise InvalidMigrationState(...)\n\nThis seems works.","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10d310e85f53e1ac19c0485e140b14fa002e71a4","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_206f2c94","line":685,"in_reply_to":"5f7c97a3_20aaccd2","updated":"2018-07-13 01:57:12.000000000","message":"Can\u0027t we do this?\n\n1. We bump the rpc api / compute service version in this change but don\u0027t do anything with the status checking.\n\n2. In the change at the end of the series that adds the new microversion and passes the support_abort_in_queue flag to nova.compute.api.API, we move the migration status checking *from* nova.compute.api.API.live_migrate_abort *to* this method with the following logic:\n\n# For backward compatibility we always allow cancelling\n# a running live migration.\nallowed_statuses \u003d [\u0027running\u0027]\nif support_abort_in_queue and can_send_version(\u00275.1\u0027):\n    # If the user has requested microversion 2.64 *and*\n    # the compute service is new enough (and RPC API isn\u0027t\n    # pinned lower), then we also support cancelling\n    # queued/preparing live migrations.\n    allowed_statues.extend([\u0027queued\u0027, \u0027preparing\u0027)\n\nif migration.status not in allowed_statuses:\n    raise InvalidMigrationState(...)\n\n--\n\nMy only concern with that is we don\u0027t ever downgrade to 5.0 if can_send_version is False. But we\u0027re all professional engineers here and it\u0027s late for me, so I\u0027m sure we can figure this out with fresh heads...","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"0e808feac2b3140909786638b955957d47519eb7","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_a6e4b443","line":685,"in_reply_to":"5f7c97a3_43fa7244","updated":"2018-07-13 04:07:41.000000000","message":"Or maybe it should be like:\n\nversion \u003d \u00275.0\u0027\nallowed_statuses \u003d [\u0027running\u0027]\n\nif support_abort_in_queue and can_send_version(\u00275.1\u0027):\n    # If the user has requested microversion 2.64 *and*\n    # the compute service is new enough (and RPC API \n    # isn\u0027t pinned lower), then we also support\n    # cancelling queued/preparing live migrations.\n    allowed_statues.extend([\u0027queued\u0027, \u0027preparing\u0027)\n    version \u003d \"5.1\"\n\nif migration.status not in allowed_statuses:\n    raise InvalidMigrationState(...)\n\ncctxt \u003d client.prepare(server\u003d_compute_host(None, instance),\n                               version\u003dversion)\ncctxt.cast(ctxt, \u0027live_migration_abort\u0027, instance\u003dinstance,\n                   migration_id\u003dmigration.id)","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b107d215d8cd4003c90ef1302f1d1f5d74edeb20","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":683,"context_line":"        # If the compute manager is old, we can only process migrations"},{"line_number":684,"context_line":"        # with running status."},{"line_number":685,"context_line":"        if not client.can_send_version(version):"},{"line_number":686,"context_line":"            if migration.status !\u003d \u0027running\u0027:"},{"line_number":687,"context_line":"                raise exception.InvalidMigrationState("},{"line_number":688,"context_line":"                    migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_6ca67bc3","line":685,"in_reply_to":"5f7c97a3_a6e4b443","updated":"2018-07-13 07:22:29.000000000","message":"+1 this looks lgtm.\n\nand we also need to check compute service version on compute/api.py right? in case rpc is pinned higher and few compute node is queen.","commit_id":"5e72ea899d92fc96b3b8ec938e079695f028b94e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b107d215d8cd4003c90ef1302f1d1f5d74edeb20","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        for Pike compatibility. All new changes should go against 5.x."},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        * 5.0  - Remove 4.x compatibility"},{"line_number":351,"context_line":"    \u0027\u0027\u0027"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    VERSION_ALIASES \u003d {"},{"line_number":354,"context_line":"        \u0027icehouse\u0027: \u00273.23\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_2c3ac334","line":351,"range":{"start_line":351,"start_character":0,"end_line":351,"end_character":7},"updated":"2018-07-13 07:22:29.000000000","message":"you need to add version 5.1 here","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"6d4b4e791fced0e0e392dc778604273f7273955b","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        for Pike compatibility. All new changes should go against 5.x."},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        * 5.0  - Remove 4.x compatibility"},{"line_number":351,"context_line":"    \u0027\u0027\u0027"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"    VERSION_ALIASES \u003d {"},{"line_number":354,"context_line":"        \u0027icehouse\u0027: \u00273.23\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_ac15734e","line":351,"range":{"start_line":351,"start_character":0,"end_line":351,"end_character":7},"in_reply_to":"5f7c97a3_2c3ac334","updated":"2018-07-13 07:27:37.000000000","message":"Ah, forgot this one","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b107d215d8cd4003c90ef1302f1d1f5d74edeb20","unresolved":false,"context_lines":[{"line_number":360,"context_line":"        \u0027ocata\u0027: \u00274.13\u0027,"},{"line_number":361,"context_line":"        \u0027pike\u0027: \u00274.18\u0027,"},{"line_number":362,"context_line":"        \u0027queens\u0027: \u00275.0\u0027,"},{"line_number":363,"context_line":"    }"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def __init__(self):"},{"line_number":366,"context_line":"        super(ComputeAPI, self).__init__()"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_ac0fb389","line":363,"range":{"start_line":363,"start_character":4,"end_line":363,"end_character":5},"updated":"2018-07-13 07:22:29.000000000","message":"ditto","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"6d4b4e791fced0e0e392dc778604273f7273955b","unresolved":false,"context_lines":[{"line_number":360,"context_line":"        \u0027ocata\u0027: \u00274.13\u0027,"},{"line_number":361,"context_line":"        \u0027pike\u0027: \u00274.18\u0027,"},{"line_number":362,"context_line":"        \u0027queens\u0027: \u00275.0\u0027,"},{"line_number":363,"context_line":"    }"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def __init__(self):"},{"line_number":366,"context_line":"        super(ComputeAPI, self).__init__()"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_8c12f760","line":363,"range":{"start_line":363,"start_character":4,"end_line":363,"end_character":5},"in_reply_to":"5f7c97a3_ac0fb389","updated":"2018-07-13 07:27:37.000000000","message":"Yeah, I missed this","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b107d215d8cd4003c90ef1302f1d1f5d74edeb20","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":683,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":684,"context_line":"        allowed_status \u003d [\u0027running\u0027]"},{"line_number":685,"context_line":"        # TODO(Kevin Zheng): the microversion check will also move to here in"},{"line_number":686,"context_line":"        # the subsequent patch."},{"line_number":687,"context_line":"        if client.can_send_version(\u00275.1\u0027):"},{"line_number":688,"context_line":"            # If the compute manager is new enough (and RPC API isn\u0027t"},{"line_number":689,"context_line":"            # pinned lower), then we also support cancelling queued/preparing"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_ec1ccbc3","line":686,"range":{"start_line":685,"start_character":8,"end_line":686,"end_character":31},"updated":"2018-07-13 07:22:29.000000000","message":"\"microversion check\" confuse me even while chatting on IRC. may you can say flag to tell whether abort live migration in in queue is supported will be added here","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"6d4b4e791fced0e0e392dc778604273f7273955b","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":683,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":684,"context_line":"        allowed_status \u003d [\u0027running\u0027]"},{"line_number":685,"context_line":"        # TODO(Kevin Zheng): the microversion check will also move to here in"},{"line_number":686,"context_line":"        # the subsequent patch."},{"line_number":687,"context_line":"        if client.can_send_version(\u00275.1\u0027):"},{"line_number":688,"context_line":"            # If the compute manager is new enough (and RPC API isn\u0027t"},{"line_number":689,"context_line":"            # pinned lower), then we also support cancelling queued/preparing"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_771de860","line":686,"range":{"start_line":685,"start_character":8,"end_line":686,"end_character":31},"in_reply_to":"5f7c97a3_ec1ccbc3","updated":"2018-07-13 07:27:37.000000000","message":"I can do that.","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c4d2874611eb766813a5ba6472d8b8490ec44200","unresolved":false,"context_lines":[{"line_number":684,"context_line":"        allowed_status \u003d [\u0027running\u0027]"},{"line_number":685,"context_line":"        # TODO(Kevin Zheng): the microversion check will also move to here in"},{"line_number":686,"context_line":"        # the subsequent patch."},{"line_number":687,"context_line":"        if client.can_send_version(\u00275.1\u0027):"},{"line_number":688,"context_line":"            # If the compute manager is new enough (and RPC API isn\u0027t"},{"line_number":689,"context_line":"            # pinned lower), then we also support cancelling queued/preparing"},{"line_number":690,"context_line":"            # live migrations."},{"line_number":691,"context_line":"            allowed_status.extend([\u0027queued\u0027, \u0027preparing\u0027])"},{"line_number":692,"context_line":"            version \u003d \u00275.1\u0027"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"        if migration.status not in allowed_status:"},{"line_number":695,"context_line":"            raise exception.InvalidMigrationState("}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_ac2c93b2","line":692,"range":{"start_line":687,"start_character":0,"end_line":692,"end_character":27},"updated":"2018-07-13 07:23:46.000000000","message":"query i asked in previous PS -\n\ndo we need to check compute service version also on compute/api.py? in case rpc is pinned higher and few compute node is queens.","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"6d4b4e791fced0e0e392dc778604273f7273955b","unresolved":false,"context_lines":[{"line_number":684,"context_line":"        allowed_status \u003d [\u0027running\u0027]"},{"line_number":685,"context_line":"        # TODO(Kevin Zheng): the microversion check will also move to here in"},{"line_number":686,"context_line":"        # the subsequent patch."},{"line_number":687,"context_line":"        if client.can_send_version(\u00275.1\u0027):"},{"line_number":688,"context_line":"            # If the compute manager is new enough (and RPC API isn\u0027t"},{"line_number":689,"context_line":"            # pinned lower), then we also support cancelling queued/preparing"},{"line_number":690,"context_line":"            # live migrations."},{"line_number":691,"context_line":"            allowed_status.extend([\u0027queued\u0027, \u0027preparing\u0027])"},{"line_number":692,"context_line":"            version \u003d \u00275.1\u0027"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"        if migration.status not in allowed_status:"},{"line_number":695,"context_line":"            raise exception.InvalidMigrationState("}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_5709cc26","line":692,"range":{"start_line":687,"start_character":0,"end_line":692,"end_character":27},"in_reply_to":"5f7c97a3_ac2c93b2","updated":"2018-07-13 07:27:37.000000000","message":"I don\u0027t know, why would anyone do that?","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"487d534c122d7e0a92ecff6c14fac74e4b478b54","unresolved":false,"context_lines":[{"line_number":696,"context_line":"                migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":697,"context_line":"                state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        objects.InstanceAction.action_start("},{"line_number":700,"context_line":"            context, instance.uuid,"},{"line_number":701,"context_line":"            instance_actions.LIVE_MIGRATION_CANCEL,"},{"line_number":702,"context_line":"            want_result\u003dFalse)"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"},{"line_number":705,"context_line":"                               version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_ec728b4a","line":702,"range":{"start_line":699,"start_character":0,"end_line":702,"end_character":30},"updated":"2018-07-13 06:55:44.000000000","message":"I\u0027m intend to move the _record_action_start here, but since it was a private method in compute.api so I\u0027m not sure what is the best way to do this, make it public in compute.api or do like this or write a new method in this file.","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b107d215d8cd4003c90ef1302f1d1f5d74edeb20","unresolved":false,"context_lines":[{"line_number":696,"context_line":"                migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":697,"context_line":"                state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        objects.InstanceAction.action_start("},{"line_number":700,"context_line":"            context, instance.uuid,"},{"line_number":701,"context_line":"            instance_actions.LIVE_MIGRATION_CANCEL,"},{"line_number":702,"context_line":"            want_result\u003dFalse)"},{"line_number":703,"context_line":""},{"line_number":704,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"},{"line_number":705,"context_line":"                               version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_8c8197c5","line":702,"range":{"start_line":699,"start_character":0,"end_line":702,"end_character":30},"in_reply_to":"5f7c97a3_ec728b4a","updated":"2018-07-13 07:22:29.000000000","message":"i think this is only exception case where we get to know that action is started at this point so doing it without any method like you doing now looks fine to me.","commit_id":"b88430bec47362c60c27df6674898e1553678ff8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efc4864f97d48313795f14e9ad27fd6874f2add8","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_17c0344a","line":693,"updated":"2018-07-13 08:03:33.000000000","message":"Sorry for the back and forth on this but after losing a bunch of sleep over it tonight, I\u0027m not sure we need the 5.1 version bump - the compute rpc api interface isn\u0027t actually changing, just the behavior / capability of the compute service.\n\nThe thing that was bothering me about this before was when we\u0027d drop back to 5.0 if we were pinned, but that shouldn\u0027t really matter - if the compute is new enough we can still send the same rpc cast we were sending before. Nothing changes about the parameters in the cast.\n\nWhen we added volume multiattach support, we added rpc api version 4.20 because the reserve_block_device_name interface actually changed:\n\nhttps://github.com/openstack/nova/blob/stable/queens/nova/compute/rpcapi.py#L340\n\nBut we didn\u0027t change the attach_volume interface because attach_volume in the compute was a behavioral change to support multiattach volumes. For the min compute service version behavior change for multiattach, we checked that in the nova.compute.api.API.attach_volume method:\n\nhttps://github.com/openstack/nova/blob/stable/queens/nova/compute/api.py#L4008\n\nAnd that\u0027s really just temporary, unlike an RPC API interface change, meaning we can drop the minimum compute service version checks in the API after a release since the API is assumed to only support n-1 computes (so rocky API only needs to officially support queens and rocky computes).\n\nSo I think we can drop this 5.1 check in this change and all of this rpcapi change you\u0027ve made for the status checking, and then all we need to do is similar to https://github.com/openstack/nova/blob/stable/queens/nova/compute/api.py#L4008 in nova.comptue.api.API.live_migrate_abort if the user requested abort with microversion \u003e\u003d 2.64 and the migration status is queued or preparing.\n\nSorry again about the back and forth on this, but the 5.1 never really sat right with me. I\u0027d say leave this for now until I can talk with dansmith about it in the morning.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"675db29ba70a24ee73adc2ce814fc720143f3c43","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_89042416","line":693,"in_reply_to":"5f7c97a3_176d942e","updated":"2018-07-13 10:10:19.000000000","message":"one thing to note, if we add the flag to know the abort in queeue supported (support_abort_in_queue), then it change the function.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8dda869cf64ae51a10614c645c0e405499946afb","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_bae5a0dd","line":693,"in_reply_to":"5f7c97a3_17c0344a","updated":"2018-07-13 13:48:16.000000000","message":"\u003e Sorry for the back and forth on this but after losing a bunch of\n \u003e sleep over it tonight, I\u0027m not sure we need the 5.1 version bump -\n \u003e the compute rpc api interface isn\u0027t actually changing, just the\n \u003e behavior / capability of the compute service.\n\nWell, the interface _is_ changing, but just not in a structural way, or a way we track. If we were really strict about this interface, we\u0027d define things to a deeper level, like what state things have to be in in order to call, etc. But, we don\u0027t. For that reason, sometimes I struggle when we _don\u0027t_ bump the api here for stuff like that.\n\nRegardless, what you say is true about what will work, how it will behave, and it matches what we do a lot of the time. A major benefit of going that way is that we can start to get the new behavior with new computes as soon as they are upgraded, since the RPC version has to be pinned to one thing, cluster-wide, until everything is upgraded.\n\nSo yeah, fine if you want to go that route.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"c3acc42fd1f236154d09ad18729af33e525518cb","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_176d942e","line":693,"in_reply_to":"5f7c97a3_17c0344a","updated":"2018-07-13 08:10:25.000000000","message":"Sorry for your insomnia :), yeah, that\u0027s also why I asked before when should it be 5.0 and when should it be 5.1, let\u0027s see what the conculsion is tomorrow.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d9ca7fde8ecf98e0aa52080158e57405b494992b","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_59f6a392","line":693,"in_reply_to":"5f7c97a3_5523ff06","updated":"2018-07-17 02:38:18.000000000","message":"i learned more on how RPC pinned and compute node upgrade work and only compute version check can be enough here. \n\nso basically, RPC version can stay same till there is actually no functional signature change.  Behavioral changes (adding or allowing new functionality/processing) can be handled from compute.api side by checking necessary versions (compute service or/and api version).","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"278231f3d59ab4bb6f37dc0b62f2efb083536def","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_edb31708","line":693,"in_reply_to":"5f7c97a3_59f6a392","updated":"2018-07-17 15:48:44.000000000","message":"It\u0027s probably worth documenting something in our contributor guide around coding for upgrade support:\n\nhttps://docs.openstack.org/nova/latest/contributor/index.html\n\nBut I don\u0027t have the energy for writing that up at the moment.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e9ba75e16672b337dc6c192b142693ac07f44367","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_5523ff06","line":693,"in_reply_to":"5f7c97a3_bae5a0dd","updated":"2018-07-13 14:30:30.000000000","message":"Ack, I\u0027ll update this change and rebase the API change with the new microversion on top and stub in the service version check.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"47967e1630caa7dc3940cec1e24444ca0685881a","unresolved":false,"context_lines":[{"line_number":690,"context_line":"                version\u003dversion)"},{"line_number":691,"context_line":"        cctxt.cast(ctxt, \u0027live_migration_force_complete\u0027, instance\u003dinstance)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def live_migration_abort(self, ctxt, instance, migration):"},{"line_number":694,"context_line":"        version \u003d \u00275.0\u0027"},{"line_number":695,"context_line":"        client \u003d self.router.client(ctxt)"},{"line_number":696,"context_line":"        allowed_status \u003d [\u0027running\u0027]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_4c3803ec","line":693,"in_reply_to":"5f7c97a3_edb31708","updated":"2018-07-18 01:19:25.000000000","message":"yeah. i will add that in doc but later sometime. adding it in my TODO.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"ff06bb7c0c99649ee85473877c4cdd3999a400f2","unresolved":false,"context_lines":[{"line_number":709,"context_line":"                migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":710,"context_line":"                state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"        objects.InstanceAction.action_start("},{"line_number":713,"context_line":"            context, instance.uuid,"},{"line_number":714,"context_line":"            instance_actions.LIVE_MIGRATION_CANCEL,"},{"line_number":715,"context_line":"            want_result\u003dFalse)"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"},{"line_number":718,"context_line":"                               version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_57c6ac6b","line":715,"range":{"start_line":712,"start_character":8,"end_line":715,"end_character":30},"updated":"2018-07-13 07:53:28.000000000","message":"I\u0027m intend to move the _record_action_start here, but since it was a private method in compute.api so I\u0027m not sure what is the best way to do this, make it public in compute.api or do like this or write a new method in this file.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"efc4864f97d48313795f14e9ad27fd6874f2add8","unresolved":false,"context_lines":[{"line_number":709,"context_line":"                migration_id\u003dmigration.id, instance_uuid\u003dinstance.uuid,"},{"line_number":710,"context_line":"                state\u003dmigration.status, method\u003d\u0027abort live migration\u0027)"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"        objects.InstanceAction.action_start("},{"line_number":713,"context_line":"            context, instance.uuid,"},{"line_number":714,"context_line":"            instance_actions.LIVE_MIGRATION_CANCEL,"},{"line_number":715,"context_line":"            want_result\u003dFalse)"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        cctxt \u003d client.prepare(server\u003d_compute_host(None, instance),"},{"line_number":718,"context_line":"                               version\u003dversion)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5f7c97a3_97e524de","line":715,"range":{"start_line":712,"start_character":8,"end_line":715,"end_character":30},"in_reply_to":"5f7c97a3_57c6ac6b","updated":"2018-07-13 08:03:33.000000000","message":"This is not what I was proposing before, and don\u0027t think we need to do any of this in rpcapi now, see above - but hold off on making changes until I confirm with Dan.","commit_id":"ef9dc3fb6aec477d1b4797e18f89b27a1444e002"}],"nova/objects/service.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b940673325c25311625904390dc37bbb9be5de9b","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    # Version 33: Add support for check on the server group with"},{"line_number":142,"context_line":"    # \u0027max_server_per_host\u0027 rules"},{"line_number":143,"context_line":"    {\u0027compute_rpc\u0027: \u00275.0\u0027},"},{"line_number":144,"context_line":"    # Version 33: Adds support to abort queued/preparing live migrations."},{"line_number":145,"context_line":"    {\u0027compute_rpc\u0027: \u00275.0\u0027},"},{"line_number":146,"context_line":")"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"5f7c97a3_14a58401","line":144,"range":{"start_line":144,"start_character":14,"end_line":144,"end_character":16},"updated":"2018-07-13 18:37:40.000000000","message":"34","commit_id":"c6a81691d70cb5efad001284a126d136c4ab969a"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"cb30e1b20e42ee8be4ec8ce06466ad3b48b47d31","unresolved":false,"context_lines":[{"line_number":7268,"context_line":"            network_info\u003dnetwork_model.NetworkInfo(["},{"line_number":7269,"context_line":"                network_model.VIF(uuids.port1), network_model.VIF(uuids.port2)"},{"line_number":7270,"context_line":"            ]))"},{"line_number":7271,"context_line":"        self.compute._waiting_live_migrations \u003d {}"},{"line_number":7272,"context_line":"        fake_migration \u003d objects.Migration("},{"line_number":7273,"context_line":"            uuid\u003duuids.migration, instance_uuid\u003dself.instance.uuid)"},{"line_number":7274,"context_line":"        fake_migration.save \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8d155ae8","line":7271,"range":{"start_line":7271,"start_character":8,"end_line":7271,"end_character":50},"updated":"2018-07-10 20:20:00.000000000","message":"OK since this is empty we\u0027ll hit a KeyError and rollback.","commit_id":"08f116ed6a966b144ad4c179a54c71e13d14e684"}]}