Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
On Mon, 24 Nov 2014, Don Slutz wrote:
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 +, Stefano Stabellini wrote:
On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
On Fri, Nov 21, 2014 at 06:48:53PM +, Stefano Stabellini
wrote:
On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
On Fri, Nov 21, 2014 at 05:11:09PM +, 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 4 bytes (256 Kib) of ram at 4201
mr.name=rtl8139.rom
e1000 vhdr enabled
xen_ram_alloc: alloc 4 bytes (256 Kib) of ram at 4205
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.stabell...@eu.citrix.com
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.
Good suggestion.
Also I think that LIBXL_MAXMEM_CONSTANT can be dropped here.
Probably, but I don't know whether there are any other hidden
allocations that would begin to fail if I removed LIBXL_MAXMEM_CONSTANT.
I think it is best to leave it as is for the moment and maybe remove it
at the beginning of the next release cycle?
I found out that
#define LIBXL_HVM_EXTRA_MEMORY 2048
Should be
#define LIBXL_HVM_EXTRA_MEMORY (LIBXL_MAXMEM_CONSTANT + 1024)
OK
if you change the size of the ROM memory for QEMU. I have only done the
static
change.
I don't understand what you mean here. Yes, if the ROM size changes in
QEMU we'll have an issue, specifically if it increases. However I don't
know how to solve that. I guess it would be another reason to keep
LIBXL_MAXMEM_CONSTANT: have an extra little buffer.
-Don Slutz
if (rc != 0) {
Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote: On Fri, Nov 21, 2014 at 06:48:53PM +, Stefano Stabellini wrote: On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote: On Fri, Nov 21, 2014 at 05:11:09PM +, 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. What if you also do PCI passthrough? Does that figure in this calculation? In the case of PCI passthrough, roms are remapped, so they shouldn't affect memory consumption. TBH not all the emulated nics need a romfile but I wouldn't want to go down at that level of details beause they become QEMU implementation details. I wouldn't want to tie libxl to a certain version of QEMU in any way. A better way would be handling memory allocation errors in QEMU but QEMU doesn't do that at the moment and the change there would be certainly more invasive that a couple of lines in libxl. memory and as a matter of fact breaks configurations with more than 4 NICs as QEMU fails to allocate memory on behalf of the guest. What if you have four different type of NICs? Say 1 rlt8193, 1 e1000, one eepro, and ne2k ? Don't you want to load the ROM for each one? The rom cannot be reused in QEMU among multiple instances of the same nic, so having four different types of nics or just one type doesn't change anything. With this fix, it is possible to assign more than 4 rtl8139 NICs to the guest. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 3e191c3..f907ca9 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -674,9 +674,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, LIBXL_NIC_TYPE_VIF_IOEMU); flexarray_append(dm_args, -device); flexarray_append(dm_args, - libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s, + libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s%s, nics[i].model, nics[i].devid, -nics[i].devid, smac)); +nics[i].devid, smac, +i ? ,romfile=\\ : )); flexarray_append(dm_args, -netdev); flexarray_append(dm_args, GCSPRINTF( type=tap,id=net%d,ifname=%s, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
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 +, Stefano Stabellini wrote:
On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
On Fri, Nov 21, 2014 at 06:48:53PM +, Stefano Stabellini wrote:
On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote:
On Fri, Nov 21, 2014 at 05:11:09PM +, 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 4 bytes (256 Kib) of ram at 4201
mr.name=rtl8139.rom
e1000 vhdr enabled
xen_ram_alloc: alloc 4 bytes (256 Kib) of ram at 4205
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.stabell...@eu.citrix.com
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,
Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
On Fri, Nov 21, 2014 at 05:11:09PM +, 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? memory and as a matter of fact breaks configurations with more than 4 NICs as QEMU fails to allocate memory on behalf of the guest. What if you have four different type of NICs? Say 1 rlt8193, 1 e1000, one eepro, and ne2k ? Don't you want to load the ROM for each one? With this fix, it is possible to assign more than 4 rtl8139 NICs to the guest. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 3e191c3..f907ca9 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -674,9 +674,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, LIBXL_NIC_TYPE_VIF_IOEMU); flexarray_append(dm_args, -device); flexarray_append(dm_args, - libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s, + libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s%s, nics[i].model, nics[i].devid, -nics[i].devid, smac)); +nics[i].devid, smac, +i ? ,romfile=\\ : )); flexarray_append(dm_args, -netdev); flexarray_append(dm_args, GCSPRINTF( type=tap,id=net%d,ifname=%s, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote: On Fri, Nov 21, 2014 at 05:11:09PM +, 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). TBH not all the emulated nics need a romfile but I wouldn't want to go down at that level of details beause they become QEMU implementation details. I wouldn't want to tie libxl to a certain version of QEMU in any way. A better way would be handling memory allocation errors in QEMU but QEMU doesn't do that at the moment and the change there would be certainly more invasive that a couple of lines in libxl. memory and as a matter of fact breaks configurations with more than 4 NICs as QEMU fails to allocate memory on behalf of the guest. What if you have four different type of NICs? Say 1 rlt8193, 1 e1000, one eepro, and ne2k ? Don't you want to load the ROM for each one? The rom cannot be reused in QEMU among multiple instances of the same nic, so having four different types of nics or just one type doesn't change anything. With this fix, it is possible to assign more than 4 rtl8139 NICs to the guest. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 3e191c3..f907ca9 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -674,9 +674,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, LIBXL_NIC_TYPE_VIF_IOEMU); flexarray_append(dm_args, -device); flexarray_append(dm_args, - libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s, + libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s%s, nics[i].model, nics[i].devid, -nics[i].devid, smac)); +nics[i].devid, smac, +i ? ,romfile=\\ : )); flexarray_append(dm_args, -netdev); flexarray_append(dm_args, GCSPRINTF( type=tap,id=net%d,ifname=%s, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
On Fri, Nov 21, 2014 at 06:48:53PM +, Stefano Stabellini wrote: On Fri, 21 Nov 2014, Konrad Rzeszutek Wilk wrote: On Fri, Nov 21, 2014 at 05:11:09PM +, 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? What if you also do PCI passthrough? Does that figure in this calculation? TBH not all the emulated nics need a romfile but I wouldn't want to go down at that level of details beause they become QEMU implementation details. I wouldn't want to tie libxl to a certain version of QEMU in any way. A better way would be handling memory allocation errors in QEMU but QEMU doesn't do that at the moment and the change there would be certainly more invasive that a couple of lines in libxl. memory and as a matter of fact breaks configurations with more than 4 NICs as QEMU fails to allocate memory on behalf of the guest. What if you have four different type of NICs? Say 1 rlt8193, 1 e1000, one eepro, and ne2k ? Don't you want to load the ROM for each one? The rom cannot be reused in QEMU among multiple instances of the same nic, so having four different types of nics or just one type doesn't change anything. With this fix, it is possible to assign more than 4 rtl8139 NICs to the guest. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 3e191c3..f907ca9 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -674,9 +674,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, LIBXL_NIC_TYPE_VIF_IOEMU); flexarray_append(dm_args, -device); flexarray_append(dm_args, - libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s, + libxl__sprintf(gc, %s,id=nic%d,netdev=net%d,mac=%s%s, nics[i].model, nics[i].devid, -nics[i].devid, smac)); +nics[i].devid, smac, +i ? ,romfile=\\ : )); flexarray_append(dm_args, -netdev); flexarray_append(dm_args, GCSPRINTF( type=tap,id=net%d,ifname=%s, ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
