Skip to content

Change machine_type using vm or vm_params module #287

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

Merged
merged 15 commits into from
Feb 26, 2024
Merged

Conversation

justinc1
Copy link
Collaborator

@justinc1 justinc1 commented Dec 18, 2023

The PR will make machine_type changable using vm_params or vm module.

Implementation detail how to change BIOS VM to UEFI:

  • first change machine_type to UEFI
  • shutdown VM
  • add NVRAM disk
  • start VM back.

Sorry you are reading this. Touching the shotdown/reboot part exploded. Many functions were returning reboot to signal VM needs to be rebooted (but caller need to know if VM was running before). Before touching implementation I added extra check to (some) integration tests to check value of vm_rebooted. This showed some bugs - (if I remember correctly) modules were returning vm_rebooted as True when VM was not even running initially. Or we tested module only on stopped VM. I think (didn't try to check) we might be also rebooting VM more than once during module invocation. PR extends some of integration tests - not all.

The VM.reboot field was removed from code. Sometimes it was meaning VM needs to be rebooted (but only if it was running), sometimes it was meaning VM was rebooted. It is replaced by flags _was_nice_shutdown_tried, _was_force_shutdown_tried, _was_start_tried`. Makes usage easier, we just need to make sure the same VM object is used all the time.

Some bugs or potential bugs remain - there are about 5 TODOs in PR. I will create a separate issue(s) for them, this PR is already to big. Beside vm and vm_params module also vm_nic module should now work correctly (regarding vm_reboot). The vm_disk and vm_boot_devices were left on TODO list.

Some changes are cosmetic. We had FROM_HYPERCORE_TO_ANSIBLE_POWER_STATE and FROM_ANSIBLE_TO_HYPERCORE_POWER_STATE mappings. But FROM_ANSIBLE_TO_HYPERCORE_POWER_STATE was not about states, it was about state changes - so I renamed it to _ACTIONS.

The module checks VM has all required disks before changing machine_type (UEFI VM is bootable only if NVRAM disk is present). But likely we want also to prevent removing NVRAM disk from UEFI VM - this is not implemented yet.

Fixes #284

Bug - VM is not get booted back.

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
@justinc1 justinc1 force-pushed the change-machine-type branch 9 times, most recently from 75b1952 to e840d20 Compare February 26, 2024 11:03
Module check needed nvram/vtpm disks are present.
Running VM is rebooted if machine_type is changed.

Fixes #284

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Use same VM object in multiple functions.
The VM does remember if it was initially running,
and know it needs to be restarted if it was shutdown.

power state vs action - rename FROM_ANSIBLE_TO_HYPERCORE_POWER_STATE
to FROM_ANSIBLE_TO_HYPERCORE_POWER_ACTION

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
01:23:45:67:89:AB is not unicast, and VM was not bootable.
VM just hang in starting state for a long time.

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
fixup! VM.update_vm_power_state needs to remember applied chang
Try to differentiate power_state vm power_action
Surprise - vm_params can send RESET to stopped VM, it does not fail
(no 500 error), but VM does not get booted (it was not running before).

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
See also #288

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Default value is wrong in 9.2.13, it was fixed in later versions

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
@justinc1 justinc1 force-pushed the change-machine-type branch from e840d20 to 1fc1ca3 Compare February 26, 2024 11:38
Test does not expect VMs will be removed while it is testing

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
@justinc1 justinc1 force-pushed the change-machine-type branch from 1fc1ca3 to 4196c1e Compare February 26, 2024 11:44
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
@justinc1 justinc1 force-pushed the change-machine-type branch from 334b558 to 4d41400 Compare February 26, 2024 16:50
@justinc1 justinc1 merged commit 2794a34 into main Feb 26, 2024
@justinc1 justinc1 deleted the change-machine-type branch February 26, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 Feature request: allow vm_params to change machine type
2 participants