Re: [libvirt] [PATCH 2/2] qemuDomainAttachMemory: Crate hugepage dir if needed
On 06/13/2017 02:03 PM, John Ferlan wrote:
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(a)redhat.com>
> ---
> 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?
It can't go there because we need to mkdir() before telling qemu about
the path. virDomainMemoryInsert is done after that. But yes, if we could
do that, it'd be awesome.
> if (qemuDomainNamespaceSetupMemory(driver, vm, mem) < 0)
> goto cleanup;
> teardowndevice = true;
> 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...
Ah, okay then.
Michal