[4.6] CLOUDSTACK-4787 - vmware diskcontrollers #1132
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
commit #7 So far only 1 controller (scsi or ide) is supported in Cloudstack for ide or scsi, this is existing limitation. Added support for 2nd IDE controller. Support adding IDE virtual disk to VM. Also added check if VM is running as IDE virtual disk cannot be attached to VM if VM is runnning.If user detaches a virtual disk on lower unit number of controller, then subsequent attach operation should find free unit number on the controller and attach the virtual disk there. commit #6 Let the controllers of existing VMs continue without flip, current busInfo retrieved from chain_info field of volume record from database would be preferred over controller settings from all configuration settings. commit #5 Editing global configuration param vmware.root.disk.controller osdefault value results in loss of previous root disk controller type. Hence root disk's controller type for legacy VMs is unknow post that modificaiton by user. If VM is stop/start then we could get this infromation from bus info of existing volume. But if user resets VM and then try to start VM. The existing bus info would be lost. Hence existing disk info is not available to depend on. Using lsilogic or generic scsi controller for ROOT disk of legacy VMs if reset. commit #4 Avoid adding additional (>1) scsi controllers to system vms. While attaching volume to legacy VM don't use osdefault optoin which applicable only for VM created with the option enabled, use legacy data disk controller type (lsilogic) commit #3 If root disk's controller type is scsi and data disk controller type condenses to any of scsi sub-types then data disk controller type would fall back to root disk controller itself. This ensures data volumes would be accessible in all cases as controller of root volume would be reliable and it means VM has the supported controller. It also avoids mix of scsi controller sub-types in a user instance. Also translating disk controller type scsi to lsilogic. commit #2 Support auto detection of recommended virtual disk controller type for specific guest OS. commit #1 Support granual controller types. Add support for controller types in template registration as well. Fix white spaces. Removed stale HEAD merge lines Removed tail of merge lines Fixed VmwareResource, removing storage commands that moved to VmwareStorageProcessor. removed stale code of controller that is present in processor Fixed check style errors. Fixed injection. Tested with Linux and windows templates. Unable to run iso based tests due to few bugs in register iso area. Signed-off-by: Sateesh Chodapuneedi <[email protected]> (cherry picked from commit a4cc987) Signed-off-by: Rohit Yadav <[email protected]>
- Adds new controller types in the UI, for selecting root disk controller while registering templates - Fixes bug to not override disk controller type if provided in the details (either vm details or from template details) (cherry picked from commit c7d6762) Signed-off-by: Rohit Yadav <[email protected]>
|
LGTM based on these tests: Result: And: Result: I have no VMware setup so I didn't test the actual feature. This just shows it didn't break anything else. |
|
@bhaisaab Let's find someone else who can review this.. |
|
Thanks @remibergsma |
|
@bhaisaab why are we merging the feature branch into a 4.6.* maintenance/minor release branch. Shouldn't it be merged into a major release branch/master? BTW, this week I am working towards a PR against ACS master, for this feature branch along with tests. |
|
@sateesh-chodapuneedi this is a real bug for many users as they are not able to use Windows 2012 templates with vmware/CloudStack, which is why we need the fix in 4.5 for 4.5.3 besides this does not involve any db changes but simply add to a list of supported controller types other than ide or scsi. |
|
Ping @abhinandanprateek @borisroman @jburwell @pdion891 and others for review |
|
code LGTM and @remibergsma his test succeed, as this is in 4.5 I think it must go in 4.6 as well, however it contains way to little tests and can not be guaranteed to continue working. @bhaisaab please consider continued maintenance by automation, i.e. both unit - and integration tests. merging |
asfgit
pushed a commit
that referenced
this pull request
Dec 12, 2015
[4.6] CLOUDSTACK-4787 - vmware diskcontrollersSame as #1131 (see this for screenshots etc) * pr/1132: CLOUDSTACK-4787: Allow users to select disk controller for VM/template CLOUDSTACK-4787 Allow selection of scsi controller type in vSphere Signed-off-by: Daan Hoogland <[email protected]>
|
@DaanHoogland thanks for merging. This is not yet in 4.5 as I'm yet to merge the 4.5 based PR #1131 |
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.
Same as #1131 (see this for screenshots etc)