On Wed, Jul 18, 2018 at 05:15:28PM +0200, Ján Tomko wrote:
On Wed, Jul 18, 2018 at 02:31:45PM +0200, Erik Skultety wrote:
> Since QEMU 2.12, QEMU understands a new vfio-pci device option 'display'
> which can be used to turn on display capabilities on vgpu-enabled
> mediated devices, IOW emulated GPU devices like QXL will no longer be
> needed with vgpu-enable mdevs.
> QEMU defaults to 'auto' for the 'display' attribute, which is not
> foolproof, so we need to play it safe here and explicitly format
> display='off' if this attribute wasn't provided in the XML explicitly.
Outdated comment, now that we set it in postParse.
>
> Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 4 +++
> .../hostdev-mdev-display-missing-graphics.xml | 35 ++++++++++++++++++
> ...v-display-spice-egl-headless.x86_64-latest.args | 37 +++++++++++++++++++
> .../hostdev-mdev-display-spice-egl-headless.xml | 40 +++++++++++++++++++++
> ...ev-mdev-display-spice-opengl.x86_64-latest.args | 36 +++++++++++++++++++
> .../hostdev-mdev-display-spice-opengl.xml | 41 ++++++++++++++++++++++
> ...dev-display-vnc-egl-headless.x86_64-latest.args | 37 +++++++++++++++++++
> .../hostdev-mdev-display-vnc-egl-headless.xml | 40 +++++++++++++++++++++
> .../hostdev-mdev-display-vnc.x86_64-latest.args | 36 +++++++++++++++++++
> .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 39 ++++++++++++++++++++
> tests/qemuxml2argvtest.c | 7 ++++
> 11 files changed, 352 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml
> create mode 100644
tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args
> create mode 100644
tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml
> create mode 100644
tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml
> create mode 100644
tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args
> create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml
> create mode 100644
tests/qemuxml2argvdata/hostdev-mdev-display-vnc.x86_64-latest.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 7c44045d61..c61da13893 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5207,6 +5207,10 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def,
> virBufferAdd(&buf, dev_str, -1);
> virBufferAsprintf(&buf, ",id=%s,sysfsdev=%s",
dev->info->alias, mdevPath);
>
> + if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT)
To play it completely safe, we could add
&& qemuCapsGet(QEMU_CAPS_VFIO_PCI_DISPLAY)
here, and allow having it set to VIR_TRISTATE_SWITCH_OFF in qemuDomainMdevDefValidate.
So, we (or the user for that matter) put display='off' to the XML as result of
the QEMU capability. If the user downgrades the version of QEMU to a version
which doesn't provide the capability and feeds in the same XML, how is that a
libvirt issue that we fail to start it? Yes, we can't crash or loose the
definition, but IMHO that is the expected behaviour, you want something the
emulator doesn't know...
But I'm not sure whether it's worth the trouble.
Regardless
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Thanks, I adjusted the commit message and will proceed with pushing the series.
Erik