Re: [Xen-devel] [PATCH 1/3] libxl:refactor stdvga option support v2

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-06-28 at 02:42 +0100, ZhouPeng wrote:
> On Wed, Jun 27, 2012 at 11:28 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 
> wrote:
> >> diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c
> >> --- a/tools/libxl/xl_cmdimpl.c  Fri May 18 16:19:21 2012 +0100
> >> +++ b/tools/libxl/xl_cmdimpl.c  Wed Jun 27 20:06:39 2012 +0800
> >> @@ -1256,7 +1256,10 @@ skip_vfb:
> >>  #undef parse_extra_args
> >>
> >>      if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> >> -        xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0);
> >> +        if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
> >> +            if (l)
> >> +                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
> >
> > I think this needs to be:
> >          if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
> >                b_info->u.hvm.vga.kind = l ? \
> >                                         LIBXL_VGA_INTERFACE_TYPE_STD : \
> >                                         LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> >
> > so that both "stdvga=0" and "stdvga=1" result in what the user asked for
> > without relying on the libxl default remaining CIRRUS.
> 
> IMHO, this make simple to be complex.
> 
> I think the check  and set-default later in libxl_create.c as a whole is 
> enough.

This is in libxl, as I said above xl should not rely on the default
remaining CIRRUS.

If the user says "stdvga=0" then xl needs to explicitly ask for CIRRUS,
despite the fact that it currently happens to be the default.

If we make the libxl default BLARGLE in the future then stdvga=0 still
needs to mean CIRRUS.

> +        if (!b_info->u.hvm.vga.kind)
> +            b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> 
> 'as a whole' here, I means if more vga type added in the future, we
> don't need to set the default one by one, redundantly.


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