-
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
Make register-vm a sync operation #655
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
/ost |
bennyz
approved these changes
Sep 18, 2022
oliel
reviewed
Sep 20, 2022
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, just one question ^
...axrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainVmResource.java
Outdated
Show resolved
Hide resolved
In 38d5120 we've changed add VM from configuration to be a blocking/sync operation. Here we do something similar for register-vm, setting it to blocking/sync by default, for the same reason - this operation doesn't involve copying the disks, only add-lease is asynchronous and it is a relatively short operation, so the caller won't wait much and at the end of the call the VM is ready to be used. This is particularly important for disaster recovery as we try to start highly available VMs right after registering them. It might have been better to change the disaster recovery scripts to specify async=false, but that would require more efforts. That change revealed an issue with the tests of ExportVmTemplate since it is monitored by polling the job but tested as if it was monitored by polling the VDSM tasks. Therefore the API tests of ExportVmTemplate are also changed here. Bug-Url: https://bugzilla.redhat.com/1968433 Signed-off-by: Arik Hadas <ahadas@redhat.com>
/ost |
getting this in since we approach the code freeze of ovirt 4.5.3 and I believe @oliel 's comment has been addressed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 38d5120 we've changed add VM from configuration to
be a blocking/sync operation. Here we do something similar for
register-vm, setting it to blocking/sync by default, for the
same reason - this operation doesn't involve copying the disks,
only add-lease is asynchronous and it is a relatively short
operation, so the caller won't wait much and at the end of the
call the VM is ready to be used.
This is particularly important for disaster recovery as we try
to start highly available VMs right after registering them. It
might have been better to change the disaster recovery scripts
to specify async=false, but that would require more efforts.
That change revealed an issue with the tests of ExportVmTemplate
since it is monitored by polling the job but tested as if it
was monitored by polling the VDSM tasks. Therefore the API
tests of ExportVmTemplate are also changed here.
Bug-Url: https://bugzilla.redhat.com/1968433
Signed-off-by: Arik Hadas ahadas@redhat.com