[libvirt] [PATCH 1/2] numatune: add check for memnode.nodeset range

[Chen, Fan]

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 at 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