Re: [libvirt] [PATCH v4 3/3] virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy

2014-11-03 Thread Martin Kletzander 

On Thu, Oct 30, 2014 at 01:44:19PM +0800, Chen Fan wrote:

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
src/util/virnuma.c | 23 ---
1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 4188ef5..613a43c 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -95,31 +95,19 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
int ret = -1;
int bit = 0;
size_t i;
-int maxnode = 0;
virBitmapPtr tmp_nodemask = NULL;

+if (!virNumaNodesetIsAvailable(numatune))
+return -1;
+
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);
}



I think this is safe, numad returning nodeset that's not on the host
would be weird error and it is easy to find in the logs.


@@ -347,10 +335,7 @@ int
virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
 virBitmapPtr nodemask ATTRIBUTE_UNUSED)
{
-if (virDomainNumatuneGetNodeset(numatune, NULL, -1)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(libvirt is compiled without NUMA tuning support));
-
+if (!virNumaNodesetIsAvailable(numatune)) {
return -1;
}


This makes the square brackets obsolete.


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 3/3] virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy

2014-11-03 Thread Chen, Fan 
On Mon, 2014-11-03 at 14:18 +0100, Martin Kletzander wrote: 
 On Thu, Oct 30, 2014 at 01:44:19PM +0800, Chen Fan wrote:
 Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
 ---
  src/util/virnuma.c | 23 ---
  1 file changed, 4 insertions(+), 19 deletions(-)
 
 diff --git a/src/util/virnuma.c b/src/util/virnuma.c
 index 4188ef5..613a43c 100644
 --- a/src/util/virnuma.c
 +++ b/src/util/virnuma.c
 @@ -95,31 +95,19 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
  int ret = -1;
  int bit = 0;
  size_t i;
 -int maxnode = 0;
  virBitmapPtr tmp_nodemask = NULL;
 
 +if (!virNumaNodesetIsAvailable(numatune))
 +return -1;
 +
  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);
  }
 
 
 I think this is safe, numad returning nodeset that's not on the host
 would be weird error and it is easy to find in the logs.
I think virNumaNodesetIsAvailable() has checked the case, but retain it 
here is ok.

 
 @@ -347,10 +335,7 @@ int
  virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
   virBitmapPtr nodemask ATTRIBUTE_UNUSED)
  {
 -if (virDomainNumatuneGetNodeset(numatune, NULL, -1)) {
 -virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(libvirt is compiled without NUMA tuning 
 support));
 -
 +if (!virNumaNodesetIsAvailable(numatune)) {
  return -1;
  }
 
 This makes the square brackets obsolete.


Thanks,
Chen


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

Re: [libvirt] [PATCH v4 3/3] virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy

2014-11-03 Thread Martin Kletzander 

On Tue, Nov 04, 2014 at 02:05:16AM +, Chen, Fan wrote:

On Mon, 2014-11-03 at 14:18 +0100, Martin Kletzander wrote:

On Thu, Oct 30, 2014 at 01:44:19PM +0800, Chen Fan wrote:
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
 src/util/virnuma.c | 23 ---
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 4188ef5..613a43c 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -95,31 +95,19 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
 int ret = -1;
 int bit = 0;
 size_t i;
-int maxnode = 0;
 virBitmapPtr tmp_nodemask = NULL;

+if (!virNumaNodesetIsAvailable(numatune))


Here you call virNumaNodesetIsAvailable() with @numatune, but ...


+return -1;
+
 tmp_nodemask = virDomainNumatuneGetNodeset(numatune, nodemask, -1);


... here you can get the automatic one ...


I think this is safe, numad returning nodeset that's not on the host
would be weird error and it is easy to find in the logs.

I think virNumaNodesetIsAvailable() has checked the case, but retain it
here is ok.



... and that's what I meant here that it might be missed.  I would be
OK with the check removed though, since that should create no new
problems, but since you added it in the next version, I'll keep it
there ;)


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v4 3/3] virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy

2014-10-29 Thread Chen Fan 
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
---
 src/util/virnuma.c | 23 ---
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 4188ef5..613a43c 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -95,31 +95,19 @@ virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
 int ret = -1;
 int bit = 0;
 size_t i;
-int maxnode = 0;
 virBitmapPtr tmp_nodemask = NULL;
 
+if (!virNumaNodesetIsAvailable(numatune))
+return -1;
+
 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);
 }
 
@@ -347,10 +335,7 @@ int
 virNumaSetupMemoryPolicy(virDomainNumatunePtr numatune,
  virBitmapPtr nodemask ATTRIBUTE_UNUSED)
 {
-if (virDomainNumatuneGetNodeset(numatune, NULL, -1)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(libvirt is compiled without NUMA tuning support));
-
+if (!virNumaNodesetIsAvailable(numatune)) {
 return -1;
 }
 
-- 
1.9.3

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