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

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



On 11/24/14 12:07, Stefano Stabellini wrote:
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>

This looks to be a good idea, just not fully done.

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;

2 return statements?

+}
+
  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));

Here (and the rest) need to check for error's.  Also I think that
LIBXL_MAXMEM_CONSTANT can be dropped here.  I found out that

#define LIBXL_HVM_EXTRA_MEMORY 2048

Should be

#define LIBXL_HVM_EXTRA_MEMORY (LIBXL_MAXMEM_CONSTANT + 1024)

if you change the size of the ROM memory for QEMU. I have only done the static
change.

   -Don Slutz


      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


 


Rackspace

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