H-LK opened a new pull request, #13857:
URL: https://github.com/apache/cloudstack/pull/13857
### Description
This PR restores the classic worker-VM-based VMDK data-disk lifecycle on
VMware 8 (`cs.vmware.api.version=8.0`), removing the FCD
(`VirtualStorageObjectManagerMO`) path for data disks. This avoids vCenter
datastore-browser inconsistencies between FCD and VMDK representations that
caused HTTP 500 errors during `attachVolume` on a running VM, as reported in
#13249.
#### Context — issue #13249
`attachVolume` fails with `HTTP response code: 500` from the vCenter
datastore browser when attaching a detached volume on a
PreSetup/DatastoreCluster storage pool to a running VMware VM.
`VirtualMachineMO.attachDisk()` → `getVmdkFileInfo()` does an HTTP GET via the
vCenter datastore browser, and vCenter routes this to a non-owner ESXi host
which cannot serve the locked file of the running VM.
This PR is a more comprehensive workaround than the minimal
descriptor-update guard proposed in #13773 (by @DaanHoogland, against the 4.20
branch). It supersedes that approach by:
1. Removing the FCD code path for data disks entirely on VMware 8 — volumes
are always created as classic VMDKs via a worker VM
(`HypervisorHostHelper.createWorkerVM` + `vmMo.createDisk`/`detachDisk`), using
`getDeprecatedLegacyDatastorePathFromVmdkFileName`.
2. Migrating legacy root-level (deprecated) and `fcd/`-base-folder VMDKs
into the VM default folder during `attachVolume` via `VirtualDiskManagerMO`
(move, or copy+delete when an adapter-type change is required), instead of
relying on the vCenter HTTP datastore browser.
3. Mapping CloudStack `ProvisioningType` → vSphere `VirtualDiskType`
(`THIN→THIN`, `SPARSE→PREALLOCATED`, `FAT→EAGER_ZEROED_THICK`) through a new
`VmwareHelper.getVirtualDiskType` helper, used both at create time and as the
`FileBackedVirtualDiskSpec.diskType` during adapter-changing copy.
4. Skipping the VMDK adapter descriptor update in `attachDisk` when the sync
already migrated the disk into the VM folder (adapter type changed), so the
vCenter datastore browser is no longer consulted for a locked file of a running
VM.
Fixes: #13249
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] 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)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [x] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
N/A
### How Has This Been Tested?
#### Unit tests
- `VmwareHelperTest.getVirtualDiskTypeMapsCloudStackProvisioningTypes` —
verifies THIN/SPARSE/FAT/null mappings to `VirtualDiskType`.
- `VmwareStorageLayoutHelperTest` (new) — three tests with Mockito
`mockConstruction` on `VirtualDiskManagerMO`:
1. `syncVolumeToVmDefaultFolderMovesRootVolumeWithVirtualDiskManager` —
root-level (deprecated legacy) `[ds] volume.vmdk` is **moved** into `[ds]
vm-name/volume.vmdk`.
2.
`syncVolumeToVmDefaultFolderCopiesRootVolumeWithTargetAdapterAndProvisioningType`
— root-level volume is **copied with adapter+provisioning spec** (`lsiLogic` /
`eagerZeroedThick`) and the original is deleted; `result.second()` is `true`.
3. `syncVolumeToVmDefaultFolderCopiesBaseFolderVolumeWithTargetAdapter` —
`fcd/`-base-folder volume is **copied with adapter spec** (`lsiLogic` / `thin`)
and the original is deleted; `result.second()` is `true`.
Verifies `moveVirtualDisk` / `copyVirtualDisk`+`deleteVirtualDisk`
ordering and the `FileBackedVirtualDiskSpec` adapterType/diskType values.
#### Compilation
All changed files compile clean (no errors). The port was verified
line-by-line identical to the workaround already shipped in the downstream
`cloudstack-4.20.3.0-vmware-datadisk-fcd-fix` branch (commit `18f2fd2c94`
"vmware: restore legacy VMDK data disk lifecycle"), which is
production-deployed.
#### How did you try to break this feature and the system with this change?
- Verified that callers of the existing `String`-returning
`syncVolumeToVmDefaultFolder` overloads still compile (they delegate `.first()`
internally).
- Verified `attachDisk(maxIops)` keeps the existing default behaviour
(`updateVmdkAdapter=true`); only the new `attachDisk(..., boolean)` overload
allows skipping the descriptor update.
- Verified `createVolume` cleanup (`detachAllDisksAndDestroy`) is wrapped in
`try/catch` with a warning so a worker-VM cleanup failure no longer aborts
volume creation.
<!-- Please read the
[CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
