On Tue, 2014-09-23 at 14:41 +0200, Michal Privoznik wrote:
On 23.09.2014 11:34, Chen Fan wrote:
> For memnode in numatune element, the range of attribute 'nodeset'
> was not validated. on my host maxnodes was 1, but when setting nodeset
> to '0-2' or more, guest also started succuss. there probably was qemu's
> bug too.
>
> Signed-off-by: Chen Fan <chen.fan.fnst(a)cn.fujitsu.com>
> ---
> src/conf/numatune_conf.c | 29 +++++++++++++++++++++++++++++
> src/conf/numatune_conf.h | 4 ++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
> index 21d9a64..a9b20aa 100644
> --- a/src/conf/numatune_conf.c
> +++ b/src/conf/numatune_conf.c
> @@ -183,6 +183,9 @@ virDomainNumatuneNodeParseXML(virDomainNumatunePtr
*numatunePtr,
> VIR_DOMAIN_CPUMASK_LEN) < 0)
> goto cleanup;
> VIR_FREE(tmp);
> +
> + if (!virDomainNumatuneNodeSetIsAvailable(numatune, cellid))
> + goto cleanup;
Well, if there already exists such configuration within an existing
domain, this will cause a failure on XML parsing when the daemon is
starting and hence domain is lost.
Right, I would move this check to VM start
routine.
> }
>
> ret = 0;
> @@ -612,3 +615,29 @@ virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr
numatune)
>
> return false;
> }
> +
> +bool
> +virDomainNumatuneNodeSetIsAvailable(virDomainNumatunePtr numatune,
> + int cellid)
> +{
> + int maxnode;
> + int bit = -1;
> + virBitmapPtr nodemask = NULL;
> +
> + nodemask = virDomainNumatuneGetNodeset(numatune, NULL, cellid);
> + if (!nodemask)
> + return false;
> +
> + if ((maxnode = virNumaGetMaxNode()) < 0)
> + return false;
This will work in real environment, but won't work in tests. You need to
mock this to get predictable max numa node.
I will add test case for this.
Thanks,
Chen
> +
> + while ((bit = virBitmapNextSetBit(nodemask, bit)) >= 0) {
> + if (bit > maxnode) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("NUMA node %d is out of range"), bit);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
> index 5254629..cab0b83 100644
> --- a/src/conf/numatune_conf.h
> +++ b/src/conf/numatune_conf.h
> @@ -102,4 +102,8 @@ bool virDomainNumatuneHasPlacementAuto(virDomainNumatunePtr
numatune);
>
> bool virDomainNumatuneHasPerNodeBinding(virDomainNumatunePtr numatune);
>
> +extern int virNumaGetMaxNode(void);
> +bool virDomainNumatuneNodeSetIsAvailable(virDomainNumatunePtr numatune,
> + int cellid);
> +
> #endif /* __NUMATUNE_CONF_H__ */
>
Michal