[libvirt] [PATCH v3 7/8] qemu: command: Enable formatting vfio-pci.display option onto cmdline
Erik Skultety
eskultet at redhat.com
Mon Jul 16 13:51:40 UTC 2018
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 at redhat.com>
> > ---
> > 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.
>
> > + } 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.
Will do.
>
> > 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?
Oh, I didn't know we had that, sure, I can go with that except for the first
test which needs _SPICE_GL, however, that is only filled after scanning the
command line options, so there is no such capability in the data set.
Erik
>
> > + 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 at redhat.com>
>
> Jano
More information about the libvir-list
mailing list