Xen devel - Aug 2010 - [PATCH 1 of 8] libxl: Fix xcinfo2xlinfo to properly set max_memkb and target_memkb

libxl: Fix xcinfo2xlinfo to properly set max_memkb and target_memkb

currently xcinfo2xlinfo reads tot_pages and uses that data to calculate
max_memkb, while tot_pages is the memory currently used by the domain
and max_pages is the theoretical maximum.

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

diff -r eccfdeb41b80 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Aug 24 18:42:59 2010 +0100
+++ b/tools/libxl/libxl.c	Wed Aug 25 20:59:35 2010 +0100
@@ -539,7 +539,8 @@ static void xcinfo2xlinfo(const xc_domai
     else
         xlinfo->shutdown_reason  = ~0;
 
-    xlinfo->max_memkb = PAGE_TO_MEMKB(xcinfo->tot_pages);
+    xlinfo->target_memkb = PAGE_TO_MEMKB(xcinfo->tot_pages);
+    xlinfo->max_memkb = PAGE_TO_MEMKB(xcinfo->max_pages);
     xlinfo->cpu_time = xcinfo->cpu_time;
     xlinfo->vcpu_max_id = xcinfo->max_vcpu_id;
     xlinfo->vcpu_online = xcinfo->nr_online_vcpus;
diff -r eccfdeb41b80 tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl	Tue Aug 24 18:42:59 2010 +0100
+++ b/tools/libxl/libxl.idl	Wed Aug 25 20:59:35 2010 +0100
@@ -36,6 +36,7 @@ libxl_dominfo = Struct("dominfo",[
 
 Otherwise set to a value guaranteed not to clash with any valid
 SHUTDOWN_* constant."""),
+    ("target_memkb",   uint64),
     ("max_memkb",   uint64),
     ("cpu_time",    uint64),
     ("vcpu_max_id", uint32),
diff -r eccfdeb41b80 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue Aug 24 18:42:59 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Wed Aug 25 20:59:35 2010 +0100
@@ -2209,7 +2209,7 @@ static void list_domains(int verbose, co
         printf("%-40s %5d %5lu %5d     %c%c%c%c%c%c  %8.1f",
                 domname,
                 info[i].domid,
-                (unsigned long) (info[i].max_memkb / 1024),
+                (unsigned long) (info[i].target_memkb / 1024),
                 info[i].vcpu_online,
                 info[i].running ? ''r'' :
''-'',
                 info[i].blocked ? ''b'' :
''-'',

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

Stefano Stabellini writes ("[Xen-devel] [PATCH 1 of 8] libxl: Fix
xcinfo2xlinfo to properly set max_memkb and
target_memkb"):
> libxl: Fix xcinfo2xlinfo to properly set max_memkb and target_memkb
> 
> currently xcinfo2xlinfo reads tot_pages and uses that data to calculate
> max_memkb, while tot_pages is the memory currently used by the domain
> and max_pages is the theoretical maximum.
> +    xlinfo->target_memkb = PAGE_TO_MEMKB(xcinfo->tot_pages);
Um, I don''t think this is right, is it ?  If "target" means
"memory
target eg for domain balloon driver".

The memory target can be read only from xenstore.  But in general if
we are sane we always set the hypervisor''s maximum to the same as the
ballooning target.  So I''m not sure having separate "max" and
"total"
values visible to libxl callers is correct.  There two values visible
to libxl callers should be "target" and "current".

Ian.

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

On Tue, 31 Aug 2010, Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] [PATCH 1 of 8] libxl: Fix
xcinfo2xlinfo to properly set max_memkb and target_memkb"):
> > libxl: Fix xcinfo2xlinfo to properly set max_memkb and target_memkb
> > 
> > currently xcinfo2xlinfo reads tot_pages and uses that data to
calculate
> > max_memkb, while tot_pages is the memory currently used by the domain
> > and max_pages is the theoretical maximum.
> 
> > +    xlinfo->target_memkb = PAGE_TO_MEMKB(xcinfo->tot_pages);
> 
> Um, I don''t think this is right, is it ?  If "target"
means "memory
> target eg for domain balloon driver".
> 
"target" means memory currently used by the domain.
I should probably rename xlinfo->target_memkb to xlinfo->current_memkb

> The memory target can be read only from xenstore.  But in general if
> we are sane we always set the hypervisor''s maximum to the same as
the
> ballooning target.  So I''m not sure having separate
"max" and "total"
> values visible to libxl callers is correct.  There two values visible
> to libxl callers should be "target" and "current".
> 
 
Unfortunately it is not that simple, in particular a slack is needed on
top of the memory target (see LIBXL_MAXMEM_CONSTANT).
But I could probably remove xlinfo->max_memkb anyway because libxl
internally can always call the libxc function directly and know exactly
what maxmem is. While libxl callers theoretically could do without this
information.

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