Re: [libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls

2015-04-16 Thread Erik Skultety 


On 04/14/2015 12:27 PM, John Ferlan wrote:
 
 
 On 04/13/2015 08:01 AM, Erik Skultety wrote:
 This patch adds checks for empty bitmaps right after the calls of
 virBitmapParse. These only include spots where set API's are called and
 where domain's XML is parsed.
 Also, it partially reverts commit 983f5a which added a check for
 invalid nodeset 0,^0 into virBitmapParse function. This change broke
 the logic, as an empty bitmap should not cause an error.

 https://bugzilla.redhat.com/show_bug.cgi?id=1210545
 ---
  src/conf/domain_conf.c   | 35 +++
  src/conf/numa_conf.c | 23 +++
  src/qemu/qemu_driver.c   |  5 +++--
  src/util/virbitmap.c |  3 ---
  src/xenconfig/xen_sxpr.c |  7 +++
  tests/virbitmaptest.c| 13 ++---
  6 files changed, 70 insertions(+), 16 deletions(-)

 
 ...
 
 diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
 index f247275..9a84e4c 100644
 --- a/tests/virbitmaptest.c
 +++ b/tests/virbitmaptest.c
 @@ -524,16 +524,23 @@ static int
  test10(const void *opaque ATTRIBUTE_UNUSED)
  {
  int ret = -1;
 -virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL;
 +virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
  
  if (virBitmapParse(0-3,5-8,11-15, 0, b1, 20)  0 ||
  virBitmapParse(4,9,10,16-19, 0, b2, 20)  0 ||
 -virBitmapParse(15, 0, b3, 20)  0)
 +virBitmapParse(15, 0, b3, 20)  0 ||
 +virBitmapParse(0,^0, 0, b4, 20)  0)
 +goto cleanup;
 +
 +if (!virBitmapIsAllClear(b4))
  goto cleanup;
  
  if (virBitmapOverlaps(b1, b2) ||
 +virBitmapOverlaps(b1, b4) ||
  virBitmapOverlaps(b2, b3) ||
 -!virBitmapOverlaps(b1, b3))
 +virBitmapOverlaps(b2, b4) ||
 +!virBitmapOverlaps(b1, b3) ||
 +virBitmapOverlaps(b3, b4))
  goto cleanup;
  
  ret = 0;

 
 My Coverity checker was unhappy today because 'b4' is never
 virBitmapFree()'d
 
 
 John
 
Oops, thanks for pushing the fix.
Erik

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

Re: [libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls

2015-04-14 Thread John Ferlan 


On 04/13/2015 08:01 AM, Erik Skultety wrote:
 This patch adds checks for empty bitmaps right after the calls of
 virBitmapParse. These only include spots where set API's are called and
 where domain's XML is parsed.
 Also, it partially reverts commit 983f5a which added a check for
 invalid nodeset 0,^0 into virBitmapParse function. This change broke
 the logic, as an empty bitmap should not cause an error.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1210545
 ---
  src/conf/domain_conf.c   | 35 +++
  src/conf/numa_conf.c | 23 +++
  src/qemu/qemu_driver.c   |  5 +++--
  src/util/virbitmap.c |  3 ---
  src/xenconfig/xen_sxpr.c |  7 +++
  tests/virbitmaptest.c| 13 ++---
  6 files changed, 70 insertions(+), 16 deletions(-)
 

...

 diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
 index f247275..9a84e4c 100644
 --- a/tests/virbitmaptest.c
 +++ b/tests/virbitmaptest.c
 @@ -524,16 +524,23 @@ static int
  test10(const void *opaque ATTRIBUTE_UNUSED)
  {
  int ret = -1;
 -virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL;
 +virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL;
  
  if (virBitmapParse(0-3,5-8,11-15, 0, b1, 20)  0 ||
  virBitmapParse(4,9,10,16-19, 0, b2, 20)  0 ||
 -virBitmapParse(15, 0, b3, 20)  0)
 +virBitmapParse(15, 0, b3, 20)  0 ||
 +virBitmapParse(0,^0, 0, b4, 20)  0)
 +goto cleanup;
 +
 +if (!virBitmapIsAllClear(b4))
  goto cleanup;
  
  if (virBitmapOverlaps(b1, b2) ||
 +virBitmapOverlaps(b1, b4) ||
  virBitmapOverlaps(b2, b3) ||
 -!virBitmapOverlaps(b1, b3))
 +virBitmapOverlaps(b2, b4) ||
 +!virBitmapOverlaps(b1, b3) ||
 +virBitmapOverlaps(b3, b4))
  goto cleanup;
  
  ret = 0;
 

My Coverity checker was unhappy today because 'b4' is never
virBitmapFree()'d


John

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

[libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls

2015-04-13 Thread Erik Skultety 
This patch adds checks for empty bitmaps right after the calls of
virBitmapParse. These only include spots where set API's are called and
where domain's XML is parsed.
Also, it partially reverts commit 983f5a which added a check for
invalid nodeset 0,^0 into virBitmapParse function. This change broke
the logic, as an empty bitmap should not cause an error.

https://bugzilla.redhat.com/show_bug.cgi?id=1210545
---
 src/conf/domain_conf.c   | 35 +++
 src/conf/numa_conf.c | 23 +++
 src/qemu/qemu_driver.c   |  5 +++--
 src/util/virbitmap.c |  3 ---
 src/xenconfig/xen_sxpr.c |  7 +++
 tests/virbitmaptest.c| 13 ++---
 6 files changed, 70 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 823e003..f7f68ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11577,6 +11577,12 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node,
 if (virBitmapParse(nodemask, 0, def-sourceNodes,
VIR_DOMAIN_CPUMASK_LEN)  0)
 goto cleanup;
+
+if (virBitmapIsAllClear(def-sourceNodes)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'nodemask': %s), nodemask);
+goto cleanup;
+}
 }
 
 ret = 0;
@@ -13265,6 +13271,13 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,
 if (virBitmapParse(tmp, 0, def-cpumask, VIR_DOMAIN_CPUMASK_LEN)  0)
 goto error;
 
+if (virBitmapIsAllClear(def-cpumask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'cpuset': %s),
+   tmp);
+goto error;
+}
+
  cleanup:
 VIR_FREE(tmp);
 ctxt-node = oldnode;
@@ -13366,6 +13379,12 @@ virDomainHugepagesParseXML(xmlNodePtr node,
 if (virBitmapParse(nodeset, 0, hugepage-nodemask,
VIR_DOMAIN_CPUMASK_LEN)  0)
 goto cleanup;
+
+if (virBitmapIsAllClear(hugepage-nodemask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'nodeset': %s), nodeset);
+goto cleanup;
+}
 }
 
 ret = 0;
@@ -13487,13 +13506,14 @@ virDomainThreadSchedParse(xmlNodePtr node,
 goto error;
 }
 
-if (!virBitmapParse(tmp, 0, sp-ids,
-VIR_DOMAIN_CPUMASK_LEN) ||
-virBitmapIsAllClear(sp-ids) ||
+if (virBitmapParse(tmp, 0, sp-ids, VIR_DOMAIN_CPUMASK_LEN)  0)
+goto error;
+
+if (virBitmapIsAllClear(sp-ids) ||
 virBitmapNextSetBit(sp-ids, -1)  minid ||
 virBitmapLastSetBit(sp-ids)  maxid) {
 
-virReportError(VIR_ERR_XML_ERROR,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Invalid value of '%s': %s),
name, tmp);
 goto error;
@@ -13861,6 +13881,13 @@ virDomainDefParseXML(xmlDocPtr xml,
 if (virBitmapParse(tmp, 0, def-cpumask,
VIR_DOMAIN_CPUMASK_LEN)  0)
 goto error;
+
+if (virBitmapIsAllClear(def-cpumask)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'cpuset': %s), tmp);
+goto error;
+}
+
 VIR_FREE(tmp);
 }
 }
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 8a0f686..7ad3f66 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -178,6 +178,12 @@ virDomainNumatuneNodeParseXML(virDomainNumaPtr numa,
 if (virBitmapParse(tmp, 0, mem_node-nodeset,
VIR_DOMAIN_CPUMASK_LEN)  0)
 goto cleanup;
+
+if (virBitmapIsAllClear(mem_node-nodeset)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'nodeset': %s), tmp);
+goto cleanup;
+}
 VIR_FREE(tmp);
 }
 
@@ -233,10 +239,19 @@ virDomainNumatuneParseXML(virDomainNumaPtr numa,
 }
 VIR_FREE(tmp);
 
-if ((tmp = virXMLPropString(node, nodeset)) 
-virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN)  0)
-goto cleanup;
-VIR_FREE(tmp);
+tmp = virXMLPropString(node, nodeset);
+if (tmp) {
+if (virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN)  0)
+goto cleanup;
+
+if (virBitmapIsAllClear(nodeset)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Invalid value of 'nodeset': %s), tmp);
+goto cleanup;
+}
+
+VIR_FREE(tmp);
+}
 }
 
 if (virDomainNumatuneSet(numa,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f37a11e..cbb6e1b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10134,8 +10134,9 @@

Re: [libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls

2015-04-13 Thread Peter Krempa 
On Mon, Apr 13, 2015 at 14:01:27 +0200, Erik Skultety wrote:
 This patch adds checks for empty bitmaps right after the calls of
 virBitmapParse. These only include spots where set API's are called and
 where domain's XML is parsed.
 Also, it partially reverts commit 983f5a which added a check for
 invalid nodeset 0,^0 into virBitmapParse function. This change broke
 the logic, as an empty bitmap should not cause an error.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1210545
 ---
  src/conf/domain_conf.c   | 35 +++
  src/conf/numa_conf.c | 23 +++
  src/qemu/qemu_driver.c   |  5 +++--
  src/util/virbitmap.c |  3 ---
  src/xenconfig/xen_sxpr.c |  7 +++
  tests/virbitmaptest.c| 13 ++---
  6 files changed, 70 insertions(+), 16 deletions(-)

ACK, thanks for adding the test.

Peter



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

Re: [libvirt] [PATCHv2] virBitmap: Place virBitmapIsAllClear check after virBitmapParse calls

2015-04-13 Thread Erik Skultety 


On 04/13/2015 02:18 PM, Peter Krempa wrote:
 On Mon, Apr 13, 2015 at 14:01:27 +0200, Erik Skultety wrote:
 This patch adds checks for empty bitmaps right after the calls of
 virBitmapParse. These only include spots where set API's are called and
 where domain's XML is parsed.
 Also, it partially reverts commit 983f5a which added a check for
 invalid nodeset 0,^0 into virBitmapParse function. This change broke
 the logic, as an empty bitmap should not cause an error.

 https://bugzilla.redhat.com/show_bug.cgi?id=1210545
 ---
  src/conf/domain_conf.c   | 35 +++
  src/conf/numa_conf.c | 23 +++
  src/qemu/qemu_driver.c   |  5 +++--
  src/util/virbitmap.c |  3 ---
  src/xenconfig/xen_sxpr.c |  7 +++
  tests/virbitmaptest.c| 13 ++---
  6 files changed, 70 insertions(+), 16 deletions(-)
 
 ACK, thanks for adding the test.
 
 Peter
 
Thank you, now pushed.
Erik

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