[PATCH] libxl: new parameters for upstream qemu's vga interfaces

[PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Change the qemu parameters for upstream qemu vgas to the
new ones (-device), introduced some years ago.

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
 tools/libxl/libxl_dm.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 43c3bec..2c6f5d9 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
 
         switch (b_info->u.hvm.vga.kind) {
         case LIBXL_VGA_INTERFACE_TYPE_STD:
-            flexarray_vappend(dm_args, "-vga", "std", NULL);
+            flexarray_append_pair(dm_args, "-device", "VGA");
             break;
         case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
-            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
-            if (b_info->video_memkb) {
-                flexarray_vappend(dm_args, "-global",
-                    GCSPRINTF("vga.vram_size_mb=%d",
-                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
-            }
+            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
+            flexarray_append_pair(dm_args, "-global",
+                GCSPRINTF("vga.vram_size_mb=%d",
+                libxl__sizekb_to_mb(b_info->video_memkb)));
             break;
         }
 
-- 
1.7.9.5

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
> Change the qemu parameters for upstream qemu vgas to the
> new ones (-device), introduced some years ago.

Ping

>
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> ---
>   tools/libxl/libxl_dm.c |   12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 43c3bec..2c6f5d9 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>   
>           switch (b_info->u.hvm.vga.kind) {
>           case LIBXL_VGA_INTERFACE_TYPE_STD:
> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
> +            flexarray_append_pair(dm_args, "-device", "VGA");
>               break;
>           case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
> -            if (b_info->video_memkb) {
> -                flexarray_vappend(dm_args, "-global",
> -                    GCSPRINTF("vga.vram_size_mb=%d",
> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
> -            }
> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
> +            flexarray_append_pair(dm_args, "-global",
> +                GCSPRINTF("vga.vram_size_mb=%d",
> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>               break;
>           }
>   

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Ian Campbell]

On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
> > Change the qemu parameters for upstream qemu vgas to the
> > new ones (-device), introduced some years ago.
> 
> Ping

Please CC the qemu maintainers (Stefano & Anthony), although these
patches touch the toolstack they are logically qemu patches and need
their ack.

> 
> >
> > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> > ---
> >   tools/libxl/libxl_dm.c |   12 +++++-------
> >   1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 43c3bec..2c6f5d9 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >   
> >           switch (b_info->u.hvm.vga.kind) {
> >           case LIBXL_VGA_INTERFACE_TYPE_STD:
> > -            flexarray_vappend(dm_args, "-vga", "std", NULL);
> > +            flexarray_append_pair(dm_args, "-device", "VGA");
> >               break;
> >           case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> > -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
> > -            if (b_info->video_memkb) {
> > -                flexarray_vappend(dm_args, "-global",
> > -                    GCSPRINTF("vga.vram_size_mb=%d",
> > -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
> > -            }
> > +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
> > +            flexarray_append_pair(dm_args, "-global",
> > +                GCSPRINTF("vga.vram_size_mb=%d",
> > +                libxl__sizekb_to_mb(b_info->video_memkb)));
> >               break;
> >           }
> >   
> 

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 10/10/2013 11:32, Ian Campbell ha scritto:
> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>> Change the qemu parameters for upstream qemu vgas to the
>>> new ones (-device), introduced some years ago.
>> Ping
> Please CC the qemu maintainers (Stefano & Anthony), although these
> patches touch the toolstack they are logically qemu patches and need
> their ack.

Thanks for your reply, I added Anthony on CC and Stefano was already on it.

>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>> ---
>>>    tools/libxl/libxl_dm.c |   12 +++++-------
>>>    1 file changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> index 43c3bec..2c6f5d9 100644
>>> --- a/tools/libxl/libxl_dm.c
>>> +++ b/tools/libxl/libxl_dm.c
>>> @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>>    
>>>            switch (b_info->u.hvm.vga.kind) {
>>>            case LIBXL_VGA_INTERFACE_TYPE_STD:
>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>                break;
>>>            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>> -            if (b_info->video_memkb) {
>>> -                flexarray_vappend(dm_args, "-global",
>>> -                    GCSPRINTF("vga.vram_size_mb=%d",
>>> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>> -            }
>>> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>> +            flexarray_append_pair(dm_args, "-global",
>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>>>                break;
>>>            }
>>>    
>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Stefano Stabellini]

On Thu, 10 Oct 2013, Fabio Fantoni wrote:
> Il 10/10/2013 11:32, Ian Campbell ha scritto:
> > On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
> > > Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
> > > > Change the qemu parameters for upstream qemu vgas to the
> > > > new ones (-device), introduced some years ago.
> > > Ping
> > Please CC the qemu maintainers (Stefano & Anthony), although these
> > patches touch the toolstack they are logically qemu patches and need
> > their ack.
> 
> Thanks for your reply, I added Anthony on CC and Stefano was already on it.
> 
> > > > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> > > > ---
> > > >    tools/libxl/libxl_dm.c |   12 +++++-------
> > > >    1 file changed, 5 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > index 43c3bec..2c6f5d9 100644
> > > > --- a/tools/libxl/libxl_dm.c
> > > > +++ b/tools/libxl/libxl_dm.c
> > > > @@ -486,15 +486,13 @@ static char **
> > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > >               switch (b_info->u.hvm.vga.kind) {
> > > >            case LIBXL_VGA_INTERFACE_TYPE_STD:
> > > > -            flexarray_vappend(dm_args, "-vga", "std", NULL);
> > > > +            flexarray_append_pair(dm_args, "-device", "VGA");
> > > >                break;
> > > >            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> > > > -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
> > > > -            if (b_info->video_memkb) {
> > > > -                flexarray_vappend(dm_args, "-global",
> > > > -                    GCSPRINTF("vga.vram_size_mb=%d",
> > > > -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
> > > > -            }
> > > > +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
> > > > +            flexarray_append_pair(dm_args, "-global",
> > > > +                GCSPRINTF("vga.vram_size_mb=%d",
> > > > +                libxl__sizekb_to_mb(b_info->video_memkb)));
> > > >                break;
> > > >            }

Aside from switching to the new parameter format, this patch also
removes the

if (b_info->video_memkb) {

check before passing the -global paramter. Is it deliberate?
If so, it would be best to write down the motivation in the commit
message.
That said, the parameter change looks OK.

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Sander Eikelenboom]

Thursday, October 10, 2013, 12:09:44 PM, you wrote:

> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>> Change the qemu parameters for upstream qemu vgas to the
>>>> new ones (-device), introduced some years ago.
>>> Ping
>> Please CC the qemu maintainers (Stefano & Anthony), although these
>> patches touch the toolstack they are logically qemu patches and need
>> their ack.

> Thanks for your reply, I added Anthony on CC and Stefano was already on it.

Should "None" be added as option as well ?


>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>> ---
>>>>    tools/libxl/libxl_dm.c |   12 +++++-------
>>>>    1 file changed, 5 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>> index 43c3bec..2c6f5d9 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>>>    
>>>>            switch (b_info->u.hvm.vga.kind) {
>>>>            case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>                break;
>>>>            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>> -            if (b_info->video_memkb) {
>>>> -                flexarray_vappend(dm_args, "-global",
>>>> -                    GCSPRINTF("vga.vram_size_mb=%d",
>>>> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>> -            }
>>>> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>>> +            flexarray_append_pair(dm_args, "-global",
>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>                break;
>>>>            }
>>>>    
>>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 10/10/2013 12:43, Stefano Stabellini ha scritto:
> On Thu, 10 Oct 2013, Fabio Fantoni wrote:
>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>> new ones (-device), introduced some years ago.
>>>> Ping
>>> Please CC the qemu maintainers (Stefano & Anthony), although these
>>> patches touch the toolstack they are logically qemu patches and need
>>> their ack.
>> Thanks for your reply, I added Anthony on CC and Stefano was already on it.
>>
>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>> ---
>>>>>     tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>     1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>> index 43c3bec..2c6f5d9 100644
>>>>> --- a/tools/libxl/libxl_dm.c
>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>> @@ -486,15 +486,13 @@ static char **
>>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>                switch (b_info->u.hvm.vga.kind) {
>>>>>             case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>>                 break;
>>>>>             case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>>> -            if (b_info->video_memkb) {
>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>> -                    GCSPRINTF("vga.vram_size_mb=%d",
>>>>> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>> -            }
>>>>> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>                 break;
>>>>>             }
> Aside from switching to the new parameter format, this patch also
> removes the
>
> if (b_info->video_memkb) {
>
> check before passing the -global paramter. Is it deliberate?
> If so, it would be best to write down the motivation in the commit
> message.
> That said, the parameter change looks OK.
>

I removed the "if" after seeing that it isn't necessary because 
b_info->video_memkb is always set (also without videoram parameter on 
xl.cfg file).
I already tested it in both cases and I didn't find any problem.

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
> Thursday, October 10, 2013, 12:09:44 PM, you wrote:
>
>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>> new ones (-device), introduced some years ago.
>>>> Ping
>>> Please CC the qemu maintainers (Stefano & Anthony), although these
>>> patches touch the toolstack they are logically qemu patches and need
>>> their ack.
>> Thanks for your reply, I added Anthony on CC and Stefano was already on it.
> Should "None" be added as option as well ?
>

There is already a nographic xl parameter that controls the 
corresponding qemu parameter, it should be the same thing.
And FWIK the none option applies only to the old -vga parameter and 
according to qemu docs/qdev-device-use.txt, the new way to do it is with 
-nodefaults (I already made a patch to add it). -nographic is probably 
also deprecated.

>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>> ---
>>>>>     tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>     1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>> index 43c3bec..2c6f5d9 100644
>>>>> --- a/tools/libxl/libxl_dm.c
>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>> @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>     
>>>>>             switch (b_info->u.hvm.vga.kind) {
>>>>>             case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>>                 break;
>>>>>             case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>>> -            if (b_info->video_memkb) {
>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>> -                    GCSPRINTF("vga.vram_size_mb=%d",
>>>>> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>> -            }
>>>>> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>                 break;
>>>>>             }
>>>>>     
>
>
>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Sander Eikelenboom]

Thursday, October 10, 2013, 2:29:46 PM, you wrote:

> Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
>> Thursday, October 10, 2013, 12:09:44 PM, you wrote:
>>
>>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>>> new ones (-device), introduced some years ago.
>>>>> Ping
>>>> Please CC the qemu maintainers (Stefano & Anthony), although these
>>>> patches touch the toolstack they are logically qemu patches and need
>>>> their ack.
>>> Thanks for your reply, I added Anthony on CC and Stefano was already on it.
>> Should "None" be added as option as well ?
>>

> There is already a nographic xl parameter that controls the 
> corresponding qemu parameter, it should be the same thing.
> And FWIK the none option applies only to the old -vga parameter and 
> according to qemu docs/qdev-device-use.txt, the new way to do it is with 
> -nodefaults (I already made a patch to add it). -nographic is probably 
> also deprecated.

In this conversation http://comments.gmane.org/gmane.comp.emulators.qemu/172385
the -nographic is said to be deprecated by peter maydell.
Didn't know -vga none was also out the door already ...

>>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>>> ---
>>>>>>     tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>>     1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>>> index 43c3bec..2c6f5d9 100644
>>>>>> --- a/tools/libxl/libxl_dm.c
>>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>>> @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>>     
>>>>>>             switch (b_info->u.hvm.vga.kind) {
>>>>>>             case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>>>                 break;
>>>>>>             case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>>>> -            if (b_info->video_memkb) {
>>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>>> -                    GCSPRINTF("vga.vram_size_mb=%d",
>>>>>> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>>> -            }
>>>>>> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>>> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>>                 break;
>>>>>>             }
>>>>>>     
>>
>>
>>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 10/10/2013 14:36, Sander Eikelenboom ha scritto:
> Thursday, October 10, 2013, 2:29:46 PM, you wrote:
>
>> Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
>>> Thursday, October 10, 2013, 12:09:44 PM, you wrote:
>>>
>>>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>>>> new ones (-device), introduced some years ago.
>>>>>> Ping
>>>>> Please CC the qemu maintainers (Stefano & Anthony), although these
>>>>> patches touch the toolstack they are logically qemu patches and need
>>>>> their ack.
>>>> Thanks for your reply, I added Anthony on CC and Stefano was already on it.
>>> Should "None" be added as option as well ?
>>>
>> There is already a nographic xl parameter that controls the
>> corresponding qemu parameter, it should be the same thing.
>> And FWIK the none option applies only to the old -vga parameter and
>> according to qemu docs/qdev-device-use.txt, the new way to do it is with
>> -nodefaults (I already made a patch to add it). -nographic is probably
>> also deprecated.
> In this conversation http://comments.gmane.org/gmane.comp.emulators.qemu/172385
> the -nographic is said to be deprecated by peter maydell.
> Didn't know -vga none was also out the door already ...

I did some test and I found out that now is impossible disable the 
emulated vga on hvm domUs.
I got cirrus vga (the default) even if vga xl parameter is not setted.
Also with nographic enabled there is the emulated vga.
I think that good solution is:
- add this patch and nodefault patch
- add "none" option to vga xl parameter (that will exclude any emulated 
vga qemu parameters)
- remove -nographic parameter in upstream qemu, making nographic xl 
paramater deprecated and equivalent to new vga="none" xl parameter.

With these changes all should be working and without qemu deprecated 
parameters (-nographic and -vga).
I'm waiting Stefano and/or Anthony replies before write the patches.

Another question is about xenfb vga that seems missed on new -device 
parameter.
I used it to have basic Spice support for pv working: 
http://lists.xen.org/archives/html/xen-devel/2013-09/msg03207.html
Anyone can update about it on newer qemu versions please?

Thanks for any reply

>>>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>>>> ---
>>>>>>>      tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>>>      1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>>>> index 43c3bec..2c6f5d9 100644
>>>>>>> --- a/tools/libxl/libxl_dm.c
>>>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>>>> @@ -486,15 +486,13 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>>>      
>>>>>>>              switch (b_info->u.hvm.vga.kind) {
>>>>>>>              case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>>>>                  break;
>>>>>>>              case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>>>>> -            if (b_info->video_memkb) {
>>>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>>>> -                    GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>> -                    libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>>>> -            }
>>>>>>> +            flexarray_append_pair(dm_args, "-device", "cirrus-vga");
>>>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>> +                libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>>>                  break;
>>>>>>>              }
>>>>>>>      
>>>
>>>
>
>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 10/10/2013 16:39, Fabio Fantoni ha scritto:
> Il 10/10/2013 14:36, Sander Eikelenboom ha scritto:
>> Thursday, October 10, 2013, 2:29:46 PM, you wrote:
>>
>>> Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
>>>> Thursday, October 10, 2013, 12:09:44 PM, you wrote:
>>>>
>>>>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>>>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>>>>> new ones (-device), introduced some years ago.
>>>>>>> Ping
>>>>>> Please CC the qemu maintainers (Stefano & Anthony), although these
>>>>>> patches touch the toolstack they are logically qemu patches and need
>>>>>> their ack.
>>>>> Thanks for your reply, I added Anthony on CC and Stefano was 
>>>>> already on it.
>>>> Should "None" be added as option as well ?
>>>>
>>> There is already a nographic xl parameter that controls the
>>> corresponding qemu parameter, it should be the same thing.
>>> And FWIK the none option applies only to the old -vga parameter and
>>> according to qemu docs/qdev-device-use.txt, the new way to do it is 
>>> with
>>> -nodefaults (I already made a patch to add it). -nographic is probably
>>> also deprecated.
>> In this conversation 
>> http://comments.gmane.org/gmane.comp.emulators.qemu/172385
>> the -nographic is said to be deprecated by peter maydell.
>> Didn't know -vga none was also out the door already ...

>
> I did some test and I found out that now is impossible disable the 
> emulated vga on hvm domUs.
> I got cirrus vga (the default) even if vga xl parameter is not setted.
> Also with nographic enabled there is the emulated vga.
> I think that good solution is:
> - add this patch and nodefault patch
update: nodefault patch is already on git
> - add "none" option to vga xl parameter (that will exclude any 
> emulated vga qemu parameters)
> - remove -nographic parameter in upstream qemu, making nographic xl 
> paramater deprecated and equivalent to new vga="none" xl parameter.
>
> With these changes all should be working and without qemu deprecated 
> parameters (-nographic and -vga).
> I'm waiting Stefano and/or Anthony replies before write the patches.
>
> Another question is about xenfb vga that seems missed on new -device 
> parameter.
> I used it to have basic Spice support for pv working: 
> http://lists.xen.org/archives/html/xen-devel/2013-09/msg03207.html
> Anyone can update about it on newer qemu versions please?
>
> Thanks for any reply

Ping...

>
>>>>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>>>>> ---
>>>>>>>>      tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>>>>      1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>>>>> index 43c3bec..2c6f5d9 100644
>>>>>>>> --- a/tools/libxl/libxl_dm.c
>>>>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>>>>> @@ -486,15 +486,13 @@ static char ** 
>>>>>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>>>>                   switch (b_info->u.hvm.vga.kind) {
>>>>>>>>              case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>>>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>>>>>                  break;
>>>>>>>>              case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>>>>>> -            if (b_info->video_memkb) {
>>>>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>>>>> - GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>>> - libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>>>>> -            }
>>>>>>>> +            flexarray_append_pair(dm_args, "-device", 
>>>>>>>> "cirrus-vga");
>>>>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>>> + libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>>>>                  break;
>>>>>>>>              }
>>>>
>>>>
>>
>>
>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 16/10/2013 14:03, Fabio Fantoni ha scritto:
> Il 10/10/2013 16:39, Fabio Fantoni ha scritto:
>> Il 10/10/2013 14:36, Sander Eikelenboom ha scritto:
>>> Thursday, October 10, 2013, 2:29:46 PM, you wrote:
>>>
>>>> Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
>>>>> Thursday, October 10, 2013, 12:09:44 PM, you wrote:
>>>>>
>>>>>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>>>>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>>>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>>>>>> new ones (-device), introduced some years ago.
>>>>>>>> Ping
>>>>>>> Please CC the qemu maintainers (Stefano & Anthony), although these
>>>>>>> patches touch the toolstack they are logically qemu patches and 
>>>>>>> need
>>>>>>> their ack.
>>>>>> Thanks for your reply, I added Anthony on CC and Stefano was 
>>>>>> already on it.
>>>>> Should "None" be added as option as well ?
>>>>>
>>>> There is already a nographic xl parameter that controls the
>>>> corresponding qemu parameter, it should be the same thing.
>>>> And FWIK the none option applies only to the old -vga parameter and
>>>> according to qemu docs/qdev-device-use.txt, the new way to do it is 
>>>> with
>>>> -nodefaults (I already made a patch to add it). -nographic is probably
>>>> also deprecated.
>>> In this conversation 
>>> http://comments.gmane.org/gmane.comp.emulators.qemu/172385
>>> the -nographic is said to be deprecated by peter maydell.
>>> Didn't know -vga none was also out the door already ...
>
>>
>> I did some test and I found out that now is impossible disable the 
>> emulated vga on hvm domUs.
>> I got cirrus vga (the default) even if vga xl parameter is not setted.
>> Also with nographic enabled there is the emulated vga.
>> I think that good solution is:
>> - add this patch and nodefault patch
> update: nodefault patch is already on git
>> - add "none" option to vga xl parameter (that will exclude any 
>> emulated vga qemu parameters)
>> - remove -nographic parameter in upstream qemu, making nographic xl 
>> parameter deprecated and equivalent to new vga="none" xl parameter.
>>
>> With these changes all should be working and without qemu deprecated 
>> parameters (-nographic and -vga).
>> I'm waiting Stefano and/or Anthony replies before write the patches.
>>
>> Another question is about xenfb vga that seems missed on new -device 
>> parameter.
>> I used it to have basic Spice support for pv working: 
>> http://lists.xen.org/archives/html/xen-devel/2013-09/msg03207.html
>> Anyone can update about it on newer qemu versions please?
>>
>> Thanks for any reply
>
> Ping...

Another ping...

>
>>
>>>>>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>>>>>> ---
>>>>>>>>>      tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>>>>>      1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>>>>>> index 43c3bec..2c6f5d9 100644
>>>>>>>>> --- a/tools/libxl/libxl_dm.c
>>>>>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>>>>>> @@ -486,15 +486,13 @@ static char ** 
>>>>>>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>>>>>                   switch (b_info->u.hvm.vga.kind) {
>>>>>>>>>              case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>>>>>> -            flexarray_vappend(dm_args, "-vga", "std", NULL);
>>>>>>>>> +            flexarray_append_pair(dm_args, "-device", "VGA");
>>>>>>>>>                  break;
>>>>>>>>>              case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
>>>>>>>>> -            if (b_info->video_memkb) {
>>>>>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>>>>>> - GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>>>> - libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>>>>>> -            }
>>>>>>>>> +            flexarray_append_pair(dm_args, "-device", 
>>>>>>>>> "cirrus-vga");
>>>>>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>>>> + libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>>>>>                  break;
>>>>>>>>>              }
>>>>>
>>>>>
>>>
>>>
>>
>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Stefano Stabellini]

On Mon, 28 Oct 2013, Fabio Fantoni wrote:
> Il 16/10/2013 14:03, Fabio Fantoni ha scritto:
> > Il 10/10/2013 16:39, Fabio Fantoni ha scritto:
> > > Il 10/10/2013 14:36, Sander Eikelenboom ha scritto:
> > > > Thursday, October 10, 2013, 2:29:46 PM, you wrote:
> > > > 
> > > > > Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
> > > > > > Thursday, October 10, 2013, 12:09:44 PM, you wrote:
> > > > > > 
> > > > > > > Il 10/10/2013 11:32, Ian Campbell ha scritto:
> > > > > > > > On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
> > > > > > > > > Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
> > > > > > > > > > Change the qemu parameters for upstream qemu vgas to the
> > > > > > > > > > new ones (-device), introduced some years ago.
> > > > > > > > > Ping
> > > > > > > > Please CC the qemu maintainers (Stefano & Anthony), although
> > > > > > > > these
> > > > > > > > patches touch the toolstack they are logically qemu patches and
> > > > > > > > need
> > > > > > > > their ack.
> > > > > > > Thanks for your reply, I added Anthony on CC and Stefano was
> > > > > > > already on it.
> > > > > > Should "None" be added as option as well ?
> > > > > > 
> > > > > There is already a nographic xl parameter that controls the
> > > > > corresponding qemu parameter, it should be the same thing.
> > > > > And FWIK the none option applies only to the old -vga parameter and
> > > > > according to qemu docs/qdev-device-use.txt, the new way to do it is
> > > > > with
> > > > > -nodefaults (I already made a patch to add it). -nographic is probably
> > > > > also deprecated.
> > > > In this conversation
> > > > http://comments.gmane.org/gmane.comp.emulators.qemu/172385
> > > > the -nographic is said to be deprecated by peter maydell.
> > > > Didn't know -vga none was also out the door already ...
> > 
> > > 
> > > I did some test and I found out that now is impossible disable the
> > > emulated vga on hvm domUs.
> > > I got cirrus vga (the default) even if vga xl parameter is not setted.
> > > Also with nographic enabled there is the emulated vga.
> > > I think that good solution is:
> > > - add this patch and nodefault patch
> > update: nodefault patch is already on git
> > > - add "none" option to vga xl parameter (that will exclude any emulated
> > > vga qemu parameters)
> > > - remove -nographic parameter in upstream qemu, making nographic xl
> > > parameter deprecated and equivalent to new vga="none" xl parameter.
> > > 
> > > With these changes all should be working and without qemu deprecated
> > > parameters (-nographic and -vga).
> > > I'm waiting Stefano and/or Anthony replies before write the patches.
> > > 
> > > Another question is about xenfb vga that seems missed on new -device
> > > parameter.
> > > I used it to have basic Spice support for pv working:
> > > http://lists.xen.org/archives/html/xen-devel/2013-09/msg03207.html
> > > Anyone can update about it on newer qemu versions please?
> > > 
> > > Thanks for any reply
> > 
> > Ping...
> 
> Another ping...


The change is good for me

> > 
> > > 
> > > > > > > > > > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> > > > > > > > > > ---
> > > > > > > > > >      tools/libxl/libxl_dm.c |   12 +++++-------
> > > > > > > > > >      1 file changed, 5 insertions(+), 7 deletions(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > > > > > > > index 43c3bec..2c6f5d9 100644
> > > > > > > > > > --- a/tools/libxl/libxl_dm.c
> > > > > > > > > > +++ b/tools/libxl/libxl_dm.c
> > > > > > > > > > @@ -486,15 +486,13 @@ static char **
> > > > > > > > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > > > > > > > >                   switch (b_info->u.hvm.vga.kind) {
> > > > > > > > > >              case LIBXL_VGA_INTERFACE_TYPE_STD:
> > > > > > > > > > -            flexarray_vappend(dm_args, "-vga", "std",
> > > > > > > > > > NULL);
> > > > > > > > > > +            flexarray_append_pair(dm_args, "-device",
> > > > > > > > > > "VGA");
> > > > > > > > > >                  break;
> > > > > > > > > >              case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> > > > > > > > > > -            flexarray_vappend(dm_args, "-vga", "cirrus",
> > > > > > > > > > NULL);
> > > > > > > > > > -            if (b_info->video_memkb) {
> > > > > > > > > > -                flexarray_vappend(dm_args, "-global",
> > > > > > > > > > - GCSPRINTF("vga.vram_size_mb=%d",
> > > > > > > > > > - libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
> > > > > > > > > > -            }
> > > > > > > > > > +            flexarray_append_pair(dm_args, "-device",
> > > > > > > > > > "cirrus-vga");
> > > > > > > > > > +            flexarray_append_pair(dm_args, "-global",
> > > > > > > > > > +                GCSPRINTF("vga.vram_size_mb=%d",
> > > > > > > > > > + libxl__sizekb_to_mb(b_info->video_memkb)));
> > > > > > > > > >                  break;
> > > > > > > > > >              }
> > > > > > 
> > > > > > 
> > > > 
> > > > 
> > > 
> > 
> 

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Fabio Fantoni]

Il 28/10/2013 17:45, Stefano Stabellini ha scritto:
> On Mon, 28 Oct 2013, Fabio Fantoni wrote:
>> Il 16/10/2013 14:03, Fabio Fantoni ha scritto:
>>> Il 10/10/2013 16:39, Fabio Fantoni ha scritto:
>>>> Il 10/10/2013 14:36, Sander Eikelenboom ha scritto:
>>>>> Thursday, October 10, 2013, 2:29:46 PM, you wrote:
>>>>>
>>>>>> Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
>>>>>>> Thursday, October 10, 2013, 12:09:44 PM, you wrote:
>>>>>>>
>>>>>>>> Il 10/10/2013 11:32, Ian Campbell ha scritto:
>>>>>>>>> On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
>>>>>>>>>> Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
>>>>>>>>>>> Change the qemu parameters for upstream qemu vgas to the
>>>>>>>>>>> new ones (-device), introduced some years ago.
>>>>>>>>>> Ping
>>>>>>>>> Please CC the qemu maintainers (Stefano & Anthony), although
>>>>>>>>> these
>>>>>>>>> patches touch the toolstack they are logically qemu patches and
>>>>>>>>> need
>>>>>>>>> their ack.
>>>>>>>> Thanks for your reply, I added Anthony on CC and Stefano was
>>>>>>>> already on it.
>>>>>>> Should "None" be added as option as well ?
>>>>>>>
>>>>>> There is already a nographic xl parameter that controls the
>>>>>> corresponding qemu parameter, it should be the same thing.
>>>>>> And FWIK the none option applies only to the old -vga parameter and
>>>>>> according to qemu docs/qdev-device-use.txt, the new way to do it is
>>>>>> with
>>>>>> -nodefaults (I already made a patch to add it). -nographic is probably
>>>>>> also deprecated.
>>>>> In this conversation
>>>>> http://comments.gmane.org/gmane.comp.emulators.qemu/172385
>>>>> the -nographic is said to be deprecated by peter maydell.
>>>>> Didn't know -vga none was also out the door already ...
>>>> I did some test and I found out that now is impossible disable the
>>>> emulated vga on hvm domUs.
>>>> I got cirrus vga (the default) even if vga xl parameter is not setted.
>>>> Also with nographic enabled there is the emulated vga.
>>>> I think that good solution is:
>>>> - add this patch and nodefault patch
>>> update: nodefault patch is already on git
>>>> - add "none" option to vga xl parameter (that will exclude any emulated
>>>> vga qemu parameters)
>>>> - remove -nographic parameter in upstream qemu, making nographic xl
>>>> parameter deprecated and equivalent to new vga="none" xl parameter.
>>>>
>>>> With these changes all should be working and without qemu deprecated
>>>> parameters (-nographic and -vga).
>>>> I'm waiting Stefano and/or Anthony replies before write the patches.
>>>>
>>>> Another question is about xenfb vga that seems missed on new -device
>>>> parameter.
>>>> I used it to have basic Spice support for pv working:
>>>> http://lists.xen.org/archives/html/xen-devel/2013-09/msg03207.html
>>>> Anyone can update about it on newer qemu versions please?
>>>>
>>>> Thanks for any reply
>>> Ping...
>> Another ping...
>
> The change is good for me

Thanks for reply.
Is this an ack for this patch?
Or/and is approval for a vga = "none" and the nographic fix for upstream 
qemu idea?
And about xenfb vga question someone can reply or I must write to 
qemu-devel?
Thanks for any reply.

>
>>>>>>>>>>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>>>>>>>>>> ---
>>>>>>>>>>>       tools/libxl/libxl_dm.c |   12 +++++-------
>>>>>>>>>>>       1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>>>>>>>>> index 43c3bec..2c6f5d9 100644
>>>>>>>>>>> --- a/tools/libxl/libxl_dm.c
>>>>>>>>>>> +++ b/tools/libxl/libxl_dm.c
>>>>>>>>>>> @@ -486,15 +486,13 @@ static char **
>>>>>>>>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>>>>>>>>                    switch (b_info->u.hvm.vga.kind) {
>>>>>>>>>>>               case LIBXL_VGA_INTERFACE_TYPE_STD:
>>>>>>>>>>> -            flexarray_vappend(dm_args, "-vga", "std",
>>>>>>>>>>> NULL);
>>>>>>>>>>> +            flexarray_append_pair(dm_args, "-device",
>>>>>>>>>>> "VGA");
>>>>>>>>>>>                   break;
>>>>>>>>>>>               case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>>>>>>>>>>> -            flexarray_vappend(dm_args, "-vga", "cirrus",
>>>>>>>>>>> NULL);
>>>>>>>>>>> -            if (b_info->video_memkb) {
>>>>>>>>>>> -                flexarray_vappend(dm_args, "-global",
>>>>>>>>>>> - GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>>>>>> - libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
>>>>>>>>>>> -            }
>>>>>>>>>>> +            flexarray_append_pair(dm_args, "-device",
>>>>>>>>>>> "cirrus-vga");
>>>>>>>>>>> +            flexarray_append_pair(dm_args, "-global",
>>>>>>>>>>> +                GCSPRINTF("vga.vram_size_mb=%d",
>>>>>>>>>>> + libxl__sizekb_to_mb(b_info->video_memkb)));
>>>>>>>>>>>                   break;
>>>>>>>>>>>               }
>>>>>>>
>>>>>

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Ian Campbell]

On Tue, 2013-10-29 at 09:51 +0100, Fabio Fantoni wrote:

> Thanks for reply.
> Is this an ack for this patch?

I've applied it.

Stefano, please can you say "Acked-by: ..." if you mean to formally ack
a patch, to save me having to guess...

Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces

[Stefano Stabellini]

On Tue, 29 Oct 2013, Fabio Fantoni wrote:
> Il 28/10/2013 17:45, Stefano Stabellini ha scritto:
> > On Mon, 28 Oct 2013, Fabio Fantoni wrote:
> > > Il 16/10/2013 14:03, Fabio Fantoni ha scritto:
> > > > Il 10/10/2013 16:39, Fabio Fantoni ha scritto:
> > > > > Il 10/10/2013 14:36, Sander Eikelenboom ha scritto:
> > > > > > Thursday, October 10, 2013, 2:29:46 PM, you wrote:
> > > > > > 
> > > > > > > Il 10/10/2013 12:51, Sander Eikelenboom ha scritto:
> > > > > > > > Thursday, October 10, 2013, 12:09:44 PM, you wrote:
> > > > > > > > 
> > > > > > > > > Il 10/10/2013 11:32, Ian Campbell ha scritto:
> > > > > > > > > > On Thu, 2013-10-10 at 11:26 +0200, Fabio Fantoni wrote:
> > > > > > > > > > > Il 30/09/2013 12:12, Fabio Fantoni ha scritto:
> > > > > > > > > > > > Change the qemu parameters for upstream qemu vgas to the
> > > > > > > > > > > > new ones (-device), introduced some years ago.
> > > > > > > > > > > Ping
> > > > > > > > > > Please CC the qemu maintainers (Stefano & Anthony), although
> > > > > > > > > > these
> > > > > > > > > > patches touch the toolstack they are logically qemu patches
> > > > > > > > > > and
> > > > > > > > > > need
> > > > > > > > > > their ack.
> > > > > > > > > Thanks for your reply, I added Anthony on CC and Stefano was
> > > > > > > > > already on it.
> > > > > > > > Should "None" be added as option as well ?
> > > > > > > > 
> > > > > > > There is already a nographic xl parameter that controls the
> > > > > > > corresponding qemu parameter, it should be the same thing.
> > > > > > > And FWIK the none option applies only to the old -vga parameter
> > > > > > > and
> > > > > > > according to qemu docs/qdev-device-use.txt, the new way to do it
> > > > > > > is
> > > > > > > with
> > > > > > > -nodefaults (I already made a patch to add it). -nographic is
> > > > > > > probably
> > > > > > > also deprecated.
> > > > > > In this conversation
> > > > > > http://comments.gmane.org/gmane.comp.emulators.qemu/172385
> > > > > > the -nographic is said to be deprecated by peter maydell.
> > > > > > Didn't know -vga none was also out the door already ...
> > > > > I did some test and I found out that now is impossible disable the
> > > > > emulated vga on hvm domUs.
> > > > > I got cirrus vga (the default) even if vga xl parameter is not setted.
> > > > > Also with nographic enabled there is the emulated vga.
> > > > > I think that good solution is:
> > > > > - add this patch and nodefault patch
> > > > update: nodefault patch is already on git
> > > > > - add "none" option to vga xl parameter (that will exclude any
> > > > > emulated
> > > > > vga qemu parameters)
> > > > > - remove -nographic parameter in upstream qemu, making nographic xl
> > > > > parameter deprecated and equivalent to new vga="none" xl parameter.
> > > > > 
> > > > > With these changes all should be working and without qemu deprecated
> > > > > parameters (-nographic and -vga).
> > > > > I'm waiting Stefano and/or Anthony replies before write the patches.
> > > > > 
> > > > > Another question is about xenfb vga that seems missed on new -device
> > > > > parameter.
> > > > > I used it to have basic Spice support for pv working:
> > > > > http://lists.xen.org/archives/html/xen-devel/2013-09/msg03207.html
> > > > > Anyone can update about it on newer qemu versions please?
> > > > > 
> > > > > Thanks for any reply
> > > > Ping...
> > > Another ping...
> > 
> > The change is good for me
> 
> Thanks for reply.
> Is this an ack for this patch?

Sorry for the late reply, I have been traveling for the last 2 weeks.
Yes, this was an ack for the patch.


> Or/and is approval for a vga = "none" and the nographic fix for upstream qemu
> idea?

I would be happy with a vga = "none" xl setting.


> And about xenfb vga question someone can reply or I must write to qemu-devel?
> Thanks for any reply.

It doesn't make too much sense to have xenfv via -device, because xenfb
is configured and initialized depending on xenstore entries.
The only advantage would be not having to set one xenstore watch in case
you don't want a xenfb device.

Do you actually have a use case for -device xenfb?


> > > > > > > > > > > > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> > > > > > > > > > > > ---
> > > > > > > > > > > >       tools/libxl/libxl_dm.c |   12 +++++-------
> > > > > > > > > > > >       1 file changed, 5 insertions(+), 7 deletions(-)
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/tools/libxl/libxl_dm.c
> > > > > > > > > > > > b/tools/libxl/libxl_dm.c
> > > > > > > > > > > > index 43c3bec..2c6f5d9 100644
> > > > > > > > > > > > --- a/tools/libxl/libxl_dm.c
> > > > > > > > > > > > +++ b/tools/libxl/libxl_dm.c
> > > > > > > > > > > > @@ -486,15 +486,13 @@ static char **
> > > > > > > > > > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > > > > > > > > > >                    switch (b_info->u.hvm.vga.kind) {
> > > > > > > > > > > >               case LIBXL_VGA_INTERFACE_TYPE_STD:
> > > > > > > > > > > > -            flexarray_vappend(dm_args, "-vga", "std",
> > > > > > > > > > > > NULL);
> > > > > > > > > > > > +            flexarray_append_pair(dm_args, "-device",
> > > > > > > > > > > > "VGA");
> > > > > > > > > > > >                   break;
> > > > > > > > > > > >               case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> > > > > > > > > > > > -            flexarray_vappend(dm_args, "-vga",
> > > > > > > > > > > > "cirrus",
> > > > > > > > > > > > NULL);
> > > > > > > > > > > > -            if (b_info->video_memkb) {
> > > > > > > > > > > > -                flexarray_vappend(dm_args, "-global",
> > > > > > > > > > > > - GCSPRINTF("vga.vram_size_mb=%d",
> > > > > > > > > > > > - libxl__sizekb_to_mb(b_info->video_memkb)), NULL);
> > > > > > > > > > > > -            }
> > > > > > > > > > > > +            flexarray_append_pair(dm_args, "-device",
> > > > > > > > > > > > "cirrus-vga");
> > > > > > > > > > > > +            flexarray_append_pair(dm_args, "-global",
> > > > > > > > > > > > +                GCSPRINTF("vga.vram_size_mb=%d",
> > > > > > > > > > > > + libxl__sizekb_to_mb(b_info->video_memkb)));
> > > > > > > > > > > >                   break;
> > > > > > > > > > > >               }
> > > > > > > > 
> > > > > > 
>