On 07/09/2018 06:24 PM, 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 | 68 +++++++++++++++++++++-
> .../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, 276 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/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 9dd22554ad..06d30565dd 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4510,9 +4510,22 @@
> guest. Currently, <code>model='vfio-pci'</code> and
> <code>model='vfio-ccw'</code> (<span class="since">Since 4.4.0</span>)
> is supported. Refer <a href="drvnodedev.html#MDEV">MDEV</a> to create
> - a mediated device on the host. There are also some implications on the
> - usage of guest's address type depending on the <code>model</code>
> - attribute, see the <code>address</code> element below.
> + a mediated device on the host.
> + <span class="since">Since 4.6.0 (QEMU 2.12)</span> an optional
> + <code>display</code> attribute may be used to enable or disable
> + support for an accelerated remote desktop backed by a mediated
> + device (such as NVIDIA vGPU or Intel GVT-g) as an alternative to
> + emulated <a href="#elementsVideo">video devices</a>. This attribute
> + is limited to <code>model='vfio-pci'</code> only. Supported values
> + are either 'on' or 'off' (default is 'off'). It is required to use a
Nit pick, I usually write it as <code>on</code>, <code>off</code>.
> + <a href="#elementsGraphics">graphical framebuffer</a> in order to
> + use this attribute, currently only supported with VNC, Spice and
> + egl-headless graphics devices.
> + <p>
> + Note: There are also some implications on the usage of guest's
> + address type depending on the <code>model</code> attribute,
> + see the <code>address</code> element below.
> + </p>
> </dd>
> </dl>
> <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8f7d273d9f..b8ec9c758a 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4576,6 +4576,11 @@
> <value>vfio-ccw</value>
> </choice>
> </attribute>
> + <optional>
> + <attribute name="display">
> + <ref name="virOnOff"/>
> + </attribute>
> + </optional>
> <element name="source">
> <ref name="mdevaddress"/>
> </element>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 54fc599e5d..027c3de329 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7619,6 +7619,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;
> @@ -7638,6 +7639,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> sgio = virXMLPropString(node, "sgio");
> rawio = virXMLPropString(node, "rawio");
> model = virXMLPropString(node, "model");
> + display = virXMLPropString(node, "display");
Don't forget to VIR_FREE(display) ;-)
>
> /* @type is passed in from the caller rather than read from the
> * xml document, because it is specified in different places for
> @@ -7725,6 +7727,15 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> model);
> goto error;
> }
> +
> + if (display &&
> + (mdevsrc->display = virTristateSwitchTypeFromString(display)) < 0) {
I guess you don't want to accept 'absent' do you? s/</<=/
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown value '%s' for <hostdev> attribute "
> + "'display'"),
> + display);
> + goto error;
Pre-existing, but this label should be renamed to 'cleanup' because it
is executed on both success and failure.
> + }
> }
>
> switch (def->source.subsys.type) {
> @@ -26530,9 +26541,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)
> + virBufferAsprintf(buf, " display='%s'",
> + virTristateSwitchTypeToString(mdevsrc->display));
> + }
> +
> }
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 2);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 31d77e49c5..2ab1faa556 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -382,6 +382,7 @@ typedef struct _virDomainHostdevSubsysMediatedDev virDomainHostdevSubsysMediated
> typedef virDomainHostdevSubsysMediatedDev *virDomainHostdevSubsysMediatedDevPtr;
> struct _virDomainHostdevSubsysMediatedDev {
> int model; /* enum virMediatedDeviceModelType */
> + int display; /* virTristateSwitchType */
Actually, it's called just "virTristateSwitch"
> char uuidstr[VIR_UUID_STRING_BUFLEN]; /* mediated device's uuid string */
> };
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index f488050bf8..a2c5201139 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6235,6 +6235,69 @@ qemuDomainVsockDefPostParse(virDomainVsockDefPtr vsock)
> }
>
>
> +static int
> +qemuDomainHostdevMdevDefPostParse(const virDomainHostdevSubsysMediatedDev *mdevsrc,
> + const virDomainDef *def)
> +{
> + if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT &&
> + 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) {
> + 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 or
> + * <graphics type='egl-headless/>' might be necessary to be added,
> + * depending on whether we're running with SPICE or VNC respectively.
> + */
> + if (!virDomainGraphicsDefHasOpenGL(def))
> + VIR_WARN("<hostdev> attribute 'display' may need the OpenGL to "
> + "be enabled");
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> +qemuDomainHostdevDefPostParse(const virDomainHostdevDef *hostdev,
> + const virDomainDef *def)
> +{
> + const virDomainHostdevSubsysMediatedDev *mdevsrc;
> +
> + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> + switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) {
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
> + break;
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> + mdevsrc = &hostdev->source.subsys.u.mdev;
> + return qemuDomainHostdevMdevDefPostParse(mdevsrc, def);
> + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> + default:
> + virReportEnumRangeError(virDomainHostdevSubsysType,
> + hostdev->source.subsys.type);
> + return -1;
> + }
> + }
> +
> + return 0;
> +}
> +
Again, these two ^^ are validate callbacks not PostParse. You are not
filling in missing information, you are checking (=validating) whether
user provided XML is valid.
> +
> static int
> qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> const virDomainDef *def,
> @@ -6285,11 +6348,14 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> ret = qemuDomainVsockDefPostParse(dev->data.vsock);
> break;
>
> + case VIR_DOMAIN_DEVICE_HOSTDEV:
> + ret = qemuDomainHostdevDefPostParse(dev->data.hostdev, def);
> + break;
> +
> case VIR_DOMAIN_DEVICE_LEASE:
> case VIR_DOMAIN_DEVICE_FS:
> case VIR_DOMAIN_DEVICE_INPUT:
> case VIR_DOMAIN_DEVICE_SOUND:
> - case VIR_DOMAIN_DEVICE_HOSTDEV:
> case VIR_DOMAIN_DEVICE_WATCHDOG:
> case VIR_DOMAIN_DEVICE_GRAPHICS:
> case VIR_DOMAIN_DEVICE_HUB:
> diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
> new file mode 100644
> index 0000000000..ea559a6444
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
> @@ -0,0 +1,35 @@
> +<domain type='qemu'>
> + <name>QEMUGuest2</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-i686</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest2'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + </controller>
> + <controller type='pci' index='0' model='pci-root'/>
> + <controller type='ide' index='0'>
> + </controller>
> + <hostdev mode='subsystem' type='mdev' model='vfio-pci' display='on'>
> + <source>
> + <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
> + </source>
> + </hostdev>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display.xml b/tests/qemuxml2argvdata/hostdev-mdev-display.xml
> new file mode 100644
> index 0000000000..f5b3575c04
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-mdev-display.xml
> @@ -0,0 +1,39 @@
> +<domain type='qemu' id='1'>
> + <name>QEMUGuest2</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-i686</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest2'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + </controller>
> + <controller type='pci' index='0' model='pci-root'/>
> + <controller type='ide' index='0'>
> + </controller>
> + <graphics type='vnc'/>
> + <hostdev mode='subsystem' type='mdev' model='vfio-pci' display='on'>
> + <source>
> + <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
> + </source>
> + </hostdev>
> + <video>
> + <model type='qxl' heads='1'/>
> + </video>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/hostdev-mdev-display-active.xml b/tests/qemuxml2xmloutdata/hostdev-mdev-display-active.xml
> new file mode 100644
> index 0000000000..63a1a00278
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/hostdev-mdev-display-active.xml
> @@ -0,0 +1,47 @@
> +<domain type='qemu' id='1'>
> + <name>QEMUGuest2</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-i686</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest2'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> + </controller>
> + <controller type='pci' index='0' model='pci-root'/>
> + <controller type='ide' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> + </controller>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <graphics type='vnc' port='-1' autoport='yes'>
> + <listen type='address'/>
> + </graphics>
> + <video>
> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> + </video>
> + <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci' display='on'>
> + <source>
> + <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
> + </source>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> + </hostdev>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/hostdev-mdev-display-inactive.xml b/tests/qemuxml2xmloutdata/hostdev-mdev-display-inactive.xml
> new file mode 100644
> index 0000000000..95a692aee5
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/hostdev-mdev-display-inactive.xml
> @@ -0,0 +1,47 @@
> +<domain type='qemu'>
> + <name>QEMUGuest2</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-i686</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest2'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
> + </controller>
> + <controller type='pci' index='0' model='pci-root'/>
> + <controller type='ide' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> + </controller>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <graphics type='vnc' port='-1' autoport='yes'>
> + <listen type='address'/>
> + </graphics>
> + <video>
> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
> + </video>
> + <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
> + <source>
> + <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
> + </source>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> + </hostdev>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index fa57221d62..49e0e03f03 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -479,6 +479,8 @@ mymain(void)
> DO_TEST("hostdev-pci-address", NONE);
> DO_TEST("hostdev-vfio", NONE);
> DO_TEST("hostdev-mdev-precreated", NONE);
> + DO_TEST_FULL("hostdev-mdev-display", WHEN_ACTIVE, GIC_NONE,
> + QEMU_CAPS_VFIO_PCI_DISPLAY);
-ENOSUCHCAP
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
