Re: [PATCH v2 for-4.5] libxl: account for romfile memory

[Don Slutz <dslutz@verizon.com>]

On 11/25/14 07:43, Stefano Stabellini wrote:
> Account for the extra memory needed for the rom files of any emulated nics:
> QEMU uses xc_domain_populate_physmap_exact to allocate the memory for
> each them. Assume 256K each.

I have seen that this is no longer planned for 4.5, but I do think that 
libxl will
still be changed for this.

so

>   int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t max_memkb)
>   {
>       GC_INIT(ctx);
>       char *mem, *endptr;
>       uint32_t memorykb;
>       char *dompath = libxl__xs_get_dompath(gc, domid);
> -    int rc = 1;
> +    int rc = 1, romsize;
>   
>       mem = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/memory/target", dompath));
>       if (!mem) {
> @@ -4550,11 +4577,18 @@ 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 = libxl__get_rom_memory_kb(gc, domid, NULL);
> +    if (rc < 0)
> +        goto out;
> +    romsize = rc;
> +    rc = xc_domain_setmaxmem(ctx->xch, domid,
> +                             max_memkb + LIBXL_MAXMEM_CONSTANT
> +                             + romsize);

Keeping LIBXL_MAXMEM_CONSTANT is wrong.  Should be dropped.

>       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 +
> +                romsize, rc);
>           goto out;
>       }
>   

And here.

> @@ -4683,7 +4717,7 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
>           int32_t target_memkb, int relative, int enforce)
>   {
>       GC_INIT(ctx);
> -    int rc = 1, abort_transaction = 0;
> +    int rc = 1, abort_transaction = 0, romsize;
>       uint32_t memorykb = 0, videoram = 0;
>       uint32_t current_target_memkb = 0, new_target_memkb = 0;
>       uint32_t current_max_memkb = 0;
> @@ -4769,12 +4803,19 @@ retry_transaction:
>   
>       if (enforce) {
>           memorykb = new_target_memkb;
> +        rc = libxl__get_rom_memory_kb(gc, domid, NULL);
> +        if (rc < 0) {
> +            abort_transaction = 1;
> +            goto out;
> +        }
> +        romsize = rc;
>           rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
> -                LIBXL_MAXMEM_CONSTANT);
> +                LIBXL_MAXMEM_CONSTANT + romsize);

And here.

>           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 +
> +                    romsize, rc);
>               abort_transaction = 1;
>               goto out;
>           }
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 74ea84b..733f4c7 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -305,7 +305,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>       libxl_domain_build_info *const info = &d_config->b_info;
>       libxl_ctx *ctx = libxl__gc_owner(gc);
>       char *xs_domid, *con_domid;
> -    int rc;
> +    int rc, romsize;
>   
>       if (xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus) != 0) {
>           LOG(ERROR, "Couldn't set max vcpu count");
> @@ -405,8 +405,12 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>           }
>       }
>   
> +	rc = libxl__get_rom_memory_kb(gc, domid, d_config);
> +	if (rc < 0)
> +		return rc;
> +	romsize = rc;
>       if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
> -        LIBXL_MAXMEM_CONSTANT) < 0) {
> +        LIBXL_MAXMEM_CONSTANT + romsize) < 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

This should be "romsize" + 1024 as far as I know.

>   #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);

     -Don Slutz