Re: [libvirt] [PATCH v3 1/2] Fix nodeinfo output on PPC64 KVM hosts
On Mon, 2015-07-06 at 17:34 +0530, Shivaprasad bhat wrote:
Thanks a lot Andrea. The cleanups are really nice. I had a chance to
test the patch and it
seems to work consistently in all sucores_per_core modes.
Only two comments written inline .
Glad you're happy with the changes!
> +nodeGetThreadsPerSubcore;
The nodeGetThreadsPerSubcore being PPC specific, is it good
to have it in nodeinfo.h ? That was the rational on which I had the
ioctl wrapper
written in virarch.c in v2.
Even though it's ppc64 specific, all the logic that uses that function
and needs to take the value of threads_per_subcore into account is
already part of that file, so I think it makes sense to have it there.
The maintainers might disagree, of course :)
> + if (virBitmapSetBit(cpu_map, cpu) < 0) {
> + printf("virBitmapSetBit(%d)\n", cpu);
Using printf here. May be you wanted virReportError(?). I think we
can
ignore the SetBit return,
given this is from directory parsing.
That was indeed not supposed to be there, good catch! I've removed it
and posted v4 of the series.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team