On 06/07/2017 11:50 AM, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1455819 > > It may happen that a domain is started without any huge pages. > However, user might try to attach a DIMM module later. DIMM > backed by huge pages (why would somebody want to mix regular and > huge pages is beyond me). Therefore we have to create the dir if > we haven't done so far. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/qemu/qemu_hotplug.c | 3 +++ > src/qemu/qemu_process.c | 20 ++++++++++++++++---- > src/qemu/qemu_process.h | 5 +++++ > 3 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 4a7d99725..62472aef8 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -2254,6 +2254,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, > priv->qemuCaps, vm->def, mem, NULL, true) < 0) > goto cleanup; > > + if (qemuProcessBuildDestroyHugepagesPath(driver, vm, mem, true) < 0) > + goto cleanup; > + If this call was done after virDomainMemoryInsert, then the new check [1] and parameter in qemuProcessBuildDestroyHugepagesPath wouldn't be necessary, but the goto here would need to be removedef, true? > if (qemuDomainNamespaceSetupMemory(driver, vm, mem) < 0) > goto cleanup; > teardowndevice = true; > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index a219a8080..823a1385f 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -3283,9 +3283,10 @@ qemuProcessReconnectCheckMemAliasOrderMismatch(virDomainObjPtr vm) > } > > > -static int > +int > qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, > virDomainObjPtr vm, > + virDomainMemoryDefPtr mem, > bool build) > { > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > @@ -3314,6 +3315,12 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, > } > } > > + if (mem && > + mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM && > + mem->pagesize && > + mem->pagesize != system_pagesize) > + shouldBuild = true; > + [1] > if (!build || > (build && shouldBuild)) { > for (i = 0; i < cfg->nhugetlbfs; i++) { > @@ -3324,6 +3331,11 @@ qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, > goto cleanup; > > if (build) { > + if (virFileExists(hugepagePath)) { > + ret = 0; > + goto cleanup; I was initially wondering why this wasn't an "if (build && virFileExists()) continue;", then it dawned on me that this code will create the path and label it for all hugetlbfs sizes if any one of those sizes exists, so there's no need... > + } > + > if (virFileMakePathWithMode(hugepagePath, 0700) < 0) { > virReportSystemError(errno, > _("Unable to create %s"), > @@ -3497,7 +3509,7 @@ qemuProcessReconnect(void *opaque) > goto cleanup; > } > > - if (qemuProcessBuildDestroyHugepagesPath(driver, obj, true) < 0) > + if (qemuProcessBuildDestroyHugepagesPath(driver, obj, NULL, true) < 0) > goto error; > > if ((qemuDomainAssignAddresses(obj->def, priv->qemuCaps, > @@ -5541,7 +5553,7 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, > NULL) < 0) > goto cleanup; > > - if (qemuProcessBuildDestroyHugepagesPath(driver, vm, true) < 0) > + if (qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, true) < 0) > goto cleanup; > > /* Ensure no historical cgroup for this VM is lying around bogus > @@ -6225,7 +6237,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, > goto endjob; > } > > - qemuProcessBuildDestroyHugepagesPath(driver, vm, false); > + qemuProcessBuildDestroyHugepagesPath(driver, vm, NULL, false); > > vm->def->id = -1; > > diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h > index 830d8cef8..b63784152 100644 > --- a/src/qemu/qemu_process.h > +++ b/src/qemu/qemu_process.h > @@ -192,4 +192,9 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver, > virDomainObjPtr vm, > qemuDomainAsyncJob asyncJob); > > +int qemuProcessBuildDestroyHugepagesPath(virQEMUDriverPtr driver, > + virDomainObjPtr vm, > + virDomainMemoryDefPtr mem, > + bool build); > + Perhaps a personal pet peeve of mine, but order wise in qemu_process.c the API is between qemuProcessStopCPUs and qemuProcessIncomingDefNew, so why not list it thusly in the .h file... John > #endif /* __QEMU_PROCESS_H__ */ > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
