From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fabio Fantoni Subject: Re: [PATCH] libxl: new parameters for upstream qemu's vga interfaces Date: Thu, 10 Oct 2013 13:58:11 +0200 Message-ID: <52569653.1010202@m2r.biz> References: <1380535957-4379-1-git-send-email-fabio.fantoni@m2r.biz> <525672D4.709@m2r.biz> <1381397544.7600.53.camel@kazak.uk.xensource.com> <52567CE8.7060003@m2r.biz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: George.Dunlap@eu.citrix.com, Anthony PERARD , xen-devel@lists.xensource.com, Ian.Jackson@eu.citrix.com, Ian Campbell List-Id: xen-devel@lists.xenproject.org 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 >>>>> --- >>>>> 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.