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

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.

> + } else { > + /* we default to 'display=off', since QEMU defaults to 'auto' which is > + * unreliable and we don't want to risk any breakages */ > + if (mdevsrc->display == VIR_TRISTATE_SWITCH_ABSENT) > + mdevsrc->display = VIR_TRISTATE_SWITCH_OFF; > + } > + > + if (mdevsrc->display != VIR_TRISTATE_SWITCH_ABSENT) > + virBufferAsprintf(&buf, ",display=%s", > + virTristateSwitchTypeToString(mdevsrc->display)); > + > if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0) > goto cleanup; > > @@ -5424,7 +5444,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", > @@ -5432,6 +5454,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, > "supported by this version of QEMU")); > return -1; > } > + > break; > case VIR_MDEV_MODEL_TYPE_VFIO_CCW: > if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW)) { These two hunks are unrelated. Feel free to push them as trivial in a separate commit.

> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 6014c81802..a0ab824038 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -1590,6 +1590,37 @@ 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-egl-headless", > + QEMU_CAPS_SPICE, > + QEMU_CAPS_EGL_HEADLESS, > + 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("hostdev-mdev-display-vnc-egl-headless", > + QEMU_CAPS_VNC, > + QEMU_CAPS_EGL_HEADLESS, > + QEMU_CAPS_DEVICE_QXL, > + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, > + QEMU_CAPS_DEVICE_VFIO_PCI, > + QEMU_CAPS_VFIO_PCI_DISPLAY); Can you use DO_TEST_CAPS_LATEST for these tests?

> + 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); Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano