On 06/27/2018 09:34 AM, Erik Skultety wrote:
QEMU 2.12 introduced a new type of display for mediated devices using vfio-pci backend which allows a mediated device to be used as a VGA compatible device as an alternative to an emulated video device. QEMU exposes this feature via a vfio device property 'display' with supported values 'on/off/auto' (default is 'off').
This patch adds the necessary bits to domain config handling in order to expose this feature. Since there's no convenient way for libvirt to come up with usable defaults for the display setting, simply because libvirt is not able to figure out which of the display implementations - dma-buf which requires OpenGL support vs vfio regions which doesn't need OpenGL (works with OpenGL enabled too) - the underlying mdev uses.
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- docs/formatdomain.html.in | 19 +++++- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 18 +++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 71 +++++++++++++++++++++- .../hostdev-mdev-display-missing-graphics.xml | 35 +++++++++++ tests/qemuxml2argvdata/hostdev-mdev-display.xml | 39 ++++++++++++ .../hostdev-mdev-display-active.xml | 47 ++++++++++++++ .../hostdev-mdev-display-inactive.xml | 47 ++++++++++++++ tests/qemuxml2xmltest.c | 2 + 10 files changed, 279 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-mdev-display-active.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-mdev-display-inactive.xml
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d399a198e..23d646cb63 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7651,6 +7651,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, char *rawio = NULL; char *backendStr = NULL; char *model = NULL; + char *display = NULL; int backend; int ret = -1; virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; @@ -7670,6 +7671,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, sgio = virXMLPropString(node, "sgio"); rawio = virXMLPropString(node, "rawio"); model = virXMLPropString(node, "model"); + display = virXMLPropString(node, "display");
/* @type is passed in from the caller rather than read from the * xml document, because it is specified in different places for @@ -7757,6 +7759,15 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, model); goto error; } + + if (display && + (mdevsrc->display = virTristateSwitchTypeFromString(display)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown value '%s' for <hostdev> attribute " + "'display'"), + display); + goto error; + } }
switch (def->source.subsys.type) { @@ -26574,9 +26585,14 @@ virDomainHostdevDefFormat(virBufferPtr buf, virTristateBoolTypeToString(scsisrc->rawio)); }
- if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) { virBufferAsprintf(buf, " model='%s'", virMediatedDeviceModelTypeToString(mdevsrc->model)); + if (mdevsrc->display > VIR_TRISTATE_SWITCH_ABSENT)
Usually just != instead of >, but whatever.
+ virBufferAsprintf(buf, " display='%s'", + virTristateSwitchTypeToString(mdevsrc->display)); + } + } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2);
Similar as before (and sorry if I asked the last time too), any ABI concerns? [...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 948b9b7fd0..d624383c61 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6127,6 +6127,72 @@ qemuDomainVsockDefPostParse(virDomainVsockDefPtr vsock) }
+static int +qemuDomainHostdevMdevDefPostParse(const virDomainHostdevSubsysMediatedDev *mdevsrc, + const virDomainDef *def) +{ + if (mdevsrc->display > VIR_TRISTATE_SWITCH_ABSENT &&
Hmmm... usually this only matters for display == YES (or != ABSENT), but then in the subsequent patch we generate NO by default. John
+ mdevsrc->model != VIR_MDEV_MODEL_TYPE_VFIO_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("<hostdev> attribute 'display' is only supported" + " with model='vfio-pci'")); + + return -1; + } + + if (mdevsrc->display == VIR_TRISTATE_SWITCH_ON) { + virDomainGraphicsDefPtr graphics = def->graphics[0]; + + if (def->ngraphics == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("graphics device is needed for attribute value " + "'display=on' in <hostdev>")); + return -1; + } + + /* We're not able to tell whether an mdev needs OpenGL or not at the + * moment, so print a warning that an extra <gl> element might be + * necessary to be added. + */ + if (!graphics->gl || + graphics->gl->enable != VIR_TRISTATE_BOOL_YES) { + VIR_WARN("<hostdev> attribute 'display' may need the OpenGL to " + "be enabled"); + } + } + + return 0; +} +
[...]