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

   ### Description
   
   When the VM setting `rootDiskController=virtio-blk` is set, there is an 
error that blocks the start of a  VM. 
   `DiskBus` renders its own value into the libvirt disk XML, and `virtio-blk` 
is not one of libvirt's target buses (`ide, scsi, virtio, xen, usb, sata, sd, 
fdc, uml`), so libvirt rejects the domain and the start fails on every host.
   
   We wanted to use `virtio-vlk` in order to get virtio disk with 
`discard='unmap'`,  plain virtio disks keep libvirt's default 
`discard='ignore'`, so a guest's TRIM never reaches the storage. On Ceph RBD 
that means deleted data keeps occupying the pool.
   
   ## Reproduction
   
   Any KVM host and template will do:
   
   ```
   cmk deploy virtualmachine zoneid=<zone> serviceofferingid=<offering> \
       templateid=<any KVM template> networkids=<network> \
       'details[0].rootDiskController=virtio-blk'
   ```
   
   The deploy job fails with `errorcode 530`, "Unable to orchestrate the start 
of VM instance", and the instance is left in the `Error` state. The planner 
retries on every host in the zone, and each agent logs:
   
   ```
   WARN  [resource.wrapper.LibvirtStartCommandWrapper] ... LibvirtException
   org.libvirt.LibvirtException: XML error: Invalid value for attribute 'bus' in
   element 'target': 'virtio-blk'.
   ```
   
   It is not version specific: the bus name is rejected by libvirt's own 
schema, so it fails the same way on libvirt 10.0.0 / QEMU 8.2.2 and on libvirt 
12.0.0 / QEMU 10.2.1.
   
   `virtio-blk` is also offered as a supported value: for a KVM resource, 
`listDetailOptions` returns it for both controller details, so the UI and API 
advertise a value that cannot start a VM:
   
   ```
   $ cmk list detailoptions resourcetype=Template resourceid=<KVM template>
   {
     "detailoptions": {
       "details": {
         ...
         "dataDiskController": [
           "osdefault",
           "ide",
           "scsi",
           "virtio",
           "virtio-blk"
         ],
         ...
         "rootDiskController": [
           "osdefault",
           "ide",
           "scsi",
           "virtio",
           "virtio-blk"
         ],
         ...
       }
     }
   }
   ```
   
   The list comes from `QueryManagerImpl.fillVMOrTemplateDetailOptions`, which 
adds these KVM-only options when the resource's hypervisor is KVM; without a 
`resourceid` the controller keys are not returned at all.
   
   ## Changes
   
   - Render the libvirt bus name instead of the enum value. 
   - Label virtio-blk disks `vd*`, as virtio disks are labelled. They 
previously fell through to the `hd*` branch, so even a valid bus would have 
produced an IDE-style target name.
   - Data disks follow a virtio-blk root, as they already follow a SCSI one, so 
a VM that opts in gets discard on all of its disks.
   
   - A data disk with no controller of its own follows the root detail when 
attaching. The attach path cannot recover the controller from the running 
domain XML, where a virtio-blk disk is indistinguishable from a virtio one; 
without this a hot-plugged disk silently loses discard until the next 
stop/start. The scan of the running disks still runs first, so a VM started on 
SCSI whose `rootDiskController` detail was later changed to `virtio-blk` keeps 
getting SCSI disks instead of a virtio disk next to its `sd*` ones.
   - Virtio-blk disks get the `iothread=` binding when the VM has iothreads 
enabled, as virtio disks do. It was gated on the `VIRTIO` enum value only, so a 
virtio-blk VM ran with `io='threads'` but no iothread on any disk.
   
   ### 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
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [x] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   N/A
   
   ### How Has This Been Tested?
   
   - Unit tests added in `LibvirtVMDefTest` and `KVMStorageProcessorTest`; 
`LibvirtVMDefTest`, `KVMStorageProcessorTest` and 
`LibvirtComputingResourceTest` pass.
   - KVM hosts on Ubuntu 24.04 (libvirt 10.0.0 / QEMU 8.2.2) and Ubuntu 26.04 
(libvirt 12.0.0 / QEMU 10.2.1), Ceph RBD primary storage.
   - Without this change, deploying a VM with `rootDiskController=virtio-blk` 
fails on both with the libvirt `bus` error shown above.
   - With this change, the same VM starts on both, and its disks are 
`bus='virtio'`, `vd*`, `discard='unmap'`.
   - With this change, on the 26.04 hosts: hot-plugged disks get the same, 
guest device names stay the same across reboot and stop/start, and `fstrim` in 
the guest returns the freed space to the Ceph pool.
   
   #### How did you try to break this feature and the system with this change?
   
   - VMs without the detail, and VMs with `rootDiskController=scsi`, deployed 
and hot-plugged as before: plain `virtio` disks still have no `discard`, SCSI 
disks are unchanged.
   - Changed `rootDiskController` to `virtio-blk` on a VM running on SCSI and 
attached a disk: it stays SCSI, since the running disks are checked before the 
detail.
   - Checked every consumer of `DiskBus`, the disk labels and the domain XML 
round trip (attach/detach, migration with storage, unmanaged import) for code 
that could treat a virtio-blk disk differently than intended.
   
   
   


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