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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 10, 2015 at 7:21 PM, John Ferlan <jferlan@xxxxxxxxxx> wrote:
>
>
> 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@xxxxxxxxxx> 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.
>

Thanks a lot for pointing out John. I am planning to test the patch on
such configuration and see how it goes. I expect, as you mentioned to discard
the offline cpus 48-63 during counting.

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

Regards,
Shiva

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]