[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] nodeinfo: fix nodeGetFreePages when max node is zero




On 09/24/2014 08:53 PM, Michal Privoznik wrote:
On 24.09.2014 14:00, Jincheng Miao wrote:

On 09/24/2014 07:40 PM, Michal Privoznik wrote:
On 24.09.2014 07:45, Jincheng Miao wrote:
In nodeGetFreePages, if startCell is given by '0',
and the max node number is '0' too. The for-loop
wouldn't be executed.
So convert it to while-loop.

Before:
virsh freepages --cellno 0 --pagesize 4
error: internal error: no suitable info found

After:
virsh freepages --cellno 0 --pagesize 4
4KiB: 472637

Signed-off-by: Jincheng Miao <jmiao redhat com>
---
  src/nodeinfo.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2459922..1fe190a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2042,18 +2042,19 @@ nodeGetFreePages(unsigned int npages,
      }

      lastCell = MIN(lastCell, startCell + cellCount);
+    cell = startCell;

-    for (cell = startCell; cell < lastCell; cell++) {
+    do {
          for (i = 0; i < npages; i++) {
              unsigned int page_size = pages[i];
              unsigned int page_free;

-            if (virNumaGetPageInfo(cell, page_size, 0, NULL,
&page_free) < 0)
+            if (virNumaGetPageInfo(cell++, page_size, 0, NULL,
&page_free) < 0)
                  goto cleanup;

              counts[ncounts++] = page_free;
          }
-    }
+    } while (cell < lastCell);

      if (!ncounts) {
          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",


There's no need to switch to do-while loop. All what is needed is cell
<= lastCell in the for loop. I'm rewriting the patch and pushing. ACK.

Wait, if change the for condition to 'cell <= lastCell', and pass
startCell == -1,
the for-loop will execute twice, and will overwrite counts[1] which is
not allocated.

Oh, right. Seems like I'm brainwashed today. This is the diff I've forgot to 'git add':

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 0a7642c..05eab6c 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2041,7 +2041,7 @@ nodeGetFreePages(unsigned int npages,
         goto cleanup;
     }

-    lastCell = MIN(lastCell, startCell + cellCount);
+    lastCell = MIN(lastCell, startCell + cellCount - 1);

Yep, this is working now with this adjustment.


     for (cell = startCell; cell <= lastCell; cell++) {
         for (i = 0; i < npages; i++) {

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]