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@xxxxxxxxxx>
> ---
> 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;
> +}
> +
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
