[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Qemu-devel] [PATCH] increase maxmem before calling xc_domain_populate_physmap




On 11/26/14 13:17, Stefano Stabellini wrote:
On Tue, 25 Nov 2014, Andrew Cooper wrote:
On 25/11/14 17:45, Stefano Stabellini wrote:
Increase maxmem before calling xc_domain_populate_physmap_exact to avoid
the risk of running out of guest memory. This way we can also avoid
complex memory calculations in libxl at domain construction time.

This patch fixes an abort() when assigning more than 4 NICs to a VM.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

diff --git a/xen-hvm.c b/xen-hvm.c
index 5c69a8d..38e08c3 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -218,6 +218,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr)
      unsigned long nr_pfn;
      xen_pfn_t *pfn_list;
      int i;
+    xc_dominfo_t info;
if (runstate_check(RUN_STATE_INMIGRATE)) {
          /* RAM already populated in Xen */
@@ -240,6 +241,13 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr)
          pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
      }
+ if (xc_domain_getinfo(xen_xc, xen_domid, 1, &info) < 0) {
xc_domain_getinfo()'s interface is mad, and provides no guarantee that
it returns the information for the domain you requested.  It also won't
return -1 on error.  The correct error handing is:

(xc_domain_getinfo(xen_xc, xen_domid, 1, &info) != 1) || (info.domid !=
xen_domid)
It might be wiser to switch to xc_domain_getinfolist

Either needs the same tests, since both return an vector of info.


~Andrew

+        hw_error("xc_domain_getinfo failed");
+    }
+    if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
+                            (nr_pfn * XC_PAGE_SIZE / 1024)) < 0) {

There are two big issues and 1 minor one with this.
1) You will allocate the videoram again.
2) You will never use the 1 MB already allocated for option ROMs.

And the minor one is that you can increase maxmem more then is needed.


Here is a better if:

-    if (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
-                            (nr_pfn * XC_PAGE_SIZE / 1024)) < 0) {
+    max_pages = info.max_memkb * 1024 / XC_PAGE_SIZE;
+    free_pages = max_pages - info.nr_pages;
+    need_pages = nr_pfn - free_pages;
+    if ((free_pages < nr_pfn) &&
+       (xc_domain_setmaxmem(xen_xc, xen_domid, info.max_memkb +
+                            (need_pages * XC_PAGE_SIZE / 1024)) < 0)) {


My testing shows a free 32 pages that I am not sure where they come from. But
the code about is passing my 8 nics of e1000.


   -Don Slutz


+        hw_error("xc_domain_setmaxmem failed");
+    }
      if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, 
pfn_list)) {
          hw_error("xen: failed to populate ram at " RAM_ADDR_FMT, ram_addr);
      }

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.