Re: [libvirt] [PATCH 1/4] qemu: validate bochs-display capability
On 9/13/19 5:20 PM, Jonathon Jongsma wrote:
When the bochs display type was added, the capability was never
checked.
Add that check in the same place as the other video device capability
checks.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
src/qemu/qemu_process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 955ba4de4c..b93af966e2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5278,7 +5278,9 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm,
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) ||
(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) {
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW)) ||
+ (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_BOCHS_DISPLAY))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("this QEMU does not support '%s' video
device"),
virDomainVideoTypeToString(video->type));
For 1-3:
Reviewed-by: Cole Robinson <crobinso(a)redhat.com>
I rebased, adjusted the 5.8.0 references and pushed.
Since patch #4 is adding a new XML element, can you rebase and send, but
with the new XML in the commit message? You can CC me and I'll review
it, but let it sit for a bit incase anyone else has comments on the new XML
Some general comments:
* Not sure if it's a rule around here, but I suggest always sending a
cover letter for > 1 patches. Personally for me an upfront summary can
help reduce cognitive load of the series, I have an idea of where it is
going without having to look at every patch to figure it out.
* Please do for 'ramfb' what fabiano did for bochs-display here:
https://www.redhat.com/archives/libvir-list/2019-October/msg00123.html
Otherwise apps don't have a good way to determine if ramfb is supported
or not.
Sidenote: Better still, you can unify the validate code and domcaps code
using an approach like I added for RNG here:
https://www.redhat.com/archives/libvir-list/2019-April/msg00457.html
Starting point would be to fold the whole qemuProcessStartValidateVideo
to be part of qemuDomainDeviceDefValidateVideo. It's only at
ProcessStart time for historical reasons IIRC, back when we didn't have
proper infrastructure for the Validate path that wouldn't make VMs
disappear on libvirtd restart.
CCing fidencio too who I've talked about this last bit in private, and
pmores who is working on nearby code currently. I promise timely reviews
for anyone who works on this and CCs me :)
- Cole