[libvirt] [PATCH v2 3/3] virLXCControllerSetupResourceLimits: Call virNuma*() iff needed

[Ján Tomko]

On Tue, Mar 31, 2015 at 01:59:10PM +0200, Michal Privoznik wrote:
> Like we are doing in qemu driver
> ($COMMIT_HASH_TO_BE_FILLED_DURING_PUSHING), lets call
> virNumaSetupMemoryPolicy() only if really needed. Problem is, if
> we numa_set_membind() child, there's no way to change it from the
> daemon afterwards. So any later attempts to change the pinning
> will fail. But in very weird way - CGroups will be set, but due
> to membind child will not allocate memory from any other node.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/lxc/lxc_controller.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index 8545f29..4b340ab 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -742,14 +742,24 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
>      virBitmapPtr nodeset = NULL;
>      virDomainNumatuneMemMode mode;
>  
> -    VIR_DEBUG("Setting up process resource limits");
> -
> -    if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
> -        goto cleanup;
> -
> -    nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1);
>      mode = virDomainNumatuneGetMode(ctrl->def->numa, -1);
>  
> +    if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
> +        virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
> +        /* Use virNuma* API iff necessary. Once set and child is exec()-ed,
> +         * there's no way for us to change it. Rely on cgroups (if available
> +         * and enabled in the config) rather then virNuma*. */
> +        VIR_DEBUG("Postponing setting up resource limits to CGroup set up phase");

'resource limits' is too vague

> +        return virLXCControllerSetupCpuAffinity(ctrl);

considering that you limit the available CPUs here.

Also, instead of copying the unrelated call here,

> +    }
> +
> +    VIR_DEBUG("Setting up process resource limits");
> +
> +    if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
> +        goto cleanup;
> +
> +    nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1);
> +
>      if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
>          goto cleanup;

Do

if (!(STRICT && ...)) {
  virNuma()
} else {
  VIR_DEBUG
}

ACK if you convert the function to only have one return call.

(And the DEBUG could be useful in QEMU driver as well)

Jan


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150408/2953e8f5/attachment-0001.sig>