Re: [PATCH v4 06/21] libxl: introduce libxl__vnuma_config_check

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2015-01-23 at 11:13 +0000, Wei Liu wrote:
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 6d3ac58..39356ba 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3394,6 +3394,11 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc,
>      libxl_bitmap_copy(CTX, &cndt->nodemap, nodemap);
>  }
>  
> +/* Check if vNUMA config is valid. Returns 0 if valid. */

And on invalid? ERROR_FOO or just !0?

I see further down it is ERROR_INVAL, perhaps say so, and perhaps
introduce ERROR_INVAL_VNUMA_CONFIGUATION or something.

> +/* Check if vNUMA configuration is valid:
> + *  1. all pnodes inside vnode_to_pnode array are valid
> + *  2. one vcpu belongs to and only belongs to one vnode
> + *  3. each vmemrange is valid and doesn't overlap with each other
> + */
> +int libxl__vnuma_config_check(libxl__gc *gc,
> +			      const libxl_domain_build_info *b_info,

Hard tabs here.


> +    for (i = 0; i < b_info->num_vnuma_nodes; i++) {
> +        uint32_t pnode;
> +
> +        p = &b_info->vnuma_nodes[i];
> +        pnode = p->pnode;
> +
> +        /* The pnode specified is not valid? */
> +        if (pnode >= nr_nodes) {

I take it that pnodes aren't (potentially) sparse?

> +            LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> +                       "Invalid pnode %d specified",
> +                       pnode);

Please use the short LOG macros (throughout).

> +            goto out;
> +        }
> +
> +        total_memkb += p->memkb;
> +    }
> +
> +    if (total_memkb != b_info->max_memkb) {
> +        LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> +                   "Amount of memory mismatch (0x%"PRIx64" != 0x%"PRIx64")",
> +                   total_memkb, b_info->max_memkb);

Did arch_setup_meminit not also check this?

Meaning it could maybe abort() or ASSERT. Not 100% sure about that being
wise though.