Re: [libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config

On 14/05/18 13:40, Martin Kletzander wrote: > On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: >> >> >> On 05/14/2018 07:24 AM, Martin Kletzander wrote: >>> On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >>>> On 11/05/18 09:42, Martin Kletzander wrote: >>>>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>>>>> Support OpenGL accelerated rendering when using SDL graphics in the >>>>>> domain config. Add associated test and documentation. >>>>>> >>>>>> Signed-off-by: Maciej Wolny <maciej.wolny(a)codethink.co.uk&gt; >>>>>> --- >>>>>> docs/formatdomain.html.in                          |  6 +++ >>>>>> docs/schemas/domaincommon.rng                      |  8 ++++ >>>>>> src/conf/domain_conf.c                             | 44 >>>>>> ++++++++++++++++++++- >>>>>> src/conf/domain_conf.h                             |  1 + >>>>> >>>>> docs, conf and schemas fit together nicely, they should be in one >>>>> patch, but. >>>>> >>>>>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>>>>> ++++++++++++++++++ >>>>>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>>>>> ++++++++++++++++++++++ >>>>>> tests/qemuxml2xmltest.c                            |  1 + >>>>> >>>>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>>>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>>>> >>>>> For the XML tests above you can use genericxml2xmltest instead of the >>>>> QEMU-specific one. >>>> >>>> The option only makes sense in QEMU afaik, hence the naming. >>>> >>> >>> Yes, for now.  If someone who's building the code without QEMU driver >>> changes >>> the behaviour, the tests will pass if you keep it in qemuxml2xml, however >>> genericxml2xml will catch that.  qemuxml2xml should be testing specifics >>> where >>> you behave based on some more information than just generic XML. >>> >>> I hope that's clear. >>> >>> Have a nice day. >>> >> >> However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >> then you'd have separate xml input and output files - is that what's >> desired? >> >> Taking a quick look just now - see the graphics-vnc-socket - do we want >> to duplicate having two input/output XML files which invariably will >> diverge? Ironically the generic one has a domain type == qemu, an >> emulator using qemu, and the socket path using QEMU - so while it's >> generic in one sense, it's not in others. Even more ironic is the qemu >> specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". >> >> Could/should generification of the xml2xml tests be considered a "bite >> sized task"? >> > > Oh, definitely.  It's only some time ago that the tests started to be usable > IIRC, so hopefully we'll migrate some XMLs here and there.  But maybe others > could chime in as well so that I don't speak for others.  I remember Pavel > having some ideas for cleaner separation of those. So, do you guys want to leave that for a separate patch set or do you want me to post a v3 with the changes Martin has requested?