Joao Martins wrote:
On 02/02/2017 10:31 PM, Jim Fehlig wrote:
> When the libxl driver is initialized, it creates a virDomainDef
> object for dom0 and adds it to the list of domains. Total memory
> for dom0 was being set from the max_memkb field of libxl_dominfo
> struct retrieved from libxl, but this field can be set to
> LIBXL_MEMKB_DEFAULT (~0ULL) if dom0 maximum memory has not been
> explicitly set by the user.
>
> This patch adds some simple parsing of the Xen commandline,
> looking for a dom0_mem parameter that also specifies a 'max' value.
> If not specified, dom0 maximum memory is effectively all physical
> host memory.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
> src/libxl/libxl_conf.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/libxl/libxl_conf.h | 3 ++
> src/libxl/libxl_driver.c | 2 +-
> 3 files changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index b5186f2..bfe0e92 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -34,6 +34,7 @@
> #include "internal.h"
> #include "virlog.h"
> #include "virerror.h"
> +#include "c-ctype.h"
> #include "datatypes.h"
> #include "virconf.h"
> #include "virfile.h"
> @@ -1530,6 +1531,80 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
>
> }
>
> +/*
> + * dom0's maximum memory can be controled by the user with the
'dom0_mem' Xen
> + * command line parameter. E.g. to set dom0's initial memory to 4G and max
> + * memory to 8G: dom0_mem=4G,max:8G
> + *
> + * If not constrained by the user, dom0 can effectively use all host memory.
> + * This function returns the configured maximum memory for dom0 in kilobytes,
> + * either the user-specified value or total physical memory as a default.
> + */
> +unsigned long long
> +libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg)
> +{
> + char **cmd_tokens = NULL;
> + char **mem_tokens = NULL;
> + size_t i;
> + size_t j;
> + unsigned long long ret;
> + libxl_physinfo physinfo;
> +
> + if (cfg->verInfo->commandline == NULL ||
> + !(cmd_tokens = virStringSplit(cfg->verInfo->commandline, "
", 0)))
> + goto physmem;
> +
> + for (i = 0; cmd_tokens[i] != NULL; i++) {
> + if (!STRPREFIX(cmd_tokens[i], "dom0_mem="))
> + continue;
> +
> + if (!(mem_tokens = virStringSplit(cmd_tokens[i], ",", 0)))
> + break;
> + for (j = 0; mem_tokens[j] != NULL; j++) {
> + if (STRPREFIX(mem_tokens[j], "max:")) {
> + char *p = mem_tokens[j] + 4;
> + unsigned long long multiplier = 1;
> +
> + while (c_isdigit(*p))
> + p++;
> + if (virStrToLong_ull(mem_tokens[j] + 4, &p, 10, &ret) <
0)
> + break;
> + if (*p) {
> + switch (*p) {
> + case 'k':
> + case 'K':
> + multiplier = 1024;
> + break;
> + case 'm':
> + case 'M':
> + multiplier = 1024 * 1024;
> + break;
> + case 'g':
> + case 'G':
> + multiplier = 1024 * 1024 * 1024;
> + break;
> + }
> + }
> + ret = (ret * multiplier) / 1024;
> + goto cleanup;
> + }
> + }
> + }
> +
> + physmem:
> + /* No 'max' specified in dom0_mem, so dom0 can use all physical memory
*/
> + libxl_physinfo_init(&physinfo);
> + libxl_get_physinfo(cfg->ctx, &physinfo);
Despite being an unlikely event, libxl_get_physinfo can fail here - I think you
need to check the return value here.
Bummer. I was hoping this function could always return a sane value for dom0 max
memory. But I'm not really sure what that would be if libxl_get_physinfo failed.
> + ret = (physinfo.total_pages * cfg->verInfo->pagesize) / 1024;
> + libxl_physinfo_dispose(&physinfo);
> +
> + cleanup:
> + virStringListFree(cmd_tokens);
> + virStringListFree(mem_tokens);
> + return ret;
> +}
> +
> +
> #ifdef LIBXL_HAVE_DEVICE_CHANNEL
> static int
> libxlPrepareChannel(virDomainChrDefPtr channel,
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 69d7885..c4ddbfe 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -173,6 +173,9 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver,
> int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
> const char *filename);
>
> +unsigned long long
> +libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg);
> +
> int
> libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
> int
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 921cc93..e54b3b7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -615,7 +615,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
> if (virDomainDefSetVcpus(vm->def, d_info.vcpu_online) < 0)
> goto cleanup;
> vm->def->mem.cur_balloon = d_info.current_memkb;
> - virDomainDefSetMemoryTotal(vm->def, d_info.max_memkb);
> + virDomainDefSetMemoryTotal(vm->def, libxlDriverGetDom0MaxmemConf(cfg));
And perhaps checking here too on whether "libxlDriverGetDom0MaxmemConf(cfg) <
0" ?
ATM, libxlDriverGetDom0MaxmemConf returns an unsigned long long. I could change
the signature to
int
libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg,
unsigned long long *maxmem)
but again I'm not really sure what to do with maxmem in case of failure. Perhaps
just set it to current_memkb?
Regards,
Jim