Skip to content

Remove restriction, in VMWare, to resize only SCSI disks #5650

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.

Sign up for GitHub

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

Merged
yadvr merged 6 commits into from
Jan 8, 2022
Merged

Remove restriction, in VMWare, to resize only SCSI disks #5650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ad6c8e
Remove restriction, in VMWare, to resize only SCSI disks, because VMW…
Nov 1, 2021
a934975
Add check with lower case to IDE controller
Nov 3, 2021
39c45cb
Merge branch 'main' of https://github.com/apache/cloudstack into remo…
Dec 9, 2021
cef5c27
Merge branch 'main' into remove-vmware-restriction-resize-only-scsi-d…
Dec 10, 2021
5294e9c
Merge branch 'main' into remove-vmware-restriction-resize-only-scsi-d…
Dec 14, 2021
f1067c6
Merge branch 'main' into remove-vmware-restriction-resize-only-scsi-d…
Dec 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 24 additions & 27 deletions ...hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,10 @@ private Answer execute(ResizeVolumeCommand cmd) {

try {
if (newSize < oldSize) {
throw new Exception(
"VMware doesn't support shrinking volume from larger size: " + oldSize / ResourceType.bytesToMiB + " GB to a smaller size: " + newSize / ResourceType.bytesToMiB + " GB");
String errorMsg = String.format("VMware doesn't support shrinking volume from larger size [%s] GB to a smaller size [%s] GB. Can't resize volume of VM [name: %s].",
oldSize / Float.valueOf(ResourceType.bytesToMiB), newSize / Float.valueOf(ResourceType.bytesToMiB), vmName);
s_logger.error(errorMsg);
throw new Exception(errorMsg);
} else if (newSize == oldSize) {
return new ResizeVolumeAnswer(cmd, true, "success", newSize * ResourceType.bytesToKiB);
}
Expand Down Expand Up @@ -827,11 +829,9 @@ private Answer execute(ResizeVolumeCommand cmd) {
vmMo = hyperHost.findVmOnPeerHyperHost(vmName);

if (vmMo == null) {
String msg = "VM " + vmName + " does not exist in VMware datacenter";

s_logger.error(msg);

throw new Exception(msg);
String errorMsg = String.format("VM [name: %s] does not exist in VMware datacenter.", vmName);
s_logger.error(errorMsg);
throw new Exception(errorMsg);
}


Expand Down Expand Up @@ -894,7 +894,7 @@ private Answer execute(ResizeVolumeCommand cmd) {
vmConfigSpec.getDeviceChange().add(deviceConfigSpec);

if (!vmMo.configureVm(vmConfigSpec)) {
throw new Exception("Failed to configure VM to resize disk. vmName: " + vmName);
throw new Exception(String.format("Failed to configure VM [name: %s] to resize disk.", vmName));
}

ResizeVolumeAnswer answer = new ResizeVolumeAnswer(cmd, true, "success", newSize * 1024);
Expand All @@ -909,11 +909,9 @@ private Answer execute(ResizeVolumeCommand cmd) {
}
return answer;
} catch (Exception e) {
s_logger.error("Unable to resize volume", e);

String error = "Failed to resize volume: " + e.getMessage();

return new ResizeVolumeAnswer(cmd, false, error);
String errorMsg = String.format("Failed to resize volume of VM [name: %s] due to: [%s].", vmName, e.getMessage());
s_logger.error(errorMsg, e);
return new ResizeVolumeAnswer(cmd, false, errorMsg);
} finally {
// OfflineVmwareMigration: 6. check if a worker was used and destroy it if needed
try {
Expand All @@ -924,34 +922,33 @@ private Answer execute(ResizeVolumeCommand cmd) {
vmMo.destroy();
}
} catch (Throwable e) {
s_logger.info("Failed to destroy worker VM: " + vmName);
s_logger.error(String.format("Failed to destroy worker VM [name: %s] due to: [%s].", vmName, e.getMessage()), e);
}
}
}

private VirtualDisk getDiskAfterResizeDiskValidations(VirtualMachineMO vmMo, String volumePath) throws Exception {
Pair<VirtualDisk, String> vdisk = vmMo.getDiskDevice(volumePath);
if (vdisk == null) {
if (s_logger.isTraceEnabled()) {
s_logger.trace("resize volume done (failed)");
}
throw new Exception("No such disk device: " + volumePath);
String errorMsg = String.format("Resize volume of VM [name: %s] failed because disk device [path: %s] doesn't exist.", vmMo.getVmName(), volumePath);
s_logger.error(errorMsg);
throw new Exception(errorMsg);
}

// IDE virtual disk cannot be re-sized if VM is running
if (vdisk.second() != null && vdisk.second().contains("ide")) {
throw new Exception("Re-sizing a virtual disk over an IDE controller is not supported in the VMware hypervisor. " +
"Please re-try when virtual disk is attached to a VM using a SCSI controller.");
if (vdisk.second() != null && vdisk.second().toLowerCase().contains("ide")) {
String errorMsg = String.format("Re-sizing a virtual disk over an IDE controller is not supported in the VMware hypervisor. "
+ "Please re-try when virtual disk is attached to VM [name: %s] using a SCSI controller.", vmMo.getVmName());
s_logger.error(errorMsg);
throw new Exception(errorMsg);
}

if (vdisk.second() != null && !vdisk.second().toLowerCase().startsWith("scsi")) {
Copy link
Copy Markdown
Member

@harikrishna-patnala harikrishna-patnala Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Nov 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@SadiJr SadiJr Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

s_logger.error("Unsupported disk device bus " + vdisk.second());
throw new Exception("Unsupported disk device bus " + vdisk.second());
}
VirtualDisk disk = vdisk.first();
if ((VirtualDiskFlatVer2BackingInfo) disk.getBacking() != null && ((VirtualDiskFlatVer2BackingInfo) disk.getBacking()).getParent() != null) {
s_logger.error("Resize is not supported because Disk device has Parent " + ((VirtualDiskFlatVer2BackingInfo) disk.getBacking()).getParent().getUuid());
throw new Exception("Resize is not supported because Disk device has Parent " + ((VirtualDiskFlatVer2BackingInfo) disk.getBacking()).getParent().getUuid());
String errorMsg = String.format("Resize of volume in VM [name: %s] is not supported because Disk device [path: %s] has Parents: [%s].",
vmMo.getVmName(), volumePath, ((VirtualDiskFlatVer2BackingInfo) disk.getBacking()).getParent().getUuid());
s_logger.error(errorMsg);
throw new Exception(errorMsg);
}
return disk;
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4453,8 +4453,8 @@ public void validateRootDiskResize(final HypervisorType hypervisorType, Long roo
} else if ((rootDiskSize << 30) > templateVO.getSize()) {
if (hypervisorType == HypervisorType.VMware && (vm.getDetails() == null || vm.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER) == null)) {
s_logger.warn("If Root disk controller parameter is not overridden, then Root disk resize may fail because current Root disk controller value is NULL.");
} else if (hypervisorType == HypervisorType.VMware && !vm.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER).toLowerCase().contains("scsi")) {
String error = "Found unsupported root disk controller: " + vm.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER);
} else if (hypervisorType == HypervisorType.VMware && vm.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER).toLowerCase().contains("ide")) {
String error = String.format("Found unsupported root disk controller [%s].", vm.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER));
s_logger.error(error);
throw new InvalidParameterValueException(error);
} else {
Expand Down