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

[John Ferlan]

On 05/14/2018 09:18 AM, Maciej Wolny wrote:
> 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 at codethink.co.uk>
>>>>>>> ---
>>>>>>> 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?
> 

My opinion is leave it as is - hard to require you to do something we're
not requiring other patches at this point. I'm not a fan of duplication
- that is the task would be to essentially copy everything into the
generic xml2xml test at this point.

John