Skip to content
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

Add vsphere virtual machine migration (Fixes #7008) #7023

Merged
merged 1 commit into from
Jun 7, 2016

Conversation

markpeek
Copy link
Contributor

@markpeek markpeek commented Jun 5, 2016

Fixes vsphere virtual machine migration issues with disk controller_type and skip_customization.

Fixes vsphere virtual machine migration issues with
disk controller_type and skip_customization.
@markpeek
Copy link
Contributor Author

markpeek commented Jun 5, 2016

@chrislovecnm @dkalleg @thetuxkeeper please review this migration change

@stack72
Copy link
Contributor

stack72 commented Jun 5, 2016

LGTM! Nice fix here :)

@thetuxkeeper
Copy link
Contributor

Looks good!
Nice feature you're using. Will keep that in mind if I change something that needs state migration (like the one you fixed :) ).
Just one question. What if we had a new version "2" (or let's say "10" in a year or two :) ), what would you have to change? Is the migration function called for each version or only once?
If only once, is it possible to prepare it for multiple version upgrades/migrations? Something like

while schema_version < target_version 
{
    migrate(current_version, current_version+1)
}

I think all at once would be too much at some time in the future, so my idea would be to do it step by step from one version to the next until we reach the latest one.
What do you think?

@chrislovecnm
Copy link
Contributor

@markpeek thanks for the work!!

@thetuxkeeper you have time to run the tests? Otherwise let me know, and I will run them this eveningish :P

@markpeek
Copy link
Contributor Author

markpeek commented Jun 6, 2016

@thetuxkeeper Here's an example on how it is done for the Google Compute Instance. Note the fallthrough between the cases in the switch statement.

@thetuxkeeper
Copy link
Contributor

@markpeek : ah, I see. Thanks for the example, didn't know that there's such a statement :)
@chrislovecnm : not today, I'll let them run tomorrow, won't hurt if we both let them run. :)

@ghost
Copy link

ghost commented Apr 25, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants