From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <Ian.Campbell@citrix.com>
Subject: Re: [PATCH v4 06/21] libxl: introduce
	libxl__vnuma_config_check
Date: Wed, 28 Jan 2015 16:13:28 +0000
Message-ID: <1422461608.5187.41.camel@citrix.com>
References: <1422011632-22018-1-git-send-email-wei.liu2@citrix.com>
	<1422011632-22018-7-git-send-email-wei.liu2@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1422011632-22018-7-git-send-email-wei.liu2@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: ufimtseva@gmail.com, andrew.cooper3@citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, JBeulich@suse.com
List-Id: xen-devel@lists.xenproject.org

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.