6:06 p.m.
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(a)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 ;-)