-
Notifications
You must be signed in to change notification settings - Fork 261
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
engine: fix reboot on powering up VM #557
Conversation
When the VM was powering UP and had the next run configuration, the cold reboot did not work. It had several reasons that have been fixed in this patch: 1. the VM was just cleared based on getVmManager().getPowerOffTimeout() 2. the VmAnalyzer:proceedDisappearedVm did not have RebootInProgress case and the default one was executed. However, the code never reached the setColdRebootFlag() as the VM was listed as asyncRunning and scheduled for rerun. 3. the next run config was never applied as the VM was present in asyncRunning and never added to movedDown list. Bug-Url: https://bugzilla.redhat.com/1912911
/ost |
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.
Looks good to me but someone more familiar with the flows should approve it.
case PoweringDown: | ||
if (getVmManager().isColdReboot()) { | ||
setColdRebootFlag(); | ||
resourceManager.removeAsyncRunningVm(dbVm.getId()); |
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.
Wondering what's the purpose of managing the "async running" VMs and what the VM removal from it here influences.
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.
If I understand correctly, if a VM is in asyncRunningVms
list, it means that the VM was just started, began migration or resuming. If such a VM goes down, it will be rerun. Otherwise, if the VM is not in asyncRunningVms
list, it means that the VM was expected to go down and it should not be rerun.
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.
I see, thank you for clarification. Then it indeed has its purpose here.
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.
Looks good.
When the VM was powering UP and had the next run configuration, the cold reboot did not work. It had several reasons that have been fixed in this patch:
getVmManager().getPowerOffTimeout()
VmAnalyzer:proceedDisappearedVm
did not haveRebootInProgress
case and the default one was executed.However, the code never reached the
setColdRebootFlag()
as the VM was listed asasyncRunning
and scheduled for rerun.asyncRunning
and never added tomovedDown
list.Bug-Url: https://bugzilla.redhat.com/1912911