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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list