subject:"\[libvirt\] \[PATCH v2 1\/3\] numatune\: add check for numatune nodeset range"

Re: [libvirt] [PATCH v2 1/3] numatune: add check for numatune nodeset range

2014-10-29 Thread Martin Kletzander


On Tue, Oct 28, 2014 at 04:22:21PM +0800, 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.f...@cn.fujitsu.com
---
src/conf/numatune_conf.c   | 21 -
src/conf/numatune_conf.h   | 19 
src/libvirt_private.syms   |  1 +
src/qemu/qemu_command.c|  3 ++
src/qemu/qemu_command.h|  1 +
src/util/virnuma.c | 55 ++
src/util/virnuma.h |  2 +
...rgv-numatune-static-nodeset-exceed-hostnode.xml | 35 ++
tests/qemuxml2argvmock.c   |  9 
tests/qemuxml2argvtest.c   |  1 +
10 files changed, 126 insertions(+), 21 deletions(-)
create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 21d9a64..d440b86 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,27 +42,6 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
  static,
  auto);

-typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
-typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
-
-struct _virDomainNumatune {
-struct {
-bool specified;
-virBitmapPtr nodeset;
-virDomainNumatuneMemMode mode;
-virDomainNumatunePlacement placement;
-} memory;   /* pinning for all the memory */
-
-struct _virDomainNumatuneNode {
-virBitmapPtr nodeset;
-virDomainNumatuneMemMode mode;
-} *mem_nodes;   /* fine tuning per guest node */
-size_t nmem_nodes;
-
-/* Future NUMA tuning related stuff should go here. */
-};
-
-
static inline bool
virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
   int cellid)
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 5254629..650b6e7 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -45,6 +45,25 @@ typedef enum {
VIR_ENUM_DECL(virDomainNumatunePlacement)
VIR_ENUM_DECL(virDomainNumatuneMemMode)

+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
+struct _virDomainNumatune {
+struct {
+bool specified;
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+virDomainNumatunePlacement placement;
+} memory;   /* pinning for all the memory */
+
+struct _virDomainNumatuneNode {
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+} *mem_nodes;   /* fine tuning per guest node */
+size_t nmem_nodes;
+
+/* Future NUMA tuning related stuff should go here. */
+};

void virDomainNumatuneFree(virDomainNumatunePtr numatune);



NACK to these two hunks.  The point of the structure being hidden in
the .c file was to abstract it.  You can provide accessors to those
members you need if they are not available already.


diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d63a8f0..16a5864 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1728,6 +1728,7 @@ virNumaGetPageInfo;
virNumaGetPages;
virNumaIsAvailable;
virNumaNodeIsAvailable;
+virNumaNodesetIsAvailable;
virNumaSetPagePoolSize;
virNumaSetupMemoryPolicy;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..9757d3e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6663,6 +6663,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
goto cleanup;
}

+if (!virNumaNodesetIsAvailable(def-numatune))
+goto cleanup;
+
for (i = 0; i  def-mem.nhugepages; i++) {
ssize_t next_bit, pos = 0;

diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index aa40c9e..f263665 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -27,6 +27,7 @@
# include domain_addr.h
# include domain_conf.h
# include vircommand.h
+# include virnuma.h
# include capabilities.h
# include qemu_conf.h
# include qemu_domain.h
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 690615f..411719d 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -312,6 +312,55 @@ virNumaGetNodeCPUs(int node,

return ret;
}
+
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+int maxnode;
+int bit = -1;
+size_t i;
+virBitmapPtr nodemask = NULL;
+
+if (!numatune)
+return true;
+
+if ((maxnode = virNumaGetMaxNode())  0)
+return false;
+
+maxnode = maxnode  NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
+
+/* verify memory and memnode element in numatune */
+

Re: [libvirt] [PATCH v2 1/3] numatune: add check for numatune nodeset range

2014-10-29 Thread Chen, Fan

On Wed, 2014-10-29 at 07:58 +0100, Martin Kletzander wrote: 
 On Tue, Oct 28, 2014 at 04:22:21PM +0800, 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.f...@cn.fujitsu.com
 ---
  src/conf/numatune_conf.c   | 21 -
  src/conf/numatune_conf.h   | 19 
  src/libvirt_private.syms   |  1 +
  src/qemu/qemu_command.c|  3 ++
  src/qemu/qemu_command.h|  1 +
  src/util/virnuma.c | 55 
  ++
  src/util/virnuma.h |  2 +
  ...rgv-numatune-static-nodeset-exceed-hostnode.xml | 35 ++
  tests/qemuxml2argvmock.c   |  9 
  tests/qemuxml2argvtest.c   |  1 +
  10 files changed, 126 insertions(+), 21 deletions(-)
  create mode 100644 
  tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml
 
 diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
 index 21d9a64..d440b86 100644
 --- a/src/conf/numatune_conf.c
 +++ b/src/conf/numatune_conf.c
 @@ -42,27 +42,6 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
static,
auto);
 
 -typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
 -typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
 -
 -struct _virDomainNumatune {
 -struct {
 -bool specified;
 -virBitmapPtr nodeset;
 -virDomainNumatuneMemMode mode;
 -virDomainNumatunePlacement placement;
 -} memory;   /* pinning for all the memory */
 -
 -struct _virDomainNumatuneNode {
 -virBitmapPtr nodeset;
 -virDomainNumatuneMemMode mode;
 -} *mem_nodes;   /* fine tuning per guest node */
 -size_t nmem_nodes;
 -
 -/* Future NUMA tuning related stuff should go here. */
 -};
 -
 -
  static inline bool
  virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
 int cellid)
 diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
 index 5254629..650b6e7 100644
 --- a/src/conf/numatune_conf.h
 +++ b/src/conf/numatune_conf.h
 @@ -45,6 +45,25 @@ typedef enum {
  VIR_ENUM_DECL(virDomainNumatunePlacement)
  VIR_ENUM_DECL(virDomainNumatuneMemMode)
 
 +typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
 +typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
 +
 +struct _virDomainNumatune {
 +struct {
 +bool specified;
 +virBitmapPtr nodeset;
 +virDomainNumatuneMemMode mode;
 +virDomainNumatunePlacement placement;
 +} memory;   /* pinning for all the memory */
 +
 +struct _virDomainNumatuneNode {
 +virBitmapPtr nodeset;
 +virDomainNumatuneMemMode mode;
 +} *mem_nodes;   /* fine tuning per guest node */
 +size_t nmem_nodes;
 +
 +/* Future NUMA tuning related stuff should go here. */
 +};
 
  void virDomainNumatuneFree(virDomainNumatunePtr numatune);
 
 
 NACK to these two hunks.  The point of the structure being hidden in
 the .c file was to abstract it.  You can provide accessors to those
 members you need if they are not available already.

Got it!

 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index d63a8f0..16a5864 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1728,6 +1728,7 @@ virNumaGetPageInfo;
  virNumaGetPages;
  virNumaIsAvailable;
  virNumaNodeIsAvailable;
 +virNumaNodesetIsAvailable;
  virNumaSetPagePoolSize;
  virNumaSetupMemoryPolicy;
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 2e5af4f..9757d3e 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -6663,6 +6663,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
  goto cleanup;
  }
 
 +if (!virNumaNodesetIsAvailable(def-numatune))
 +goto cleanup;
 +
  for (i = 0; i  def-mem.nhugepages; i++) {
  ssize_t next_bit, pos = 0;
 
 diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
 index aa40c9e..f263665 100644
 --- a/src/qemu/qemu_command.h
 +++ b/src/qemu/qemu_command.h
 @@ -27,6 +27,7 @@
  # include domain_addr.h
  # include domain_conf.h
  # include vircommand.h
 +# include virnuma.h
  # include capabilities.h
  # include qemu_conf.h
  # include qemu_domain.h
 diff --git a/src/util/virnuma.c b/src/util/virnuma.c
 index 690615f..411719d 100644
 --- a/src/util/virnuma.c
 +++ b/src/util/virnuma.c
 @@ -312,6 +312,55 @@ virNumaGetNodeCPUs(int node,
 
  return ret;
  }
 +
 +bool
 +virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
 +{
 +int maxnode;
 +int bit = -1;
 +size_t i;
 +virBitmapPtr nodemask = NULL;

[libvirt] [PATCH v2 1/3] numatune: add check for numatune nodeset range

2014-10-28 Thread 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 chen.fan.f...@cn.fujitsu.com
---
 src/conf/numatune_conf.c   | 21 -
 src/conf/numatune_conf.h   | 19 
 src/libvirt_private.syms   |  1 +
 src/qemu/qemu_command.c|  3 ++
 src/qemu/qemu_command.h|  1 +
 src/util/virnuma.c | 55 ++
 src/util/virnuma.h |  2 +
 ...rgv-numatune-static-nodeset-exceed-hostnode.xml | 35 ++
 tests/qemuxml2argvmock.c   |  9 
 tests/qemuxml2argvtest.c   |  1 +
 10 files changed, 126 insertions(+), 21 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-static-nodeset-exceed-hostnode.xml

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 21d9a64..d440b86 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -42,27 +42,6 @@ VIR_ENUM_IMPL(virDomainNumatunePlacement,
   static,
   auto);
 
-typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
-typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
-
-struct _virDomainNumatune {
-struct {
-bool specified;
-virBitmapPtr nodeset;
-virDomainNumatuneMemMode mode;
-virDomainNumatunePlacement placement;
-} memory;   /* pinning for all the memory */
-
-struct _virDomainNumatuneNode {
-virBitmapPtr nodeset;
-virDomainNumatuneMemMode mode;
-} *mem_nodes;   /* fine tuning per guest node */
-size_t nmem_nodes;
-
-/* Future NUMA tuning related stuff should go here. */
-};
-
-
 static inline bool
 virDomainNumatuneNodeSpecified(virDomainNumatunePtr numatune,
int cellid)
diff --git a/src/conf/numatune_conf.h b/src/conf/numatune_conf.h
index 5254629..650b6e7 100644
--- a/src/conf/numatune_conf.h
+++ b/src/conf/numatune_conf.h
@@ -45,6 +45,25 @@ typedef enum {
 VIR_ENUM_DECL(virDomainNumatunePlacement)
 VIR_ENUM_DECL(virDomainNumatuneMemMode)
 
+typedef struct _virDomainNumatuneNode virDomainNumatuneNode;
+typedef virDomainNumatuneNode *virDomainNumatuneNodePtr;
+
+struct _virDomainNumatune {
+struct {
+bool specified;
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+virDomainNumatunePlacement placement;
+} memory;   /* pinning for all the memory */
+
+struct _virDomainNumatuneNode {
+virBitmapPtr nodeset;
+virDomainNumatuneMemMode mode;
+} *mem_nodes;   /* fine tuning per guest node */
+size_t nmem_nodes;
+
+/* Future NUMA tuning related stuff should go here. */
+};
 
 void virDomainNumatuneFree(virDomainNumatunePtr numatune);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d63a8f0..16a5864 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1728,6 +1728,7 @@ virNumaGetPageInfo;
 virNumaGetPages;
 virNumaIsAvailable;
 virNumaNodeIsAvailable;
+virNumaNodesetIsAvailable;
 virNumaSetPagePoolSize;
 virNumaSetupMemoryPolicy;
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2e5af4f..9757d3e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6663,6 +6663,9 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
 goto cleanup;
 }
 
+if (!virNumaNodesetIsAvailable(def-numatune))
+goto cleanup;
+
 for (i = 0; i  def-mem.nhugepages; i++) {
 ssize_t next_bit, pos = 0;
 
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index aa40c9e..f263665 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -27,6 +27,7 @@
 # include domain_addr.h
 # include domain_conf.h
 # include vircommand.h
+# include virnuma.h
 # include capabilities.h
 # include qemu_conf.h
 # include qemu_domain.h
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 690615f..411719d 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -312,6 +312,55 @@ virNumaGetNodeCPUs(int node,
 
 return ret;
 }
+
+bool
+virNumaNodesetIsAvailable(virDomainNumatunePtr numatune)
+{
+int maxnode;
+int bit = -1;
+size_t i;
+virBitmapPtr nodemask = NULL;
+
+if (!numatune)
+return true;
+
+if ((maxnode = virNumaGetMaxNode())  0)
+return false;
+
+maxnode = maxnode  NUMA_NUM_NODES ? maxnode : NUMA_NUM_NODES;
+
+/* verify memory and memnode element in numatune */
+nodemask = virDomainNumatuneGetNodeset(numatune, NULL, -1);
+if (nodemask) {
+while ((bit = virBitmapNextSetBit(nodemask, bit)) = 0) {
+if (bit  maxnode) {
+goto error;
+

Re: [libvirt] [PATCH v2 1/3] numatune: add check for numatune nodeset range

Re: [libvirt] [PATCH v2 1/3] numatune: add check for numatune nodeset range

[libvirt] [PATCH v2 1/3] numatune: add check for numatune nodeset range

3 matches

Site Navigation

Mail list logo

Footer information