Remove restriction, in VMWare, to resize only SCSI disks by SadiJr · Pull Request #5650 · apache/cloudstack

SadiJr · 2021-11-01T21:56:22Z

Description

In VMWare, there are different types of SCSI controllers, such as BusLogic Parallel, LSI Logic Parallel, LSI Logic SAS, etc. All of these sub-types support the disk resize operation, the only exception is the IDE controller. However, in ACS, there is a logical restriction to allow only resizing of volumes of the specific SCSI type; thus, not allowing resizing of the SCSI sub-types. This PR corrects this behavior blocking the resize of volumes which uses IDE controller only.
More information can be found at:

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

How Has This Been Tested?

I tested it in a local lab:

I created a VM with lsisas1068 SAS controller.
I tried to resize the volume and got this error message: "Found unsupported root disk controller: lsisas1068".
With the changes from this PR, I was able to resize the volume correctly.

…are supports resize of subtypes of SCSI, like LSILOGIC, LSILOGIC SAS, etc

shwstppr · 2021-11-02T04:30:52Z

@blueorangutan package

harikrishna-patnala · 2021-11-02T04:50:34Z

+            throw new Exception(errorMsg);
        }

-        if (vdisk.second() != null && !vdisk.second().toLowerCase().startsWith("scsi")) {


I think we have to keep a counter check for IDE controller here, instead of just removing the SCSI check.
I see we are checking it in UserVMManagerImpl but there are other places where resize volume is initiated. So it is better to keep a check in resource layer. Can you please add that.

@SadiJr Alternatively, you can check for specific controller types from com.cloud.hypervisor.vmware.mo.ScsiDiskControllerType (or) com.cloud.hypervisor.vmware.mo.DiskControllerType

@harikrishna-patnala The IDE controller check is done on the top line (

cloudstack/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

Line 943 in 4ad6c8e

if (vdisk.second() != null && vdisk.second().contains("ide")) {

), so I see no reason to redo it here. @sureshanaparti is an option, however, if in the future new types of SCSI controllers are added, this code would have to be revised to accommodate the changes. Rather than checking for allowed controllers, it is much more practical to check for disallowed controllers.

blueorangutan · 2021-11-02T06:03:31Z

Packaging result: ✖️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1657

SadiJr · 2021-11-03T18:35:07Z

@blueorangutan package

blueorangutan · 2021-11-03T18:36:04Z

@SadiJr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-11-03T19:16:28Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1672

shwstppr · 2021-11-03T20:24:53Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-11-03T20:26:03Z

@shwstppr a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

yadvr · 2021-11-18T08:23:18Z

@blueorangutan package

blueorangutan · 2021-11-18T08:24:05Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-11-18T10:28:42Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 1726

…ve-vmware-restriction-resize-only-scsi-disks

…isks

DaanHoogland · 2021-12-16T12:28:10Z

@blueorangutan package

blueorangutan · 2021-12-16T12:29:03Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-12-16T14:17:23Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1913

sureshanaparti · 2021-12-16T14:58:23Z

@blueorangutan test centos7 vmware-67u3

yadvr · 2021-12-30T11:00:03Z

@blueorangutan package

blueorangutan · 2021-12-30T11:01:03Z

@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-12-30T12:42:42Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 2042

GabrielBrascher

Code LGTM

…isks

yadvr · 2021-12-30T17:36:51Z

@blueorangutan package

blueorangutan · 2021-12-30T17:37:03Z

@rohityadavcloud a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-12-30T19:13:45Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2049

DaanHoogland · 2021-12-31T09:05:50Z

@blueorangutan test centos7 vmware-67

blueorangutan · 2021-12-31T09:06:03Z

@DaanHoogland unsupported parameters provided. Supported mgmt server os are: centos7, centos6, alma8, ubuntu18, suse15, ubuntu20, rocky8, centos8. Supported hypervisors are: kvm-centos6, kvm-centos7, kvm-centos8, kvm-rocky8, kvm-alma8, kvm-ubuntu18, kvm-ubuntu20, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82

DaanHoogland · 2021-12-31T09:14:06Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-12-31T09:15:03Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

harikrishna-patnala

Code LGTM

blueorangutan · 2021-12-31T19:14:12Z

Trillian test result (tid-2759)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34692 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5650-t2759-vmware-67u3.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

Remove restriction, in VMWare, to resize only SCSI disks, because VMW…

4ad6c8e

…are supports resize of subtypes of SCSI, like LSILOGIC, LSILOGIC SAS, etc

SadiJr changed the title ~~Remove restriction, in VMWare, to resize only SCSI disks, because VMW…~~ Remove restriction, in VMWare, to resize only SCSI disks Nov 1, 2021

harikrishna-patnala reviewed Nov 2, 2021

View reviewed changes

Add check with lower case to IDE controller

a934975

SadiJr added 3 commits December 9, 2021 17:51

Merge branch 'main' of https://github.com/apache/cloudstack into remo…

39c45cb

…ve-vmware-restriction-resize-only-scsi-disks

Merge branch 'main' into remove-vmware-restriction-resize-only-scsi-d…

cef5c27

…isks

Merge branch 'main' into remove-vmware-restriction-resize-only-scsi-d…

5294e9c

…isks

sureshanaparti requested a review from nvazquez December 16, 2021 14:58

GabrielBrascher approved these changes Dec 30, 2021

View reviewed changes

Merge branch 'main' into remove-vmware-restriction-resize-only-scsi-d…

f1067c6

…isks

yadvr changed the base branch from main to 4.16 December 30, 2021 17:36

yadvr changed the base branch from 4.16 to main December 30, 2021 17:36

harikrishna-patnala approved these changes Dec 31, 2021

View reviewed changes

yadvr approved these changes Jan 8, 2022

View reviewed changes

yadvr merged commit 5d7ea30 into apache:main Jan 8, 2022

yadvr added this to the 4.17.0.0 milestone Jan 8, 2022

yadvr added component:vmware type:enhancement labels Jan 8, 2022

Conversation

SadiJr commented Nov 1, 2021

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

shwstppr commented Nov 2, 2021

Uh oh!

harikrishna-patnala Nov 2, 2021

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Nov 2, 2021

Choose a reason for hiding this comment

Uh oh!

SadiJr Nov 3, 2021

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Nov 2, 2021

Uh oh!

SadiJr commented Nov 3, 2021

Uh oh!

blueorangutan commented Nov 3, 2021

Uh oh!

blueorangutan commented Nov 3, 2021

Uh oh!

shwstppr commented Nov 3, 2021

Uh oh!

blueorangutan commented Nov 3, 2021

Uh oh!

yadvr commented Nov 18, 2021

Uh oh!

blueorangutan commented Nov 18, 2021

Uh oh!

blueorangutan commented Nov 18, 2021

Uh oh!

DaanHoogland commented Dec 16, 2021

Uh oh!

blueorangutan commented Dec 16, 2021

Uh oh!

blueorangutan commented Dec 16, 2021

Uh oh!

sureshanaparti commented Dec 16, 2021

Uh oh!

yadvr commented Dec 30, 2021

Uh oh!

blueorangutan commented Dec 30, 2021

Uh oh!

blueorangutan commented Dec 30, 2021

Uh oh!

GabrielBrascher left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Dec 30, 2021

Uh oh!

blueorangutan commented Dec 30, 2021

Uh oh!

blueorangutan commented Dec 30, 2021

Uh oh!

DaanHoogland commented Dec 31, 2021

Uh oh!

blueorangutan commented Dec 31, 2021

Uh oh!

DaanHoogland commented Dec 31, 2021

Uh oh!

blueorangutan commented Dec 31, 2021

Uh oh!

harikrishna-patnala left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Dec 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!