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 <jfeh...@suse.com>
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.mart...@oracle.com> > --- > > 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). > + > + 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. > + } > + } > + } > + > + physmem: > + /* No 'max' specified in dom0_mem, so dom0 can use all physical memory */ > + libxl_physinfo_init(&physinfo); > + if (libxl_get_physinfo(cfg->ctx, &physinfo)) { > + VIR_WARN("libxl_get_physinfo failed"); > + goto cleanup; > + } > + *maxmem = (physinfo.total_pages * cfg->verInfo->pagesize) / 1024; > + libxl_physinfo_dispose(&physinfo); > + ret = 0; > + > + 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..4942171 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -174,6 +174,10 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg, > const char *filename); > > int > +libxlDriverGetDom0MaxmemConf(libxlDriverConfigPtr cfg, > + unsigned long long *maxmem); > + > +int > libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev); > int > libxlMakeNic(virDomainDefPtr def, > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index 040b986..1558d5b 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -576,6 +576,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver) > virDomainObjPtr vm = NULL; > virDomainDefPtr oldDef = NULL; > libxl_dominfo d_info; > + unsigned long long maxmem; > int ret = -1; > > libxl_dominfo_init(&d_info); > @@ -615,7 +616,9 @@ 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); > + if (libxlDriverGetDom0MaxmemConf(cfg, &maxmem) < 0) > + maxmem = d_info.current_memkb; > + virDomainDefSetMemoryTotal(vm->def, maxmem); > > ret = 0; > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list