Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 24 Nov 2014, Don Slutz wrote:
> On 11/24/14 10:26, Stefano Stabellini wrote:
> > On Mon, 24 Nov 2014, Konrad Rzeszutek Wilk wrote:
> > > On Mon, Nov 24, 2014 at 12:17:12PM +0000, Stefano Stabellini wrote:
> > > > On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
> > > > > On Fri, Nov 21, 2014 at 06:48:53PM +0000, Stefano Stabellini wrote:
> > > > > > On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
> > > > > > > On Fri, Nov 21, 2014 at 05:11:09PM +0000, Stefano Stabellini
> > > > > > > wrote:
> > > > > > > > The rom is used for pxebooting. We don't need to allow
> > > > > > > > pxebooting from
> > > > > > > > more than one network card.  Loading a romfile for every NIC
> > > > > > > > wastes
> > > > > > > Why not? Why can't we PXE boot from each network card?
> > > > > > I take it back: you are right, at the moment it is actually possible
> > > > > > to
> > > > > > pxe boot from the fourth nic for example. Maybe we could just load
> > > > > > the
> > > > > > first four romfiles and skip the others (four is the limit before
> > > > > > QEMU
> > > > > > fails to allocate any more memory).
> > > > > The limit is in the count of ROMs or the memory consumption?
> > > > The limit is memory consumption.
> > > Um, how big are those ROMs? Gigs?
> > I think they are 60K each in the case of rtl8139.
> > However they are accounted on top of the ram of the guest, that's why
> > the allocation fails. Strictly speaking I guess it shouldn't work even
> > the first time around.
> 
> My extra QEMU debug says:
> 
> xen_ram_alloc: alloc 40000 bytes (256 Kib) of ram at 42010000
> mr.name=rtl8139.rom
> e1000 vhdr enabled
> xen_ram_alloc: alloc 40000 bytes (256 Kib) of ram at 42050000
> mr.name=e1000.rom
> 
> And that matches the max of 4.
> 
> #define LIBXL_MAXMEM_CONSTANT 1024
> 
> is what controls this.
> 
> > Maybe the bug is in libxl memory allocation, that doesn't account for
> > rom sizes.
> 
> Yes.

Here is a better patch.

---

libxl: account for romfile memory

Account for memory needed for emulated network card rom files.
Assume 256K for each romfile.

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

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index de23fec..2edb194 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4527,6 +4527,33 @@ out:
 
 
/******************************************************************************/
 
+int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, 
libxl_domain_config *d_config)
+{
+    int i, count_rom, rc;
+    libxl_domain_config local_d_config;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+
+    if (d_config == NULL) {
+        libxl_domain_config_init(&local_d_config);
+        rc = libxl_retrieve_domain_configuration(ctx, domid, &local_d_config);
+        if (rc < 0)
+            return rc;
+        d_config = &local_d_config;
+    }
+
+    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV)
+        return 0;
+
+    for (i = 0, count_rom = 0; i < d_config->num_nics; i++) {
+        if (d_config->nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU)
+            count_rom++;
+    }
+
+    return count_rom*LIBXL_ROMSIZE_KB;
+
+    return 0;
+}
+
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
 {
     GC_INIT(ctx);
@@ -4550,11 +4577,14 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t 
domid, uint32_t max_memkb)
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "memory_static_max must be 
greater than or or equal to memory_dynamic_max\n");
         goto out;
     }
-    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb + 
LIBXL_MAXMEM_CONSTANT);
+    rc = xc_domain_setmaxmem(ctx->xch, domid,
+                             max_memkb + LIBXL_MAXMEM_CONSTANT
+                             + libxl__get_rom_memory_kb(gc, domid, NULL));
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                 "xc_domain_setmaxmem domid=%d memkb=%d failed "
-                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT, rc);
+                "rc=%d\n", domid, max_memkb + LIBXL_MAXMEM_CONSTANT +
+                libxl__get_rom_memory_kb(gc, domid, NULL), rc);
         goto out;
     }
 
@@ -4770,11 +4800,12 @@ retry_transaction:
     if (enforce) {
         memorykb = new_target_memkb;
         rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
-                LIBXL_MAXMEM_CONSTANT);
+                LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, 
NULL));
         if (rc != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
                     "xc_domain_setmaxmem domid=%d memkb=%d failed "
-                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
+                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT +
+                    libxl__get_rom_memory_kb(gc, domid, NULL), rc);
             abort_transaction = 1;
             goto out;
         }
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 74ea84b..ca254f9 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -406,7 +406,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
     }
 
     if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
-        LIBXL_MAXMEM_CONSTANT) < 0) {
+        LIBXL_MAXMEM_CONSTANT + libxl__get_rom_memory_kb(gc, domid, d_config)) 
< 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
         return ERROR_FAIL;
     }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4361421..33826ea 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -90,6 +90,7 @@
 #define LIBXL_XENCONSOLE_LIMIT 1048576
 #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
 #define LIBXL_MAXMEM_CONSTANT 1024
+#define LIBXL_ROMSIZE_KB 256
 #define LIBXL_PV_EXTRA_MEMORY 1024
 #define LIBXL_HVM_EXTRA_MEMORY 2048
 #define LIBXL_MIN_DOM0_MEM (128*1024)
@@ -1023,6 +1024,12 @@ _hidden char * libxl__domain_pvcontrol_read(libxl__gc 
*gc,
 _hidden int libxl__domain_pvcontrol_write(libxl__gc *gc, xs_transaction_t t,
                                           uint32_t domid, const char *cmd);
 
+/* Returns the amount of extra mem required to allocate roms or an libxl
+ * error code on error.
+ * The *d_config parameter is optional.
+ */
+_hidden int libxl__get_rom_memory_kb(libxl__gc *gc, uint32_t domid, 
libxl_domain_config *d_config);
+
 /* from xl_device */
 _hidden char *libxl__device_disk_string_of_backend(libxl_disk_backend backend);
 _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);

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