Re: [libvirt] [PATCH v3 7/8] qemu: command: Enable formatting vfio-pci.display option onto cmdline

On Fri, Jul 13, 2018 at 03:06:05PM +0200, Ján Tomko wrote: > On Wed, Jul 11, 2018 at 03:58:27PM +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. > > > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; > > --- > > src/qemu/qemu_command.c | 25 ++++++++++++- > > .../hostdev-mdev-display-missing-graphics.xml | 35 ++++++++++++++++++ > > .../hostdev-mdev-display-spice-egl-headless.args | 32 +++++++++++++++++ > > .../hostdev-mdev-display-spice-egl-headless.xml | 40 +++++++++++++++++++++ > > .../hostdev-mdev-display-spice-opengl.args | 31 ++++++++++++++++ > > .../hostdev-mdev-display-spice-opengl.xml | 41 ++++++++++++++++++++++ > > .../hostdev-mdev-display-vnc-egl-headless.args | 32 +++++++++++++++++ > > .../hostdev-mdev-display-vnc-egl-headless.xml | 40 +++++++++++++++++++++ > > .../qemuxml2argvdata/hostdev-mdev-display-vnc.args | 31 ++++++++++++++++ > > .../qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 39 ++++++++++++++++++++ > > tests/qemuxml2argvtest.c | 31 ++++++++++++++++ > > 11 files changed, 376 insertions(+), 1 deletion(-) > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.args > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.args > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.args > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml > > create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-display-vnc.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 48e224cabc..5f6b340f8f 100644 > > --- a/src/qemu/qemu_command.c > > +++ b/src/qemu/qemu_command.c > > @@ -5207,6 +5207,26 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def, > > virBufferAdd(&buf, dev_str, -1); > > virBufferAsprintf(&buf, ",id=%s,sysfsdev=%s", dev->info->alias, mdevPath); > > > > + /* QEMU 2.12 added support for vfio-pci display type, we need to perform > > + * some additional checks here */ > > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_DISPLAY)) { > > + if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT) { > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > > + _("display property of device vfio-pci is " > > + "not supported by this version of QEMU")); > > + goto cleanup; > > + } > > qemuCaps checks belong to *Validate functions. Well, we still need to come up with the default value, but only in case QEMU has the cap and we already agreed upon not doing this in postparse,

otherwise we'd format something into the offline config even if there was no input from the user, so I'd like to keep it this way, otherwise the capability check would have to be at 2 places instead of 1.