On 06/27/2018 09:34 AM, Erik Skultety wrote: Kinda sparse for what I assume is a new "feature" (and I won't comment about you're missing a news.xml change :-P)
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/qemu/qemu_command.c | 23 +++++++++++- src/qemu/qemu_domain.c | 3 +- .../hostdev-mdev-display-spice-egl-headless.args | 32 +++++++++++++++++ .../hostdev-mdev-display-spice-egl-headless.xml | 41 ++++++++++++++++++++++ .../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 | 41 ++++++++++++++++++++++ .../qemuxml2argvdata/hostdev-mdev-display-vnc.args | 31 ++++++++++++++++ .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 39 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 29 +++++++++++++++ 11 files changed, 341 insertions(+), 2 deletions(-) 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 fc80a6c3a6..a2a27b5b9b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5162,6 +5162,16 @@ 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' to stay safe from future changes */
I have a vague recollection of a longer explanation for this during the previous review - maybe put some more details here why we had to use display=off, especially since the commit message is sparse.
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY) && + mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT) + mdevsrc->display = VIR_TRISTATE_SWITCH_OFF; + + if (mdevsrc->display > VIR_TRISTATE_SWITCH_ABSENT) + virBufferAsprintf(&buf, ",display=%s", + virTristateSwitchTypeToString(mdevsrc->display)); + if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0) goto cleanup;
@@ -5379,7 +5389,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", @@ -5387,6 +5399,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;
s/ // Too many spaces for return -1
+ } + break; case VIR_MDEV_MODEL_TYPE_VFIO_CCW: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW)) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d624383c61..39920da442 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6141,7 +6141,7 @@ qemuDomainHostdevMdevDefPostParse(const virDomainHostdevSubsysMediatedDev *mdevs }
if (mdevsrc->display == VIR_TRISTATE_SWITCH_ON) { - virDomainGraphicsDefPtr graphics = def->graphics[0]; + virDomainGraphicsDefPtr graphics = NULL;
if (def->ngraphics == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -6154,6 +6154,7 @@ qemuDomainHostdevMdevDefPostParse(const virDomainHostdevSubsysMediatedDev *mdevs * moment, so print a warning that an extra <gl> element might be * necessary to be added. */ + graphics = def->graphics[0]; if (!graphics->gl || graphics->gl->enable != VIR_TRISTATE_BOOL_YES) { VIR_WARN("<hostdev> attribute 'display' may need the OpenGL to "
Thus hunk belongs in the previous patch... John [...] FYI: The examples certainly help point out usage of gl enable=yes conditions and when egl-headless comes into play, I'm still confused over the RFC comments, but that shouldn't really surprise anyone ;-)