It's pointless to check the same thing multiple times. Fix the indentation along the way too. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/domain_conf.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 544f63a2a9..62bd17739c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4513,20 +4513,10 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, if (dev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) return 0; - if (model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && - dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - virReportError(VIR_ERR_XML_ERROR, - _("Unsupported address type '%s' with mediated " - "device model '%s'"), - virDomainDeviceAddressTypeToString(dev->info->type), - virMediatedDeviceModelTypeToString(model)); - return -1; - } - if ((model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && - dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) || + dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) || (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && - dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { virReportError(VIR_ERR_XML_ERROR, _("Unsupported address type '%s' with mediated " "device model '%s'"), -- 2.14.3

It should be the command line helper who takes care of the iteration rather than the caller. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/qemu/qemu_command.c | 60 +++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5ec4f2ba..1667b09a8b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5323,10 +5323,10 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, } break; case VIR_MDEV_MODEL_TYPE_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected vfio type '%d'"), subsys->u.mdev.model); + default: + virReportEnumRangeError(virMediatedDeviceModelType, + subsys->u.mdev.model); return -1; - break; } virCommandAddArg(cmd, "-device"); @@ -8042,26 +8042,41 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, static int qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg, virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, - virDomainGraphicsDefPtr graphics) + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { - switch (graphics->type) { - case VIR_DOMAIN_GRAPHICS_TYPE_SDL: - return qemuBuildGraphicsSDLCommandLine(cfg, cmd, qemuCaps, graphics); + size_t i; - case VIR_DOMAIN_GRAPHICS_TYPE_VNC: - return qemuBuildGraphicsVNCCommandLine(cfg, cmd, qemuCaps, graphics); + for (i = 0; i < def->ngraphics; i++) { + virDomainGraphicsDefPtr graphics = def->graphics[i]; - case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: - return qemuBuildGraphicsSPICECommandLine(cfg, cmd, qemuCaps, graphics); + switch (graphics->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + if (qemuBuildGraphicsSDLCommandLine(cfg, cmd, + qemuCaps, graphics) < 0) + return -1; - case VIR_DOMAIN_GRAPHICS_TYPE_RDP: - case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: - case VIR_DOMAIN_GRAPHICS_TYPE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(graphics->type)); - return -1; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + if (qemuBuildGraphicsVNCCommandLine(cfg, cmd, + qemuCaps, graphics) < 0) + return -1; + + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + if (qemuBuildGraphicsSPICECommandLine(cfg, cmd, + qemuCaps, graphics) < 0) + return -1; + + break; + case VIR_DOMAIN_GRAPHICS_TYPE_RDP: + case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP: + break; + case VIR_DOMAIN_GRAPHICS_TYPE_LAST: + default: + virReportEnumRangeError(virDomainGraphicsType, graphics->type); + return -1; + } } return 0; @@ -10131,11 +10146,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->ngraphics; ++i) { - if (qemuBuildGraphicsCommandLine(cfg, cmd, qemuCaps, - def->graphics[i]) < 0) - goto error; - } + if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps) < 0) + goto error; if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error; -- 2.14.3

This helper will later help us to make corresponding changes when building QEMU cmdline, depending on what implementation of vfio-pci display should be used - dmabuf (requires OpenGL) vs vfio region mapping (doesn't need OpenGL). Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/domain_conf.c | 24 ++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 28 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62bd17739c..c868d8de08 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29927,3 +29927,27 @@ virDomainDefHasManagedPR(const virDomainDef *def) return false; } + + +/** + * virDomainDefHasSpiceGL: + * @def: domain definition + * + * Returns true if we have a SPICE graphic frame buffer with OpenGL enabled, + * false otherwise + */ +bool +virDomainDefHasSpiceGL(const virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->ngraphics; i++) { + virDomainGraphicsDefPtr gfx = def->graphics[i]; + + if (gfx->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + gfx->data.spice.gl == VIR_TRISTATE_BOOL_YES) + return true; + } + + return false; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6cc8f8a29b..8493dfdd76 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3592,4 +3592,7 @@ virDomainDiskGetDetectZeroesMode(virDomainDiskDiscard discard, bool virDomainDefHasManagedPR(const virDomainDef *def); +bool +virDomainDefHasSpiceGL(const virDomainDef *def); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6001635916..b18f2fd5ea 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -278,6 +278,7 @@ virDomainDefHasDeviceAddress; virDomainDefHasManagedPR; virDomainDefHasMemballoon; virDomainDefHasMemoryHotplug; +virDomainDefHasSpiceGL; virDomainDefHasUSB; virDomainDefHasVcpusOffline; virDomainDefLifecycleActionAllowed; -- 2.14.3

QEMU introduced a new type of display for mediated devices using vfio-pci backend which controls whether a mediated device can be used as a native rendering device as an alternative to an emulated video device. This patch adds the necessary bits to domain config handling in order to expose this feature. It's worth noting that even though QEMU supports and defaults to the value 'auto', this patch only introduces values 'on' and 'off' defaulting to 'off' (for safety), since there's no convenient way for libvirt to come up with relevant defaults based on the fact, that libvirt doesn't know whether the underlying device uses dma-buf or vfio region mapping. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- docs/formatdomain.html.in | 16 ++++++-- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 18 +++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 43 +++++++++++++++++++ .../hostdev-mdev-display-missing-graphics.xml | 35 ++++++++++++++++ .../hostdev-mdev-display-spice-no-opengl.xml | 41 ++++++++++++++++++ .../hostdev-mdev-display-spice-opengl.xml | 41 ++++++++++++++++++ .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 39 ++++++++++++++++++ .../hostdev-mdev-display-spice-no-opengl.xml | 47 +++++++++++++++++++++ .../hostdev-mdev-display-spice-opengl.xml | 48 ++++++++++++++++++++++ .../hostdev-mdev-display-vnc.xml | 47 +++++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 13 files changed, 380 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-mdev-display-spice-no-opengl.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-mdev-display-spice-opengl.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-mdev-display-vnc.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b5a6e33bfe..deecc3166a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4437,9 +4437,19 @@ 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.4.0 (QEMU 2.12)</span> libvirt added + a new optional <code>display</code> attribute 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'). + <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 703a1bb6f8..345bd3c757 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4507,6 +4507,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 c868d8de08..7844b6d272 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7606,6 +7606,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; @@ -7625,6 +7626,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 @@ -7712,6 +7714,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) { @@ -26297,9 +26308,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) + 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 8493dfdd76..123a676ab9 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 */ 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 2c51e4c0d8..27088d4456 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4277,10 +4277,35 @@ qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net) } +static int +qemuDomainDeviceDefValidateHostdevMdev(const virDomainHostdevSubsysMediatedDev *mdevsrc, + const virDomainDef *def) +{ + if (mdevsrc->display) { + if (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; + } else if (def->ngraphics == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("graphics device is needed for attribute value " + "'display=on' in <hostdev>")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, const virDomainDef *def) { + const virDomainHostdevSubsysMediatedDev *mdevsrc; + /* forbid capabilities mode hostdev in this kind of hypervisor */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4290,6 +4315,24 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, return -1; } + 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 qemuDomainDeviceDefValidateHostdevMdev(mdevsrc, def); + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: + default: + virReportEnumRangeError(virDomainHostdevSubsysType, + hostdev->source.subsys.type); + return -1; + } + } + return 0; } 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-spice-no-opengl.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.xml new file mode 100644 index 0000000000..64a9d7b70c --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.xml @@ -0,0 +1,41 @@ +<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> + <graphics type='spice'> + <listen type='none'/> + </graphics> + <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/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml new file mode 100644 index 0000000000..b7f2fdf63e --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml @@ -0,0 +1,41 @@ +<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> + <graphics type='spice'> + <gl enable='yes' rendernode='/dev/dri/foo'/> + </graphics> + <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/qemuxml2argvdata/hostdev-mdev-display-vnc.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml new file mode 100644 index 0000000000..f37e08e1b9 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml @@ -0,0 +1,39 @@ +<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> + <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-spice-no-opengl.xml b/tests/qemuxml2xmloutdata/hostdev-mdev-display-spice-no-opengl.xml new file mode 100644 index 0000000000..ce56724629 --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-mdev-display-spice-no-opengl.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='spice'> + <listen type='none'/> + </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-spice-opengl.xml b/tests/qemuxml2xmloutdata/hostdev-mdev-display-spice-opengl.xml new file mode 100644 index 0000000000..6848213a1c --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-mdev-display-spice-opengl.xml @@ -0,0 +1,48 @@ +<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='spice'> + <listen type='none'/> + <gl enable='yes' rendernode='/dev/dri/foo'/> + </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-vnc.xml b/tests/qemuxml2xmloutdata/hostdev-mdev-display-vnc.xml new file mode 100644 index 0000000000..94c11b1199 --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-mdev-display-vnc.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' 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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5671114de0..f9a9967a7c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -477,6 +477,9 @@ mymain(void) DO_TEST("hostdev-pci-address", NONE); DO_TEST("hostdev-vfio", NONE); DO_TEST("hostdev-mdev-precreated", NONE); + DO_TEST("hostdev-mdev-display-spice-opengl", NONE); + DO_TEST("hostdev-mdev-display-spice-no-opengl", NONE); + DO_TEST("hostdev-mdev-display-vnc", NONE); DO_TEST("pci-rom", NONE); DO_TEST("pci-rom-disabled", NONE); DO_TEST("pci-rom-disabled-invalid", NONE); -- 2.14.3

On Mon, Jun 04, 2018 at 07:39:00PM -0400, John Ferlan wrote: QEMU has already realized that and they're trying to change it to 'off' too, see [1]. Noted, I will do that. It doesn't really work reliably even if QEMU has much more info about the device that libvirt could possibly have and there's still the factor that you can't predict user's intention with the device, so 'auto' is not really the smartest default option in this case (given the 3rd party vendors and 2 different approaches), hence the 'off' default for libvirt, 'auto' would be IMHO too tricky and dangerous in terms of guaranteeing backwards compatibility. ... Right, worth mentioning that currently only works for QEMU+vnc/spice combo, thanks. ... I'm sorry, I'm failing to see how it could change, the way I designed it is that whenever QEMU supports the capability we always default to 'off' which means that it'll always get formatted explicitly as 'off', even if the attribute was missing in the source XML. The only problem would an older libvirt who doesn't know what to do with that attribute, so it would ignore it which could cause issues with a newer QEMU without the patch linked below, but the ABI stability check wouldn't help anyway. Right, good point, I preferred validation instead of post parse so as not to risk 'loosing' a domain, but it doesn't make any sense wanting a display and not having any graphical framebuffer, I'll change that. Sure, I'll make the PostParse change. [1] https://www.redhat.com/archives/libvir-list/2018-May/msg02128.html Erik

QEMU 2.12 introduced a new vfio-pci device option 'display=on/off/auto'. Initially, libvirt is only going to support values on/off only, as we don't want to predict what the intended usage of the mediated device will be and most importantly, what kind of vfio display implementation to use - dmabuf vs vfio region mapping as there's no convenient way for us to find out what implementation the device's underlying driver supports. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 6 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e2e76e4dd8..7c08991103 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -493,6 +493,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 305 */ "vhost-vsock", + "vfio-pci.display", ); @@ -1179,6 +1180,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsPCIAssign[] = { static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVfioPCI[] = { { "bootindex", QEMU_CAPS_VFIO_PCI_BOOTINDEX }, + { "display", QEMU_CAPS_VFIO_PCI_DISPLAY }, }; static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsSCSIDisk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f2aecefb9b..07cbe58789 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -477,6 +477,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 305 */ QEMU_CAPS_DEVICE_VHOST_VSOCK, /* -device vhost-vsock-* */ + QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index f511bcb58c..3dba3e3b27 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -167,6 +167,7 @@ <flag name='hda-output'/> <flag name='blockdev-del'/> <flag name='vhost-vsock'/> + <flag name='vfio-pci.display'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>343099</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index 37813ad0b1..13634db45f 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -164,6 +164,7 @@ <flag name='hda-output'/> <flag name='blockdev-del'/> <flag name='vhost-vsock'/> + <flag name='vfio-pci.display'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419968</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 3191e9ba95..5ff96afc86 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -131,6 +131,7 @@ <flag name='screendump_device'/> <flag name='blockdev-del'/> <flag name='vhost-vsock'/> + <flag name='vfio-pci.display'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>371055</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 964b9e8fa6..5aa6b2bc3d 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -206,6 +206,7 @@ <flag name='blockdev-del'/> <flag name='vmgenid'/> <flag name='vhost-vsock'/> + <flag name='vfio-pci.display'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390813</microcodeVersion> -- 2.14.3

If QEMU supports vfio-pci.display option, we default to 'off' as we're not trying to guess what a user's intentions with the mdev are. Perform this decision as part of driver-specific post parse callback. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 27088d4456..b9f152022b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5887,6 +5887,35 @@ qemuDomainVsockDefPostParse(virDomainVsockDefPtr vsock) } +static int +qemuDomainHostdevDefMdevPostParse(virDomainHostdevSubsysMediatedDevPtr mdevsrc, + virQEMUCapsPtr qemuCaps) +{ + /* QEMU 2.12 added support for vfio-pci display type, we default to + * 'display=off' to stay safe from future changes */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY) && + mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT) + mdevsrc->display = VIR_TRISTATE_SWITCH_OFF; + + return 0; +} + + +static int +qemuDomainHostdevDefPostParse(virDomainHostdevDefPtr hostdev, + virQEMUCapsPtr qemuCaps) +{ + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV && + qemuDomainHostdevDefMdevPostParse(&subsys->u.mdev, qemuCaps) < 0) + return -1; + + return 0; +} + + static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, const virDomainDef *def, @@ -5942,6 +5971,9 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_HOSTDEV: + ret = qemuDomainHostdevDefPostParse(dev->data.hostdev, qemuCaps); + break; + case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: -- 2.14.3

Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/qemu/qemu_command.c | 24 +++++++++++++++- .../hostdev-mdev-display-spice-no-opengl.args | 32 ++++++++++++++++++++++ .../hostdev-mdev-display-spice-opengl.args | 31 +++++++++++++++++++++ .../qemuxml2argvdata/hostdev-mdev-display-vnc.args | 32 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 23 ++++++++++++++++ 5 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1667b09a8b..8a1a4dc72b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5088,6 +5088,10 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def, virBufferAdd(&buf, dev_str, -1); virBufferAsprintf(&buf, ",id=%s,sysfsdev=%s", dev->info->alias, mdevPath); + if (mdevsrc->display) + virBufferAsprintf(&buf, ",display=%s", + virTristateSwitchTypeToString(mdevsrc->display)); + if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0) goto cleanup; @@ -5305,7 +5309,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", @@ -5313,6 +5319,15 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, "supported by this version of QEMU")); return -1; } + + if (mdevsrc->display && + !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; + } + break; case VIR_MDEV_MODEL_TYPE_VFIO_CCW: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW)) { @@ -5335,6 +5350,13 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, return -1; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); + + /* we need to add '-display egl-headless' if 'display=on' for + * vfio-pci and OpenGL is disabled (either not supported or user + * forgot to add 'gl=on' to SPICE or simply wants to use VNC...) + */ + if (mdevsrc->display && !virDomainDefHasSpiceGL(def)) + virCommandAddArgList(cmd, "-display", "egl-headless", NULL); } } diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.args b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.args new file mode 100644 index 0000000000..b3fe29cbf4 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-no-opengl.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest2 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest2/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-spice port=0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,\ +addr=0x2 \ +-device vfio-pci,id=hostdev0,\ +sysfsdev=/sys/bus/mdev/devices/53764d0e-85a0-42b4-af5c-2046b460b1dc,display=on,\ +bus=pci.0,addr=0x3 \ +-display egl-headless diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args new file mode 100644 index 0000000000..170b4e4ed0 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest2 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest2/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-spice port=0,gl=on,rendernode=/dev/dri/foo \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,\ +addr=0x2 \ +-device vfio-pci,id=hostdev0,\ +sysfsdev=/sys/bus/mdev/devices/53764d0e-85a0-42b4-af5c-2046b460b1dc,display=on,\ +bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc.args b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc.args new file mode 100644 index 0000000000..126ac8e6a6 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest2 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest2/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-vnc 127.0.0.1:0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,\ +addr=0x2 \ +-device vfio-pci,id=hostdev0,\ +sysfsdev=/sys/bus/mdev/devices/53764d0e-85a0-42b4-af5c-2046b460b1dc,display=on,\ +bus=pci.0,addr=0x3 \ +-display egl-headless diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ddd2b88c0a..9cd92fbfbe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1557,6 +1557,29 @@ mymain(void) QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address", QEMU_CAPS_DEVICE_VFIO_PCI); + DO_TEST("hostdev-mdev-display-spice-opengl", + QEMU_CAPS_SPICE, + QEMU_CAPS_SPICE_GL, + QEMU_CAPS_SPICE_RENDERNODE, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_VFIO_PCI_DISPLAY); + DO_TEST("hostdev-mdev-display-spice-no-opengl", + QEMU_CAPS_SPICE, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_VFIO_PCI_DISPLAY); + DO_TEST("hostdev-mdev-display-vnc", + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_VFIO_PCI_DISPLAY); + DO_TEST_PARSE_ERROR("hostdev-mdev-display-missing-graphics", + QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_VFIO_PCI_DISPLAY); DO_TEST("pci-rom", NONE); DO_TEST("pci-rom-disabled", NONE); DO_TEST("pci-rom-disabled-invalid", NONE); -- 2.14.3

On Mon, Jun 04, 2018 at 07:40:42PM -0400, John Ferlan wrote: So, w/r/t comments in patch 6, I understand that we'd keep an open door to changing the default (no display attr means use 'auto' as it would in most cases in libvirt). However, I don't believe that we'd want to make 'auto' a default ever because you can't predict whether the user wants to use the device as GPGPU or graphics, if they leave the display attr unset and we'd default to auto in the future, we might risk that they know they can't use it for graphics and they didn't even intend to, but we tried and failed because of some underlying reasons. And I'm not even sure we'd ever introduce 'auto' to libvirt for reasons I just mentioned + the cover letter and the discussion that happened upstream regarding the VNC console. What I know though, is that libvirt is never going to use QEMU's 'auto', since it doesn't make sense, either we take care of everything or the user does, regardless of QEMU. Anyway, the way I imagine 'auto' could work in libvirt is that user references an mdev, but doesn't care (or doesn't know) what technology it uses underneath and explicitly says "libvirt please take care of that on my behalf, I want to use the display, but have 0 clue on what to enable". That's a different kind of 'auto' compared to when we'd try to read user's mind. I tried to play it very safe here, as the vgpu display thing can be very tricky, not even thinking about future migrations. But if there's a strong disagreement with this kind of approach, then I'll of course need to incorporate your comments in patch 6 and this one to get this successfully merged. Erik Will fix this. Thanks, Erik

On Fri, Jun 08, 2018 at 12:00:50PM -0400, John Ferlan wrote: Alright, we could skip the "offline" XML that's a fair argument, but we should format something into the "live" one, as that one serves as the base for the "migration" XML, I'd rather us not sending a missing display argument to a newer libvirt on the destination for safety reasons, I believe that turning something 'off' rather than 'on' by default is good. Erik