Home
Reading
Searching
Subscribe
Sponsors
Statistics
Posting
Contact
Spam
Lists
Links
About
Hosting
Filtering
Features Download
Marketing
Archives
FAQ
Blog
 
Gmane

From: Don Slutz <dslutz <at> verizon.com>
Subject: Re: [PATCH for-4.5] libxl: do not load roms for any NICs except the first to avoid wasting memory
Newsgroups: gmane.comp.emulators.xen.devel
Date: Monday 24th November 2014 20:35:58 UTC (over 4 years ago)
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 

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);
 
CD: 4ms