[libvirt] [PATCH 4/4] libxl: implement virDomainGetNumaParameters

[Jim Fehlig]

On 06/28/2013 08:33 AM, Dario Faggioli wrote:
> Although, having it depending on Xen >= 4.3 (by using the proper
> libxl feature flag).
>
> Xen currently implements a NUMA placement policy which is basically
> the same as the 'interleaved' policy of `numactl', although it can
> be applied on a subset of the available nodes. We therefore hardcode
> "interleave" as 'numa_mode', and we use the newly introduced libxl
> interface to figure out what nodes a domain spans ('numa_nodeset').
>
> With this change, it is now possible to query the NUMA node
> affinity of a running domain:
>
> [raistlin at Zhaman ~]$ sudo virsh --connect xen:/// list
>   Id    Name                           State
> ----------------------------------------------------
>   23    F18_x64                        running
>
> [raistlin at Zhaman ~]$ sudo virsh --connect xen:/// numatune 23
> numa_mode      : interleave
> numa_nodeset   : 1
>
> Signed-off-by: Dario Faggioli <dario.faggioli at citrix.com>
> ---
>   src/libxl/libxl_driver.c |  138 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 138 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 53af609..9bd6d99 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -28,6 +28,7 @@
>   
>   #include <math.h>
>   #include <libxl.h>
> +#include <libxl_utils.h>
>   #include <fcntl.h>
>   #include <regex.h>
>   
> @@ -4514,6 +4515,140 @@ libxlDomainSetSchedulerParameters(virDomainPtr dom, virTypedParameterPtr params,
>       return libxlDomainSetSchedulerParametersFlags(dom, params, nparams, 0);
>   }
>   
> +/* NUMA node affinity information is available through libxl
> + * starting from Xen 4.3. */
> +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> +
> +/* Number of Xen NUMA parameters */
> +#define LIBXL_NUMA_NPARAM 2

Is there a similar definition in Xen?  E.g. would future changes to libxl adding 
more parameters, but neglecting to update here, cause problems?

> +
> +static int
> +libxlDomainGetNumaParameters(virDomainPtr dom,
> +                             virTypedParameterPtr params,
> +                             int *nparams,
> +                             unsigned int flags)
> +{
> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
> +    libxlDomainObjPrivatePtr priv;
> +    virDomainObjPtr vm;
> +
> +    libxl_bitmap nodemap;
> +    virBitmapPtr nodes = NULL;
> +    char *nodeset = NULL;
> +
> +    int rc, ret = -1;
> +    int i, j;

No need for the extra whitespace between these local variables.

> +
> +    /* In Xen 4.3, it is possible to query the NUMA node affinity of a domain
> +     * via libxl, but not to change it. We therefore only allow AFFECT_LIVE. */
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +    libxlDriverLock(driver);
> +    vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> +    libxlDriverUnlock(driver);
> +
> +    if (!vm) {
> +        virReportError(VIR_ERR_NO_DOMAIN, "%s",
> +                       _("no domain with matching uuid"));
> +        goto cleanup;
> +    }
> +
> +    if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("Domain is not running"));
> +        goto cleanup;
> +    }
> +
> +    priv = vm->privateData;
> +
> +    libxl_bitmap_init(&nodemap);
> +
> +    if ((*nparams) == 0) {
> +        *nparams = LIBXL_NUMA_NPARAM;
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    for (i = 0; i < LIBXL_NUMA_NPARAM && i < *nparams; i++) {
> +        virMemoryParameterPtr param = &params[i];
> +
> +        switch (i) {
> +        case 0:
> +            /* NUMA mode */
> +
> +            /* Xen implements something that is is really close to numactl's
> +             * 'interleave' policy (see `man 8 numactl' for details). */
> +            if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
> +                                        VIR_TYPED_PARAM_INT,
> +                                        VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE) < 0)
> +                goto cleanup;
> +
> +            break;
> +
> +        case 1:
> +            /* Node affinity */
> +
> +            /* Let's allocate both libxl and libvirt bitmaps */
> +            if (libxl_node_bitmap_alloc(priv->ctx, &nodemap, 0) ||
> +                !(nodes = virBitmapNew(libxl_get_max_nodes(priv->ctx)))) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +
> +            rc = libxl_domain_get_nodeaffinity(priv->ctx, vm->def->id,
> +                                               &nodemap);

Fits on one line, albeit with nothing to spare :).

> +            if (rc != 0) {
> +                virReportSystemError(-rc, "%s",
> +                                     _("unable to get numa affinity"));
> +                goto cleanup;
> +            }
> +
> +            /* First, we convert libxl_bitmap into virBitmap. After that,
> +             * we format virBitmap as a string that can be returned. */
> +            virBitmapClearAll(nodes);
> +            libxl_for_each_set_bit(j, nodemap) {
> +                if (virBitmapSetBit(nodes, j)) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("Node %d out of range"), j);
> +                    goto cleanup;
> +                }
> +            }
> +
> +            nodeset = virBitmapFormat(nodes);
> +            if (!nodeset && VIR_STRDUP(nodeset, "") < 0)
> +                goto cleanup;
> +
> +            if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
> +                                        VIR_TYPED_PARAM_STRING, nodeset) < 0)
> +                goto cleanup;
> +
> +            nodeset = NULL;
> +
> +            break;
> +
> +        default:
> +            break;
> +        }
> +    }
> +
> +    if (*nparams > LIBXL_NUMA_NPARAM)
> +        *nparams = LIBXL_NUMA_NPARAM;
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(nodeset);
> +    virBitmapFree(nodes);
> +    libxl_bitmap_dispose(&nodemap);
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +#endif
> +
>   static int
>   libxlDomainIsActive(virDomainPtr dom)
>   {
> @@ -4741,6 +4876,9 @@ static virDriver libxlDriver = {
>       .domainGetSchedulerParametersFlags = libxlDomainGetSchedulerParametersFlags, /* 0.9.2 */
>       .domainSetSchedulerParameters = libxlDomainSetSchedulerParameters, /* 0.9.0 */
>       .domainSetSchedulerParametersFlags = libxlDomainSetSchedulerParametersFlags, /* 0.9.2 */
> +#ifdef LIBXL_HAVE_DOMAIN_NODEAFFINITY
> +    .domainGetNumaParameters = libxlDomainGetNumaParameters, /* 1.1.1 */
> +#endif
>       .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>       .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
>       .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */

Looks good with the exception of these minor nits, but needs to go along with an 
updated 3/4.

Regards,
Jim