[Xen-devel] Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>]

Stefano Stabellini wrote:
> On Tue, 8 Dec 2009, Andres Lagar-Cavilla wrote:
>   
> > Hi,
> > couple of comments:
> > - PV domains without videoram won't be able to use this
> >     
> PV domains just have videoram = 0.
>   
But you abort libxl_set_memory_target if the videoram node is not found. 
Which won't be for PVs with no videoram...
+    videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, 
"%s/memory/videoram", dompath));
+    if (!videoram_s)
+        return -1;

Andres
>   
> > - Further, doesn't the PV domain build function need to use 
> > target_memkb? (That's my read of what xend does at least)
> >     
> Yes, you are right, I'll fix this.
>
>   
> > - Finally, LIBXL_MAXMEM_CONSTANT looks like an "evil constant we should 
> > avoid". Where did it come from?
> >     
> I decided to introduce this constant after a discussion with developers
> of the memory management functions in xapi: after thorough testing they
> found that adding 1 MB to maxmem increases the robustness of the system.
>
> BTW the current value of the constant is wrong because it should be
> expressed in KB, I'll send a patch to fix this later today.
>
>   
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel