abh1sar opened a new pull request, #14162:
URL: https://github.com/apache/cloudstack/pull/14162

   ## Description
   
   This PR fixes 3 issues in the `importVM` workflow:
   
   #### 1. Root disk import doesn't work for RBD pools unlike data disk import
   
   `importVolume` / `listVolumesForImport` already accept RBD. 
`LibvirtGetVolumesOnStorageCommandWrapper` has handled it all along, including 
building the `rbd:` URI for `qemu-img info`. But `importVm importsource=shared` 
rejects it, because the neighbouring `LibvirtCheckVolumeCommandWrapper` was 
written with a file-only pool list. An operator on Ceph can import a data disk 
but not a root disk.
   
   The failure is also misleading. The agent answers "Unsupported Storage 
Pool", but the operator sees:
   
   ```
   CloudRuntimeException: Disk not found or is invalid
   ```
   
   and the VM is rolled back.
   
   Adds `StoragePoolType.RBD` to the check wrapper's supported list, takes the 
virtual size from the disk libvirt already resolved (`qemu-img` cannot open a 
bare RBD image name), skips the QCOW2 header check for raw RBD images, and 
builds the `rbd:` URI via the existing `KVMPhysicalDisk.RBDStringBuilder`.
   
   `diskpath` on `importVm` and `path` on `importVolume` are the file name on 
file-based pools and the image name on RBD. Both parameter descriptions now say 
so.
   
   #### 2. An imported volume records the wrong image format
   
   `importVolume` and `updateImportedVolume` both stamped the cluster default 
format for the hypervisor, so a volume imported from an RBD pool was recorded 
`QCOW2` while a natively deployed volume on the same pool is `RAW`. Both entry 
points were affected: root disks via `importVm importsource=shared`, and data 
disks via `importVolume`.
   
   They now record the format the hypervisor reported for the image, from the 
check answer for a root disk and from the volume listed on the pool for a data 
disk, falling back to the hypervisor default when none is reported. This also 
corrects a raw image imported from a file-based pool, so it is a general 
correctness fix rather than an RBD-specific one.
   
   #### 3. The import is planned outside the pool's cluster
   
   `importKVMInstanceFromDiskImage` planned with pod and cluster unset, so the 
planner could pick any host in the zone by capacity, even one in a cluster with 
no access to the pool the caller named. The volume check then ran against 
whichever pool that cluster does have, and the import failed with "Disk not 
found or is invalid" although the image was fine.
   This is not RBD-specific. Reproduced on a two-cluster zone with plain NFS.
   
   The plan now takes pod and cluster from the pool the caller passed. Both are 
null for a zone-wide pool, which every host can see, so that case plans exactly 
as before.
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally 
changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to 
reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be 
closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- 
******************************************************************************* 
-->
   <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE 
DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- 
******************************************************************************* 
-->
   
   ### 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)
   - [x] 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
   - [x] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   ### How Has This Been Tested?
   
   1. Deployed a VM with a root and data disk on RBD pool
   2. Unmanage it 
   3. Import the VM 
   4. Checked the guest OS
   5. Took volume snapshots on imported volumes
   6. No failures or errors were observed
   7. Verified the format of the root and data disk was raw
   8. Verified on a two cluster setup that importVM always selects the host 
that has access to the storage pool.
   
   
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to 
-->
   
   #### How did you try to break this feature and the system with this change?
   
   <!-- see how your change affects other areas of the code, etc. -->
   
   <!-- 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]

Reply via email to