Re: [Xen-devel] [PATCH] xl: create VFB for PV guest when VNC is specified

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH] xl: create VFB for PV guest when VNC is 
specified"):
> On Mon, 2013-12-16 at 12:17 +0000, Ian Jackson wrote:
> > This should be xrealloc, and wrapped to 80 columns.  But see below.
> 
> This also suggests that we might be trying to support both the
> "toplevel" vnc options and the "vfb = []" style at the same time.
> 
> IMHO the vfb option should take precedence -- i.e. we should ignore the
> toplevel options if it is present.

Oh, yes.  Good point.

> The alternative would be some sort of union of the toplevel options and
> the first vfb given, but that sounds a bit mad...
> 
> I suppose there is also the question of what xend did here.

Perhaps Konrad can let us know...

> > There's a lot of this kind of boilerplate.  I'm tempted to suggest
> > making a macro to do this.  Searching for "devid =" found 4 call sites
> > if that line is included in the macro; searching for "realloc" found
> > me 6 call sites if it isn't.  And that's before your two additional
> > ones.  Ian C, what do you think ?
> 
> Some helpers for dealing with allocating and resizing/appending to libxl
> Array types might be a useful addition to the library itself, i.e. idl
> generated? Otherwise an xl macro might indeed be handy.
> 
> I'm not sure what the 4 vs. 6 "if that line is/isn't included in" is
> referring too though.

There are four instances of this kind of boilerplate which contain
            SPONGE->devid = d_config->num_SPONGEs;
and six that lack it.

> > This duplicates the HVM VNC option parsing.  It should be factored out
> > into a subroutine.
> 
> Ack.
> 
> That makes me wonder if the top level options shouldn't be populating
> &b_info->u.pv.vnc.* (and if that shouldn't have been a non-keyed field)
> while vfb = fills in d_config->vfbs. That is a bigger change on the
> libxl side though and maybe doesn't make quite as much sense as with
> the .

As with the "." ?

Ian.

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