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

[Ján Tomko]

On Mon, Jul 16, 2018 at 03:51:40PM +0200, Erik Skultety wrote:
>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,

By putting it in postParse you get the benefit of preserving this
default for future starts (if we ever can and decide to change the
default) and more importantly it gets displayed in the domain XML.

>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.

You can create a domain with pretty minimal XML and we will fill in all
the sensible defaults, there's no reason to treat this attribute
differently.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180717/67b830a3/attachment-0001.sig>