-
Notifications
You must be signed in to change notification settings - Fork 288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CA-380580: cross-pool migration: no CPU checks for halted VMs #5132
Conversation
CPU checks are needed only for running VMs that are being migrated, to check for compatibility with the remote-host's CPUs. Signed-off-by: Rob Hoes <rob.hoes@citrix.com>
@@ -1456,7 +1457,7 @@ let migrate_send' ~__context ~vm ~dest ~live:_ ~vdi_map ~vif_map ~vgpu_map | |||
in | |||
inter_pool_metadata_transfer ~__context ~remote ~vm ~vdi_map | |||
~vif_map ~vgpu_map ~dry_run:false ~live:true ~copy | |||
~check_cpu:(not force) | |||
~check_cpu:((not force) && power_state <> `Halted) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about 'Suspended' and 'Paused'? In theory you can move that around freely until you resume (at which point you do the CPU check).
Should the condition here we power_state = 'Running
instead? (assuming that 'unpause' would do the CPU check, if not then Running||Paused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an odd use case... The checks take into account that one may want to start/resume/unpause/shutdown the VM while it is being migrated to another pool. This is why the checks are always done with ~live:true
, which checks for available memory on the target host, even if the VM is halted. The CPU checks would be similar, except that you can't actually do it for a halted VM.
Is this risking moving a halted VM to a host where it can't be started later? |
No, CPU feature checks are for running VMs, because those would have made assumptions about the available CPU features when they started. |
…CPU check to the target host (#6175) Rebased the work from 2023 merged in #5111 and #5132, that caused issues and was partially fixed in #5148, but was completely reverted in #5147. I've integrated the fix from #5148 and additionally the fix suggested by @minglumlu in CA-380715 that was not merged at the time due to time constraints. This series passed the tests that were originally failing: sxm-unres (Job ID 4177739), vGPUSXMM60CrossPool (4177750), and also passed the Ring3 BST+BVT (209341). I can run more migration tests if needed - I've heard @Vincent-lau has requested for these to be separated into its own suite instead of being only in Core and Distribution regression tests.
CPU checks are needed only for running VMs that are being migrated, to check for compatibility with the remote-host's CPUs.