[libvirt] [PATCH] nodeinfo: fix nodeGetFreePages when max node is zero
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", -- 1.9.3
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. Michal
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.
Michal
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); for (cell = startCell; cell <= lastCell; cell++) { for (i = 0; i < npages; i++) { Michal
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
participants (2)
-
Jincheng Miao -
Michal Privoznik