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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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@xxxxxxx>
> > > > > > > > > > > > ---
> > > > > > > > > > > >       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;
> > > > > > > > > > > >               }
> > > > > > > > 
> > > > > > 
> 

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