From: Peter Krempa <pkrempa@redhat.com> The currently existing checks are broken: - only QEMU_CAPS_DEVICE_VHOST_USER_GPU is checked for vhostuser backends (vhost-user-vga is actually separately packaged) - the check for the 3d accelerated (-gl) versions checks only if one of them exists (the commandline formatter picks a non-gl afterwards) - 'virtio-vga'/'virtio-gpu' is not checked at all The code also doesn't yet check if, when the user passes the new 'device' property manually the config actually makes sense. To fix all of the above introduce a table of supported frontend devices as well as properties that need to be checked for them. This requires fixing a recently-introduced test case which shows a nonsensical situation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_validate.c | 88 +++++++++++++------ ...VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.args | 35 -------- ..._VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.err | 1 + ..._VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.xml | 46 ---------- tests/qemuxmlconftest.c | 1 + 5 files changed, 65 insertions(+), 106 deletions(-) delete mode 100644 tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.args create mode 100644 tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.err delete mode 100644 tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.xml diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 7c0ea402c3..439d4b1916 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2853,6 +2853,26 @@ qemuValidateDomainDeviceDefHostdev(const virDomainHostdevDef *hostdev, } +struct qemuValidateDeviceVideoVirtioData { + virQEMUCapsFlags cap; /* capability for the device */ + bool primary; /* this device can be only the primary device */ + bool vhostuser; /* uses the vhostuser protocol */ + bool gl; /* supports 3d accel */ +}; + + +struct qemuValidateDeviceVideoVirtioData qemuValidateDeviceVideoVirtioTable[] = { + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_DEFAULT] = { .cap = QEMU_CAPS_LAST }, + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_VGA] = { .cap = QEMU_CAPS_DEVICE_VIRTIO_VGA, .primary = true }, + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_GPU] = { .cap = QEMU_CAPS_DEVICE_VIRTIO_GPU }, + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_VGA_GL] = { .cap = QEMU_CAPS_VIRTIO_VGA_GL, .gl = true, .primary = true }, + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_GPU_GL] = { .cap = QEMU_CAPS_VIRTIO_GPU_GL_PCI, .gl = true }, + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_VHOST_USER_VGA] = { .cap = QEMU_CAPS_DEVICE_VHOST_USER_VGA, .vhostuser = true, .primary = true }, + [VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_VHOST_USER_GPU] = { .cap = QEMU_CAPS_DEVICE_VHOST_USER_GPU, .vhostuser = true }, +}; +G_STATIC_ASSERT(G_N_ELEMENTS(qemuValidateDeviceVideoVirtioTable) == VIR_DOMAIN_VIDEO_VIRTIO_DEVICE_LAST); + + static int qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video, const virDomainDef *def, @@ -2873,6 +2893,49 @@ qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video, return -1; } + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + struct qemuValidateDeviceVideoVirtioData *data = &qemuValidateDeviceVideoVirtioTable[video->virtiodevice]; + + if (data->cap == QEMU_CAPS_LAST) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks any suitable 'virtio' video device frontend")); + return -1; + } + + if (!virQEMUCapsGet(qemuCaps, data->cap)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("this QEMU doesn't support the '%1$s' virtio video device frontend"), + virDomainVideoVirtioDeviceTypeToString(video->virtiodevice)); + return -1; + } + + if (data->primary && !video->primary) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the '%1$s' virtio video device frontend can be only used as primary video device"), + virDomainVideoVirtioDeviceTypeToString(video->virtiodevice)); + return -1; + } + + /* this check deliberately allows downgrade to the non-gl frontend as + * that was possible due to a bug in capability checking and the + * commandline formatter. More details are explained in + * 'qemuDomainDeviceVideoDefPostParse' which selects the frontend */ + if (data->gl && + !(video->accel && video->accel->accel3d == VIR_TRISTATE_BOOL_YES)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the '%1$s' virtio video device frontend can be only with enabled 3d acceleration"), + virDomainVideoVirtioDeviceTypeToString(video->virtiodevice)); + return -1; + } + + if (data->vhostuser != (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the '%1$s' virtio video device doesn't match the selected vhostuser backend"), + virDomainVideoVirtioDeviceTypeToString(video->virtiodevice)); + return -1; + } + } + if (video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { if (!video->primary) { @@ -2944,31 +3007,6 @@ qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video, } } - if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) { - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU does not support 'vhost-user' video device")); - return -1; - } - } else if (video->accel) { - if (video->accel->accel3d == VIR_TRISTATE_BOOL_YES) { - if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("3d acceleration is supported only with 'virtio' video device")); - return -1; - } - - if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_GL_PCI) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_VGA_GL))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("3d acceleration is not supported by this QEMU binary")); - return -1; - } - } - } - if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { if (video->blob != VIR_TRISTATE_SWITCH_ABSENT) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_BLOB)) { diff --git a/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.args b/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.args deleted file mode 100644 index cd19fd5d17..0000000000 --- a/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.args +++ /dev/null @@ -1,35 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ -USER=test \ -LOGNAME=test \ -XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ -XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ -XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -/usr/bin/qemu-system-x86_64 \ --name guest=QEMUGuest1,debug-threads=on \ --S \ --object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ --machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ --accel tcg \ --cpu qemu64 \ --m size=1048576k \ --object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \ --overcommit mem-lock=off \ --smp 1,sockets=1,cores=1,threads=1 \ --uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ --display none \ --no-user-config \ --nodefaults \ --chardev socket,id=charmonitor,fd=@mon-fd@,server=on,wait=off \ --mon chardev=charmonitor,id=monitor,mode=control \ --rtc base=utc \ --no-shutdown \ --boot strict=on \ --device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ --audiodev '{"id":"audio1","driver":"none"}' \ --device '{"driver":"virtio-vga-gl","id":"video0","max_outputs":1,"bus":"pci.0","addr":"0x2"}' \ --device '{"driver":"virtio-gpu-pci","id":"video1","max_outputs":1,"bus":"pci.0","addr":"0x4"}' \ --device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}' \ --sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ --msg timestamp=on diff --git a/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.err b/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.err new file mode 100644 index 0000000000..b94bf12d69 --- /dev/null +++ b/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.err @@ -0,0 +1 @@ +unsupported configuration: this QEMU binary lacks any suitable 'virtio' video device frontend diff --git a/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.xml b/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.xml deleted file mode 100644 index 587a1c1d0e..0000000000 --- a/tests/qemuxmlconfdata/video-virtio-vga-gpu-gl.x86_64-latest.QEMU_CAPS_VIRTIO_GPU_GL_PCI-disabled-ABI_UPDATE.xml +++ /dev/null @@ -1,46 +0,0 @@ -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>1048576</memory> - <currentMemory unit='KiB'>1048576</currentMemory> - <vcpu placement='static'>1</vcpu> - <os> - <type arch='x86_64' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <cpu mode='custom' match='exact' check='none'> - <model fallback='forbid'>qemu64</model> - </cpu> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu-system-x86_64</emulator> - <controller type='ide' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> - </controller> - <controller type='usb' index='0' model='piix3-uhci'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> - </controller> - <controller type='pci' index='0' model='pci-root'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <audio id='1' type='none'/> - <video> - <model type='virtio' heads='1' primary='yes' device='virtio-vga-gl'> - <acceleration accel3d='yes'/> - </model> - <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> - </video> - <video> - <model type='virtio' heads='1'> - <acceleration accel3d='yes'/> - </model> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> - </video> - <memballoon model='virtio'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> - </memballoon> - </devices> -</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 856aa046b6..a1389775b9 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2664,6 +2664,7 @@ mymain(void) ARG_CAPS_VER, "latest", ARG_QEMU_CAPS_DEL, QEMU_CAPS_VIRTIO_GPU_GL_PCI, QEMU_CAPS_LAST, ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, + ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR, ARG_END); DO_TEST_CAPS_LATEST("video-virtio-edid-none"); DO_TEST_CAPS_LATEST("video-virtio-edid-off"); -- 2.54.0