All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
@ 2014-04-19 12:16 Fabio Fantoni
  2014-04-22 13:11 ` Ian Jackson
  2014-05-02 11:41 ` Ian Campbell
  0 siblings, 2 replies; 20+ messages in thread
From: Fabio Fantoni @ 2014-04-19 12:16 UTC (permalink / raw)
  To: xen-devel
  Cc: anthony.perard, Fabio Fantoni, Ian.Jackson, Ian.Campbell,
	Stefano.Stabellini

Reading one qemu-devel post seems that setting video memory of
cirrus vga with upstream qemu is wrong even if not show errors.

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>

---

Note:
- when this patch will be accepted upstream should be
  backported also on xen 4.4
- with this qemu parameters seems correct but for further
  confirmation I posted a question about it:
http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
---
 tools/libxl/libxl_dm.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8abed7b..90f19b7 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -508,9 +508,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
             flexarray_append_pair(dm_args, "-device", "VGA");
             break;
         case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
-            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
-            flexarray_append_pair(dm_args, "-global",
-                GCSPRINTF("vga.vram_size_mb=%d",
+            flexarray_append_pair(dm_args, "-device",
+                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
                 libxl__sizekb_to_mb(b_info->video_memkb)));
             break;
         case LIBXL_VGA_INTERFACE_TYPE_NONE:
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-04-19 12:16 [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu Fabio Fantoni
@ 2014-04-22 13:11 ` Ian Jackson
  2014-04-22 13:21   ` Fabio Fantoni
  2014-05-02 11:41 ` Ian Campbell
  1 sibling, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2014-04-22 13:11 UTC (permalink / raw)
  To: Fabio Fantoni; +Cc: anthony.perard, xen-devel, Ian.Campbell, Stefano.Stabellini

Fabio Fantoni writes ("[PATCH] libxl: fix cirrus vga video memory setting with upstream qemu"):
> Reading one qemu-devel post seems that setting video memory of
> cirrus vga with upstream qemu is wrong even if not show errors.

Can you point us to the qemu-devel post ?

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-04-22 13:11 ` Ian Jackson
@ 2014-04-22 13:21   ` Fabio Fantoni
  2014-04-24 10:33     ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-04-22 13:21 UTC (permalink / raw)
  To: Ian Jackson; +Cc: anthony.perard, xen-devel, Ian.Campbell, Stefano.Stabellini

Il 22/04/2014 15:11, Ian Jackson ha scritto:
> Fabio Fantoni writes ("[PATCH] libxl: fix cirrus vga video memory setting with upstream qemu"):
>> Reading one qemu-devel post seems that setting video memory of
>> cirrus vga with upstream qemu is wrong even if not show errors.
> Can you point us to the qemu-devel post ?
Is a topic about qemu patch posted on qemu-devel and xen-devel, this is 
the post of one qemu developer that reply to my first aswer:
http://lists.xen.org/archives/html/xen-devel/2014-04/msg02564.html
I also posted for confirmation about qemu parameters of this patch and 
I'm wating for reply:
http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
>
> Thanks,
> Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-04-22 13:21   ` Fabio Fantoni
@ 2014-04-24 10:33     ` Fabio Fantoni
  2014-05-02  9:11       ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-04-24 10:33 UTC (permalink / raw)
  To: Ian Jackson; +Cc: anthony.perard, xen-devel, Ian.Campbell, Stefano.Stabellini

Il 22/04/2014 15:21, Fabio Fantoni ha scritto:
> Il 22/04/2014 15:11, Ian Jackson ha scritto:
>> Fabio Fantoni writes ("[PATCH] libxl: fix cirrus vga video memory 
>> setting with upstream qemu"):
>>> Reading one qemu-devel post seems that setting video memory of
>>> cirrus vga with upstream qemu is wrong even if not show errors.
>> Can you point us to the qemu-devel post ?
> Is a topic about qemu patch posted on qemu-devel and xen-devel, this 
> is the post of one qemu developer that reply to my first aswer:
> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02564.html
> I also posted for confirmation about qemu parameters of this patch and 
> I'm wating for reply:
> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
>>
>> Thanks,
>> Ian.
>
I tried now to force videoram increase to with qemu parameters and 
without increase xen videoram and give xen memory error, then the 
parameter of this patch looks correct.
I'll post also the patch to make thevideoram settable with stdvga, now 
missing with upstream qemu.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-04-24 10:33     ` Fabio Fantoni
@ 2014-05-02  9:11       ` Fabio Fantoni
  0 siblings, 0 replies; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-02  9:11 UTC (permalink / raw)
  To: Ian Jackson; +Cc: anthony.perard, xen-devel, Ian.Campbell, Stefano.Stabellini

Il 24/04/2014 12:33, Fabio Fantoni ha scritto:
> Il 22/04/2014 15:21, Fabio Fantoni ha scritto:
>> Il 22/04/2014 15:11, Ian Jackson ha scritto:
>>> Fabio Fantoni writes ("[PATCH] libxl: fix cirrus vga video memory 
>>> setting with upstream qemu"):
>>>> Reading one qemu-devel post seems that setting video memory of
>>>> cirrus vga with upstream qemu is wrong even if not show errors.
>>> Can you point us to the qemu-devel post ?
>> Is a topic about qemu patch posted on qemu-devel and xen-devel, this 
>> is the post of one qemu developer that reply to my first aswer:
>> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02564.html
>> I also posted for confirmation about qemu parameters of this patch 
>> and I'm wating for reply:
>> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
>>>
>>> Thanks,
>>> Ian.
>>
> I tried now to force videoram increase to with qemu parameters and 
> without increase xen videoram and give xen memory error, then the 
> parameter of this patch looks correct.
> I'll post also the patch to make thevideoram settable with stdvga, now 
> missing with upstream qemu.
Ping

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-04-19 12:16 [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu Fabio Fantoni
  2014-04-22 13:11 ` Ian Jackson
@ 2014-05-02 11:41 ` Ian Campbell
  2014-05-02 19:44   ` Don Slutz
  1 sibling, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-05-02 11:41 UTC (permalink / raw)
  To: Fabio Fantoni; +Cc: anthony.perard, xen-devel, Ian.Jackson, Stefano.Stabellini

On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:
> Reading one qemu-devel post seems that setting video memory of
> cirrus vga with upstream qemu is wrong even if not show errors.

You later provided links but I think the conversation should be
referenced here.

Is this change correct for all versions of mainline qemu which people
might be using with Xen?

Please can you also explain what "wrong" means? Does it crash? Does it
silently ignore the setting? Does it do something else "wrong"? How bad
is it?

> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> 
> ---
> 
> Note:
> - when this patch will be accepted upstream should be
>   backported also on xen 4.4

I think this very much depends on what you mean by "wrong" above.

> - with this qemu parameters seems correct but for further
>   confirmation I posted a question about it:
> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html

Any reply to this question?

Anthony, Stefano: Opinions on this change?

> ---
>  tools/libxl/libxl_dm.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8abed7b..90f19b7 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -508,9 +508,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>              flexarray_append_pair(dm_args, "-device", "VGA");
>              break;
>          case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> -            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
> -            flexarray_append_pair(dm_args, "-global",
> -                GCSPRINTF("vga.vram_size_mb=%d",
> +            flexarray_append_pair(dm_args, "-device",
> +                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
>                  libxl__sizekb_to_mb(b_info->video_memkb)));
>              break;
>          case LIBXL_VGA_INTERFACE_TYPE_NONE:

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-02 11:41 ` Ian Campbell
@ 2014-05-02 19:44   ` Don Slutz
  2014-05-02 20:04     ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Don Slutz @ 2014-05-02 19:44 UTC (permalink / raw)
  To: Ian Campbell, Fabio Fantoni
  Cc: anthony.perard, xen-devel, Ian.Jackson, Stefano.Stabellini

On 05/02/14 07:41, Ian Campbell wrote:
> On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:
>> Reading one qemu-devel post seems that setting video memory of
>> cirrus vga with upstream qemu is wrong even if not show errors.

Fabio,
   You can add my:

Reviewed-by: Don Slutz <dslutz@verizon.com>

I have a similar code change locally (part of my pending
list of to dos) (I just changed the global arg...).

> You later provided links but I think the conversation should be
> referenced here.

I was part of the conversation.  When I was looking into upstreaming
a change I have (pci_min_hole) to xen & qemu, I was asked by QEMU
to report if it was not used (i.e. non x86 cpu's).  While I was testing my
change to QEMU under xen I noticed:

Warning: "-global vga.vram_size_mb=16" not used


In /var/log/xen/qemu-dm-<guest>.log

Here is the cross post to xen-devel:


  [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused -global


http://lists.xen.org/archives/html/xen-devel/2014-03/msg03128.html



> Is this change correct for all versions of mainline qemu which people
> might be using with Xen?

What I know is that "-global cirrus-vga.vgamem_mb=32" does work
with upstream QEMU 1.5.0, 1.6.0, 1.7.0 and 2.0.0.  I had added a debug
output into the QEMU version above that show the amount of video ram
that gets allocated and so the testing was quick and easy.  My
understanding of QEMU is that when specified this way:

  "-device cirrus-vga,vgamem_mb=32"

the error checking in QEMU will report when it does not like it:

qemu-system-x86_64: Property '.vram_size_mb' not found


(unlike -global).  So while I have not tested it, I would guess that
any QEMU that accepts "-device cirrus-vga" will also either
accept the change or report and error and not start. (Note: the
change from "-vga cirrus" to "-device cirrus-vga" was done since
4.3.0 and has yet to generate a bug).

> Please can you also explain what "wrong" means? Does it crash? Does it
> silently ignore the setting? Does it do something else "wrong"? How bad
> is it?

For me it just silently ignores the setting.  Which means that the
cirrus vga has the default memory and so will not support any
display setting that needs more memory (the most likely reason
videoram=32 is specified in the config file).

I feel it is bad enough that a backport to 4.4 makes sense to
me.

Hope this helps.
     -Don Slutz

>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>
>> ---
>>
>> Note:
>> - when this patch will be accepted upstream should be
>>    backported also on xen 4.4
> I think this very much depends on what you mean by "wrong" above.
>
>> - with this qemu parameters seems correct but for further
>>    confirmation I posted a question about it:
>> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
> Any reply to this question?
>
> Anthony, Stefano: Opinions on this change?
>
>> ---
>>   tools/libxl/libxl_dm.c |    5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 8abed7b..90f19b7 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -508,9 +508,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>               flexarray_append_pair(dm_args, "-device", "VGA");
>>               break;
>>           case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>> -            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>> -            flexarray_append_pair(dm_args, "-global",
>> -                GCSPRINTF("vga.vram_size_mb=%d",
>> +            flexarray_append_pair(dm_args, "-device",
>> +                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
>>                   libxl__sizekb_to_mb(b_info->video_memkb)));
>>               break;
>>           case LIBXL_VGA_INTERFACE_TYPE_NONE:
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-02 19:44   ` Don Slutz
@ 2014-05-02 20:04     ` Fabio Fantoni
  2014-05-07 12:20       ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-02 20:04 UTC (permalink / raw)
  To: Don Slutz
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Ian Campbell, Stefano Stabellini


[-- Attachment #1.1: Type: text/plain, Size: 4655 bytes --]

2014-05-02 21:44 GMT+02:00 Don Slutz <dslutz@verizon.com>:

> On 05/02/14 07:41, Ian Campbell wrote:
>
>> On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:
>>
>>> Reading one qemu-devel post seems that setting video memory of
>>> cirrus vga with upstream qemu is wrong even if not show errors.
>>>
>>
> Fabio,
>   You can add my:
>
> Reviewed-by: Don Slutz <dslutz@verizon.com>
>
> I have a similar code change locally (part of my pending
> list of to dos) (I just changed the global arg...).
>
>
>  You later provided links but I think the conversation should be
>> referenced here.
>>
>
> I was part of the conversation.  When I was looking into upstreaming
> a change I have (pci_min_hole) to xen & qemu, I was asked by QEMU
> to report if it was not used (i.e. non x86 cpu's).  While I was testing my
> change to QEMU under xen I noticed:
>
> Warning: "-global vga.vram_size_mb=16" not used
>
>
> In /var/log/xen/qemu-dm-<guest>.log
>
> Here is the cross post to xen-devel:
>
>
>  [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused
> -global
>
>
> http://lists.xen.org/archives/html/xen-devel/2014-03/msg03128.html
>
>
>
>
>  Is this change correct for all versions of mainline qemu which people
>> might be using with Xen?
>>
>
> What I know is that "-global cirrus-vga.vgamem_mb=32" does work
> with upstream QEMU 1.5.0, 1.6.0, 1.7.0 and 2.0.0.  I had added a debug
> output into the QEMU version above that show the amount of video ram
> that gets allocated and so the testing was quick and easy.  My
> understanding of QEMU is that when specified this way:
>
>  "-device cirrus-vga,vgamem_mb=32"
>
> the error checking in QEMU will report when it does not like it:
>
> qemu-system-x86_64: Property '.vram_size_mb' not found
>
>
> (unlike -global).  So while I have not tested it, I would guess that
> any QEMU that accepts "-device cirrus-vga" will also either
> accept the change or report and error and not start. (Note: the
> change from "-vga cirrus" to "-device cirrus-vga" was done since
> 4.3.0 and has yet to generate a bug).


Thanks for your reply, probably with my bad english I not understand good
this part of your reply.
I changed to -device following this official qemu doc:
http://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/qdev-device-use.txt;hb=master
I did several tests of additional vga's parameters without using the -global in
recent days with cirrus, stdvga and qxl, I've never seen errors and the
amount of videoram by domUs seem ok.


>
>
>  Please can you also explain what "wrong" means? Does it crash? Does it
>> silently ignore the setting? Does it do something else "wrong"? How bad
>> is it?
>>
>
> For me it just silently ignores the setting.  Which means that the
> cirrus vga has the default memory and so will not support any
> display setting that needs more memory (the most likely reason
> videoram=32 is specified in the config file).
>
> I feel it is bad enough that a backport to 4.4 makes sense to
> me.
>
> Hope this helps.
>     -Don Slutz
>
>  Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>
>>> ---
>>>
>>> Note:
>>> - when this patch will be accepted upstream should be
>>>    backported also on xen 4.4
>>>
>> I think this very much depends on what you mean by "wrong" above.
>>
>>  - with this qemu parameters seems correct but for further
>>>    confirmation I posted a question about it:
>>> http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
>>>
>> Any reply to this question?
>>
>> Anthony, Stefano: Opinions on this change?
>>
>>  ---
>>>   tools/libxl/libxl_dm.c |    5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> index 8abed7b..90f19b7 100644
>>> --- a/tools/libxl/libxl_dm.c
>>> +++ b/tools/libxl/libxl_dm.c
>>> @@ -508,9 +508,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc
>>> *gc,
>>>               flexarray_append_pair(dm_args, "-device", "VGA");
>>>               break;
>>>           case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>> -            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>> -            flexarray_append_pair(dm_args, "-global",
>>> -                GCSPRINTF("vga.vram_size_mb=%d",
>>> +            flexarray_append_pair(dm_args, "-device",
>>> +                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
>>>                   libxl__sizekb_to_mb(b_info->video_memkb)));
>>>               break;
>>>           case LIBXL_VGA_INTERFACE_TYPE_NONE:
>>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>
>
>

[-- Attachment #1.2: Type: text/html, Size: 8226 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-02 20:04     ` Fabio Fantoni
@ 2014-05-07 12:20       ` Fabio Fantoni
  2014-05-08 10:10         ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-07 12:20 UTC (permalink / raw)
  To: Don Slutz
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Ian Campbell, Stefano Stabellini

Il 02/05/2014 22:04, Fabio Fantoni ha scritto:
> 2014-05-02 21:44 GMT+02:00 Don Slutz <dslutz@verizon.com 
> <mailto:dslutz@verizon.com>>:
>
>     On 05/02/14 07:41, Ian Campbell wrote:
>
>         On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:
>
>             Reading one qemu-devel post seems that setting video memory of
>             cirrus vga with upstream qemu is wrong even if not show
>             errors.
>
>
>     Fabio,
>       You can add my:
>
>     Reviewed-by: Don Slutz <dslutz@verizon.com
>     <mailto:dslutz@verizon.com>>
>
>     I have a similar code change locally (part of my pending
>     list of to dos) (I just changed the global arg...).
>
>
>         You later provided links but I think the conversation should be
>         referenced here.
>
>
>     I was part of the conversation.  When I was looking into upstreaming
>     a change I have (pci_min_hole) to xen & qemu, I was asked by QEMU
>     to report if it was not used (i.e. non x86 cpu's).  While I was
>     testing my
>     change to QEMU under xen I noticed:
>
>     Warning: "-global vga.vram_size_mb=16" not used
>
>
>     In /var/log/xen/qemu-dm-<guest>.log
>
>     Here is the cross post to xen-devel:
>
>
>      [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about
>     unused -global
>
>
>     http://lists.xen.org/archives/html/xen-devel/2014-03/msg03128.html
>
>
>
>
>         Is this change correct for all versions of mainline qemu which
>         people
>         might be using with Xen?
>
>
>     What I know is that "-global cirrus-vga.vgamem_mb=32" does work
>     with upstream QEMU 1.5.0, 1.6.0, 1.7.0 and 2.0.0.  I had added a debug
>     output into the QEMU version above that show the amount of video ram
>     that gets allocated and so the testing was quick and easy.  My
>     understanding of QEMU is that when specified this way:
>
>      "-device cirrus-vga,vgamem_mb=32"
>
>     the error checking in QEMU will report when it does not like it:
>
>     qemu-system-x86_64: Property '.vram_size_mb' not found
>
>
>     (unlike -global).  So while I have not tested it, I would guess that
>     any QEMU that accepts "-device cirrus-vga" will also either
>     accept the change or report and error and not start. (Note: the
>     change from "-vga cirrus" to "-device cirrus-vga" was done since
>     4.3.0 and has yet to generate a bug).
>
>
> Thanks for your reply, probably with my bad english I not understand 
> good this part of your reply.
> I changed to -device following this official qemu doc:
> http://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/qdev-device-use.txt;hb=master
> I did several tests of additional vga's parameters without using 
> the-global in recent days with cirrus, stdvga and qxl, I've never seen 
> errors and the amount of videoram by domUs seem ok.

Ping

>
>
>         Please can you also explain what "wrong" means? Does it crash?
>         Does it
>         silently ignore the setting? Does it do something else
>         "wrong"? How bad
>         is it?
>
>
>     For me it just silently ignores the setting.  Which means that the
>     cirrus vga has the default memory and so will not support any
>     display setting that needs more memory (the most likely reason
>     videoram=32 is specified in the config file).
>
>     I feel it is bad enough that a backport to 4.4 makes sense to
>     me.
>
>     Hope this helps.
>         -Don Slutz
>
>             Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz
>             <mailto:fabio.fantoni@m2r.biz>>
>
>             ---
>
>             Note:
>             - when this patch will be accepted upstream should be
>                backported also on xen 4.4
>
>         I think this very much depends on what you mean by "wrong" above.
>
>             - with this qemu parameters seems correct but for further
>                confirmation I posted a question about it:
>             http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
>
>         Any reply to this question?
>
>         Anthony, Stefano: Opinions on this change?
>
>             ---
>               tools/libxl/libxl_dm.c |    5 ++---
>               1 file changed, 2 insertions(+), 3 deletions(-)
>
>             diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>             index 8abed7b..90f19b7 100644
>             --- a/tools/libxl/libxl_dm.c
>             +++ b/tools/libxl/libxl_dm.c
>             @@ -508,9 +508,8 @@ static char **
>             libxl__build_device_model_args_new(libxl__gc *gc,
>                           flexarray_append_pair(dm_args, "-device",
>             "VGA");
>                           break;
>                       case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>             -            flexarray_append_pair(dm_args, "-device",
>             "cirrus-vga");
>             -            flexarray_append_pair(dm_args, "-global",
>             -                GCSPRINTF("vga.vram_size_mb=%d",
>             +            flexarray_append_pair(dm_args, "-device",
>             +                GCSPRINTF("cirrus-vga,vgamem_mb=%d",
>                               libxl__sizekb_to_mb(b_info->video_memkb)));
>                           break;
>                       case LIBXL_VGA_INTERFACE_TYPE_NONE:
>
>
>
>         _______________________________________________
>         Xen-devel mailing list
>         Xen-devel@lists.xen.org <mailto:Xen-devel@lists.xen.org>
>         http://lists.xen.org/xen-devel
>
>
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-07 12:20       ` Fabio Fantoni
@ 2014-05-08 10:10         ` Ian Campbell
  2014-05-08 10:41           ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-05-08 10:10 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Don Slutz, Stefano Stabellini

On Wed, 2014-05-07 at 14:20 +0200, Fabio Fantoni wrote:
> Il 02/05/2014 22:04, Fabio Fantoni ha scritto:
> > 2014-05-02 21:44 GMT+02:00 Don Slutz <dslutz@verizon.com 
> > <mailto:dslutz@verizon.com>>:
> >
> >     On 05/02/14 07:41, Ian Campbell wrote:
> >
> >         On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:
> >
> >             Reading one qemu-devel post seems that setting video memory of
> >             cirrus vga with upstream qemu is wrong even if not show
> >             errors.
> >
> >
> >     Fabio,
> >       You can add my:
> >
> >     Reviewed-by: Don Slutz <dslutz@verizon.com
> >     <mailto:dslutz@verizon.com>>
> >
> >     I have a similar code change locally (part of my pending
> >     list of to dos) (I just changed the global arg...).
> >
> >
> >         You later provided links but I think the conversation should be
> >         referenced here.
> >
> >
> >     I was part of the conversation.  When I was looking into upstreaming
> >     a change I have (pci_min_hole) to xen & qemu, I was asked by QEMU
> >     to report if it was not used (i.e. non x86 cpu's).  While I was
> >     testing my
> >     change to QEMU under xen I noticed:
> >
> >     Warning: "-global vga.vram_size_mb=16" not used
> >
> >
> >     In /var/log/xen/qemu-dm-<guest>.log
> >
> >     Here is the cross post to xen-devel:
> >
> >
> >      [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about
> >     unused -global
> >
> >
> >     http://lists.xen.org/archives/html/xen-devel/2014-03/msg03128.html
> >
> >
> >
> >
> >         Is this change correct for all versions of mainline qemu which
> >         people
> >         might be using with Xen?
> >
> >
> >     What I know is that "-global cirrus-vga.vgamem_mb=32" does work
> >     with upstream QEMU 1.5.0, 1.6.0, 1.7.0 and 2.0.0.  I had added a debug
> >     output into the QEMU version above that show the amount of video ram
> >     that gets allocated and so the testing was quick and easy.  My
> >     understanding of QEMU is that when specified this way:
> >
> >      "-device cirrus-vga,vgamem_mb=32"
> >
> >     the error checking in QEMU will report when it does not like it:
> >
> >     qemu-system-x86_64: Property '.vram_size_mb' not found
> >
> >
> >     (unlike -global).  So while I have not tested it, I would guess that
> >     any QEMU that accepts "-device cirrus-vga" will also either
> >     accept the change or report and error and not start. (Note: the
> >     change from "-vga cirrus" to "-device cirrus-vga" was done since
> >     4.3.0 and has yet to generate a bug).
> >
> >
> > Thanks for your reply, probably with my bad english I not understand 
> > good this part of your reply.
> > I changed to -device following this official qemu doc:
> > http://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/qdev-device-use.txt;hb=master
> > I did several tests of additional vga's parameters without using 
> > the-global in recent days with cirrus, stdvga and qxl, I've never seen 
> > errors and the amount of videoram by domUs seem ok.
> 
> Ping

I'm not sure who this was addressed to but for my part I am waiting for
a refreshed patch which addresses the questions asked in
<1399030886.32736.63.camel@kazak.uk.xensource.com> in the commit
message. Those questions have been partially answered here but it seem
that at least Don's comments about which versions it worked with were
wrt a different patch using a different variation on the options.

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-08 10:10         ` Ian Campbell
@ 2014-05-08 10:41           ` Fabio Fantoni
  2014-05-08 11:33             ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-08 10:41 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Anthony PERARD, xen-devel, Ian Jackson, Don Slutz, Stefano Stabellini

Il 08/05/2014 12:10, Ian Campbell ha scritto:
> On Wed, 2014-05-07 at 14:20 +0200, Fabio Fantoni wrote:
>> Il 02/05/2014 22:04, Fabio Fantoni ha scritto:
>>> 2014-05-02 21:44 GMT+02:00 Don Slutz <dslutz@verizon.com
>>> <mailto:dslutz@verizon.com>>:
>>>
>>>      On 05/02/14 07:41, Ian Campbell wrote:
>>>
>>>          On Sat, 2014-04-19 at 14:16 +0200, Fabio Fantoni wrote:
>>>
>>>              Reading one qemu-devel post seems that setting video memory of
>>>              cirrus vga with upstream qemu is wrong even if not show
>>>              errors.
>>>
>>>
>>>      Fabio,
>>>        You can add my:
>>>
>>>      Reviewed-by: Don Slutz <dslutz@verizon.com
>>>      <mailto:dslutz@verizon.com>>
>>>
>>>      I have a similar code change locally (part of my pending
>>>      list of to dos) (I just changed the global arg...).
>>>
>>>
>>>          You later provided links but I think the conversation should be
>>>          referenced here.
>>>
>>>
>>>      I was part of the conversation.  When I was looking into upstreaming
>>>      a change I have (pci_min_hole) to xen & qemu, I was asked by QEMU
>>>      to report if it was not used (i.e. non x86 cpu's).  While I was
>>>      testing my
>>>      change to QEMU under xen I noticed:
>>>
>>>      Warning: "-global vga.vram_size_mb=16" not used
>>>
>>>
>>>      In /var/log/xen/qemu-dm-<guest>.log
>>>
>>>      Here is the cross post to xen-devel:
>>>
>>>
>>>       [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about
>>>      unused -global
>>>
>>>
>>>      http://lists.xen.org/archives/html/xen-devel/2014-03/msg03128.html
>>>
>>>
>>>
>>>
>>>          Is this change correct for all versions of mainline qemu which
>>>          people
>>>          might be using with Xen?
>>>
>>>
>>>      What I know is that "-global cirrus-vga.vgamem_mb=32" does work
>>>      with upstream QEMU 1.5.0, 1.6.0, 1.7.0 and 2.0.0.  I had added a debug
>>>      output into the QEMU version above that show the amount of video ram
>>>      that gets allocated and so the testing was quick and easy.  My
>>>      understanding of QEMU is that when specified this way:
>>>
>>>       "-device cirrus-vga,vgamem_mb=32"
>>>
>>>      the error checking in QEMU will report when it does not like it:
>>>
>>>      qemu-system-x86_64: Property '.vram_size_mb' not found
>>>
>>>
>>>      (unlike -global).  So while I have not tested it, I would guess that
>>>      any QEMU that accepts "-device cirrus-vga" will also either
>>>      accept the change or report and error and not start. (Note: the
>>>      change from "-vga cirrus" to "-device cirrus-vga" was done since
>>>      4.3.0 and has yet to generate a bug).
>>>
>>>
>>> Thanks for your reply, probably with my bad english I not understand
>>> good this part of your reply.
>>> I changed to -device following this official qemu doc:
>>> http://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/qdev-device-use.txt;hb=master
>>> I did several tests of additional vga's parameters without using
>>> the-global in recent days with cirrus, stdvga and qxl, I've never seen
>>> errors and the amount of videoram by domUs seem ok.
>> Ping
> I'm not sure who this was addressed to but for my part I am waiting for
> a refreshed patch which addresses the questions asked in
> <1399030886.32736.63.camel@kazak.uk.xensource.com> in the commit
> message. Those questions have been partially answered here but it seem
> that at least Don's comments about which versions it worked with were
> wrt a different patch using a different variation on the options.
>
> Ian.
>

About qemu version when vgamem_mb property is added is qemu 1.3, I 
already did detailed reply about it in stdvga patch:
http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
Or you mean something other?

Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-08 10:41           ` Fabio Fantoni
@ 2014-05-08 11:33             ` Ian Campbell
  2014-05-08 15:03               ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-05-08 11:33 UTC (permalink / raw)
  To: Fabio Fantoni; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Don Slutz

On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:

> About qemu version when vgamem_mb property is added is qemu 1.3, I 
> already did detailed reply about it in stdvga patch:
> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
> Or you mean something other?

"in the commit message" is the bit you keep missing.


> 
> Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-08 11:33             ` Ian Campbell
@ 2014-05-08 15:03               ` Fabio Fantoni
  2014-05-08 15:19                 ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-08 15:03 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Don Slutz

Il 08/05/2014 13:33, Ian Campbell ha scritto:
> On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
>
>> About qemu version when vgamem_mb property is added is qemu 1.3, I
>> already did detailed reply about it in stdvga patch:
>> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
>> Or you mean something other?
> "in the commit message" is the bit you keep missing.

I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with google 
or thunderbird on mail archive, I not understand what post you refer :(
The question about qemu versions is above, the patch is tested and on 
domUs show correct video memory size, for example now I'm using windows 
7 domUs with stdvga and videoram=64 and on windows in standard video 
device properties show 64 mb of memory (default is 16 mb).
The only think that I not understand is part of latest Don Slutz reply 
even if he did "Reviewed-by" on patch.

Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-08 15:03               ` Fabio Fantoni
@ 2014-05-08 15:19                 ` Ian Campbell
  2014-05-09  8:01                   ` Fabio Fantoni
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-05-08 15:19 UTC (permalink / raw)
  To: Fabio Fantoni; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Don Slutz

On Thu, 2014-05-08 at 17:03 +0200, Fabio Fantoni wrote:
> Il 08/05/2014 13:33, Ian Campbell ha scritto:
> > On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
> >
> >> About qemu version when vgamem_mb property is added is qemu 1.3, I
> >> already did detailed reply about it in stdvga patch:
> >> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
> >> Or you mean something other?
> > "in the commit message" is the bit you keep missing.
> 
> I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with google 
> or thunderbird on mail archive, I not understand what post you refer :(

mid.gmane.org is a good way to find these things. e.g.:
http://mid.gmane.org/<1399030886.32736.63.camel@kazak.uk.xensource.com>

Note that the <> which I quoted *are* part of the message id.

The questions there were:
        Is this change correct for all versions of mainline qemu which
        people might be using with Xen?
        
        Please can you also explain what "wrong" means? Does it crash?
        Does it silently ignore the setting? Does it do something else
        "wrong"? How bad is it?
        
        > - with this qemu parameters seems correct but for further
        >   confirmation I posted a question about it:
        > http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html

        Any reply to this question?

> The question about qemu versions is above,

As I keep saying: These questions should be answered by the *commit
message*.

>  the patch is tested and on 
> domUs show correct video memory size, for example now I'm using windows 
> 7 domUs with stdvga and videoram=64 and on windows in standard video 
> device properties show 64 mb of memory (default is 16 mb).

More information which would be useful in the commit message.

> The only think that I not understand is part of latest Don Slutz reply 
> even if he did "Reviewed-by" on patch.
> 
> Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-08 15:19                 ` Ian Campbell
@ 2014-05-09  8:01                   ` Fabio Fantoni
  2014-05-09  9:09                     ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-09  8:01 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Anthony PERARD, xen-devel, Ian Jackson, Don Slutz

Il 08/05/2014 17:19, Ian Campbell ha scritto:
> On Thu, 2014-05-08 at 17:03 +0200, Fabio Fantoni wrote:
>> Il 08/05/2014 13:33, Ian Campbell ha scritto:
>>> On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
>>>
>>>> About qemu version when vgamem_mb property is added is qemu 1.3, I
>>>> already did detailed reply about it in stdvga patch:
>>>> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
>>>> Or you mean something other?
>>> "in the commit message" is the bit you keep missing.
>> I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with google
>> or thunderbird on mail archive, I not understand what post you refer :(
> mid.gmane.org is a good way to find these things. e.g.:
> http://mid.gmane.org/<1399030886.32736.63.camel@kazak.uk.xensource.com>
>
> Note that the <> which I quoted *are* part of the message id.
>
> The questions there were:
>          Is this change correct for all versions of mainline qemu which
>          people might be using with Xen?

vgamem_mb property was added in qemu 1.3 (same for stdvga patch), xen 
4.4 from source use qemu 1.6, and from distributions package is newer 
all case I saw, then FWIK there should be no problems. Xen 4.4 if I 
remember good change other important qemu value for hvm domUs that 
require qemu>=1.6. (1.6.1 because 1.6.0 have critical regression for all 
hvm domUs)

>          
>          Please can you also explain what "wrong" means? Does it crash?
>          Does it silently ignore the setting? Does it do something else
>          "wrong"? How bad is it?

It silently ignore the setting.


>          
>          > - with this qemu parameters seems correct but for further
>          >   confirmation I posted a question about it:
>          > http://lists.xen.org/archives/html/xen-devel/2014-04/msg02606.html
>
>          Any reply to this question?

Don Slutz(who  found the problem of global parameters wrong silently ignored on qemu) responded:
http://lists.xen.org/archives/html/xen-devel/2014-05/msg00369.html
One part of his reply is strange but he gave the reviewed-by and the patch is tested and working,so probably  I did not understand  that part.


>
>> The question about qemu versions is above,
> As I keep saying: These questions should be answered by the *commit
> message*.
>
>>   the patch is tested and on
>> domUs show correct video memory size, for example now I'm using windows
>> 7 domUs with stdvga and videoram=64 and on windows in standard video
>> device properties show 64 mb of memory (default is 16 mb).
> More information which would be useful in the commit message.

I'll add more details on commit message of this patch and also the 
stdvga one.

>
>> The only think that I not understand is part of latest Don Slutz reply
>> even if he did "Reviewed-by" on patch.
>>
>> Thanks for any reply and sorry for my bad english.
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-09  8:01                   ` Fabio Fantoni
@ 2014-05-09  9:09                     ` Ian Campbell
  2014-05-09 14:03                       ` Stefano Stabellini
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2014-05-09  9:09 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: Anthony PERARD, Stefano Stabellini, xen-devel, Ian Jackson, Don Slutz

On Fri, 2014-05-09 at 10:01 +0200, Fabio Fantoni wrote:
> Il 08/05/2014 17:19, Ian Campbell ha scritto:
> > On Thu, 2014-05-08 at 17:03 +0200, Fabio Fantoni wrote:
> >> Il 08/05/2014 13:33, Ian Campbell ha scritto:
> >>> On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
> >>>
> >>>> About qemu version when vgamem_mb property is added is qemu 1.3, I
> >>>> already did detailed reply about it in stdvga patch:
> >>>> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
> >>>> Or you mean something other?
> >>> "in the commit message" is the bit you keep missing.
> >> I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with google
> >> or thunderbird on mail archive, I not understand what post you refer :(
> > mid.gmane.org is a good way to find these things. e.g.:
> > http://mid.gmane.org/<1399030886.32736.63.camel@kazak.uk.xensource.com>
> >
> > Note that the <> which I quoted *are* part of the message id.
> >
> > The questions there were:
> >          Is this change correct for all versions of mainline qemu which
> >          people might be using with Xen?
> 
> vgamem_mb property was added in qemu 1.3 (same for stdvga patch),

How was the video RAM size controller in qemu prior to that? Or was it
not possible?

>  xen 
> 4.4 from source use qemu 1.6, and from distributions package is newer 
> all case I saw, then FWIK there should be no problems. Xen 4.4 if I 
> remember good change other important qemu value for hvm domUs that 
> require qemu>=1.6. (1.6.1 because 1.6.0 have critical regression for all 
> hvm domUs)

Stefano/Anthony -- can you confirm that we already depend on qemu >=
1.6.x (or otherwise that it is OK to depend on qemu >= 1.3)?

An ack from you guys on this change would be appreciated.

It's also been proposed that we backport this to stable branches, how
far back would it be safe/acceptable for us to go with that?
[...]

> I'll add more details on commit message of this patch and also the 
> stdvga one.

Thanks.
> 
> >
> >> The only think that I not understand is part of latest Don Slutz reply
> >> even if he did "Reviewed-by" on patch.
> >>
> >> Thanks for any reply and sorry for my bad english.
> >
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-09  9:09                     ` Ian Campbell
@ 2014-05-09 14:03                       ` Stefano Stabellini
  2014-05-09 14:08                         ` Ian Campbell
  2014-05-09 14:24                         ` Fabio Fantoni
  0 siblings, 2 replies; 20+ messages in thread
From: Stefano Stabellini @ 2014-05-09 14:03 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, Ian Jackson, Don Slutz, Fabio Fantoni,
	Stefano Stabellini, Anthony PERARD

On Fri, 9 May 2014, Ian Campbell wrote:
> On Fri, 2014-05-09 at 10:01 +0200, Fabio Fantoni wrote:
> > Il 08/05/2014 17:19, Ian Campbell ha scritto:
> > > On Thu, 2014-05-08 at 17:03 +0200, Fabio Fantoni wrote:
> > >> Il 08/05/2014 13:33, Ian Campbell ha scritto:
> > >>> On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
> > >>>
> > >>>> About qemu version when vgamem_mb property is added is qemu 1.3, I
> > >>>> already did detailed reply about it in stdvga patch:
> > >>>> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
> > >>>> Or you mean something other?
> > >>> "in the commit message" is the bit you keep missing.
> > >> I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with google
> > >> or thunderbird on mail archive, I not understand what post you refer :(
> > > mid.gmane.org is a good way to find these things. e.g.:
> > > http://mid.gmane.org/<1399030886.32736.63.camel@kazak.uk.xensource.com>
> > >
> > > Note that the <> which I quoted *are* part of the message id.
> > >
> > > The questions there were:
> > >          Is this change correct for all versions of mainline qemu which
> > >          people might be using with Xen?
> > 
> > vgamem_mb property was added in qemu 1.3 (same for stdvga patch),
> 
> How was the video RAM size controller in qemu prior to that? Or was it
> not possible?
> 
> >  xen 
> > 4.4 from source use qemu 1.6, and from distributions package is newer 
> > all case I saw, then FWIK there should be no problems. Xen 4.4 if I 
> > remember good change other important qemu value for hvm domUs that 
> > require qemu>=1.6. (1.6.1 because 1.6.0 have critical regression for all 
> > hvm domUs)
> 
> Stefano/Anthony -- can you confirm that we already depend on qemu >=
> 1.6.x (or otherwise that it is OK to depend on qemu >= 1.3)?

>From upstream QEMU POV anything older than 1.7 is "ancient" and
unmaintained.

>From Xen POV the upstream QEMU version that we released with Xen 4.2 was
based on QEMU v1.0.1, the one we released with Xen 4.3 was based on
v1.3.0.

Given that upstream QEMU with Xen 4.2 was a tech preview and that if I
am not mistaken we don't maintain Xen 4.2 anymore, it should be OK to
depend on QEMU >= 1.3.


> An ack from you guys on this change would be appreciated.

I think the change is OK but I wonder if we should write down somewhere,
maybe in the 4.5 release notes or on the wiki, that we depend on QEMU >=
1.3.


> It's also been proposed that we backport this to stable branches, how
> far back would it be safe/acceptable for us to go with that?
> [...]

Anything newer than 4.2 should be OK.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-09 14:03                       ` Stefano Stabellini
@ 2014-05-09 14:08                         ` Ian Campbell
  2014-05-09 14:24                         ` Fabio Fantoni
  1 sibling, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2014-05-09 14:08 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Jackson, Don Slutz, Fabio Fantoni,
	Stefano Stabellini, Anthony PERARD

On Fri, 2014-05-09 at 15:03 +0100, Stefano Stabellini wrote:
> Given that upstream QEMU with Xen 4.2 was a tech preview and that if I
> am not mistaken we don't maintain Xen 4.2 anymore, it should be OK to
> depend on QEMU >= 1.3.

Thanks for confirming.

> > An ack from you guys on this change would be appreciated.
> 
> I think the change is OK but I wonder if we should write down somewhere,
> maybe in the 4.5 release notes or on the wiki, that we depend on QEMU >=
> 1.3.

I think the appropriate place is the README in a similar place to the
build dependencies. I suppose it needs to be worded as semioptional to
account for the fact that we supply one which is certainly OK. 

> > It's also been proposed that we backport this to stable branches, how
> > far back would it be safe/acceptable for us to go with that?
> > [...]
> 
> Anything newer than 4.2 should be OK.

Since, as you say, qemu-xen was preview in 4.2 I don't think backporting
fixes to it would be appropriate anyway, regardless of the support
status of 4.2 generally.

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-09 14:03                       ` Stefano Stabellini
  2014-05-09 14:08                         ` Ian Campbell
@ 2014-05-09 14:24                         ` Fabio Fantoni
  2014-05-09 14:48                           ` Fabio Fantoni
  1 sibling, 1 reply; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-09 14:24 UTC (permalink / raw)
  To: Stefano Stabellini, Ian Campbell
  Cc: Anthony PERARD, Stefano Stabellini, xen-devel, Ian Jackson, Don Slutz

Il 09/05/2014 16:03, Stefano Stabellini ha scritto:
> On Fri, 9 May 2014, Ian Campbell wrote:
>> On Fri, 2014-05-09 at 10:01 +0200, Fabio Fantoni wrote:
>>> Il 08/05/2014 17:19, Ian Campbell ha scritto:
>>>> On Thu, 2014-05-08 at 17:03 +0200, Fabio Fantoni wrote:
>>>>> Il 08/05/2014 13:33, Ian Campbell ha scritto:
>>>>>> On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
>>>>>>
>>>>>>> About qemu version when vgamem_mb property is added is qemu 1.3, I
>>>>>>> already did detailed reply about it in stdvga patch:
>>>>>>> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
>>>>>>> Or you mean something other?
>>>>>> "in the commit message" is the bit you keep missing.
>>>>> I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with google
>>>>> or thunderbird on mail archive, I not understand what post you refer :(
>>>> mid.gmane.org is a good way to find these things. e.g.:
>>>> http://mid.gmane.org/<1399030886.32736.63.camel@kazak.uk.xensource.com>
>>>>
>>>> Note that the <> which I quoted *are* part of the message id.
>>>>
>>>> The questions there were:
>>>>           Is this change correct for all versions of mainline qemu which
>>>>           people might be using with Xen?
>>> vgamem_mb property was added in qemu 1.3 (same for stdvga patch),
>> How was the video RAM size controller in qemu prior to that? Or was it
>> not possible?
>>
>>>   xen
>>> 4.4 from source use qemu 1.6, and from distributions package is newer
>>> all case I saw, then FWIK there should be no problems. Xen 4.4 if I
>>> remember good change other important qemu value for hvm domUs that
>>> require qemu>=1.6. (1.6.1 because 1.6.0 have critical regression for all
>>> hvm domUs)
>> Stefano/Anthony -- can you confirm that we already depend on qemu >=
>> 1.6.x (or otherwise that it is OK to depend on qemu >= 1.3)?
>  From upstream QEMU POV anything older than 1.7 is "ancient" and
> unmaintained.
>
>  From Xen POV the upstream QEMU version that we released with Xen 4.2 was
> based on QEMU v1.0.1, the one we released with Xen 4.3 was based on
> v1.3.0.
>
> Given that upstream QEMU with Xen 4.2 was a tech preview and that if I
> am not mistaken we don't maintain Xen 4.2 anymore, it should be OK to
> depend on QEMU >= 1.3.

If I remember good the machine/pv qemu parameters part changed by 
Anthony in xen 4.4 require at least qemu 1.6.
In that case xen >=4.4 need already qemu>=1.6.1 (1.6.1 because 1.6.0 
have critical regression for all
hvm domUs)

>
>
>> An ack from you guys on this change would be appreciated.
> I think the change is OK but I wonder if we should write down somewhere,
> maybe in the 4.5 release notes or on the wiki, that we depend on QEMU >=
> 1.3.
>
>
>> It's also been proposed that we backport this to stable branches, how
>> far back would it be safe/acceptable for us to go with that?
>> [...]
> Anything newer than 4.2 should be OK.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu
  2014-05-09 14:24                         ` Fabio Fantoni
@ 2014-05-09 14:48                           ` Fabio Fantoni
  0 siblings, 0 replies; 20+ messages in thread
From: Fabio Fantoni @ 2014-05-09 14:48 UTC (permalink / raw)
  To: Stefano Stabellini, Ian Campbell
  Cc: Anthony PERARD, Stefano Stabellini, xen-devel, Ian Jackson, Don Slutz

Il 09/05/2014 16:24, Fabio Fantoni ha scritto:
> Il 09/05/2014 16:03, Stefano Stabellini ha scritto:
>> On Fri, 9 May 2014, Ian Campbell wrote:
>>> On Fri, 2014-05-09 at 10:01 +0200, Fabio Fantoni wrote:
>>>> Il 08/05/2014 17:19, Ian Campbell ha scritto:
>>>>> On Thu, 2014-05-08 at 17:03 +0200, Fabio Fantoni wrote:
>>>>>> Il 08/05/2014 13:33, Ian Campbell ha scritto:
>>>>>>> On Thu, 2014-05-08 at 12:41 +0200, Fabio Fantoni wrote:
>>>>>>>
>>>>>>>> About qemu version when vgamem_mb property is added is qemu 1.3, I
>>>>>>>> already did detailed reply about it in stdvga patch:
>>>>>>>> http://lists.xen.org/archives/html/xen-devel/2014-05/msg00259.html
>>>>>>>> Or you mean something other?
>>>>>>> "in the commit message" is the bit you keep missing.
>>>>>> I not found 1399030886.32736.63.camel@kazak.uk.xensource.com with 
>>>>>> google
>>>>>> or thunderbird on mail archive, I not understand what post you 
>>>>>> refer :(
>>>>> mid.gmane.org is a good way to find these things. e.g.:
>>>>> http://mid.gmane.org/<1399030886.32736.63.camel@kazak.uk.xensource.com> 
>>>>>
>>>>>
>>>>> Note that the <> which I quoted *are* part of the message id.
>>>>>
>>>>> The questions there were:
>>>>>           Is this change correct for all versions of mainline qemu 
>>>>> which
>>>>>           people might be using with Xen?
>>>> vgamem_mb property was added in qemu 1.3 (same for stdvga patch),
>>> How was the video RAM size controller in qemu prior to that? Or was it
>>> not possible?
>>>
>>>>   xen
>>>> 4.4 from source use qemu 1.6, and from distributions package is newer
>>>> all case I saw, then FWIK there should be no problems. Xen 4.4 if I
>>>> remember good change other important qemu value for hvm domUs that
>>>> require qemu>=1.6. (1.6.1 because 1.6.0 have critical regression 
>>>> for all
>>>> hvm domUs)
>>> Stefano/Anthony -- can you confirm that we already depend on qemu >=
>>> 1.6.x (or otherwise that it is OK to depend on qemu >= 1.3)?
>>  From upstream QEMU POV anything older than 1.7 is "ancient" and
>> unmaintained.
>>
>>  From Xen POV the upstream QEMU version that we released with Xen 4.2 
>> was
>> based on QEMU v1.0.1, the one we released with Xen 4.3 was based on
>> v1.3.0.
>>
>> Given that upstream QEMU with Xen 4.2 was a tech preview and that if I
>> am not mistaken we don't maintain Xen 4.2 anymore, it should be OK to
>> depend on QEMU >= 1.3.
>
> If I remember good the machine/pv qemu parameters part changed by 
> Anthony in xen 4.4 require at least qemu 1.6.
> In that case xen >=4.4 need already qemu>=1.6.1 (1.6.1 because 1.6.0 
> have critical regression for all
> hvm domUs)

Found:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=2bc047635b51abd41c917aa2b813211ee0de2c38
http://git.qemu.org/?p=qemu.git;a=commit;h=a97d6fe6fbb97630d77253d20bdce78f76d01850
 From a fast look seems that require qemu>=1.6 only for the case of 
xen_platform_pci=0

I remember also another critical regression in qemu 1.5 and solved in 
qemu 1.6 that cause qemu crash in some cases hvm domUs with high ram 
(even without pci passthrough).

These are probably a good reasons to recommend qemu>=1.6.1 with xen>=4.4

>
>>
>>
>>> An ack from you guys on this change would be appreciated.
>> I think the change is OK but I wonder if we should write down somewhere,
>> maybe in the 4.5 release notes or on the wiki, that we depend on QEMU >=
>> 1.3.
>>
>>
>>> It's also been proposed that we backport this to stable branches, how
>>> far back would it be safe/acceptable for us to go with that?
>>> [...]
>> Anything newer than 4.2 should be OK.
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-05-09 14:48 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-19 12:16 [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu Fabio Fantoni
2014-04-22 13:11 ` Ian Jackson
2014-04-22 13:21   ` Fabio Fantoni
2014-04-24 10:33     ` Fabio Fantoni
2014-05-02  9:11       ` Fabio Fantoni
2014-05-02 11:41 ` Ian Campbell
2014-05-02 19:44   ` Don Slutz
2014-05-02 20:04     ` Fabio Fantoni
2014-05-07 12:20       ` Fabio Fantoni
2014-05-08 10:10         ` Ian Campbell
2014-05-08 10:41           ` Fabio Fantoni
2014-05-08 11:33             ` Ian Campbell
2014-05-08 15:03               ` Fabio Fantoni
2014-05-08 15:19                 ` Ian Campbell
2014-05-09  8:01                   ` Fabio Fantoni
2014-05-09  9:09                     ` Ian Campbell
2014-05-09 14:03                       ` Stefano Stabellini
2014-05-09 14:08                         ` Ian Campbell
2014-05-09 14:24                         ` Fabio Fantoni
2014-05-09 14:48                           ` Fabio Fantoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.