[libvirt] [PATCH v4 0/2] Fix nodeinfo output on PPC64 KVM hosts

[John Ferlan]

On 07/10/2015 09:04 AM, Shivaprasad bhat wrote:
> Hi John,
> 
> Thanks for the comments.
> 
> On Thu, Jul 9, 2015 at 9:16 PM, John Ferlan <jferlan at redhat.com> wrote:
>>
>>
>> On 07/07/2015 03:25 AM, Andrea Bolognani wrote:
>>> Changes from v3 to v4:
>>>
>>>   * removed a printf() statement;
>>>
>>>   * fixed typo in a commit message.
>>>
>>> Shivaprasad G Bhat (2):
>>>   Fix nodeinfo output on PPC64 KVM hosts
>>>   Add testcase for PPC64 kvm host nodeinfo
>>>
>>
>>
>> Never saw the v4 2/2 come through (nor do I see it in the archive);
>> however, I assume it's the same as the v3 patch:
>>
> 
> Its because of the size mailer daemon has blocked it. The 2/2 is pretty big.
> 

Right - my assumption too. Perhaps any future/followup patches just
reference that 2/2 that would need to be pushed as well, but not include
in the send... Seeing as it's held up anyway.

> 
>> http://www.redhat.com/archives/libvir-list/2015-July/msg00155.html
>>
>> Given it is and what I found reviewing the following:
>>
>> http://www.redhat.com/archives/libvir-list/2015-July/msg00219.html
>>
>> regarding nodeinfo.c not really using the tests/nodeinfodata local path
>> instead the running host's sysfs (/sys/devices/system) path.
> 
> I see nodeinfo referencing the tests-sysfs correctly on my laptop.
> From my gdb
> 473        if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
> (gdb)
> 472    while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
> (gdb)
> 480    if (direrr < 0)
> (gdb) p node
> $2 = 0x6268b0 "/home/shivaprasad/code/libvirt/tests/nodeinfodata/linux-test9/node/node0"
> 

As you could see in the series I referenced - there are a number of
nodeinfo.c API's which don't process the sysfs properly, e.g. they
assume /sys/devices/system.

I haven't been fully convinced that the patch which ends up as patch9 in
my series won't have some sort of negative affect somewhere down the
line. Consider if your "*/cpu/present" contained "0-47,64-95" instead of
"0-95" - what "expectations" would you have in this patch series?

The point being if the expectation is that 48-63 would/should have some
specific state and they don't, then I can certainly see the need for the
other patch. Since you had a reason to be in the code, I figured to pick
your brain over this logic while the code was still fresh in your mind!
 It's more a datapoint for the need of the filtering patch.

Once that patch is in place, the call added by that patch to
nodeGetPresentCPUBitmap could certainly have altered results if the
PPC64 host it was running on didn't have 96 CPU's.

Hopefully this makes sense (my multitasking abilities lately have been
suspect)!

John