[libvirt] [PATCH v4 09/10] qemu: command: Enable formatting vfio-pci.display option onto cmdline

[Erik Skultety]

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 at 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 at redhat.com>
>

Thanks, I adjusted the commit message and will proceed with pushing the series.

Erik