Re: [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 
> > ---
> >   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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Michal Privoznik]

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



  }

  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.



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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Chen Fan]

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 
---
 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;
 }
 
 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;
+
+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__ */
-- 
1.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list