| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-10 | |||
| 18:25:57 | sean-k-mooney | well live migration are only done if the vm is running | |
| 18:26:04 | sean-k-mooney | that what makes the a live migration | |
| 18:26:26 | sean-k-mooney | so the api i asked you to try is only used for live migration but you are doing a cold migration | |
| 18:26:26 | spatel | I talled to you couple month back NOVA has bug which doesn't copy full disk and that cause instance never get full migrate... | |
| 18:26:52 | spatel | I am worried about that bug... (someone did migration without checking with me) now i want to stop this :( | |
| 18:26:54 | sean-k-mooney | i vaguly rememeber that yes | |
| 18:27:26 | sean-k-mooney | mriedem: is there any way to stop a cold migration that is in flight? | |
| 18:27:34 | sean-k-mooney | mriedem: can we revert it | |
| 18:27:50 | mriedem | nope | |
| 18:27:51 | spatel | nova start block migration and then kill it source disk without verifying full disk migration.. that was the bug | |
| 18:27:52 | sean-k-mooney | when the instance is in task_sate resize_migrating | |
| 18:28:32 | sean-k-mooney | mriedem: what about reseting the state on the vm? | |
| 18:29:49 | mriedem | sean-k-mooney: that would reset the task_state to None so you'd maybe fail somewhere along the way in compute if it tries to update the task_state and is using instance.save(expected_task_state=...) | |
| 18:30:04 | mriedem | but clearly that's not any kind of supported way to interrupt a cold migration | |
| 18:30:22 | sean-k-mooney | mriedem: yes i know | |
| 18:30:54 | spatel | cold migration = when machine is shutdown it does cold migration ? | |
| 18:31:00 | spatel | is that true? | |
| 18:31:17 | sean-k-mooney | spatel: by machine you mean the vm? | |
| 18:31:21 | mriedem | NTT (i think it was NTT) had a spec up for several releases to add support to abort a cold migration, with the main reason being "we shove logs into a 100GB+ local disk and migrating those VMs takes a long time so we'd maybe like to abort the cold migration" | |
| 18:31:27 | spatel | yes machine = VM | |
| 18:31:52 | sean-k-mooney | mriedem: ya i vaguly rememebr that | |
| 18:32:16 | sean-k-mooney | spatel: do you have nova configured to auto confim migrations/resize | |
| 18:32:37 | sean-k-mooney | spatel: if not then nova will not delete the souce disk unless you confim the resize. | |
| 18:33:11 | spatel | NO i don't have auto confim | |
| 18:33:13 | sean-k-mooney | at which point you can either boot up the vm and check its ok or you can do "nova resize-revert <server uuid>" | |
| 18:33:53 | spatel | oh! can i boot instance without confirm ? | |
| 18:34:20 | sean-k-mooney | i think so. i have not tried this in quite a while. | |
| 18:35:48 | sean-k-mooney | maybe we cant but i know that has also been asked for in the past | |
| 18:36:28 | sean-k-mooney | spatel: in anycase when the migration eventually complete you can revert it which will be quick as teh data will not have been delete on the souce node. | |
| 18:36:47 | openstackgerrit | Adam Spiers proposed openstack/nova master: Add extra spec parameter and image property for memory encryption https://review.opendev.org/664420 | |
| 18:37:02 | aspiers | efried: is this roughly along the right lines? ^^^ | |
| 18:38:01 | spatel | i will revert it without confirm it | |
| 18:38:03 | efried | aspiers: I'm going to have to look a bit later, I'm afraid. | |
| 18:38:27 | aspiers | efried: np. Don't need a full review yet, just confirmation I'm barking up the right tree | |
| 18:55:45 | spatel | sean-k-mooney: revert works! | |
| 18:55:53 | spatel | what a pain :) | |
| 18:56:20 | spatel | sean-k-mooney: thank you again for saving my a... :) | |
| 19:14:01 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Spec to pre-filter disabled computes with placement https://review.opendev.org/657884 | |
| 19:53:07 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881 | |
| 19:53:08 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Remove finish_revert_migration TypeError try/except https://review.opendev.org/664013 | |
| 19:53:09 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Include direct-physical in compute manager events check https://review.opendev.org/664431 | |
| 19:53:48 | artom | sean-k-mooney, mriedem, ^^ did a thing. Still lots of cleanup to do, but maybe check the compute manager/libvirt driver parts to see if the general approach is palatable? | |
| 19:59:44 | sean-k-mooney | that likely wont be backportable. due to the chagne in the signiture of finish_revert_migration or at least its more invasive then it needs to be | |
| 19:59:47 | sean-k-mooney | still reading it | |
| 20:00:48 | artom | finish_revert_migration was changed with the previous approach as well (was adding vifs_already_plugged) | |
| 20:01:19 | sean-k-mooney | yes but we can actully make this work without adding any new parmater to finish_revert_migration | |
| 20:01:57 | sean-k-mooney | if we take the approch i was suggesting. we actully did not need to pass vif_already_plugged | |
| 20:02:16 | artom | Why is adding a parameter bad though? That's not RPC, don't think it'll affect backportability... | |
| 20:02:40 | artom | It's compute manager talking to its virt driver, both are updated at the same time | |
| 20:02:40 | sean-k-mooney | its chaning the virt dirver api right | |
| 20:02:49 | artom | Yeah | |
| 20:02:55 | sean-k-mooney | so it would break out of tree drivers without the try catch | |
| 20:03:10 | artom | Well, yeah | |
| 20:03:13 | sean-k-mooney | * try excpet | |
| 20:04:43 | artom | I don't see a way around passing the virt driver *something* | |
| 20:05:27 | sean-k-mooney | we have all the info we need in the network_info object since that is a list of vifs | |
| 20:05:31 | sean-k-mooney | so we can calualte if we need to wait in the driver or manager form that without passing anything | |
| 20:05:55 | sean-k-mooney | which is part of the reason i suggested adding the function in the vif model class | |
| 20:07:17 | artom | The problem is that finish_revert_migration isn't always called for revert resize... | |
| 20:07:43 | artom | So even if network_info has all it needs in terms of event "types" | |
| 20:07:55 | artom | We still need a way to tell the virt driver when to actually use that information | |
| 20:08:42 | sean-k-mooney | im not following | |
| 20:09:19 | artom | All of this stuff, it's only applicable in the revert resize case, right? | |
| 20:09:36 | sean-k-mooney | and revert migration yes | |
| 20:09:42 | sean-k-mooney | *cold migration | |
| 20:10:17 | artom | Is it? Does the instance remain shutoff on the source with the VIFs still wired while it's cold migrating? | |
| 20:10:25 | sean-k-mooney | yes | |
| 20:11:06 | sean-k-mooney | to both questions | |
| 20:11:08 | artom | So, the only other instance of driver.finish_revert_migration that I've found is https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1001 | |
| 20:11:43 | artom | So maybe you're right, driver.finish_revert_migration is called if and only iff we need to do the compute manager/virt driver events checking thing | |
| 20:11:57 | sean-k-mooney | yes cold migration uses the resize_migrating task state | |
| 20:12:50 | sean-k-mooney | right it only used in two case | |
| 20:13:10 | sean-k-mooney | if we are migrating/resizing instance.task_state == task_states.RESIZE_MIGRATING | |
| 20:13:20 | artom | So in that case yeah, we can stuff the logic inside the VIF model | |
| 20:13:21 | sean-k-mooney | which is that auto rollback if things go wrong | |
| 20:13:45 | artom | Kinda weird for something so specific to go in the model, but I guess it makes sense | |
| 20:13:51 | sean-k-mooney | or if the user revert the migraion/resize form the api after we succeed in the migratio/resize instead of confiming | |
| 20:14:48 | sean-k-mooney | artom: well i had planned to stuff the event info form neutron in that data stcutrue if we implement the spec in anycase | |
| 20:16:00 | sean-k-mooney | artom: we can always add a untility module or make it a free fucntion if you like but i think the encapsulation makes sense | |
| 20:17:04 | artom | Until confirmed (or reverted) | |
| 20:17:12 | sean-k-mooney | yep | |
| 20:17:19 | sean-k-mooney | and we default to auto confirm disabled | |
| 20:18:05 | sean-k-mooney | i more or less had this conversation with stephenfin and mriedem a few hours ago :) | |
| 20:18:20 | artom | (Which is weird UX, btw - because you 'nova migrate', but then you 'nova resize-revert') | |
| 20:18:32 | sean-k-mooney | yep we know :) | |
| 20:18:41 | artom | Yeah, didn't think I was teaching anyone anything | |
| 20:19:54 | sean-k-mooney | hehe its just funny to me because you are reiterating what stephenfin said more or less like 6 hours ago | |
| 20:20:32 | artom | Better late than never, I suppose | |
| 20:20:43 | sean-k-mooney | artom: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-10.log.html#t2019-06-10T14:53:27 :) | |
| 20:21:43 | artom | *snerk* Yeah | |
| 20:24:25 | sean-k-mooney | so are you open to trying to remove the api change to the driver api and filtering the events as i was suggesting ? it shoudl reduce the size of the patch a resonable amount | |
| 20:24:43 | artom | sean-k-mooney, yeah | |
| 20:25:04 | artom | Still opposed to passing 'revert' to _create_domain_and_network though :) | |
| 20:25:42 | artom | But less oot driver pain is indeed better | |
| 20:26:29 | sean-k-mooney | well you might be able to tell form somthing else but why dont you like passing revert to _create_domain_and_network | |
| 20:26:41 | sean-k-mooney | i dont think you replied to the commets did you? | |
| 20:27:19 | artom | It's higher up logic that should live higher up | |
| 20:27:24 | artom | I kinda replied | |
| 20:27:45 | artom | So what I came up with is passing a list of events to wait for | |
| 20:28:25 | artom | I say higher up | |