Re: [Qemu-devel] [PATCH] vfio/pci: Default display option to "off"

[Erik Skultety <eskultet@redhat.com>]

On Tue, May 29, 2018 at 09:24:08AM -0600, Alex Williamson wrote:
> [Cc +Erik,libvirt]
>
> Sorry, should have cc'd libvirt with this initially since display
> support is under development.  I think "off" is the better
> compatibility option, but perhaps the damage is done since it was the
> 2.12 default.  Thanks,

Thanks for CC'ing me. So, the approach I took with the initial design to be
sent as an RFC is to default to display=off in libvirt. My implementation is
lacking support for display=auto, solely because the way libvirt would need to
get all the information about devices was cumbersome - whether it uses dmabuf
or vfio regions ergo not guessing whether a missing gl=on signals dmabuf or it
was just a mistake. Therefore, we're going to default to use egl-headless unless
explicitly asked to turn on OpenGL. Back to the 'auto' value, it would also mean
for libvirt to make policy decisions which is not what libvirt's supposed to do
plus in this case it's difficult for any layer to guess what user's intention
with the device was, whether it was supposed to be used for graphics or not.
We can add support for 'auto' once there are compelling reasons for us to do so
and the overall consensus is in favour of implementing that, however, we
wouldn't change the display=off default anyway, since it's much safer for us.

Erik

>
> Alex
>
> On Tue, 29 May 2018 09:18:10 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
> > Commit a9994687cb9b ("vfio/display: core & wireup") added display
> > support to vfio-pci with the default being "auto", which breaks
> > existing VMs when the vGPU requires GL support but had no previous
> > requirement for a GL compatible configuration.  "Off" is the safer
> > default as we impose no new requirements to VM configurations.
> >
> > Fixes: a9994687cb9b ("vfio/display: core & wireup")
> > Cc: qemu-stable@nongnu.org
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  hw/vfio/pci.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> > index 84e27c7bb2d1..18c493b49ec1 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -3160,7 +3160,7 @@ static Property vfio_pci_dev_properties[] = {
> >      DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
> >      DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev),
> >      DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
> > -                            display, ON_OFF_AUTO_AUTO),
> > +                            display, ON_OFF_AUTO_OFF),
> >      DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice,
> >                         intx.mmap_timeout, 1100),
> >      DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
> >
>