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(a)redhat.com&gt; > --- > 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 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(a)redhat.com&gt; >>> --- >>> 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