[libvirt] [PATCH v2 4/5] qemu: Add QEMU_CAPS_SDL_GL to qemu capabilities

Martin Kletzander mkletzan at redhat.com
Fri May 11 11:47:29 UTC 2018


On Fri, May 11, 2018 at 06:34:54AM -0400, John Ferlan wrote:
>On 05/11/2018 04:26 AM, Martin Kletzander wrote:
>> On Thu, May 10, 2018 at 04:17:52PM -0400, John Ferlan wrote:
>>>
>>>
>>> On 05/10/2018 06:53 AM, Maciej Wolny wrote:
>>>> Support OpenGL acceleration capability when using SDL graphics.
>>>>
>>>> Signed-off-by: Maciej Wolny <maciej.wolny at codethink.co.uk>
>>>> ---
>>>>  src/qemu/qemu_capabilities.c                         | 2 ++
>>>>  src/qemu/qemu_capabilities.h                         | 1 +
>>>>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies | 9 +++++++++
>>>>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml     | 3 ++-
>>>>  4 files changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>
>>> As I rather lengthily noted in the v1 - I'm assuming you handed
>>> edited the .replies file.  What that perhaps works and gets you
>>> the answer, the fact that none of other files were adjusted leads
>>> me to the hand editing belief.
>>>
>>>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>>>> index 920a59617..23f917b66 100644
>>>> --- a/src/qemu/qemu_capabilities.c
>>>> +++ b/src/qemu/qemu_capabilities.c
>>>> @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>>>>                "disk-write-cache",
>>>>                "nbd-tls",
>>>>                "tpm-crb",
>>>> +              "sdl-gl",
>>>>      );
>>>>
>>>>
>>>> @@ -2456,6 +2457,7 @@ static struct virQEMUCapsCommandLineProps
>>>> virQEMUCapsCommandLine[] = {
>>>>      { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS },
>>>>      { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT },
>>>>      { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST },
>>>> +    { "sdl", "gl", QEMU_CAPS_SDL_GL },
>>>>  };
>>>
>>> Rather than this, I'll apply the following diff:
>>>
>>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>>> index 23f917b66e..28079fa7ab 100644
>>> --- a/src/qemu/qemu_capabilities.c
>>> +++ b/src/qemu/qemu_capabilities.c
>>> @@ -2457,7 +2457,6 @@ static struct virQEMUCapsCommandLineProps
>>> virQEMUCapsCommandLine[] = {
>>>     { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS },
>>>     { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT },
>>>     { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST },
>>> -    { "sdl", "gl", QEMU_CAPS_SDL_GL },
>>> };
>>>
>>> static int
>>> @@ -3828,6 +3827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
>>>     if (qemuCaps->version >= 2004000)
>>>         virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT);
>>>
>>> +    /* sdl -gl option is supported from v2.4.0 (qemu commit id
>>> 0b71a5d5) */
>>> +    if (qemuCaps->version >= 2004000)
>>> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_SDL_GL);
>>> +
>>>     /* Since 2.4.50 ARM virt machine supports gic-version option */
>>>     if (qemuCaps->version >= 2004050)
>>>         virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACH_VIRT_GIC_VERSION);
>>>
>>>
>>
>> NO! Why? We only result to setting capabilities by version if there is
>> no other
>> way to do that.  If query-commandline-options (or whatever that name is)
>> works
>> for this capability, why would you even consider this change?
>>
>> NACK to the diff you added.
>
>Are you sure? Did you read my responses from v1:
>
>https://www.redhat.com/archives/libvir-list/2018-May/msg00250.html
>https://www.redhat.com/archives/libvir-list/2018-May/msg00671.html
>
>I actually spent some time trying to figure out which magic incantation
>of the virQEMUCaps* would work. I even tried various forms in
>virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from
>qemu 2.4 and beyond.  Again, my "assumption" is that it was hand edited
>into the 2.4 replies that was added here mainly because none of the
>other available sdl * options were addressed - just the one that was wanted.
>
>Looking at the QEMU source code - AFAICT the gl option is only "known"
>or handled within parse_options of vl.c.  I found nothing in the .json
>files where I'd usually find some mechanism that would allow
>introspection for the "-sdl gl" option. There is something in
>qapi/ui.json that has 'gl' that looks like it could be used in some new
>command syntax, but that seems to imply QEMU 2.12 only.
>
>Please enlighten me/us with code that will work before just pushing the
>NACK button.
>

I'm so sorry for that.  I totally missed the part that the reply was
hand-edited.  In that case, unfortunately, the only way is to use the version
_again_.  So my apologies once again, especially if that sounded rude (which it
does to me now when I'm reading after myself).  I am adding a capability
currently and I'm dealing with part of code that I reworked few times due to a
similar misunderstanding from a previous developer.

So feel free to squash that in if Maciej is OK with it as well (which he should
be, otherwise it won't work for anyone anyway).  ACK.

>Tks,
>
>John
-------------- 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/20180511/47dc0eec/attachment-0001.sig>


More information about the libvir-list mailing list