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

2018-05-10 Thread Maciej Wolny 
Support OpenGL acceleration capability when using SDL graphics.

Signed-off-by: Maciej Wolny 
---
 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(-)

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 },
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index abd6eff14..01d6be818 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache 
param */
 QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */
 QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */
+QEMU_CAPS_SDL_GL, /* -sdl gl */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
index 68ecb0c17..5614fc63c 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
@@ -3575,6 +3575,15 @@
 },
 {
   "parameters": [
+{
+  "name": "gl",
+  "type": "boolean"
+}
+  ],
+  "option": "sdl"
+},
+{
+  "parameters": [
   ],
   "option": "acpi"
 },
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index f3834d5bc..3a968542b 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -153,9 +153,10 @@
   
   
   
+  
   2004000
   0
-  75406
+  75544
   
   x86_64
   
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-10 Thread John Ferlan 


On 05/10/2018 06:53 AM, Maciej Wolny wrote:
> Support OpenGL acceleration capability when using SDL graphics.
> 
> Signed-off-by: Maciej Wolny 
> ---
>  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);


>  
>  static int
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index abd6eff14..01d6be818 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
> syntax-check */
>  QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache 
> param */
>  QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */
>  QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */
> +QEMU_CAPS_SDL_GL, /* -sdl gl */
>  
>  QEMU_CAPS_LAST /* this must always be the last item */
>  } virQEMUCapsFlags;
> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies 
> b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
> index 68ecb0c17..5614fc63c 100644
> --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
> +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
> @@ -3575,6 +3575,15 @@
>  },
>  {
>"parameters": [
> +{
> +  "name": "gl",
> +  "type": "boolean"
> +}
> +  ],
> +  "option": "sdl"
> +},
> +{
> +  "parameters": [
>],
>"option": "acpi"
>  },

I'll remove this hand edit...

> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml 
> b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
> index f3834d5bc..3a968542b 100644
> --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
> @@ -153,9 +153,10 @@
>
>
>
> +  
>2004000
>0
> -  75406
> +  75544

This hunk would be reverted

>
>x86_64
>
> 

and run VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest to generate
a bunch of caps_2*.*.xml files across all the arches.

I've made the adjustments, but just need to make sure you're OK with that and
that no one else has some sort of agita over the solution or some other way
to try and get the answer without changing virQEMUCapsInitQMPMonitor.

Reviewed-by: John Ferlan 

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-11 Thread Martin Kletzander 

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

signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-11 Thread John Ferlan 


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

Tks,

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-11 Thread Martin Kletzander 

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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-11 Thread John Ferlan 


On 05/11/2018 07:47 AM, Martin Kletzander wrote:
> 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 
> ---
>  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.
> 

We're good - don't worry - but thanks for the note. I certainly get

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

2018-05-11 Thread Maciej Wolny 
On 11/05/18 11:34, John Ferlan wrote:> 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.

It was hand-edited. I'm still not sure how this work.. I ran the tool to
generate a .replies file; but based on that, how do I get the replies for
all historical version of QEMU, do I need to have each of the versions of
QEMU installed on my system.

-- milloni

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-11 Thread Maciej Wolny 


On 10/05/18 21:17, 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 
>> ---
>>  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);
> 
> 
>>  
>>  static int
>> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>> index abd6eff14..01d6be818 100644
>> --- a/src/qemu/qemu_capabilities.h
>> +++ b/src/qemu/qemu_capabilities.h
>> @@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
>> syntax-check */
>>  QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache 
>> param */
>>  QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */
>>  QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */
>> +QEMU_CAPS_SDL_GL, /* -sdl gl */
>>  
>>  QEMU_CAPS_LAST /* this must always be the last item */
>>  } virQEMUCapsFlags;
>> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies 
>> b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
>> index 68ecb0c17..5614fc63c 100644
>> --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
>> +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.replies
>> @@ -3575,6 +3575,15 @@
>>  },
>>  {
>>"parameters": [
>> +{
>> +  "name": "gl",
>> +  "type": "boolean"
>> +}
>> +  ],
>> +  "option": "sdl"
>> +},
>> +{
>> +  "parameters": [
>>],
>>"option": "acpi"
>>  },
> 
> I'll remove this hand edit...
> 
>> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml 
>> b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
>> index f3834d5bc..3a968542b 100644
>> --- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
>> +++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
>> @@ -153,9 +153,10 @@
>>
>>
>>
>> +  
>>2004000
>>0
>> -  75406
>> +  75544
> 
> This hunk would be reverted
> 
>>
>>x86_64
>>
>>
> 
> and run VIR_TEST_REGENERATE_OUTPUT=1 tests/qemucapabilitiestest to generate
> a bunch of caps_2*.*.xml files across all the arches.
> 
> I've made the adjustments, but just need to make sure you're OK with that and
> that no one else has some sort of agita over the solution or some other way
> to try and get the answer without changing virQEMUCapsInitQMPMonitor.

I'm OK with that.

> 
> Reviewed-by: John Ferlan 
> 
> John
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-05-14 Thread Martin Kletzander 

On Fri, May 11, 2018 at 03:14:26PM +0100, Maciej Wolny wrote:

On 11/05/18 11:34, John Ferlan wrote:> 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.


It was hand-edited. I'm still not sure how this work.. I ran the tool to
generate a .replies file; but based on that, how do I get the replies for
all historical version of QEMU, do I need to have each of the versions of
QEMU installed on my system.



Unfortunately, yes.  There was some work done so taht we could automate that,
but nobody actually finished the work.  Anyway, since QEMU does not expose that
capability in the QMP communication (what libvirt uses to check for the
support), we need to guess it based on the release version :(


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list