Xen devel - Oct 2010 - [PATCH]: libxl: fix dom0 minimum memory threshold check

libxl_set_memory_target tries to set the memory target for a given
domain to new_target_memkb. The function includes a check to make sure
that dom0''s memory is not reduced below a minimal threshold ie.
LIBXL_MIN_DOM0_MEM. However, this check is performed before the
new_target_memkb variable is properly initialised - when the value is
always zero. This means that the check always fails. Fix this by moving
the test to happen after the proper initialisation of new_target_memkb.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 0dc0bc411035 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Oct 21 18:51:36 2010 +0100
+++ b/tools/libxl/libxl.c	Fri Oct 22 13:44:56 2010 +0100
@@ -2927,12 +2927,6 @@ retry_transaction:
         abort = 1;
         goto out;
     }
-    if (!domid && new_target_memkb < LIBXL_MIN_DOM0_MEM) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                "new target for dom0 is below the minimum
threshold\n");
-        abort = 1;
-        goto out;
-    }
 
     if (relative)
         new_target_memkb = current_target_memkb + target_memkb;
@@ -2946,6 +2940,13 @@ retry_transaction:
         goto out;
     }
 
+    if (!domid && new_target_memkb < LIBXL_MIN_DOM0_MEM) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                "new target %d for dom0 is below the minimum
threshold\n",
+                 new_target_memkb);
+        abort = 1;
+        goto out;
+    }
     videoram_s = libxl__xs_read(&gc, t, libxl__sprintf(&gc,
                 "%s/memory/videoram", dompath));
     videoram = videoram_s ? atoi(videoram_s) : 0;



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 22 Oct 2010, Gianni Tedesco wrote:
> libxl_set_memory_target tries to set the memory target for a given
> domain to new_target_memkb. The function includes a check to make sure
> that dom0''s memory is not reduced below a minimal threshold ie.
> LIBXL_MIN_DOM0_MEM. However, this check is performed before the
> new_target_memkb variable is properly initialised - when the value is
> always zero. This means that the check always fails. Fix this by moving
> the test to happen after the proper initialisation of new_target_memkb.
> 
> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> 
I wonder what caused this block misplacement, maybe patch or hg import?
Anyway:

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2010-10-22 at 15:04 +0100, Stefano Stabellini
wrote:
> On Fri, 22 Oct 2010, Gianni Tedesco wrote:
> > libxl_set_memory_target tries to set the memory target for a given
> > domain to new_target_memkb. The function includes a check to make sure
> > that dom0''s memory is not reduced below a minimal threshold
ie.
> > LIBXL_MIN_DOM0_MEM. However, this check is performed before the
> > new_target_memkb variable is properly initialised - when the value is
> > always zero. This means that the check always fails. Fix this by
moving
> > the test to happen after the proper initialisation of
new_target_memkb.
> > 
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> > 
> 
> I wonder what caused this block misplacement, maybe patch or hg import?
> Anyway:
FWIW, I blame hg, I''ve seen it do similar things before!

Gianni



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Gianni Tedesco writes ("[Xen-devel] [PATCH]: libxl: fix dom0 minimum memory
threshold check"):
> libxl_set_memory_target tries to set the memory target for a given
> domain to new_target_memkb. The function includes a check to make sure
> that dom0''s memory is not reduced below a minimal threshold ie.
> LIBXL_MIN_DOM0_MEM. However, this check is performed before the
> new_target_memkb variable is properly initialised - when the value is
> always zero. This means that the check always fails. Fix this by moving
> the test to happen after the proper initialisation of new_target_memkb.
Applied, thanks.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel