Re: [PATCH] libxl: fix cirrus vga video memory setting with upstream qemu

[Ian Campbell <Ian.Campbell@citrix.com>]

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.