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]