Joao Martins wrote: > On 02/08/2017 10:24 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@xxxxxxxx> > > Argh, sorry. Failed to mention one (non-blocking) observation in the patch about > the parsing of "dom0_mem". Regardless, > > Acked-by: Joao Martins <joao.m.martins@xxxxxxxxxx> > >> --- >> >> V2: Check return value of libxl_get_physinfo and >> libxlDriverGetDom0MaxmemConf. >> >> src/libxl/libxl_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ >> src/libxl/libxl_conf.h | 4 +++ >> src/libxl/libxl_driver.c | 5 +++- >> 3 files changed, 85 insertions(+), 1 deletion(-) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index 28b05e1..7e30c7d 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" >> @@ -1533,6 +1534,82 @@ 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. >> + */ >> +int >> +libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg, >> + unsigned long long *maxmem) >> +{ >> + char **cmd_tokens = NULL; >> + char **mem_tokens = NULL; >> + size_t i; >> + size_t j; >> + libxl_physinfo physinfo; >> + int ret = -1; >> + >> + 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; > Hmm, perhaps worth a comment here (or in the command at the top of the function) > because if the parameter doesn't have a suffix it defaults to kilobytes (as per > Xen command line docs). I've added another sentence to the comment at the top of the function. > >> + >> + while (c_isdigit(*p)) >> + p++; >> + if (virStrToLong_ull(mem_tokens[j] + 4, &p, 10, maxmem) < 0) >> + break; >> + if (*p) { >> + switch (*p) { >> + case 'm': >> + case 'M': >> + multiplier = 1024; >> + break; >> + case 'g': >> + case 'G': >> + multiplier = 1024 * 1024; >> + break; >> + } >> + } >> + *maxmem = *maxmem * multiplier; >> + ret = 0; >> + goto cleanup; > This seems to match xen's parsing of the parameter that is for "max=4GG" or > "max=4GM" the latter suffix will be ignored (thus considering 4G only). I was > looking at the Xen's code of dom0_mem parsing and it's documentation - and > noticed that we don't support Terabytes in the parsing above. Though this is > only since Xen 4.5. Good point. I've included support for the terabyte suffix. How about the below diff squashed in? Regards, Jim --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1564,6 +1564,8 @@ 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 + * Supported unit suffixes are [bBkKmMgGtT]. If not specified the default + * unit is kilobytes. * * If not constrained by the user, dom0 can effectively use all host memory. * This function returns the configured maximum memory for dom0 in kilobytes, @@ -1609,6 +1611,10 @@ libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg, case 'G': multiplier = 1024 * 1024; break; + case 't': + case 'T': + multiplier = 1024 * 1024 * 1024; + break; } } *maxmem = *maxmem * multiplier; -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list