Re: [Qemu-devel] [PATCH v2 1/1] pc-dimm: Change PCDIMMDevice->node from UINT32 to INT32, and initialize it as -1.

[tangchen <tangchen@cn.fujitsu.com>]

Hi ,

Would anybody help to review this patch ?

Thanks. :)

On 08/19/2014 09:55 AM, Tang Chen wrote:
> If user doesn't specify numa options, nb_numa_nodes will be 0. But PCDIMMDevice->node
> is also initialized to 0. As a result, the following check will fail:
>
> pc_dimm_realize()
> {
> 	......
> 	if (dimm->node >= nb_numa_nodes) {
>              error_setg(errp, "'DIMM property " PC_DIMM_NODE_PROP " has value %"
>                         PRIu32 "' which exceeds the number of numa nodes: %d",
>                         dimm->node, nb_numa_nodes);
>              return;
> 	}
> 	......
> }
>
> But this is not an error.
>
> PCDIMMDevice->node should be initialized to -1. This is for users who do not use
> NUMA.
>
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
>
> Change log v1 -> v2:
> 1. Simplify the comment.
> 2. Move the definition of NO_NODE_ID near where it is used.
>
>   hw/mem/pc-dimm.c         | 7 ++++++-
>   include/hw/mem/pc-dimm.h | 2 +-
>   2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index ec8b1a3..34109a2 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -195,9 +195,14 @@ out:
>       return ret;
>   }
>   
> +/* Default value for PCDIMMDevice->node (unless specified by user).
> + * In this case, SRAT won't be created.
> + */
> +#define NO_NODE_ID -1
> +
>   static Property pc_dimm_properties[] = {
>       DEFINE_PROP_UINT64(PC_DIMM_ADDR_PROP, PCDIMMDevice, addr, 0),
> -    DEFINE_PROP_UINT32(PC_DIMM_NODE_PROP, PCDIMMDevice, node, 0),
> +    DEFINE_PROP_INT32(PC_DIMM_NODE_PROP, PCDIMMDevice, node, NO_NODE_ID),
>       DEFINE_PROP_INT32(PC_DIMM_SLOT_PROP, PCDIMMDevice, slot,
>                         PC_DIMM_UNASSIGNED_SLOT),
>       DEFINE_PROP_END_OF_LIST(),
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index 761eeef..82abb2f 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -53,7 +53,7 @@ typedef struct PCDIMMDevice {
>   
>       /* public */
>       uint64_t addr;
> -    uint32_t node;
> +    int32_t node;
>       int32_t slot;
>       HostMemoryBackend *hostmem;
>   } PCDIMMDevice;