On 07/09/2018 06:24 PM, Erik Skultety wrote:
Since QEMU 2.12, QEMU understands a new vfio-pci device option 'display' which can be used to turn on display capabilities on vgpu-enabled mediated devices, IOW emulated GPU devices like QXL will no longer be needed with vgpu-enable mdevs. Since QEMU 2.12 defaults to 'auto' for the 'display' attribute, which often doesn't work as reliably, we need to play it safe here and explicitly format display='off' if this attribute wasn't provided in the XML explicitly.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_command.c | 24 ++++++++++++- .../hostdev-mdev-display-spice-egl-headless.args | 32 +++++++++++++++++ .../hostdev-mdev-display-spice-egl-headless.xml | 40 +++++++++++++++++++++ .../hostdev-mdev-display-spice-opengl.args | 31 ++++++++++++++++ .../hostdev-mdev-display-spice-opengl.xml | 41 ++++++++++++++++++++++ .../hostdev-mdev-display-vnc-egl-headless.args | 32 +++++++++++++++++ .../hostdev-mdev-display-vnc-egl-headless.xml | 40 +++++++++++++++++++++ .../qemuxml2argvdata/hostdev-mdev-display-vnc.args | 31 ++++++++++++++++ .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 39 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 31 ++++++++++++++++ 10 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8026efbe87..6192253a9c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5178,6 +5178,17 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def, virBufferAdd(&buf, dev_str, -1); virBufferAsprintf(&buf, ",id=%s,sysfsdev=%s", dev->info->alias, mdevPath);
+ /* QEMU 2.12 added support for vfio-pci display type, we default to + * 'display=off', since QEMU defaults to 'auto' which is unreliable and + * we don't want to risk any breakages */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY) &&
Perhaps you forgot to include a commit that introduces this capability into the series?
+ mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT) + mdevsrc->display = VIR_TRISTATE_SWITCH_OFF;
Isn't if ironic that @def is 'const virDomainDef*' but we are actually changing it? I'm starting to understand why 'const' is pointless in C. (read as not your fault, I just wanted to point this out)
+ + if (mdevsrc->display > VIR_TRISTATE_SWITCH_ABSENT)
Err, s/>/!=/
+ virBufferAsprintf(&buf, ",display=%s", + virTristateSwitchTypeToString(mdevsrc->display)); + if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0) goto cleanup;
@@ -5395,7 +5406,9 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
/* MDEV */ if (virHostdevIsMdevDevice(hostdev)) { - switch ((virMediatedDeviceModelType) subsys->u.mdev.model) { + virDomainHostdevSubsysMediatedDevPtr mdevsrc = &subsys->u.mdev; + + switch ((virMediatedDeviceModelType) mdevsrc->model) { case VIR_MDEV_MODEL_TYPE_VFIO_PCI: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -5403,6 +5416,15 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, "supported by this version of QEMU")); return -1; } + + if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("display property of device vfio-pci is " + "not supported by this version of QEMU")); + return -1; + }
Any reason why this is not checked for in qemuBuildHostdevMediatedDevStr? Because with this check it is possible to work around it - through hotplug code.
+ break; case VIR_MDEV_MODEL_TYPE_VFIO_CCW: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW)) {
Michal