Re: [libvirt] [PATCH 2/2] numatune: move up verification codes in virNumaSetupMemoryPolicy

[Chen, Fan]

On Tue, 2014-09-23 at 14:42 +0200, Michal Privoznik wrote: 
> On 23.09.2014 11:34, Chen Fan wrote:
> > use virDomainNumatuneNodeSetIsAvailable() to verify momory.nodeset
> > whether is out of range. and move up the verification.
> >
> > Signed-off-by: Chen Fan 
> > ---
> >   src/conf/numatune_conf.c |  3 +++
> >   src/util/virnuma.c   | 15 ---
> >   2 files changed, 3 insertions(+), 15 deletions(-)
> 
> I'd expect a test case for this.
Ok, I will add it.

thanks,
Chen

> 
> >
> > diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
> > index a9b20aa..8b43167 100644
> > --- a/src/conf/numatune_conf.c
> > +++ b/src/conf/numatune_conf.c
> > @@ -278,6 +278,9 @@ virDomainNumatuneParseXML(virDomainNumatunePtr 
> > *numatunePtr,
> >nodeset) < 0)
> >   goto cleanup;
> >
> > +if (!virDomainNumatuneNodeSetIsAvailable(*numatunePtr, -1))
> > +goto cleanup;
> > +
> >   if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) < 0)
> >   goto cleanup;
> >
> > diff --git a/src/util/virnuma.c b/src/util/virnuma.c
> > index 1a34398..4766f16 100644
> > --- a/src/util/virnuma.c
> > +++ b/src/util/virnuma.c
> > @@ -95,31 +95,16 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
> >   int ret = -1;
> >   int bit = 0;
> >   size_t i;
> > -int maxnode = 0;
> >   virBitmapPtr tmp_nodemask = NULL;
> >
> >   tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1);
> >   if (!tmp_nodemask)
> >   return 0;
> >
> > -if (numa_available() < 0) {
> > -virReportError(VIR_ERR_INTERNAL_ERROR,
> > -   "%s", _("Host kernel is not aware of NUMA."));
> > -return -1;
> > -}
> > -
> > -maxnode = numa_max_node();
> > -maxnode = maxnode < NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
> > -
> >   /* Convert nodemask to NUMA bitmask. */
> >   nodemask_zero(&mask);
> >   bit = -1;
> >   while ((bit = virBitmapNextSetBit(tmp_nodemask, bit)) >= 0) {
> > -if (bit > maxnode) {
> > -virReportError(VIR_ERR_INTERNAL_ERROR,
> > -   _("NUMA node %d is out of range"), bit);
> > -return -1;
> > -}
> >   nodemask_set(&mask, bit);
> >   }
> >
> >
> 
> Yet again, this suffers the same problem that 1/2 does: domain may be lost.
> 
> Michal


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

Re: [libvirt] [PATCH 2/2] numatune: move up verification codes in virNumaSetupMemoryPolicy

[Michal Privoznik]

On 23.09.2014 11:34, Chen Fan wrote:

use virDomainNumatuneNodeSetIsAvailable() to verify momory.nodeset
whether is out of range. and move up the verification.

Signed-off-by: Chen Fan 
---
  src/conf/numatune_conf.c |  3 +++
  src/util/virnuma.c   | 15 ---
  2 files changed, 3 insertions(+), 15 deletions(-)


I'd expect a test case for this.



diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index a9b20aa..8b43167 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -278,6 +278,9 @@ virDomainNumatuneParseXML(virDomainNumatunePtr *numatunePtr,
   nodeset) < 0)
  goto cleanup;

+if (!virDomainNumatuneNodeSetIsAvailable(*numatunePtr, -1))
+goto cleanup;
+
  if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) < 0)
  goto cleanup;

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 1a34398..4766f16 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -95,31 +95,16 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
  int ret = -1;
  int bit = 0;
  size_t i;
-int maxnode = 0;
  virBitmapPtr tmp_nodemask = NULL;

  tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1);
  if (!tmp_nodemask)
  return 0;

-if (numa_available() < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("Host kernel is not aware of NUMA."));
-return -1;
-}
-
-maxnode = numa_max_node();
-maxnode = maxnode < NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
-
  /* Convert nodemask to NUMA bitmask. */
  nodemask_zero(&mask);
  bit = -1;
  while ((bit = virBitmapNextSetBit(tmp_nodemask, bit)) >= 0) {
-if (bit > maxnode) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("NUMA node %d is out of range"), bit);
-return -1;
-}
  nodemask_set(&mask, bit);
  }




Yet again, this suffers the same problem that 1/2 does: domain may be lost.

Michal

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

[libvirt] [PATCH 2/2] numatune: move up verification codes in virNumaSetupMemoryPolicy

[Chen Fan]

use virDomainNumatuneNodeSetIsAvailable() to verify momory.nodeset
whether is out of range. and move up the verification.

Signed-off-by: Chen Fan 
---
 src/conf/numatune_conf.c |  3 +++
 src/util/virnuma.c   | 15 ---
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index a9b20aa..8b43167 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -278,6 +278,9 @@ virDomainNumatuneParseXML(virDomainNumatunePtr *numatunePtr,
  nodeset) < 0)
 goto cleanup;
 
+if (!virDomainNumatuneNodeSetIsAvailable(*numatunePtr, -1))
+goto cleanup;
+
 if (virDomainNumatuneNodeParseXML(numatunePtr, ncells, ctxt) < 0)
 goto cleanup;
 
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 1a34398..4766f16 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -95,31 +95,16 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
 int ret = -1;
 int bit = 0;
 size_t i;
-int maxnode = 0;
 virBitmapPtr tmp_nodemask = NULL;
 
 tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1);
 if (!tmp_nodemask)
 return 0;
 
-if (numa_available() < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("Host kernel is not aware of NUMA."));
-return -1;
-}
-
-maxnode = numa_max_node();
-maxnode = maxnode < NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
-
 /* Convert nodemask to NUMA bitmask. */
 nodemask_zero(&mask);
 bit = -1;
 while ((bit = virBitmapNextSetBit(tmp_nodemask, bit)) >= 0) {
-if (bit > maxnode) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("NUMA node %d is out of range"), bit);
-return -1;
-}
 nodemask_set(&mask, bit);
 }
 
-- 
1.9.3

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