| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-17 | |||
| 11:13:02 | gibi___ | stephenfin: thanks | |
| 11:14:06 | songwenping__ | stephenfin: thanks, wait for the release patch merged. | |
| 11:29:03 | gibi___ | stephenfin: I have comments in https://review.opendev.org/c/openstack/nova/+/800078 | |
| 11:33:55 | gibi___ | stephenfin, lyarwood : I'm done with the alembic series, I'm mostly +2. | |
| 11:33:59 | gibi___ | stephenfin: thanks for working on it | |
| 11:37:09 | lyarwood | ACK I think I still had some left to review in that series, I'll try to finish it today | |
| 11:38:21 | gibi___ | lyarwood: yepp, that is why I pinged you, as I saw you were doing active review previously on that series | |
| 11:38:50 | lyarwood | ah I see, thanks | |
| 11:50:36 | Gowthami__ | <lyarwood> Thank you . Will try with 0.5.2 and also increase the resources too. | |
| 11:51:16 | lyarwood | Gowthami__: yeah FWIW if you do try 0.5.2 you need to raise the resouces anyway https://github.com/cirros-dev/cirros/issues/53 | |
| 11:57:40 | gibi___ | aarents: I have still concerns about https://review.opendev.org/c/openstack/nova/+/764435/5/nova/virt/libvirt/driver.py#9970 | |
| 12:00:06 | sean-k-mooney | gibi___: my understainding is we are nver ment to attempt to rollback a live migration once we have actully started it in qemu | |
| 12:00:24 | sean-k-mooney | we can rollback if we call migrate on libvirt and it imideatly returns with an error | |
| 12:00:40 | sean-k-mooney | but once it start we dont rollback unless it times out | |
| 12:01:20 | sean-k-mooney | besided timeout to we have other cases where we rollback after the migration has started today? | |
| 12:03:06 | sean-k-mooney | im not sure that aarents patch will solve the issue they are trying to solve in this case either | |
| 12:03:31 | sean-k-mooney | but for different reasons, the migration may continue as they said and the vm can end up on the destionation | |
| 12:04:02 | sean-k-mooney | so reverting the db state may or may not be the correct thing to do | |
| 12:04:52 | sean-k-mooney | for example im concerned about what happens with post copy | |
| 12:06:00 | sean-k-mooney | the instance would be still migrating but running on the dest and we would have already executed part or all of post_live_migrateion assuimg we recived the post_copy_reume event before the monitor connection died | |
| 12:06:18 | sean-k-mooney | whihc shoudl mean the host is already updated. | |
| 12:07:08 | aarents | gibi___: Hum, but I think I call live_migration_abort() of libvirt driver not from manager and it only call libvirt.api but now I have doubt | |
| 12:07:58 | sean-k-mooney | in the libvirt dirver it just does https://review.opendev.org/plugins/gitiles/openstack/nova/+/refs/changes/35/764435/5/nova/virt/libvirt/driver.py#9434 | |
| 12:08:18 | aarents | sean-k-mooney: yes thanks for the link | |
| 12:12:28 | sean-k-mooney | so i think that is fine it will jsut call libvirt | |
| 12:12:44 | sean-k-mooney | althoguh if the monitor connect is down it may not be able to mange the vm | |
| 12:13:50 | aarents | sean-k-mooney: yes in that case it will not work | |
| 12:14:23 | sean-k-mooney | which is the case you are trying to fix right. in the even the monitor connection drops you want to about the migration job | |
| 12:14:34 | sean-k-mooney | you can tell libvirt to about the migration | |
| 12:14:44 | sean-k-mooney | but it may or may not be able ot comply | |
| 12:15:17 | sean-k-mooney | i assume the except Exception: is to catch the libvirt error that is raised when that happens | |
| 12:16:41 | gibi_ | aarents: oops sorry I jumped to the wrong driver.live_migration_abort call. | |
| 12:16:45 | aarents | this will work only with network,RPC,DB issues not for libvirt issue | |
| 12:17:11 | sean-k-mooney | aarents: so for those cases im not sure we want to abort the migration | |
| 12:17:24 | sean-k-mooney | aarents: unless you want to abort all other operation when that happens | |
| 12:17:28 | aarents | sean-k-mooney: or it may work if there is only one flap from libvirt | |
| 12:17:33 | sean-k-mooney | spawns, deletes ectra | |
| 12:20:00 | sean-k-mooney | its a larger chagne but to me what feels like a more robost change would be to suspend the green thread if the connection is closed and resume it when we reconnect and only try to send the rpc call then | |
| 12:20:55 | sean-k-mooney | realistically if the rpc bus is down there is notight on the compute we can do to update the db state | |
| 12:24:18 | aarents | sean-k-mooney: honestly, the change is just ensuring to kill job regardless if state in can or cannot update in DB, we loss some instances due to that as explain in bug | |
| 12:24:19 | gibi_ | hm, so assuming we have the RPC down. the patch aborts the libvirt job. then raises the exception as today. That exception expected to update the instance and migration states which will not happen while the RPC is down. the nova compute RPC call to update the DB will time out eventually I guess. | |
| 12:24:55 | aarents | gibi_: yes | |
| 12:24:56 | gibi_ | so the nova DB will still see the migration as runnig | |
| 12:25:02 | gibi_ | but the compute already aborted it | |
| 12:25:19 | gibi_ | does the conductor time out the migration eventually too? | |
| 12:25:20 | sean-k-mooney | aarents: have you tested this with post-copy enabled | |
| 12:25:49 | sean-k-mooney | gibi_: i think the timeout happened at the comptue level | |
| 12:25:54 | sean-k-mooney | not the conductor | |
| 12:26:18 | gibi_ | sean-k-mooney: so there is no cleanup triggered be the conductor for this aborted migration | |
| 12:26:26 | gibi_ | s/be/by/ | |
| 12:26:36 | sean-k-mooney | im not sure | |
| 12:26:42 | aarents | gibi_: yes there will be inconsitency that need operator intervention, but vm will be safe because still referenced in source host & running on source host | |
| 12:26:55 | gibi_ | aarents: I see. that was the missing piece | |
| 12:27:04 | sean-k-mooney | aarents: again i dont know if that is always correct | |
| 12:27:17 | gibi_ | aarents: so this change does not try to fix an DB inconsistency but try to save the VM | |
| 12:27:18 | sean-k-mooney | you have ignored my post-copy question | |
| 12:28:00 | aarents | sean-k-mooney: good question | |
| 12:28:24 | aarents | gibi_: exactly, I was not clear | |
| 12:28:24 | gibi_ | sean-k-mooney: so you suggest that the save move would be to abort the non post-copy migrations and let the post-copy migrations run forward | |
| 12:28:33 | gibi_ | /save/safe/ | |
| 12:28:37 | sean-k-mooney | gibi_: yes | |
| 12:28:47 | sean-k-mooney | but only afte we are in the post copy phase | |
| 12:28:53 | gibi_ | sean-k-mooney: I guess we document that post-copy means no way back | |
| 12:29:14 | sean-k-mooney | well we can abort until we enter the post copy phase | |
| 12:29:27 | aarents | sean-k-mooneyI don't have so much experiance about post copy in operation | |
| 12:29:33 | sean-k-mooney | but when we hit post copy suspend we call post_live_migration | |
| 12:29:44 | sean-k-mooney | and update the host and prot bindings | |
| 12:31:05 | sean-k-mooney | aarents: we dont have access to the last known state of the instance at thsi point do we | |
| 12:31:09 | sean-k-mooney | form a libvirt perspecitiv | |
| 12:31:41 | sean-k-mooney | assuming not then i would make the abort condtional on "not postcopy_enabled" | |
| 12:31:57 | sean-k-mooney | to be on the safe side | |
| 12:33:04 | sean-k-mooney | gibi_: looking at https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py i dont see anything that looks like cleanup logic once a migration has started | |
| 12:33:11 | aarents | sean-k-mooney: no we don't have access to the instance state | |
| 12:34:21 | sean-k-mooney | we handel messigng time ectra form check_can_live_migrate_destination and other cases but there seams to be no overall timeout enforced by the conductor | |
| 12:34:49 | sean-k-mooney | which kind of makes sense since the live migration timout option are virt diriver specific | |
| 12:35:16 | sean-k-mooney | as is whereter we abort or force complete when the timeout expires | |
| 12:35:19 | gibi_ | sean-k-mooney: ack, I got it now that aarents' goal is to save the VM running state instead of avoiding the DB inconsistency in case of RPC/DB issue | |
| 12:36:35 | gibi_ | sean-k-mooney, aarents: I'm fine with the patch with addition of the "not postcopy_enabled" condition as sean-k-mooney suggests | |
| 12:37:13 | sean-k-mooney | gibi_: i think i would be oke with it with that added also | |
| 12:37:19 | gibi_ | cool | |
| 12:37:37 | aarents | gibi_: sean-k-mooney yep this "not postcopy_enabled" condition make sense | |
| 12:38:30 | aarents | I will add that, thanks | |
| 12:39:19 | lyarwood | https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainAbortJob FWIW | |
| 12:39:26 | lyarwood | In case the job is a migration in a post-copy mode, virDomainAbortJob will report an error (see virDomainMigrateStartPostCopy for more details). | |
| 12:40:20 | lyarwood | https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrateStartPostCopy has some more context | |
| 12:40:29 | lyarwood | On the other hand once the guest is running on the destination host, the migration can no longer be rolled back because none of the hosts has complete state. If this happens, libvirt will leave the domain paused on both hosts with VIR_DOMAIN_PAUSED_POSTCOPY_FAILED reason. It's up to the upper layer to decide what to do in such case. Because of this, libvirt will refuse to cancel post-copy migration via virDomainAbortJob. | |
| 12:41:38 | lyarwood | so tbh I don't think we need to check anything | |
| 12:42:42 | lyarwood | oh wait I missed that live_migration_abort is raising the error back, sigh | |
| 12:45:25 | sean-k-mooney | ya although we are catching and ignoring that with a log in aarents patch | |
| 12:45:44 | sean-k-mooney | i guess we could rely on that behavior but i would prefer to have a comment to that effect honestly | |
| 12:45:49 | sean-k-mooney | jsut to not forget that | |
| 12:46:13 | sean-k-mooney | as the next time i see the abbort ill get suspicios about post copy again. | |
| 12:49:01 | gibi_ | aarents: are you seeing this ^^ :) | |
| 12:50:17 | aarents | gibi_: Yes so I will add a comment that say that abort may not work in case of post copy ? | |
| 12:50:51 | gibi_ | aarents: I guess you need to catch the error returned from abort and ignore it | |
| 12:52:03 | aarents | So I drop the warning | |
| 12:52:25 | aarents | ? | |
| 12:53:22 | gibi_ | aarents: sorry, so you already catching the error from abort, that is OK | |
| 12:53:27 | gibi_ | keep the warning too | |
| 12:53:32 | gibi_ | just add a note as sean-k-mooney requested | |