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. Hopefully this makes sense (my multitasking abilities lately have been suspect)! John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list