On 06/07/2017 11:50 AM, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1455819 > > Currently, the per-domain path for huge pages mmap() for qemu is > created iff domain has memoryBacking and hugepages in it > configured. However, this alone is not enough because there can > be a DIMM module with hugepages configured too. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/qemu/qemu_process.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 32ba8e373..a219a8080 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -3289,11 +3289,33 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, > bool build) > { > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > + const long system_pagesize = virGetSystemPageSizeKB(); > char *hugepagePath = NULL; > size_t i; > + bool shouldBuild = false; > int ret = -1; > None of the subsequent new checks would be necessary if build was false... Maybe it'd be better to have a helper function... if (build) shouldBuild = qemuProcessNeedHugepagesPath(...); if (shouldBuild || !build) { ... } > - if (vm->def->mem.nhugepages) { > + if (vm->def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) > + shouldBuild = true; Not part of the commit message, but that's an easy fix. Actually it seems to me the algorithm prior to these changes is solely based on whether hugepages were defined in the guest missing that there's multiple ways to configure... > + > + for (i = 0; !shouldBuild && i < vm->def->mem.nhugepages; i++) { > + if (vm->def->mem.hugepages[i].size != system_pagesize) { > + shouldBuild = true; > + break; I would think because of the !shouldBuild as an end condition that the break; wouldn't be necessary... Even more unnecessary if you have a helper and it returns true right here. FWIW: This particular check seems to be "additional" to the commit message as well and is the much shorter check than found in other places that use virGetSystemPageSizeKB to compare pagesize values. > + } > + } > + > + for (i = 0; !shouldBuild && vm->def->nmems; i++) { ^^ Reduce one space... > + if (vm->def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && > + vm->def->mems[i]->pagesize && > + vm->def->mems[i]->pagesize != system_pagesize) { > + shouldBuild = true; > + break; ditto on break; and helper comment. John > + } > + } > + > + if (!build || > + (build && shouldBuild)) { > for (i = 0; i < cfg->nhugetlbfs; i++) { > VIR_FREE(hugepagePath); > hugepagePath = qemuGetDomainHugepagePath(vm->def, &cfg->hugetlbfs[i]); > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
