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 at 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